From: Frederic Lecaille <flecaille@haproxy.com>
Date: Wed, 4 Nov 2026 16:30:26 +0000 (+0100)
Subject: CLEANUP: quic: fix and complete the comments in quic_retransmit.c
X-Git-Url: https://git.kaiwu.me/$%7BISSUES%7D/static/gitweb.js?a=commitdiff_plain;h=1ce318349e8986628671ca2ca6929d2d46d25eff;p=haproxy.git

CLEANUP: quic: fix and complete the comments in quic_retransmit.c

Three comments are inaccurate:

  - qc_prep_fast_retrans() is documented as taking a <qel> argument, but
    it has been replaced by <pktns> for a while;

  - the packet lookup loop of qc_prep_fast_retrans() is said to skip the
    coalesced packets, while QUIC_FL_TX_PACKET_COALESCED is not tested
    there at all. Only the Handshake lookup loop skips them;

  - qc_pkt_with_only_acked_frms() is documented as returning 1 for a
    packet made only of acknowledged frames, but it also returns 1 for a
    packet without any frame. qc_prep_fast_retrans() relies on this to
    skip the empty packets.

Three comment blocks are added where the intent is not obvious: the
<tmp> list used by qc_dup_pkt_frms() to prepend the duplicated frames
ahead of the pending ones, the <pkt> variable which is deliberately not
reset before the <start> label so that a second probe datagram is built
from the same frames when no other packet may be retransmitted, and the
Initial packet number space which may still be probed with a PING frame
when the anti-amplification limit allows to send a full-size datagram.

Only comments are touched, no need to backport.
---

diff --git a/src/quic_retransmit.c b/src/quic_retransmit.c
index 891532240..a96767dc5 100644
--- a/src/quic_retransmit.c
+++ b/src/quic_retransmit.c
@@ -106,6 +106,9 @@ static void qc_dup_pkt_frms(struct quic_conn *qc,
 			TRACE_DEVEL("duplicated from lost packet", QUIC_EV_CONN_PRSAFRM, qc);
 		}
 
+		/* Append to <tmp> so that the duplicated frames are prepended
+		 * to <out_frm_list> below, ahead of the frames already pending.
+		 */
 		LIST_APPEND(&tmp, &dup_frm->list);
 	}
 
@@ -114,8 +117,8 @@ static void qc_dup_pkt_frms(struct quic_conn *qc,
 	TRACE_LEAVE(QUIC_EV_CONN_PRSAFRM, qc);
 }
 
-/* Boolean function which returns 1 if <pkt> TX packet is only made of
- * already acknowledged frames.
+/* Boolean function which returns 1 if <pkt> TX packet has no retransmittable
+ * frame, that is if it is empty or only made of already acknowledged frames.
  */
 static inline int qc_pkt_with_only_acked_frms(struct quic_tx_packet *pkt)
 {
@@ -128,7 +131,10 @@ static inline int qc_pkt_with_only_acked_frms(struct quic_tx_packet *pkt)
 	return 1;
 }
 
-/* Prepare a fast retransmission from <qel> encryption level */
+/* Prepare a fast retransmission for the packets of <pktns> packet number space.
+ * The frames to be retransmitted are duplicated into <frms1>, then into <frms2>
+ * if not NULL, so that they may be sent in two distinct datagrams.
+ */
 void qc_prep_fast_retrans(struct quic_conn *qc,
                           struct quic_pktns *pktns,
                           struct list *frms1, struct list *frms2)
@@ -142,6 +148,10 @@ void qc_prep_fast_retrans(struct quic_conn *qc,
 
 	BUG_ON(frms1 == frms2);
 
+	/* <pkt> is deliberately not reset at the <start> label: if no other
+	 * packet may be retransmitted, the frames of <pkt> are duplicated
+	 * again into <frms2> so that a second probe datagram is sent.
+	 */
 	pkt = NULL;
 	node = eb64_first(pkts);
  start:
@@ -150,7 +160,9 @@ void qc_prep_fast_retrans(struct quic_conn *qc,
 
 		p = eb64_entry(node, struct quic_tx_packet, pn_node);
 		node = eb64_next(node);
-		/* Skip the empty and coalesced packets */
+		/* Skip the empty packets and those made only of already
+		 * acknowledged frames. Coalesced packets are kept here.
+		 */
 		TRACE_PRINTF(TRACE_LEVEL_PROTO, QUIC_EV_CONN_SPPKTS, qc, 0, 0, 0,
 		             "--> pn=%llu (%d %d %d)", (ull)p->pn_node.key,
 		             LIST_ISEMPTY(&p->frms), !!(p->flags & QUIC_FL_TX_PACKET_COALESCED),
@@ -242,6 +254,10 @@ void qc_prep_hdshk_fast_retrans(struct quic_conn *qc,
 			TRACE_PROTO("anti-amplification limit would be reached", QUIC_EV_CONN_SPPKTS, qc, pkt);
 			if (pkt->next)
 				TRACE_PROTO("anti-amplification limit would be reached", QUIC_EV_CONN_SPPKTS, qc, pkt->next);
+			/* The Initial packet number space may still be probed
+			 * with a PING frame if a full-size datagram fits in what
+			 * the anti-amplification limit allows to send.
+			 */
 			if (qel == iqel && may_send >= QUIC_INITIAL_PACKET_MINLEN)
 				TRACE_PROTO("will probe Initial packet number space", QUIC_EV_CONN_SPPKTS, qc);
 			else