MINOR: quic: Dynamic packet reordering threshold

Let's say that the largest packet number acknowledged by the peer is #10, when inspecting
the non already acknowledged packets to detect if they are lost or not, this is the
case a least if the difference between this largest packet number and and their
packet numbers are bigger or equal to the packet reordering threshold as defined
by the RFC 9002. This latter must not be less than QUIC_LOSS_PACKET_THRESHOLD(3).
Which such a value, packets #7 and oldest are detected as lost if non acknowledged,
contrary to packet number #8 or #9.

So, the packet loss detection is very sensitive to such a network characteristic
where non acknowledged packets are distant from each others by their packet number
differences.

Do not use this static value anymore for the packet reordering threshold which is used
as a criteria to detect packet loss. In place, make it depend on the difference
between the number of the last transmitted packet and the number of the oldest
one among the packet which are still in flight before being inspected to be
deemed as lost.

Add new tune.quic.reorder-ratio setting to apply a ratio in percent to this
dynamic packet reorder threshold.

Should be backported to 2.6.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 937d2a4b79cd5fb6d5bfdb571d26aa53275cd840..47f5153104555fd2839d20381f5b872ee90956c7 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3616,6 +3616,12 @@ tune.quic.max-frame-loss <number>
 
   The default value is 10.
 
+tune.quic.reorder-ratio <0..100, in percent>
+  The ratio applied to the packet reordering threshold calculated. It may
+  trigger a high packet loss detection when too small.
+
+  The default value is 50.
+
 tune.quic.retry-threshold <number>
   Dynamically enables the Retry feature for all the configured QUIC listeners
   as soon as this number of half open connections is reached. A half open

diff --git a/include/haproxy/global-t.h b/include/haproxy/global-t.h
index 0f2a3feb486905fe682e4c798987728e33fdc230..25536df14539aaa0e35161a018bed2b88f953b61 100644 (file)
--- a/include/haproxy/global-t.h
+++ b/include/haproxy/global-t.h
@@ -195,6 +195,7 @@ struct global {
                unsigned int quic_frontend_max_idle_timeout;
                unsigned int quic_frontend_max_streams_bidi;
                unsigned int quic_retry_threshold;
+               unsigned int quic_reorder_ratio;
                unsigned int quic_streams_buf;
                unsigned int quic_max_frame_loss;
 #endif /* USE_QUIC */

diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h
index a1125fa2903154befd15adef0ad83b8f5b5caf7e..8aec6f09c2ba2671223285a43c563cb10f40224d 100644 (file)
--- a/include/haproxy/quic_conn-t.h
+++ b/include/haproxy/quic_conn-t.h
@@ -92,6 +92,8 @@ typedef unsigned long long ull;
 #define QUIC_RETRY_DURATION_SEC       10
 /* Default Retry threshold */
 #define QUIC_DFLT_RETRY_THRESHOLD     100 /* in connection openings */
+/* Default ratio value applied to a dynamic Packet reorder threshold. */
+#define QUIC_DFLT_REORDER_RATIO        50 /* in percent */
 /* Default limit of loss detection on a single frame. If exceeded, connection is closed. */
 #define QUIC_DFLT_MAX_FRAME_LOSS       10
 

diff --git a/src/cfgparse-quic.c b/src/cfgparse-quic.c
index e3917c8a3dd4d426d875bae5b13aebe722755d8e..3b38efa72081012cd3c261502647791de088ee91 100644 (file)
--- a/src/cfgparse-quic.c
+++ b/src/cfgparse-quic.c
@@ -239,6 +239,14 @@ static int cfg_parse_quic_tune_setting(char **args, int section_type,
                global.tune.quic_frontend_max_streams_bidi = arg;
        else if (strcmp(suffix, "max-frame-loss") == 0)
                global.tune.quic_max_frame_loss = arg;
+       else if (strcmp(suffix, "reorder-ratio") == 0) {
+               if (arg > 100) {
+                       memprintf(err, "'%s' expects an integer argument between 0 and 100.", args[0]);
+                       return -1;
+               }
+
+               global.tune.quic_reorder_ratio = arg;
+       }
        else if (strcmp(suffix, "retry-threshold") == 0)
                global.tune.quic_retry_threshold = arg;
        else {
@@ -275,6 +283,7 @@ static struct cfg_kw_list cfg_kws = {ILH, {
        { CFG_GLOBAL, "tune.quic.frontend.max-streams-bidi", cfg_parse_quic_tune_setting },
        { CFG_GLOBAL, "tune.quic.frontend.max-idle-timeout", cfg_parse_quic_time },
        { CFG_GLOBAL, "tune.quic.max-frame-loss", cfg_parse_quic_tune_setting },
+       { CFG_GLOBAL, "tune.quic.reorder-ratio", cfg_parse_quic_tune_setting },
        { CFG_GLOBAL, "tune.quic.retry-threshold", cfg_parse_quic_tune_setting },
        { CFG_GLOBAL, "tune.quic.zero-copy-fwd-send", cfg_parse_quic_zero_copy_fwd_snd },
        { 0, NULL, NULL }

diff --git a/src/haproxy.c b/src/haproxy.c
index d11ef021e613c4a362ebdd851bdfe4e55679249d..a9b0190a5143a5fb766e1a8246c3fe41a37bfdeb 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -198,6 +198,7 @@ struct global global = {
                .quic_backend_max_idle_timeout = QUIC_TP_DFLT_BACK_MAX_IDLE_TIMEOUT,
                .quic_frontend_max_idle_timeout = QUIC_TP_DFLT_FRONT_MAX_IDLE_TIMEOUT,
                .quic_frontend_max_streams_bidi = QUIC_TP_DFLT_FRONT_MAX_STREAMS_BIDI,
+               .quic_reorder_ratio = QUIC_DFLT_REORDER_RATIO,
                .quic_retry_threshold = QUIC_DFLT_RETRY_THRESHOLD,
                .quic_max_frame_loss = QUIC_DFLT_MAX_FRAME_LOSS,
                .quic_streams_buf = 30,

diff --git a/src/quic_loss.c b/src/quic_loss.c
index 7d47b6af0411ab4673eb452989ed9703d93fe1f9..fa0e4ba575f31cab34109a4654fa038b9000381f 100644 (file)
--- a/src/quic_loss.c
+++ b/src/quic_loss.c
@@ -157,6 +157,7 @@ void qc_packet_loss_lookup(struct quic_pktns *pktns, struct quic_conn *qc,
        struct eb64_node *node;
        struct quic_loss *ql;
        unsigned int loss_delay;
+       uint64_t pktthresh;
 
        TRACE_ENTER(QUIC_EV_CONN_PKTLOSS, qc);
        TRACE_PROTO("TX loss", QUIC_EV_CONN_PKTLOSS, qc, pktns);
@@ -171,6 +172,27 @@ void qc_packet_loss_lookup(struct quic_pktns *pktns, struct quic_conn *qc,
                QUIC_LOSS_TIME_THRESHOLD_MULTIPLICAND / QUIC_LOSS_TIME_THRESHOLD_DIVISOR;
 
        node = eb64_first(pkts);
+
+       /* RFC 9002 6.1.1. Packet Threshold
+        * The RECOMMENDED initial value for the packet reordering threshold
+        * (kPacketThreshold) is 3, based on best practices for TCP loss detection
+        * [RFC5681] [RFC6675]. In order to remain similar to TCP, implementations
+        * SHOULD NOT use a packet threshold less than 3; see [RFC5681].
+
+        * Some networks may exhibit higher degrees of packet reordering, causing a
+        * sender to detect spurious losses. Additionally, packet reordering could be
+        * more common with QUIC than TCP because network elements that could observe
+        * and reorder TCP packets cannot do that for QUIC and also because QUIC
+        * packet numbers are encrypted.
+        */
+
+       /* Dynamic packet reordering threshold calculation depending on the distance
+        * (in packets) between the last transmitted packet and the oldest still in
+        * flight before loss detection.
+        */
+       pktthresh = pktns->tx.next_pn - 1 - eb64_entry(node, struct quic_tx_packet, pn_node)->pn_node.key;
+       /* Apply a ratio to this threshold and add it to QUIC_LOSS_PACKET_THRESHOLD. */
+       pktthresh = pktthresh * global.tune.quic_reorder_ratio / 100 + QUIC_LOSS_PACKET_THRESHOLD;
        while (node) {
                struct quic_tx_packet *pkt;
                int64_t largest_acked_pn;
@@ -185,7 +207,7 @@ void qc_packet_loss_lookup(struct quic_pktns *pktns, struct quic_conn *qc,
                time_sent = pkt->time_sent;
                loss_time_limit = tick_add(time_sent, loss_delay);
                if (tick_is_le(loss_time_limit, now_ms) ||
-                       (int64_t)largest_acked_pn >= pkt->pn_node.key + QUIC_LOSS_PACKET_THRESHOLD) {
+                       (int64_t)largest_acked_pn >= pkt->pn_node.key + pktthresh) {
                        eb64_delete(&pkt->pn_node);
                        LIST_APPEND(lost_pkts, &pkt->list);
                        ql->nb_lost_pkt++;