BUG/MINOR: quic: Wrong RTT computation (srtt and rrt_var)

Due to the fact that several variable values (rtt_var, srtt) were stored as multiple
of their real values, some calculations were less accurate as expected.

Stop storing 4*rtt_var values, and 8*srtt values.
Adjust all the impacted statements.

Must be backported as far as 2.6.

diff --git a/include/haproxy/quic_loss-t.h b/include/haproxy/quic_loss-t.h
index 8ab41b2562516be011f1eb6e43841f16ff05f2ca..d1fea9f7e113e09cfacd03a9b30923259a1343e6 100644 (file)
--- a/include/haproxy/quic_loss-t.h
+++ b/include/haproxy/quic_loss-t.h
@@ -44,13 +44,13 @@
  */
 
 struct quic_loss {
-       /* The most recent RTT measurement. */
+       /* The most recent RTT measurement (ms) */
        unsigned int latest_rtt;
-       /* Smoothed RTT << 3 */
+       /* Smoothed RTT (ms) */
        unsigned int srtt;
-       /* RTT variation << 2 */
+       /* RTT variation (ms) */
        unsigned int rtt_var;
-       /* Minimum RTT. */
+       /* Minimum RTT (ms) */
        unsigned int rtt_min;
        /* Number of NACKed sent PTO. */
        unsigned int pto_count;

diff --git a/include/haproxy/quic_loss.h b/include/haproxy/quic_loss.h
index ae2d1489c90f65d2766c52dbe0d7dd8fac1a835d..9f0b085474b9768c4879b486f9d0b15d91cef409 100644 (file)
--- a/include/haproxy/quic_loss.h
+++ b/include/haproxy/quic_loss.h
@@ -35,8 +35,8 @@
 static inline void quic_loss_init(struct quic_loss *ql)
 {
        ql->latest_rtt = 0;
-       ql->srtt = QUIC_LOSS_INITIAL_RTT << 3;
-       ql->rtt_var = (QUIC_LOSS_INITIAL_RTT >> 1) << 2;
+       ql->srtt = QUIC_LOSS_INITIAL_RTT;
+       ql->rtt_var = QUIC_LOSS_INITIAL_RTT / 2;
        ql->rtt_min = 0;
        ql->pto_count = 0;
        ql->nb_lost_pkt = 0;
@@ -57,8 +57,8 @@ static inline int quic_loss_persistent_congestion(struct quic_loss *ql,
        if (!period)
                return 0;
 
-       congestion_period = (ql->srtt >> 3) +
-               QUIC_MAX(ql->rtt_var, QUIC_TIMER_GRANULARITY) + max_ack_delay;
+       congestion_period = ql->srtt +
+               QUIC_MAX(4 * ql->rtt_var, QUIC_TIMER_GRANULARITY) + max_ack_delay;
        congestion_period *= QUIC_LOSS_PACKET_THRESHOLD;
 
        return period >= congestion_period;
@@ -69,7 +69,7 @@ static inline unsigned int quic_pto(struct quic_conn *qc)
 {
        struct quic_loss *ql = &qc->path->loss;
 
-       return (ql->srtt >> 3) + QUIC_MAX(ql->rtt_var, QUIC_TIMER_GRANULARITY) +
+       return ql->srtt + QUIC_MAX(4 * ql->rtt_var, QUIC_TIMER_GRANULARITY) +
                (HA_ATOMIC_LOAD(&qc->state) >= QUIC_HS_ST_COMPLETE ? qc->max_ack_delay : 0);
 }
 

diff --git a/src/quic_cli.c b/src/quic_cli.c
index 2b9ba64d608789ca170a9ee0a093e03d91b58e18..71a6175d5504941ce8738ad4e9f8091bf9f0fdd1 100644 (file)
--- a/src/quic_cli.c
+++ b/src/quic_cli.c
@@ -203,7 +203,7 @@ static void dump_quic_full(struct show_quic_ctx *ctx, struct quic_conn *qc)
 
        chunk_appendf(&trash, "  srtt=%-4u rttvar=%-4u rttmin=%-4u ptoc=%-4u cwnd=%-6llu"
                              " mcwnd=%-6llu sentpkts=%-6llu lostpkts=%-6llu\n",
-                     qc->path->loss.srtt >> 3, qc->path->loss.rtt_var >> 2,
+                     qc->path->loss.srtt, qc->path->loss.rtt_var,
                      qc->path->loss.rtt_min, qc->path->loss.pto_count, (ullong)qc->path->cwnd,
                      (ullong)qc->path->mcwnd, (ullong)qc->cntrs.sent_pkt, (ullong)qc->path->loss.nb_lost_pkt);
 

diff --git a/src/quic_loss.c b/src/quic_loss.c
index fbfec77394ec3796625cc063ff7688fa7a7ab8ab..c386fa3da780f48a649bfe3fe35936a967dae227 100644 (file)
--- a/src/quic_loss.c
+++ b/src/quic_loss.c
@@ -25,9 +25,8 @@ void quic_loss_srtt_update(struct quic_loss *ql,
        ql->latest_rtt = rtt;
        if (!ql->rtt_min) {
                /* No previous measurement. */
-               ql->srtt = rtt << 3;
-               /* rttval <- rtt / 2 or 4*rttval <- 2*rtt. */
-               ql->rtt_var = rtt << 1;
+               ql->srtt = rtt;
+               ql->rtt_var = rtt / 2;
                ql->rtt_min = rtt;
        }
        else {
@@ -37,13 +36,11 @@ void quic_loss_srtt_update(struct quic_loss *ql,
                /* Specific to QUIC (RTT adjustment). */
                if (ack_delay && rtt >= ql->rtt_min + ack_delay)
                        rtt -= ack_delay;
-               diff = (ql->srtt >> 3) - rtt;
+               diff = ql->srtt - rtt;
                if (diff < 0)
                        diff = -diff;
-               /* 4*rttvar = 3*rttvar + |diff| */
-               ql->rtt_var += diff - (ql->rtt_var >> 2);
-               /* 8*srtt = 7*srtt + rtt */
-               ql->srtt += rtt - (ql->srtt >> 3);
+               ql->rtt_var = (3 * ql->rtt_var + diff) / 4;
+               ql->srtt = (7 * ql->srtt + rtt) / 8;
        }
 
        TRACE_PROTO("TX loss srtt update", QUIC_EV_CONN_RTTUPDT, qc,,, ql);
@@ -93,8 +90,8 @@ struct quic_pktns *quic_pto_pktns(struct quic_conn *qc,
 
        BUG_ON(LIST_ISEMPTY(&qc->pktns_list));
        duration =
-               (ql->srtt >> 3) +
-               (QUIC_MAX(ql->rtt_var, QUIC_TIMER_GRANULARITY) << ql->pto_count);
+               ql->srtt +
+               (QUIC_MAX(4 * ql->rtt_var, QUIC_TIMER_GRANULARITY) << ql->pto_count);
 
        /* RFC 9002 6.2.2.1. Before Address Validation
         *
@@ -170,7 +167,7 @@ void qc_packet_loss_lookup(struct quic_pktns *pktns, struct quic_conn *qc,
                goto out;
 
        ql = &qc->path->loss;
-       loss_delay = QUIC_MAX(ql->latest_rtt, ql->srtt >> 3);
+       loss_delay = QUIC_MAX(ql->latest_rtt, ql->srtt);
        loss_delay = QUIC_MAX(loss_delay, MS_TO_TICKS(QUIC_TIMER_GRANULARITY)) *
                QUIC_LOSS_TIME_THRESHOLD_MULTIPLICAND / QUIC_LOSS_TIME_THRESHOLD_DIVISOR;
 

diff --git a/src/quic_trace.c b/src/quic_trace.c
index 372063ba1391ead0d9b836fbdd8fbe5e458f3598..84f747fed2072925dafb297411dc28dac1c17e4d 100644 (file)
--- a/src/quic_trace.c
+++ b/src/quic_trace.c
@@ -419,7 +419,7 @@ static void quic_trace(enum trace_level level, uint64_t mask, const struct trace
                        if (ql)
                                chunk_appendf(&trace_buf,
                                              " srtt=%ums rttvar=%ums min_rtt=%ums",
-                                             ql->srtt >> 3, ql->rtt_var >> 2, ql->rtt_min);
+                                             ql->srtt, ql->rtt_var, ql->rtt_min);
                }
                if (mask & QUIC_EV_CONN_CC) {
                        const struct quic_cc_event *ev = a2;