feature(transport/cubic): dynamic changing of alpha value

This implements changing CUBIC_ALPHA to `1.0` when `w_est >= cwnd_prior` as per <https://datatracker.ietf.org/doc/html/rfc9438#section-4.3-11>.

- added `self.alpha` field
- added `self.cwnd_prior` field
- existing functions now use `self.alpha` instead of constant value
- `self.alpha` changes and resets as per RFC
- added test helpers
- added test for alpha
- made tests pass

Author: omansfeld

neqo-transport/src/cc/cubic.rs

@@ -92,6 +92,17 @@ pub fn convert_to_f64(v: usize) -> f64 {
 
 #[derive(Debug, Default)]
 pub struct Cubic {
+    /// > CUBIC additive increase factor used in the Reno-friendly region \[to achieve
+    /// > approximately the same average congestion window size as Reno\].
+    ///
+    /// <https://datatracker.ietf.org/doc/html/rfc9438#name-constants-of-interest>
+    alpha: f64,
+    /// > Size of cwnd in \[bytes\] at the time of setting `ssthresh` most recently, either upon
+    /// > exiting the first slow start or just before `cwnd` was reduced in the last congestion
+    /// > event.
+    ///
+    /// <https://datatracker.ietf.org/doc/html/rfc9438#section-4.1.2-2.4>
+    cwnd_prior: f64,
     /// > An estimate for the congestion window \[...\] in the Reno-friendly region -- that
     /// > is, an estimate for the congestion window of Reno.
     ///
@@ -205,24 +216,50 @@ impl Cubic {
         max_datagram_size: f64,
         now: Instant,
     ) {
+        self.alpha = CUBIC_ALPHA;
         self.t_epoch = Some(now);
         self.reno_acked_bytes = new_acked_bytes;
         self.w_est = curr_cwnd;
         // If `w_max < cwnd_epoch` we take the cubic root from a negative value in `calc_k()`. That
         // could only happen if somehow `cwnd` get's increased between calling `reduce_cwnd()` and
         // `start_epoch()`. This could happen if we exit slow start without packet loss, thus never
         // had a congestion event and called `reduce_cwnd()` which means `w_max` was never set and
-        // is still it's default `0.0` value. For those cases we reset/initialize `w_max` here and
-        // appropiately set `k` to `0.0` (`k` is the time for `cwnd` to reach `w_max`).
+        // is still it's default `0.0` value. For those cases we reset/initialize `w_max` and
+        // `cwnd_prior` here and appropiately set `k` to `0.0` (`k` is the time for `cwnd`
+        // to reach `w_max`). We also set `alpha` to `1.0` as per the below RFC section,
+        // since `w_est >= cwnd_prior` is true here.
+        //
+        // <https://datatracker.ietf.org/doc/html/rfc9438#section-4.3-11>
         self.k = if self.w_max <= curr_cwnd {
+            self.cwnd_prior = curr_cwnd;
             self.w_max = curr_cwnd;
+            debug_assert!(
+            self.w_est >= self.cwnd_prior,
+            "w_est < cwnd_prior, so we are not allowed to set alpha = 1 (w_est: {}, cwnd_prior: {})", self.w_est, self.cwnd_prior
+        );
+            self.alpha = 1.0;
             0.0
         } else {
             self.calc_k(curr_cwnd, max_datagram_size)
         };
         qtrace!("[{self}] New epoch");
     }
 
+    #[cfg(test)]
+    pub const fn w_est(&self) -> f64 {
+        self.w_est
+    }
+
+    #[cfg(test)]
+    pub const fn cwnd_prior(&self) -> f64 {
+        self.cwnd_prior
+    }
+
+    #[cfg(test)]
+    pub const fn alpha(&self) -> f64 {
+        self.alpha
+    }
+
     #[cfg(test)]
     pub const fn w_max(&self) -> f64 {
         self.w_max
@@ -301,15 +338,24 @@ impl WindowAdjustment for Cubic {
         // <https://datatracker.ietf.org/doc/html/rfc9438#section-4.3-9>
 
         // We first calculate the increase in segments and floor it to only include whole segments.
-        let increase = (CUBIC_ALPHA * self.reno_acked_bytes / curr_cwnd).floor();
-
+        let increase = (self.alpha * self.reno_acked_bytes / curr_cwnd).floor();
         // Only apply the increase if it is at least by one segment.
         if increase > 0.0 {
             self.w_est += increase * max_datagram_size;
             // Because we floored the increase to whole segments we cannot just zero
             // `reno_acked_bytes` but have to calculate the actual bytes used.
-            let acked_bytes_used = increase * curr_cwnd / CUBIC_ALPHA;
+            let acked_bytes_used = increase * curr_cwnd / self.alpha;
             self.reno_acked_bytes -= acked_bytes_used;
+
+            // > Once w_est has grown to reach the cwnd at the time of most recently setting
+            // > ssthresh -- that is, w_est >= cwnd_prior -- the sender SHOULD set CUBIC_ALPHA to
+            // > 1 to ensure that it can achieve the same congestion window increment rate
+            // > as Reno, which uses AIMD(1, 0.5).
+            //
+            // <https://datatracker.ietf.org/doc/html/rfc9438#section-4.3-11>
+            if self.w_est >= self.cwnd_prior {
+                self.alpha = 1.0;
+            }
         }
 
         // > When receiving a new ACK in congestion avoidance (where cwnd could be greater than
@@ -381,6 +427,7 @@ impl WindowAdjustment for Cubic {
         max_datagram_size: usize,
     ) -> (usize, usize) {
         let curr_cwnd_f64 = convert_to_f64(curr_cwnd);
+        self.cwnd_prior = curr_cwnd_f64;
         // Fast Convergence
         //
         // > During a congestion event, if the current cwnd is less than w_max, this indicates

neqo-transport/src/cc/tests/cubic.rs

@@ -15,15 +15,16 @@ use std::{
     time::{Duration, Instant},
 };
 
+use neqo_common::qinfo;
 use test_fixture::now;
 
 use super::{IP_ADDR, MTU, RTT};
 use crate::{
     cc::{
         classic_cc::ClassicCongestionControl,
         cubic::{
-            convert_to_f64, Cubic, CUBIC_ALPHA, CUBIC_BETA_USIZE_DIVIDEND,
-            CUBIC_BETA_USIZE_DIVISOR, CUBIC_C, CUBIC_FAST_CONVERGENCE_FACTOR,
+            convert_to_f64, Cubic, CUBIC_BETA_USIZE_DIVIDEND, CUBIC_BETA_USIZE_DIVISOR, CUBIC_C,
+            CUBIC_FAST_CONVERGENCE_FACTOR,
         },
         CongestionControl as _,
     },
@@ -82,13 +83,34 @@ fn packet_lost(cc: &mut ClassicCongestionControl<Cubic>, pn: u64) {
     cc.on_packets_lost(None, None, PTO, &[p_lost], now());
 }
 
-fn expected_tcp_acks(cwnd_rtt_start: usize, mtu: usize) -> u64 {
+fn expected_tcp_acks(cwnd_rtt_start: usize, mtu: usize, alpha: f64) -> u64 {
     (f64::from(i32::try_from(cwnd_rtt_start).unwrap())
         / f64::from(i32::try_from(mtu).unwrap())
-        / CUBIC_ALPHA)
+        / alpha)
         .round() as u64
 }
 
+/// Calculates the expected increase to `w_est` given the number of acked bytes, the current
+/// congestion window and the state of `alpha`.
+///
+/// Returns the expected increase to `w_est` and the acked bytes used for it as `(w_est_increase,
+/// acked_bytes_used)`.
+fn expected_w_est_increase(
+    curr_cwnd: f64,
+    max_datagram_size: f64,
+    acked_bytes: f64,
+    alpha: f64,
+) -> (f64, f64) {
+    let increase = (alpha * (acked_bytes / curr_cwnd)).floor();
+    if increase > 0.0 {
+        let w_est_increase = increase * max_datagram_size;
+        let acked_bytes_used = increase * curr_cwnd / alpha;
+        (w_est_increase, acked_bytes_used)
+    } else {
+        (0.0, 0.0)
+    }
+}
+
 #[test]
 fn tcp_phase() {
     let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
@@ -97,7 +119,6 @@ fn tcp_phase() {
     cubic.set_ssthresh(1);
 
     let mut now = now();
-    let start_time = now;
     // helper variables to remember the next packet number to be sent/acked.
     let mut next_pn_send = 0;
     let mut next_pn_ack = 0;
@@ -110,23 +131,33 @@ fn tcp_phase() {
     // The phase will end when cwnd calculated with cubic equation is equal to TCP estimate:
     // CUBIC_C * (n * RTT / CUBIC_ALPHA)^3 * MAX_DATAGRAM_SIZE = n * MAX_DATAGRAM_SIZE
     // from this n = sqrt(CUBIC_ALPHA^3/ (CUBIC_C * RTT^3)).
-    let num_tcp_increases = (CUBIC_ALPHA.powi(3) / (CUBIC_C * RTT.as_secs_f64().powi(3)))
+
+    // Because `cubic::Cubic::alpha` is uninialized here (it's initialized in
+    // `cubic::Cubic::start_epoch` and we never had an ack yet) we set it to `1.0` which would be
+    // the value it'd be initialized to after the first ack under this test's conditions.
+    let alpha: f64 = 1.0;
+    let num_tcp_increases = (alpha.powi(3) / (CUBIC_C * RTT.as_secs_f64().powi(3)))
         .sqrt()
         .floor() as u64;
 
-    for _ in 0..num_tcp_increases {
+    for i in 0..num_tcp_increases {
         let cwnd_rtt_start = cubic.cwnd();
         // Expected acks during a period of RTT / CUBIC_ALPHA.
-        let acks = expected_tcp_acks(cwnd_rtt_start, cubic.max_datagram_size());
+        let acks = expected_tcp_acks(cwnd_rtt_start, cubic.max_datagram_size(), alpha);
         // The time between acks if they are ideally paced over a RTT.
         let time_increase =
             RTT / u32::try_from(cwnd_rtt_start / cubic.max_datagram_size()).unwrap();
 
-        for _ in 0..acks {
+        for j in 0..acks {
             now += time_increase;
             ack_packet(&mut cubic, next_pn_ack, now);
             next_pn_ack += 1;
             next_pn_send = fill_cwnd(&mut cubic, next_pn_send, now);
+            qinfo!(
+                "round {i}, ACK {j}, cwnd: {}, alpha: {}",
+                cubic.cwnd(),
+                cubic.cc_algorithm().alpha(),
+            );
         }
 
         assert_eq!(cubic.cwnd() - cwnd_rtt_start, cubic.max_datagram_size());
@@ -150,16 +181,19 @@ fn tcp_phase() {
 
     // Make sure that the increase is not according to TCP equation, i.e., that it took
     // less than RTT / CUBIC_ALPHA.
-    let expected_ack_tcp_increase = expected_tcp_acks(cwnd_rtt_start, cubic.max_datagram_size());
+    let expected_ack_tcp_increase = expected_tcp_acks(
+        cwnd_rtt_start,
+        cubic.max_datagram_size(),
+        cubic.cc_algorithm().alpha(),
+    );
     assert!(num_acks < expected_ack_tcp_increase);
 
     // This first increase after a TCP phase may be shorter than what it would take by a regular
     // cubic phase, because of the proper byte counting and the credit it already had before
-    // entering this phase. Therefore We will perform another round and compare it to expected
-    // increase using the cubic equation.
+    // entering this phase. Therefore We will perform another round and compare it to the expected
+    // number of acks needed for TCP.
 
     let cwnd_rtt_start_after_tcp = cubic.cwnd();
-    let elapsed_time = now - start_time;
 
     // calculate new time_increase.
     let time_increase =
@@ -174,26 +208,12 @@ fn tcp_phase() {
         next_pn_send = fill_cwnd(&mut cubic, next_pn_send, now);
     }
 
-    let expected_ack_tcp_increase2 =
-        expected_tcp_acks(cwnd_rtt_start_after_tcp, cubic.max_datagram_size());
+    let expected_ack_tcp_increase2 = expected_tcp_acks(
+        cwnd_rtt_start_after_tcp,
+        cubic.max_datagram_size(),
+        cubic.cc_algorithm().alpha(),
+    );
     assert!(num_acks2 < expected_ack_tcp_increase2);
-
-    // The time needed to increase cwnd by MAX_DATAGRAM_SIZE using the cubic equation will be
-    // calculated from: W_cubic(elapsed_time + t_to_increase) - W_cubic(elapsed_time) =
-    // MAX_DATAGRAM_SIZE => CUBIC_C * (elapsed_time + t_to_increase)^3 * MAX_DATAGRAM_SIZE +
-    // CWND_INITIAL - CUBIC_C * elapsed_time^3 * MAX_DATAGRAM_SIZE + CWND_INITIAL =
-    // MAX_DATAGRAM_SIZE => t_to_increase = cbrt((1 + CUBIC_C * elapsed_time^3) / CUBIC_C) -
-    // elapsed_time (t_to_increase is in seconds)
-    // number of ack needed is t_to_increase / time_increase.
-    let expected_ack_cubic_increase =
-        (((CUBIC_C.mul_add((elapsed_time).as_secs_f64().powi(3), 1.0) / CUBIC_C).cbrt()
-            - elapsed_time.as_secs_f64())
-            / time_increase.as_secs_f64())
-        .ceil() as u64;
-    // num_acks is very close to the calculated value. The exact value is hard to calculate
-    // because the proportional increase (i.e. curr_cwnd_f64 / (target - curr_cwnd_f64) *
-    // MAX_DATAGRAM_SIZE_F64) and the byte counting.
-    assert_eq!(num_acks2, expected_ack_cubic_increase + 2);
 }
 
 #[test]
@@ -366,3 +386,101 @@ fn congestion_event_congestion_avoidance_no_overflow() {
     // Now ack packet that was send earlier.
     ack_packet(&mut cubic, 0, now().checked_sub(PTO).unwrap());
 }
+
+/// This tests the dynamic changing of the `alpha` value outlined in RFC 9438 section 4.3.
+///
+/// <https://datatracker.ietf.org/doc/html/rfc9438#section-4.3-11>
+#[test]
+fn alpha_changes_for_high_w_est_values() {
+    const NORMAL_ALPHA: f64 = 3.0 * (1.0 - 0.7) / (1.0 + 0.7);
+    const INCREASED_ALPHA: f64 = 1.0;
+    let mut cc = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut next_pn_to_send = 0;
+    let mut last_sent_pn;
+    let mut first_sent_pn;
+    let mut w_est_projected = convert_to_f64(cc.cwnd_initial()); // initial value
+    let max_datagram_size = convert_to_f64(cc.max_datagram_size());
+    let mut w_est_increase;
+    let mut acked_bytes_used;
+    let mut acked_bytes = 0.0;
+
+    // Set ssthresh to something small to make sure that cc is in the congection avoidance phase.
+    cc.set_ssthresh(1);
+
+    // Send enough packets to have at least one congestion window increase
+    next_pn_to_send = fill_cwnd(&mut cc, next_pn_to_send, now());
+    last_sent_pn = next_pn_to_send - 1;
+    for pn in 0..=last_sent_pn {
+        // Calculate the projected increase of w_est
+        acked_bytes += max_datagram_size;
+        (w_est_increase, acked_bytes_used) = expected_w_est_increase(
+            convert_to_f64(cc.cwnd()),
+            max_datagram_size,
+            acked_bytes,
+            INCREASED_ALPHA,
+        );
+        acked_bytes -= acked_bytes_used;
+        w_est_projected += w_est_increase;
+
+        // Actually process the ACK
+        ack_packet(&mut cc, pn, now());
+
+        qinfo!(
+            "pn acked: {pn}, alpha: {}, w_est: {}, w_est_projected: {w_est_projected}",
+            cc.cc_algorithm().alpha(),
+            cc.cc_algorithm().w_est()
+        );
+        // Since we never had a congestion event we started with the initial values for `w_est =
+        // cwnd_prior = current_cwnd`, thus `w_est >= cwnd_prior` should be `true`, `alpha`
+        // should be set to it's increased value and `w_est` should be growing accordingly.
+        assert!(cc.cc_algorithm().w_est() >= cc.cc_algorithm().cwnd_prior());
+        assert_within(cc.cc_algorithm().alpha(), INCREASED_ALPHA, f64::EPSILON);
+        assert_within(cc.cc_algorithm().w_est(), w_est_projected, f64::EPSILON);
+    }
+
+    // Trigger a congestion event, which calls `reduce_cwnd` where `cwnd_prior` is updated to the
+    // current congestion window before reducing it. The next ACK after a congestion event will call
+    // `start_epoch` where `alpha` is set to it's default value and `w_est` is set to the newly
+    // reduced congestion window. Thus we now have `w_est < cwnd_prior` and `alpha ==
+    // NORMAL_ALPHA`.
+    packet_lost(&mut cc, last_sent_pn);
+    w_est_projected = convert_to_f64(cc.cwnd()); // update the value after the congestion event
+    acked_bytes = 0.0; // reset the acked bytes counter for the next epoch
+
+    // Send and ack packets until the congestion window grew so much that `w_est` is as big as
+    // `cwnd_prior`.
+    loop {
+        first_sent_pn = next_pn_to_send;
+        next_pn_to_send = fill_cwnd(&mut cc, next_pn_to_send, now());
+        last_sent_pn = next_pn_to_send - 1;
+        for pn in first_sent_pn..=last_sent_pn {
+            // Calculate the projected increase of w_est
+            acked_bytes += max_datagram_size;
+            (w_est_increase, acked_bytes_used) = expected_w_est_increase(
+                convert_to_f64(cc.cwnd()),
+                max_datagram_size,
+                acked_bytes,
+                NORMAL_ALPHA,
+            );
+            acked_bytes -= acked_bytes_used;
+            w_est_projected += w_est_increase;
+
+            // Actually process the ACK
+            ack_packet(&mut cc, pn, now());
+            qinfo!(
+                "pn acked: {pn}, alpha: {}, w_est: {}, w_est_projected: {w_est_projected}",
+                cc.cc_algorithm().alpha(),
+                cc.cc_algorithm().w_est()
+            );
+        }
+        if cc.cc_algorithm().w_est() >= cc.cc_algorithm().cwnd_prior() {
+            break;
+        }
+        // Make sure `w_est` grew by the amount expected with the normal `alpha` value
+        assert_within(cc.cc_algorithm().w_est(), w_est_projected, f64::EPSILON);
+    }
+
+    // Now `w_est` should be as big as `cwnd_prior`, thus `alpha` should have it's increased value.
+    assert!(cc.cc_algorithm().w_est() >= cc.cc_algorithm().cwnd_prior());
+    assert_within(cc.cc_algorithm().alpha(), INCREASED_ALPHA, f64::EPSILON);
+}