From 8252b0b9355076d5511fe5e38b047fc3448a401d Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Thu, 29 Nov 2018 15:26:49 -0500 Subject: [PATCH] Release 1.17.6 - Add failsafe: resume sending packets after some time The change puts a 1-second limit on the amount of time the engine will not send packets after some packets are delayed. This makes the library robust in case the user does not unblock the engine explicitly using lsquic_engine_send_unsent_packets() call. - [BUGFIX] Handle corner cases in send controller when packets are a) delayed or b) dropped during repackaging. - [BUGFIX] Memory leak: destroy buffered packets during controller cleanup. --- CHANGELOG | 14 ++++++++++++ include/lsquic.h | 2 +- src/liblsquic/lsquic_engine.c | 37 ++++++++++++++++++++++++++----- src/liblsquic/lsquic_packet_out.h | 1 + src/liblsquic/lsquic_send_ctl.c | 27 ++++++++++++++++++++-- 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 0a41a039d..4592be1e3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,17 @@ +2018-11-29 + - 1.17.6 + - Add failsafe: resume sending packets after some time + + The change puts a 1-second limit on the amount of time the engine + will not send packets after some packets are delayed. This makes + the library robust in case the user does not unblock the engine + explicitly using lsquic_engine_send_unsent_packets() call. + + - [BUGFIX] Handle corner cases in send controller when packets are + a) delayed or b) dropped during repackaging. + - [BUGFIX] Memory leak: destroy buffered packets during controller + cleanup. + 2018-11-16 - 1.17.3 - [BUGFIX] Do not send STOP_WAITING frames when using Q044 diff --git a/include/lsquic.h b/include/lsquic.h index 152b1e131..faf72c46c 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 1 #define LSQUIC_MINOR_VERSION 17 -#define LSQUIC_PATCH_VERSION 3 +#define LSQUIC_PATCH_VERSION 6 /** * Engine flags: diff --git a/src/liblsquic/lsquic_engine.c b/src/liblsquic/lsquic_engine.c index 5b8919c79..a53ac3c67 100644 --- a/src/liblsquic/lsquic_engine.c +++ b/src/liblsquic/lsquic_engine.c @@ -61,6 +61,8 @@ #define LSQUIC_LOGGER_MODULE LSQLM_ENGINE #include "lsquic_logger.h" +#define MIN(a, b) ((a) < (b) ? (a) : (b)) + /* The batch of outgoing packets grows and shrinks dynamically */ #define MAX_OUT_BATCH_SIZE 1024 @@ -168,6 +170,7 @@ struct lsquic_engine lsquic_time_t last_sent; unsigned n_conns; lsquic_time_t deadline; + lsquic_time_t resume_sending_at; struct out_batch out_batch; }; @@ -1025,6 +1028,7 @@ send_batch (lsquic_engine_t *engine, struct conns_out_iter *conns_iter, if (n_sent < (int) n_to_send) { engine->pub.enp_flags &= ~ENPUB_CAN_SEND; + engine->resume_sending_at = now + 1000000; LSQ_DEBUG("cannot send packets"); EV_LOG_GENERIC_EVENT("cannot send packets"); } @@ -1272,6 +1276,15 @@ process_connections (lsquic_engine_t *engine, conn_iter_f next_conn, TAILQ_INIT(&ticked_conns); reset_deadline(engine, now); + if (!(engine->pub.enp_flags & ENPUB_CAN_SEND) + && now > engine->resume_sending_at) + { + LSQ_NOTICE("failsafe activated: resume sending packets again after " + "timeout"); + EV_LOG_GENERIC_EVENT("resume sending packets again after timeout"); + engine->pub.enp_flags |= ENPUB_CAN_SEND; + } + i = 0; while ((conn = next_conn(engine)) ) @@ -1412,8 +1425,8 @@ lsquic_engine_quic_versions (const lsquic_engine_t *engine) int lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff) { - const lsquic_time_t *next_time; - lsquic_time_t now; + const lsquic_time_t *next_attq_time; + lsquic_time_t now, next_time; if (((engine->flags & ENG_PAST_DEADLINE) && lsquic_mh_count(&engine->conns_out)) @@ -1423,12 +1436,24 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff) return 1; } - next_time = attq_next_time(engine->attq); - if (!next_time) - return 0; + next_attq_time = attq_next_time(engine->attq); + if (engine->pub.enp_flags & ENPUB_CAN_SEND) + { + if (next_attq_time) + next_time = *next_attq_time; + else + return 0; + } + else + { + if (next_attq_time) + next_time = MIN(*next_attq_time, engine->resume_sending_at); + else + next_time = engine->resume_sending_at; + } now = lsquic_time_now(); - *diff = (int) ((int64_t) *next_time - (int64_t) now); + *diff = (int) ((int64_t) next_time - (int64_t) now); return 1; } diff --git a/src/liblsquic/lsquic_packet_out.h b/src/liblsquic/lsquic_packet_out.h index 0d5981fd3..9edeb84de 100644 --- a/src/liblsquic/lsquic_packet_out.h +++ b/src/liblsquic/lsquic_packet_out.h @@ -94,6 +94,7 @@ typedef struct lsquic_packet_out PO_IPv6 = (1 <<20), /* Set if pmi_allocate was passed is_ipv6=1, * otherwise unset. */ + PO_LIMITED = (1 <<21), /* Used to credit sc_next_limit if needed. */ } po_flags; enum quic_ft_bit po_frame_types:16; /* Bitmask of QUIC_FRAME_* */ unsigned short po_data_sz; /* Number of usable bytes in data */ diff --git a/src/liblsquic/lsquic_send_ctl.c b/src/liblsquic/lsquic_send_ctl.c index d4585f591..6f29f29b0 100644 --- a/src/liblsquic/lsquic_send_ctl.c +++ b/src/liblsquic/lsquic_send_ctl.c @@ -858,7 +858,8 @@ send_ctl_next_packno (lsquic_send_ctl_t *ctl) void lsquic_send_ctl_cleanup (lsquic_send_ctl_t *ctl) { - lsquic_packet_out_t *packet_out; + lsquic_packet_out_t *packet_out, *next; + unsigned n; lsquic_senhist_cleanup(&ctl->sc_senhist); while ((packet_out = TAILQ_FIRST(&ctl->sc_scheduled_packets))) { @@ -881,6 +882,16 @@ lsquic_send_ctl_cleanup (lsquic_send_ctl_t *ctl) TAILQ_REMOVE(&ctl->sc_lost_packets, packet_out, po_next); send_ctl_destroy_packet(ctl, packet_out); } + for (n = 0; n < sizeof(ctl->sc_buffered_packets) / + sizeof(ctl->sc_buffered_packets[0]); ++n) + { + for (packet_out = TAILQ_FIRST(&ctl->sc_buffered_packets[n].bpq_packets); + packet_out; packet_out = next) + { + next = TAILQ_NEXT(packet_out, po_next); + send_ctl_destroy_packet(ctl, packet_out); + } + } pacer_cleanup(&ctl->sc_pacer); #if LSQUIC_SEND_STATS LSQ_NOTICE("stats: n_total_sent: %u; n_resent: %u; n_delayed: %u", @@ -1083,6 +1094,7 @@ lsquic_packet_out_t * lsquic_send_ctl_next_packet_to_send (lsquic_send_ctl_t *ctl) { lsquic_packet_out_t *packet_out; + int dec_limit; get_packet: packet_out = TAILQ_FIRST(&ctl->sc_scheduled_packets); @@ -1093,10 +1105,12 @@ lsquic_send_ctl_next_packet_to_send (lsquic_send_ctl_t *ctl) !(packet_out->po_frame_types & (1 << QUIC_FRAME_ACK))) { if (ctl->sc_next_limit) - --ctl->sc_next_limit; + dec_limit = 1; else return NULL; } + else + dec_limit = 0; send_ctl_sched_remove(ctl, packet_out); if (packet_out->po_flags & PO_REPACKNO) @@ -1116,6 +1130,13 @@ lsquic_send_ctl_next_packet_to_send (lsquic_send_ctl_t *ctl) } ctl->sc_bytes_out += packet_out_total_sz(packet_out); + if (dec_limit) + { + --ctl->sc_next_limit; + packet_out->po_flags |= PO_LIMITED; + } + else + packet_out->po_flags &= ~PO_LIMITED; return packet_out; } @@ -1126,6 +1147,8 @@ lsquic_send_ctl_delayed_one (lsquic_send_ctl_t *ctl, { send_ctl_sched_prepend(ctl, packet_out); ctl->sc_bytes_out -= packet_out_total_sz(packet_out); + if (packet_out->po_flags & PO_LIMITED) + ++ctl->sc_next_limit; LSQ_DEBUG("packet %"PRIu64" has been delayed", packet_out->po_packno); #if LSQUIC_SEND_STATS ++ctl->sc_stats.n_delayed;