From 456c3997b203ab12306e9010ffb5507c905970e0 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Wed, 9 Oct 2024 11:59:32 +0200 Subject: [PATCH] BUG/MEDIUM: quic: properly decount out-of-order ACK on stream release Out-of-order STREAM ACK are buffered in its related streambuf tree. On insertion, overlapping or contiguous ranges are merged together. The total size of buffered ack range is stored in streambuf member and reported to QUIC MUX layer on streambuf release. The objective is to ensure QUIC MUX layer can allocate Tx buffers conveniently to preserve a good transfer throughput. Streamdesc is the overall container of many streambufs. It may also been released when its upper QCS instance is freed, after all stream data have been emitted. In this case, the active streambuf is also released via custom code. However, in this code path, was not reported to the QUIC MUX layer. This bug caused wrong estimation for the QUIC MUX txbuf window, with bytes reamining even after all ACK reception. This may cause transfer freeze on other connection streams, with RESET_STREAM emission on timeout client. To fix this, reuse the existing qc_stream_buf_release() function on streamdesc release. This ensures that notify_room is correctly used. No need to backport. --- src/quic_stream.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/quic_stream.c b/src/quic_stream.c index 7f06d63eb..293a2457a 100644 --- a/src/quic_stream.c +++ b/src/quic_stream.c @@ -128,15 +128,14 @@ void qc_stream_desc_release(struct qc_stream_desc *stream, if (final_size < tail_offset) b_sub(buf, tail_offset - final_size); - if (!b_data(buf)) { - /* This will ensure notify_room is triggered. */ + /* Release active buffer, or delete it immediatly if there is + * no data to acknowledge. Both functions will reset active + * buf pointer and invoke if necessary. + */ + if (!b_data(buf)) qc_stream_buf_free(stream, &stream_buf); - } - else if (stream->notify_room && b_room(buf)) { - stream->notify_room(stream, b_room(buf)); - } - - stream->buf = NULL; + else + qc_stream_buf_release(stream); } if (qc_stream_desc_done(stream)) {