From: Frederic Lecaille <flecaille@haproxy.com>
Date: Mon, 5 Feb 2024 13:07:51 +0000 (+0100)
Subject: BUG/MINOR: quic: Wrong ack ranges handling when reaching the limit.
X-Git-Tag: v3.0-dev3~88
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0ce61d2f6ddc0232226d135fdfd28b7ad3dfcc0f;p=thirdparty%2Fhaproxy.git

BUG/MINOR: quic: Wrong ack ranges handling when reaching the limit.

Acknowledgements ranges are used to build ACK frames. To avoid allocating too
much such objects, a limit was set to 32(QUIC_MAX_ACK_RANGES) by this commit:

	MINOR: quic: Do not allocate too much ack ranges

But there is an inversion when removing the oldest range from its tree.
eb64_first() must be used in place of eb64_last(). Note that this patch
only does this modification in addition to rename <last> variable to <first>.

This bug leads such a h2load command to block when a request ends up not
being acknowledged by haproxy even if correctly served:

	/opt/nghttp2/build/bin/h2load --alpn-list h3 -t 1 -c 1 -m 1 -n 100 \
		https://127.0.0.1/?s=5m

There is a remaining question to be answered. In such a case, haproxy refuses to
reopen the stream, this is a good thing but should not haproxy ackownledge the
request (because correctly parsed again).

Note that to be easily reproduced, this setting had to be applied to the client
network interface:

    tc qdisc add dev eth1 root netem delay 100ms 1s loss random

Must be backported as far as 2.6.
---

diff --git a/src/quic_ack.c b/src/quic_ack.c
index 224a4334c3..9551c4d4a1 100644
--- a/src/quic_ack.c
+++ b/src/quic_ack.c
@@ -81,12 +81,12 @@ struct quic_arng_node *quic_insert_new_range(struct quic_conn *qc,
 	TRACE_ENTER(QUIC_EV_CONN_RXPKT, qc);
 
 	if (arngs->sz >= QUIC_MAX_ACK_RANGES) {
-		struct eb64_node *last;
+		struct eb64_node *first;
 
-		last = eb64_last(&arngs->root);
-		BUG_ON(last == NULL);
-		eb64_delete(last);
-		pool_free(pool_head_quic_arng, last);
+		first = eb64_first(&arngs->root);
+		BUG_ON(fist == NULL);
+		eb64_delete(first);
+		pool_free(pool_head_quic_arng, first);
 		arngs->sz--;
 	}