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>
(cherry picked from commit 0937c7e85bc541a15ec0d51e2397a9799bdbcf33)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/quic_tx.c b/src/quic_tx.c
index 0c9106d..d6aed56 100644 (file)
--- 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 <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));
@@ -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 <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
@@ -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 <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;
@@ -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;
                }
        }