]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: quic: Lack of precision when computing K (cubic only cc)
authorFrederic Lecaille <flecaille@haproxy.com>
Wed, 24 Jul 2024 14:16:26 +0000 (16:16 +0200)
committerFrederic Lecaille <flecaille@haproxy.com>
Wed, 24 Jul 2024 16:24:39 +0000 (18:24 +0200)
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

index 51d11b11c2cac244257422a68594b6ce29c57109..fc21aa0f530fb55662b500c96d5405636bb770b0 100644 (file)
@@ -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;
                }