From 428530e38e78354aedf615b8eb21c3319eab11e2 Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Tue, 19 Mar 2019 16:23:50 -0400 Subject: [PATCH] Release 1.19.6 - [BUGFIX] Ensure that Largest Observed does not decrease in ACKs we send. ACK frames placed in packets in buffered queues (optimization introduced in 1.17.15) can be preceded by an ACK frame generated later. In this case, the older ACK frame should not be sent out, as Chromium- based servers flags decrease in the ACK frame's Largest Observed value as an error. --- CHANGELOG | 9 +++++++++ include/lsquic.h | 2 +- src/liblsquic/lsquic_full_conn.c | 2 +- src/liblsquic/lsquic_send_ctl.c | 14 ++++++++++++++ src/liblsquic/lsquic_send_ctl.h | 9 ++++++++- 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 17c6c6706..cd478ba13 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,12 @@ +2019-03-19 + - 1.19.6 + - [BUGFIX] Ensure that Largest Observed does not decrease in ACKs we + send. ACK frames placed in packets in buffered queues (optimization + introduced in 1.17.15) can be preceded by an ACK frame generated later. + In this case, the older ACK frame should not be sent out, as Chromium- + based servers flags decrease in the ACK frame's Largest Observed value + as an error. + 2019-03-05 - 1.19.5 - [BUGFIX] Use correct public key from PUBS based on KEXS index. diff --git a/include/lsquic.h b/include/lsquic.h index 3153ab18d..40f277481 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 1 #define LSQUIC_MINOR_VERSION 19 -#define LSQUIC_PATCH_VERSION 5 +#define LSQUIC_PATCH_VERSION 6 /** * Engine flags: diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c index 87bd637ec..edecb6dbc 100644 --- a/src/liblsquic/lsquic_full_conn.c +++ b/src/liblsquic/lsquic_full_conn.c @@ -980,7 +980,7 @@ full_conn_ci_write_ack (struct lsquic_conn *lconn, EV_LOG_GENERATED_ACK_FRAME(LSQUIC_LOG_CONN_ID, conn->fc_conn.cn_pf, packet_out->po_data + packet_out->po_data_sz, w); verify_ack_frame(conn, packet_out->po_data + packet_out->po_data_sz, w); - lsquic_send_ctl_scheduled_ack(&conn->fc_send_ctl); + lsquic_send_ctl_scheduled_ack(&conn->fc_send_ctl, packet_out->po_ack2ed); packet_out->po_frame_types |= 1 << QUIC_FRAME_ACK; lsquic_send_ctl_incr_pack_sz(&conn->fc_send_ctl, packet_out, w); packet_out->po_regen_sz += w; diff --git a/src/liblsquic/lsquic_send_ctl.c b/src/liblsquic/lsquic_send_ctl.c index 9247962a9..1bda2b24c 100644 --- a/src/liblsquic/lsquic_send_ctl.c +++ b/src/liblsquic/lsquic_send_ctl.c @@ -1857,6 +1857,20 @@ lsquic_send_ctl_schedule_buffered (lsquic_send_ctl_t *ctl, while ((packet_out = TAILQ_FIRST(&packet_q->bpq_packets)) && lsquic_send_ctl_can_send(ctl)) { + if ((packet_out->po_frame_types & QUIC_FTBIT_ACK) + && packet_out->po_ack2ed < ctl->sc_largest_acked) + { + LSQ_DEBUG("Remove out-of-order ACK from buffered packet"); + lsquic_packet_out_chop_regen(packet_out); + if (packet_out->po_data_sz == 0) + { + LSQ_DEBUG("Dropping now-empty buffered packet"); + TAILQ_REMOVE(&packet_q->bpq_packets, packet_out, po_next); + --packet_q->bpq_count; + send_ctl_destroy_packet(ctl, packet_out); + continue; + } + } if (bits != lsquic_packet_out_packno_bits(packet_out)) { used = packno_bits2len(lsquic_packet_out_packno_bits(packet_out)); diff --git a/src/liblsquic/lsquic_send_ctl.h b/src/liblsquic/lsquic_send_ctl.h index da3eecd6d..d2731253f 100644 --- a/src/liblsquic/lsquic_send_ctl.h +++ b/src/liblsquic/lsquic_send_ctl.h @@ -81,6 +81,12 @@ typedef struct lsquic_send_ctl { * else and it is not insanely long.) */ lsquic_packno_t sc_largest_ack2ed; + /* sc_largest_acked is the largest packet number in PNS_APP packet number + * space sent by peer for which we generated (not necessarily sent) an ACK. + * This information is used to drop stale ACK frames from packets in + * buffered queues. + */ + lsquic_packno_t sc_largest_acked; lsquic_time_t sc_loss_to; struct { @@ -174,8 +180,9 @@ lsquic_send_ctl_reschedule_packets (lsquic_send_ctl_t *); #define lsquic_send_ctl_lost_ack(ctl) ((ctl)->sc_flags & SC_LOST_ACK) -#define lsquic_send_ctl_scheduled_ack(ctl) do { \ +#define lsquic_send_ctl_scheduled_ack(ctl, packno) do { \ (ctl)->sc_flags &= ~SC_LOST_ACK; \ + (ctl)->sc_largest_acked = packno; \ } while (0) void