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.
litespeedtech · Mar 19, 2019 · 428530e · 428530e
1 parent 03d03a4
commit 428530e
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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