From 432866d24270808ee73d72f6887b462f19afcef6 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 (cherry picked from commit 0937c7e85bc541a15ec0d51e2397a9799bdbcf33) Signed-off-by: Christopher Faulet --- src/quic_tx.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/quic_tx.c b/src/quic_tx.c index 0c9106d..d6aed56 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -1810,6 +1810,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)); @@ -1856,6 +1857,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 @@ -1921,7 +1923,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; @@ -1946,7 +1949,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; } } -- 1.7.10.4