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 <cc> 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 <probe> argument
but rather <pto_probe> 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 <probe> argument to ensure
this.

This should be backported up to 2.6.

(cherry picked from commit 0376e66112f17d79cfd1824ae75d7288eb383059)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 6d0cdabc754d94c1e5036ca4ce4b01085afcdfa1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/quic_tx.c b/src/quic_tx.c
index edeb395..28c7fc1 100644 (file)
--- 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 <probe> argument instead of using QEL <pto_probe> 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 <probe> argument instead of using QEL <pto_probe> 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 <probe> argument instead of using QEL <pto_probe> 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.