From 96b2641fc8ce58eb1875e7b525c57e58e4b794c3 Mon Sep 17 00:00:00 2001 From: Frederic Lecaille Date: Mon, 25 Nov 2024 11:14:20 +0100 Subject: [PATCH] BUG/MAJOR: quic: fix wrong packet building due to already acked frames If a packet build was asked to probe the peer with frames which have just been acked, the frames build run by qc_build_frms() could be cancelled by qc_stream_frm_is_acked() whose aim is to check that current frames to be built have not been already acknowledged. In this case the packet build run by qc_do_build_pkt() is not interrupted, leading to the build of an empty packet which should be ack-eliciting. This is a bug detected by the BUG_ON() statement in qc_do_build_pk(): BUG_ON(qel->pktns->tx.pto_probe && !(pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING)); Thank you to @Tristan971 for having reported this issue in GH #2709 This is an old bug which must be backported as far as 2.6. --- src/quic_tx.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/quic_tx.c b/src/quic_tx.c index 37f46fe731ad..bc85f0d8b3c0 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -2012,7 +2012,23 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, goto comp_pkt_len; } - if (!ack_frm_len && !qel->pktns->tx.pto_probe) + if (qel->pktns->tx.pto_probe) { + /* If a probing packet was asked and could not be built, + * this is not because there was not enough room, but due to + * its frames which were already acknowledeged. + * See qc_stream_frm_is_acked()) called by qc_build_frms(). + * Note that qc_stream_frm_is_acked() logs a trace in this + * case mentionning some frames were already acknowledged. + * + * That said, the consequence must be the same: cancelling + * the packet build as if there was not enough room in the + * TX buffer. + */ + qel->pktns->tx.pto_probe--; + goto no_room; + } + + if (!ack_frm_len) goto no_room; } }