From: Frederic Lecaille <flecaille@haproxy.com>
Date: Fri, 19 Jul 2024 14:06:55 +0000 (+0200)
Subject: BUG/MINOR: quic: Non optimal first datagram.
X-Git-Tag: v3.1-dev4~20
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=402ce29e9e8e2d8b32c65e21a95e33f2f4d6373c;p=thirdparty%2Fhaproxy.git

BUG/MINOR: quic: Non optimal first datagram.

This bug arrived with this commit:

     b068e758f MINOR: quic: simplify rescheduling for handshake

This commit introduced a bad side effect. Haproxy always replied by an ACK-only
datagram when it received the first client Initial packet. Then it handled
the CRYPTO data insided. And finally, it sent its own CRYPTO data. This broke
the packet coalescing rule whose aim is to optimally build and send as more
as QUIC packets by datagram.

To fix this, simply partially reverts this commit, to make the low level I/O
task return again if some CRYPTO were received. This will delay the
acknowledgement which will be sent with the CRYPTO data from the same
datagram again.

Must be backported to 3.0.
---

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 73b6430d2e..9c6a090461 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -755,6 +755,7 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state)
 	st = qc->state;
 	TRACE_PROTO("connection state", QUIC_EV_CONN_IO_CB, qc, &st);
 
+	/* TASK_HEAVY is set when received CRYPTO data have to be handled. */
 	if (HA_ATOMIC_LOAD(&tl->state) & TASK_HEAVY) {
 		qc_ssl_provide_all_quic_data(qc, qc->xprt_ctx);
 		HA_ATOMIC_AND(&tl->state, ~TASK_HEAVY);
@@ -774,6 +775,16 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state)
 	if (!qc_treat_rx_pkts(qc))
 		goto out;
 
+	/* TASK_HEAVY is set when received CRYPTO data have to be handled. These data
+	 * must be acknowledged asap and replied to with others CRYPTO data. In this
+	 * case, it is more optimal to delay the ACK to send it with the CRYPTO data
+	 * from the same datagram.
+	 */
+	if (HA_ATOMIC_LOAD(&tl->state) & TASK_HEAVY) {
+		tasklet_wakeup(tl);
+		goto out;
+	}
+
 	if (qc->flags & QUIC_FL_CONN_TO_KILL) {
 		TRACE_DEVEL("connection to be killed", QUIC_EV_CONN_PHPKTS, qc);
 		goto out;