From 7d554ca6295033cbf1b82a15980ae8c0a0c94ec3 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 11 Aug 2025 18:04:59 +0200 Subject: [PATCH] BUG/MINOR: quic: don't coalesce probing and ACK packet of same type Haproxy QUIC stack suffers from a limitation : it's not possible to emit a packet which contains probing data and a ACK frame in it. Thus, in case qc_do_build_pkt() is invoked which both values as true, probing has the priority and ACK is ignored. However, this has the undesired side-effect of possibly generating two coalesced packets of the same type in the same datagram : the first one with the probing data and the second with an ACK frame. This is caused by qc_prep_pkts() loop which may call qc_do_build_pkt() multiple times with the same QEL instance. This case is normally use when a full datagram has been built but there is still content to emit on the current encryption level. To fix this, alter qc_prep_pkts() loop : if both probing and ACK is requested, force the datagram to be written after packet encoding. This will result in a datagram containing the packet with probing data as final entry. A new datagram is started for the next packet which will can contain the ACK frame. This also has some impact on INITIAL padding. Indeed, if packet must be the last due to probing emission, qc_prep_pkts() will also activate padding to ensure final datagram is at least 1.200 bytes long. Note that coalescing two packets of the same type is not invalid according to QUIC RFC. However it could cause issue with some shaky implementations, so it is considered as a bug. This must be backported up to 2.6. --- src/quic_tx.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/quic_tx.c b/src/quic_tx.c index 99a51d92c..695841732 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -648,6 +648,7 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf, enum quic_pkt_type pkt_type; struct quic_tx_packet *cur_pkt; enum qc_build_pkt_err err; + int final_packet = 0; TRACE_PROTO("TX prep pkts", QUIC_EV_CONN_PHPKTS, qc, qel); @@ -706,10 +707,19 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf, padding = 1; } + + /* TODO currently it's not possible to emit an ACK and probing data simultaneously (see qc_do_build_pkt()). + * As a side-effect, this could cause coalescing of two packets of the same type which should be avoided. + * To implement this, a new datagram is forced by invokation of qc_txb_store(). This must then be checked + * if padding is required as in this case this will be the last packet of the current datagram. + */ + if (probe && (must_ack || (qel->pktns->flags & QUIC_FL_PKTNS_ACK_REQUIRED))) + final_packet = 1; + pkt_type = quic_enc_level_pkt_type(qc, qel); cur_pkt = qc_build_pkt(&pos, end, qel, tls_ctx, frms, qc, ver, dglen, pkt_type, must_ack, - padding && !next_qel, + padding && (!next_qel || final_packet), probe, cc, &err); if (!cur_pkt) { switch (err) { @@ -787,7 +797,7 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf, if (probe && qel == qc->ael) break; - if (LIST_ISEMPTY(frms)) { + if (LIST_ISEMPTY(frms) && !final_packet) { /* Everything sent. Continue within the same datagram. */ prv_pkt = cur_pkt; } -- 2.47.2