From 3b851a326b96286c5887566ceee4a9f925adcc58 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Wed, 6 Nov 2024 17:18:45 +0100 Subject: [PATCH] BUG/MEDIUM: quic: do not consider ACK on released stream as error When an ACK is received by haproxy, a lookup is performed to retrieve the related emitted frames. For STREAM type frames, a lookup is performed under quic_conn stream_desc tree. Indeed, the corresponding stream instance could be already released if multiple ACK were received refering to the same stream offset, which can happen notably if retransmission occured. qc_handle_newly_acked_frm() implements this logic. If the case with an already released stream is encounted, an error is returned. In the end, this error is propagated via qc_parse_pkt_frms() into qc_treat_rx_pkts(), despite being in fact a perfectly valid case. Fix this by adjusting ACK handling function to return a success value for the particular case of released stream instead. The impact of this bug is unknown, but it can have several consequences. * if the packet with the ACK contains other frames after it, their content will be skipped * the packet won't be acknowledged by haproxy, even if it contains other frames and is ack-eliciting. This may cause unneeded retransmission by the client. * RTT sampling information related to this ACK is ignored by haproxy Finally, it also caused the increment of the quic_conn counter dropped_parsing (droppars in "show quic" output) which should be reserved only for real error cases. This regression is present since the following patch : e7578084b0536e3e5988be7f09091c85beb8fa9d MINOR: quic: implement dedicated type for out-of-order stream ACK Before, qc_handle_newly_acked_frm() return type was always ignored. As such, no backport is needed. --- src/quic_rx.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/quic_rx.c b/src/quic_rx.c index 3204f3fe5..69a36ec77 100644 --- a/src/quic_rx.c +++ b/src/quic_rx.c @@ -232,8 +232,7 @@ static int qc_handle_newly_acked_frm(struct quic_conn *qc, struct quic_frame *fr if (!node) { TRACE_DEVEL("acked stream for released stream", QUIC_EV_CONN_ACKSTRM, qc, strm_frm); qc_release_frm(qc, frm); - /* early return */ - goto leave; + /* return as success */ } else { stream = eb64_entry(node, struct qc_stream_desc, by_id);