]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MAJOR: quic: fix wrong packet building due to already acked frames
authorFrederic Lecaille <flecaille@haproxy.com>
Mon, 25 Nov 2024 10:14:20 +0000 (11:14 +0100)
committerFrederic Lecaille <flecaille@haproxy.com>
Mon, 25 Nov 2024 17:55:45 +0000 (18:55 +0100)
If a packet build was asked to probe the peer with frames which have just
been acked, the frames build run by qc_build_frms() could be cancelled  by
qc_stream_frm_is_acked() whose aim is to check that current frames to
be built have not been already acknowledged. In this case the packet build run
by qc_do_build_pkt() is not interrupted, leading to the build of an empty packet
which should be ack-eliciting.

This is a bug detected by the BUG_ON() statement in qc_do_build_pk():

    BUG_ON(qel->pktns->tx.pto_probe &&
           !(pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING));

Thank you to @Tristan971 for having reported this issue in GH #2709

This is an old bug which must be backported as far as 2.6.

src/quic_tx.c

index 37f46fe731add6efdf651a39b1287ed041706720..bc85f0d8b3c0b65ceb2a4dd9a748f88794601254 100644 (file)
@@ -2012,7 +2012,23 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
                                goto comp_pkt_len;
                        }
 
-                       if (!ack_frm_len && !qel->pktns->tx.pto_probe)
+                       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
+                                * its frames which were already acknowledeged.
+                                * See qc_stream_frm_is_acked()) called by qc_build_frms().
+                                * Note that qc_stream_frm_is_acked() logs a trace in this
+                                * case mentionning some frames were already acknowledged.
+                                *
+                                * That said, the consequence must be the same: cancelling
+                                * the packet build as if there was not enough room in the
+                                * TX buffer.
+                                */
+                                qel->pktns->tx.pto_probe--;
+                                goto no_room;
+                       }
+
+                       if (!ack_frm_len)
                                goto no_room;
                }
        }