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.
This commit is contained in:
Frdric Lcaille 2023-09-05 15:24:11 +02:00 committed by Willy Tarreau
parent cf768f7456
commit fb4294be55
5 changed files with 19 additions and 22 deletions

View File

@ -44,13 +44,13 @@
*/ */
struct quic_loss { struct quic_loss {
/* The most recent RTT measurement. */ /* The most recent RTT measurement (ms) */
unsigned int latest_rtt; unsigned int latest_rtt;
/* Smoothed RTT << 3 */ /* Smoothed RTT (ms) */
unsigned int srtt; unsigned int srtt;
/* RTT variation << 2 */ /* RTT variation (ms) */
unsigned int rtt_var; unsigned int rtt_var;
/* Minimum RTT. */ /* Minimum RTT (ms) */
unsigned int rtt_min; unsigned int rtt_min;
/* Number of NACKed sent PTO. */ /* Number of NACKed sent PTO. */
unsigned int pto_count; unsigned int pto_count;

View File

@ -35,8 +35,8 @@
static inline void quic_loss_init(struct quic_loss *ql) static inline void quic_loss_init(struct quic_loss *ql)
{ {
ql->latest_rtt = 0; ql->latest_rtt = 0;
ql->srtt = QUIC_LOSS_INITIAL_RTT << 3; ql->srtt = QUIC_LOSS_INITIAL_RTT;
ql->rtt_var = (QUIC_LOSS_INITIAL_RTT >> 1) << 2; ql->rtt_var = QUIC_LOSS_INITIAL_RTT / 2;
ql->rtt_min = 0; ql->rtt_min = 0;
ql->pto_count = 0; ql->pto_count = 0;
ql->nb_lost_pkt = 0; ql->nb_lost_pkt = 0;
@ -57,8 +57,8 @@ static inline int quic_loss_persistent_congestion(struct quic_loss *ql,
if (!period) if (!period)
return 0; return 0;
congestion_period = (ql->srtt >> 3) + congestion_period = ql->srtt +
QUIC_MAX(ql->rtt_var, QUIC_TIMER_GRANULARITY) + max_ack_delay; QUIC_MAX(4 * ql->rtt_var, QUIC_TIMER_GRANULARITY) + max_ack_delay;
congestion_period *= QUIC_LOSS_PACKET_THRESHOLD; congestion_period *= QUIC_LOSS_PACKET_THRESHOLD;
return period >= congestion_period; 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; 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); (HA_ATOMIC_LOAD(&qc->state) >= QUIC_HS_ST_COMPLETE ? qc->max_ack_delay : 0);
} }

View File

@ -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" chunk_appendf(&trash, " srtt=%-4u rttvar=%-4u rttmin=%-4u ptoc=%-4u cwnd=%-6llu"
" mcwnd=%-6llu sentpkts=%-6llu lostpkts=%-6llu\n", " 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, 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); (ullong)qc->path->mcwnd, (ullong)qc->cntrs.sent_pkt, (ullong)qc->path->loss.nb_lost_pkt);

View File

@ -25,9 +25,8 @@ void quic_loss_srtt_update(struct quic_loss *ql,
ql->latest_rtt = rtt; ql->latest_rtt = rtt;
if (!ql->rtt_min) { if (!ql->rtt_min) {
/* No previous measurement. */ /* No previous measurement. */
ql->srtt = rtt << 3; ql->srtt = rtt;
/* rttval <- rtt / 2 or 4*rttval <- 2*rtt. */ ql->rtt_var = rtt / 2;
ql->rtt_var = rtt << 1;
ql->rtt_min = rtt; ql->rtt_min = rtt;
} }
else { else {
@ -37,13 +36,11 @@ void quic_loss_srtt_update(struct quic_loss *ql,
/* Specific to QUIC (RTT adjustment). */ /* Specific to QUIC (RTT adjustment). */
if (ack_delay && rtt >= ql->rtt_min + ack_delay) if (ack_delay && rtt >= ql->rtt_min + ack_delay)
rtt -= ack_delay; rtt -= ack_delay;
diff = (ql->srtt >> 3) - rtt; diff = ql->srtt - rtt;
if (diff < 0) if (diff < 0)
diff = -diff; diff = -diff;
/* 4*rttvar = 3*rttvar + |diff| */ ql->rtt_var = (3 * ql->rtt_var + diff) / 4;
ql->rtt_var += diff - (ql->rtt_var >> 2); ql->srtt = (7 * ql->srtt + rtt) / 8;
/* 8*srtt = 7*srtt + rtt */
ql->srtt += rtt - (ql->srtt >> 3);
} }
TRACE_PROTO("TX loss srtt update", QUIC_EV_CONN_RTTUPDT, qc,,, ql); 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)); BUG_ON(LIST_ISEMPTY(&qc->pktns_list));
duration = duration =
(ql->srtt >> 3) + ql->srtt +
(QUIC_MAX(ql->rtt_var, QUIC_TIMER_GRANULARITY) << ql->pto_count); (QUIC_MAX(4 * ql->rtt_var, QUIC_TIMER_GRANULARITY) << ql->pto_count);
/* RFC 9002 6.2.2.1. Before Address Validation /* 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; goto out;
ql = &qc->path->loss; 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)) * loss_delay = QUIC_MAX(loss_delay, MS_TO_TICKS(QUIC_TIMER_GRANULARITY)) *
QUIC_LOSS_TIME_THRESHOLD_MULTIPLICAND / QUIC_LOSS_TIME_THRESHOLD_DIVISOR; QUIC_LOSS_TIME_THRESHOLD_MULTIPLICAND / QUIC_LOSS_TIME_THRESHOLD_DIVISOR;

View File

@ -419,7 +419,7 @@ static void quic_trace(enum trace_level level, uint64_t mask, const struct trace
if (ql) if (ql)
chunk_appendf(&trace_buf, chunk_appendf(&trace_buf,
" srtt=%ums rttvar=%ums min_rtt=%ums", " 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) { if (mask & QUIC_EV_CONN_CC) {
const struct quic_cc_event *ev = a2; const struct quic_cc_event *ev = a2;