]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: quic: fix received ACK stream calculation
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 24 Feb 2022 16:39:57 +0000 (17:39 +0100)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 24 Feb 2022 17:37:39 +0000 (18:37 +0100)
Adjust the handling of ACK for STREAM frames. When receiving a ACK, the
corresponding frames from the acknowledged packet are retrieved. If a
frame is of type STREAM, we compare the frame STREAM offset with the
last offset known of the qcs instance.

The comparison was incomplete as it did not treat a acked offset smaller
than the known offset. Previously, the acked frame was incorrectly
buffered in the qcs.tx.acked_frms. On reception of future ACKs, when
trying to process the buffered acks via qcs_try_to_consume, the loop is
interrupted on the smallest offset different from the qcs known offset :
in this case it will be the previous smaller range. This is a real bug
as it prevents all buffered ACKs to be processed, eventually filling the
qcs sending buffer and cause the transfer to stall.

Fix this by properly properly handle smaller acked offset. First check
if the offset length is greater than the qcs offset and mark as
acknowledged the difference on the qcs. If not, the frame is not
buffered and simply ignored.

src/xprt_quic.c

index cefa4c6902103d447a340208ce7dd12a62928bd8..52f843dd22d9fa2c0a291621e302a55abc975c89 100644 (file)
@@ -1396,14 +1396,19 @@ static int qcs_try_to_consume(struct qcs *qcs)
                struct quic_stream *strm;
 
                strm = eb64_entry(&frm_node->node, struct quic_stream, offset);
-               if (strm->offset.key != qcs->tx.ack_offset)
+               if (strm->offset.key > qcs->tx.ack_offset)
                        break;
 
-               b_del(strm->buf, strm->len);
-               qcs->tx.ack_offset += strm->len;
+               if (strm->offset.key + strm->len > qcs->tx.ack_offset) {
+                       const size_t diff = strm->offset.key + strm->len -
+                                           qcs->tx.ack_offset;
+                       qcs->tx.ack_offset += diff;
+                       b_del(strm->buf, diff);
+                       ret = 1;
+               }
+
                frm_node = eb64_next(frm_node);
                eb64_delete(&strm->offset);
-               ret = 1;
        }
 
        return ret;
@@ -1423,16 +1428,22 @@ static inline void qc_treat_acked_tx_frm(struct quic_conn *qc,
                struct qcs *qcs = frm->stream.qcs;
                struct quic_stream *strm = &frm->stream;
 
-               if (qcs->tx.ack_offset == strm->offset.key) {
-                       b_del(strm->buf, strm->len);
-                       qcs->tx.ack_offset += strm->len;
+               if (strm->offset.key <= qcs->tx.ack_offset) {
+                       if (strm->offset.key + strm->len > qcs->tx.ack_offset) {
+                               const size_t diff = strm->offset.key + strm->len -
+                                                   qcs->tx.ack_offset;
+                               qcs->tx.ack_offset += diff;
+                               b_del(strm->buf, diff);
+                               stream_acked = 1;
+                       }
+
                        LIST_DELETE(&frm->list);
                        pool_free(pool_head_quic_frame, frm);
-                       stream_acked = 1;
                }
                else {
                        eb64_insert(&qcs->tx.acked_frms, &strm->offset);
                }
+
                stream_acked |= qcs_try_to_consume(qcs);
        }
        break;