From: Frederic Lecaille Date: Tue, 13 Feb 2024 18:38:46 +0000 (+0100) Subject: MINOR: quic: Dynamic packet reordering threshold X-Git-Tag: v3.0-dev4~60 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=eeeb81bb499677ccaf83858dbdde21c9f3db4341;p=thirdparty%2Fhaproxy.git 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 937d2a4b79..47f5153104 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -3616,6 +3616,12 @@ tune.quic.max-frame-loss 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 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 0f2a3feb48..25536df145 100644 --- 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 a1125fa290..8aec6f09c2 100644 --- 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 e3917c8a3d..3b38efa720 100644 --- 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 d11ef021e6..a9b0190a51 100644 --- 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 7d47b6af04..fa0e4ba575 100644 --- 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++;