From a6d40e09f7c9ebb99d5f08630a6482fbb7d04d26 Mon Sep 17 00:00:00 2001 From: Frederic Lecaille Date: Wed, 24 Jul 2024 16:16:26 +0200 Subject: [PATCH] BUG/MINOR: quic: Lack of precision when computing K (cubic only cc) K cubic variable is stored in ms. But it was a formula with the second as unit for the window difference parameter which was used to compute K without considering the loss of information. Then the result was converted in ms (K *= 1000). This leaded to a lack of precision and multiples of 1000 as values. To fix this, use the same formula but with the window difference in ms as parameter passed to the cubic function and remove the conversion. Must be backported as far as 2.6. --- src/quic_cc_cubic.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/quic_cc_cubic.c b/src/quic_cc_cubic.c index 51d11b11c..fc21aa0f5 100644 --- a/src/quic_cc_cubic.c +++ b/src/quic_cc_cubic.c @@ -243,13 +243,24 @@ static inline void quic_cubic_update(struct quic_cc *cc, uint32_t acked) c->W_target = path->cwnd; } else { + uint64_t wnd_diff; + /* K value computing (in seconds): * K = cubic_root((W_max - cwnd_epoch)/C) (Figure 2) - * Note that K is stored in milliseconds. + * Note that K is stored in milliseconds and that + * 8000 * 125000 = 1000^3. + * + * Supporting 2^40 windows, shifted by 10, leaves ~13 bits of unused + * precision. We exploit this precision for our NS conversion by + * multiplying by 8000 without overflowing, then later by 125000 + * after the divide so that we limit the precision loss to the minimum + * before the cubic_root() call." */ - c->K = cubic_root(((c->last_w_max - path->cwnd) << CUBIC_SCALE_FACTOR_SHIFT) / (CUBIC_C_SCALED * path->mtu)); - /* Convert to milliseconds. */ - c->K *= 1000; + wnd_diff = (c->last_w_max - path->cwnd) << CUBIC_SCALE_FACTOR_SHIFT; + wnd_diff *= 8000ULL; + wnd_diff /= CUBIC_C_SCALED * path->mtu; + wnd_diff *= 125000ULL; + c->K = cubic_root(wnd_diff); c->W_target = c->last_w_max; }