From 0376e66112f17d79cfd1824ae75d7288eb383059 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. --- 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 7c3843fc1..48979aa00 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -1856,6 +1856,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)); @@ -1902,6 +1903,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 @@ -1967,7 +1969,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; @@ -1992,7 +1995,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; } } @@ -2067,7 +2070,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.