From 0937c7e85bc541a15ec0d51e2397a9799bdbcf33 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Tue, 12 Aug 2025 11:30:03 +0200 Subject: [PATCH] BUG/MINOR: quic: do not emit probe data if CONNECTION_CLOSE requested If connection closing is activated, qc_prep_pkts() can only built a datagram with a single packet. This is because we consider that only a single CONNECTION_CLOSE frame is relevant at this stage. This is handled both by qc_prep_pkts() which ensure that only a single packet datagram is built and also qc_do_build_pkt() which prevents the invokation of qc_build_frms() if is set. However, there is an incoherency for probing. First, qc_prep_pkts() deactivates it if connection closing is requested. But qc_do_build_pkt() may still emit probing frame as it does not check its argument but rather QEL field directly. This can results in a packet mixing a PING and a CONNECTION close frames, which is useless. Fix this by adjusting qc_do_build_pkt() : closing argument is also checked on PING probing emission. Note that there is still shaky code here as qc_do_build_pkt() should rely only on argument to ensure this. This should be backported up to 2.6. (cherry picked from commit 0376e66112f17d79cfd1824ae75d7288eb383059) Signed-off-by: Christopher Faulet (cherry picked from commit 6d0cdabc754d94c1e5036ca4ce4b01085afcdfa1) Signed-off-by: Christopher Faulet --- src/quic_tx.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/quic_tx.c b/src/quic_tx.c index edeb395..28c7fc1 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -1960,6 +1960,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, /* Build an ACK frame if required. */ ack_frm_len = 0; /* Do not ack and probe at the same time. */ + /* TODO qc_do_build_pkt() must rely on its argument instead of using QEL field. */ if ((must_ack || (qel->pktns->flags & QUIC_FL_PKTNS_ACK_REQUIRED)) && !qel->pktns->tx.pto_probe) { struct quic_arngs *arngs = &qel->pktns->rx.arngs; BUG_ON(eb_is_empty(&qel->pktns->rx.arngs.root)); @@ -2006,6 +2007,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, goto comp_pkt_len; } + /* TODO qc_do_build_pkt() must rely on its argument instead of using QEL field. */ 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 @@ -2071,7 +2073,8 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, else if (len_frms && len_frms < QUIC_PACKET_PN_MAXLEN) { len += padding_len = QUIC_PACKET_PN_MAXLEN - len_frms; } - else if (LIST_ISEMPTY(&frm_list)) { + else if (LIST_ISEMPTY(&frm_list) && !cc) { + /* TODO qc_do_build_pkt() must rely on its argument instead of using QEL field. */ if (qel->pktns->tx.pto_probe) { /* If we cannot send a frame, we send a PING frame. */ add_ping_frm = 1; @@ -2096,7 +2099,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, } else { /* If there is no frame at all to follow, add at least a PADDING frame. */ - if (!ack_frm_len && !cc) + if (!ack_frm_len) len += padding_len = QUIC_PACKET_PN_MAXLEN - *pn_len; } } @@ -2171,7 +2174,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, goto no_room; } - BUG_ON(qel->pktns->tx.pto_probe && + BUG_ON(qel->pktns->tx.pto_probe && !cc && !(pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING)); /* If this packet is ack-eliciting and we are probing let's * decrement the PTO probe counter. -- 1.7.10.4