From 2ed07212e5d9e880f87770af270b2a9a012cfc81 Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Wed, 11 Nov 2020 07:50:03 -0500 Subject: [PATCH] Release 2.24.3 - [BUGFIX] Get rough RTT estimate on receipt of Handshake packet. This prevents BBR on the client from miscalculating pacing rate, slowing down sending of ACK packets. - [BUGFIX] Packets sent during handshake are app-limited. - [BUGFIX] Bandwidth sampler starts in app-limited mode. - [BUGFIX] Memory leak: free QPACK handler context in stream dtor. - Logging improvements. --- CHANGELOG | 10 ++++ docs/conf.py | 2 +- include/lsquic.h | 2 +- src/liblsquic/lsquic_bw_sampler.c | 1 + src/liblsquic/lsquic_full_conn.c | 1 + src/liblsquic/lsquic_full_conn_ietf.c | 8 ++++ src/liblsquic/lsquic_send_ctl.c | 67 ++++++++++++++++++++++++--- src/liblsquic/lsquic_send_ctl.h | 5 ++ src/liblsquic/lsquic_stream.c | 2 + 9 files changed, 90 insertions(+), 8 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 9f7a6f50f..d985ce5db 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,13 @@ +2020-11-11 + - 2.24.3 + - [BUGFIX] Get rough RTT estimate on receipt of Handshake packet. + This prevents BBR on the client from miscalculating pacing rate, + slowing down sending of ACK packets. + - [BUGFIX] Packets sent during handshake are app-limited. + - [BUGFIX] Bandwidth sampler starts in app-limited mode. + - [BUGFIX] Memory leak: free QPACK handler context in stream dtor. + - Logging improvements. + 2020-11-05 - 2.24.2 - [BUGFIX] Allow peer to migrate when its SCID is zero-length. diff --git a/docs/conf.py b/docs/conf.py index ab18cdfaa..8df0ab0e5 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -26,7 +26,7 @@ # The short X.Y version version = u'2.24' # The full version, including alpha/beta/rc tags -release = u'2.24.2' +release = u'2.24.3' # -- General configuration --------------------------------------------------- diff --git a/include/lsquic.h b/include/lsquic.h index 5ab0d5fbb..de6e1c166 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 2 #define LSQUIC_MINOR_VERSION 24 -#define LSQUIC_PATCH_VERSION 2 +#define LSQUIC_PATCH_VERSION 3 /** * Engine flags: diff --git a/src/liblsquic/lsquic_bw_sampler.c b/src/liblsquic/lsquic_bw_sampler.c index 0885b4a00..93596a35b 100644 --- a/src/liblsquic/lsquic_bw_sampler.c +++ b/src/liblsquic/lsquic_bw_sampler.c @@ -37,6 +37,7 @@ lsquic_bw_sampler_init (struct bw_sampler *sampler, struct lsquic_conn *conn, sampler->bws_malo = malo; sampler->bws_conn = conn; sampler->bws_retx_frames = retx_frames; + sampler->bws_flags |= BWS_APP_LIMITED; LSQ_DEBUG("init"); return 0; } diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c index e12db364a..bcb6790cb 100644 --- a/src/liblsquic/lsquic_full_conn.c +++ b/src/liblsquic/lsquic_full_conn.c @@ -3545,6 +3545,7 @@ full_conn_ci_tick (lsquic_conn_t *lconn, lsquic_time_t now) if (!handshake_done_or_doing_sess_resume(conn)) { process_hsk_stream_write_events(conn); + lsquic_send_ctl_maybe_app_limited(&conn->fc_send_ctl, &conn->fc_path); goto end_write; } diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c index 415092115..6f9f48ca3 100644 --- a/src/liblsquic/lsquic_full_conn_ietf.c +++ b/src/liblsquic/lsquic_full_conn_ietf.c @@ -7069,12 +7069,16 @@ process_regular_packet (struct ietf_full_conn *conn, if (pns == PNS_INIT) conn->ifc_conn.cn_esf.i->esfi_set_iscid(conn->ifc_conn.cn_enc_session, packet_in); + else if (pns == PNS_HSK) + lsquic_send_ctl_maybe_calc_rough_rtt(&conn->ifc_send_ctl, pns - 1); if ((pns == PNS_INIT && (conn->ifc_flags & IFC_IGNORE_INIT)) || (pns == PNS_HSK && (conn->ifc_flags & IFC_IGNORE_HSK))) { /* Don't bother decrypting */ LSQ_DEBUG("ignore %s packet", pns == PNS_INIT ? "Initial" : "Handshake"); + EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "ignore %s packet", + lsquic_pns2str[pns]); return 0; } @@ -8087,7 +8091,11 @@ ietf_full_conn_ci_tick (struct lsquic_conn *lconn, lsquic_time_t now) if (0 == s) process_crypto_stream_write_events(conn); if (!(conn->ifc_mflags & MF_DOING_0RTT)) + { + lsquic_send_ctl_maybe_app_limited(&conn->ifc_send_ctl, + CUR_NPATH(conn)); goto end_write; + } } maybe_conn_flush_special_streams(conn); diff --git a/src/liblsquic/lsquic_send_ctl.c b/src/liblsquic/lsquic_send_ctl.c index 9bdee1fec..f33098a05 100644 --- a/src/liblsquic/lsquic_send_ctl.c +++ b/src/liblsquic/lsquic_send_ctl.c @@ -57,6 +57,12 @@ #define LSQUIC_LOG_CONN_ID lsquic_conn_log_cid(ctl->sc_conn_pub->lconn) #include "lsquic_logger.h" +#if __GNUC__ +# define UNLIKELY(cond) __builtin_expect(cond, 0) +#else +# define UNLIKELY(cond) cond +#endif + #define MAX_RESUBMITTED_ON_RTO 2 #define MAX_RTO_BACKOFFS 10 #define DEFAULT_RETX_DELAY 500000 /* Microseconds */ @@ -742,6 +748,12 @@ take_rtt_sample (lsquic_send_ctl_t *ctl, const lsquic_time_t measured_rtt = now - sent; if (packno > ctl->sc_max_rtt_packno && lack_delta < measured_rtt) { + if (UNLIKELY(ctl->sc_flags & SC_ROUGH_RTT)) + { + memset(&ctl->sc_conn_pub->rtt_stats, 0, + sizeof(ctl->sc_conn_pub->rtt_stats)); + ctl->sc_flags &= ~SC_ROUGH_RTT; + } ctl->sc_max_rtt_packno = packno; lsquic_rtt_stats_update(&ctl->sc_conn_pub->rtt_stats, measured_rtt, lack_delta); LSQ_DEBUG("packno %"PRIu64"; rtt: %"PRIu64"; delta: %"PRIu64"; " @@ -1117,12 +1129,6 @@ lsquic_send_ctl_got_ack (lsquic_send_ctl_t *ctl, __builtin_prefetch(packet_out); #endif -#if __GNUC__ -# define UNLIKELY(cond) __builtin_expect(cond, 0) -#else -# define UNLIKELY(cond) cond -#endif - #if __GNUC__ if (UNLIKELY(LSQ_LOG_ENABLED(LSQ_LOG_DEBUG))) #endif @@ -1520,9 +1526,22 @@ send_ctl_all_bytes_out (const struct lsquic_send_ctl *ctl) int lsquic_send_ctl_pacer_blocked (struct lsquic_send_ctl *ctl) { +#ifdef NDEBUG return (ctl->sc_flags & SC_PACE) && !lsquic_pacer_can_schedule(&ctl->sc_pacer, ctl->sc_n_in_flight_all); +#else + if (ctl->sc_flags & SC_PACE) + { + const int blocked = !lsquic_pacer_can_schedule(&ctl->sc_pacer, + ctl->sc_n_in_flight_all); + LSQ_DEBUG("pacer blocked: %d, in_flight_all: %u", blocked, + ctl->sc_n_in_flight_all); + return blocked; + } + else + return 0; +#endif } @@ -3214,6 +3233,42 @@ lsquic_send_ctl_set_token (struct lsquic_send_ctl *ctl, } +void +lsquic_send_ctl_maybe_calc_rough_rtt (struct lsquic_send_ctl *ctl, + enum packnum_space pns) +{ + const struct lsquic_packet_out *packet_out; + lsquic_time_t min_sent, rtt; + struct lsquic_packets_tailq *const *q; + struct lsquic_packets_tailq *const queues[] = { + &ctl->sc_lost_packets, + &ctl->sc_unacked_packets[pns], + }; + + if ((ctl->sc_flags & SC_ROUGH_RTT) + || lsquic_rtt_stats_get_srtt(&ctl->sc_conn_pub->rtt_stats)) + return; + + min_sent = UINT64_MAX; + for (q = queues; q < queues + sizeof(queues) / sizeof(queues[0]); ++q) + TAILQ_FOREACH(packet_out, *q, po_next) + if (min_sent > packet_out->po_sent) + min_sent = packet_out->po_sent; + + /* If we do not have an RTT estimate yet, get a rough estimate of it, + * because now we will ignore packets that carry acknowledgements and + * RTT estimation may be delayed. + */ + if (min_sent < UINT64_MAX) + { + rtt = lsquic_time_now() - min_sent; + lsquic_rtt_stats_update(&ctl->sc_conn_pub->rtt_stats, rtt, 0); + ctl->sc_flags |= SC_ROUGH_RTT; + LSQ_DEBUG("set rough RTT to %"PRIu64" usec", rtt); + } +} + + void lsquic_send_ctl_empty_pns (struct lsquic_send_ctl *ctl, enum packnum_space pns) { diff --git a/src/liblsquic/lsquic_send_ctl.h b/src/liblsquic/lsquic_send_ctl.h index 196c2aaf4..2b4de2da6 100644 --- a/src/liblsquic/lsquic_send_ctl.h +++ b/src/liblsquic/lsquic_send_ctl.h @@ -53,6 +53,7 @@ enum send_ctl_flags { SC_ACK_RECV_INIT= 1 << 19, SC_ACK_RECV_HSK = SC_ACK_RECV_INIT << PNS_HSK, SC_ACK_RECV_APP = SC_ACK_RECV_INIT << PNS_APP, + SC_ROUGH_RTT = 1 << 22, }; typedef struct lsquic_send_ctl { @@ -366,6 +367,10 @@ lsquic_send_ctl_set_token (struct lsquic_send_ctl *, void lsquic_send_ctl_empty_pns (struct lsquic_send_ctl *, enum packnum_space); +void +lsquic_send_ctl_maybe_calc_rough_rtt (struct lsquic_send_ctl *, + enum packnum_space); + void lsquic_send_ctl_repath (struct lsquic_send_ctl *ctl, const struct network_path *old, const struct network_path *new, diff --git a/src/liblsquic/lsquic_stream.c b/src/liblsquic/lsquic_stream.c index b5638f53d..ba68762ac 100644 --- a/src/liblsquic/lsquic_stream.c +++ b/src/liblsquic/lsquic_stream.c @@ -701,6 +701,8 @@ lsquic_stream_destroy (lsquic_stream_t *stream) destroy_uh(stream); free(stream->sm_buf); free(stream->sm_header_block); + if (stream->sm_hblock_ctx) + free(stream->sm_hblock_ctx); LSQ_DEBUG("destroyed stream"); SM_HISTORY_DUMP_REMAINING(stream); free(stream);