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.

(cherry picked from commit eeeb81bb499677ccaf83858dbdde21c9f3db4341)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 8ab981b..2f828bb 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3538,6 +3538,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 bb39baf..9b3cd78 100644 (file)
--- a/include/haproxy/global-t.h
+++ b/include/haproxy/global-t.h
@@ -194,6 +194,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 a1125fa..8aec6f0 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 e3917c8..3b38efa 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 f56efa0..4c739f4 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 7d47b6a..fa0e4ba 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++;