feat(transport/cc): spurious congestion event detection

neqo-transport/src/cc/mod.rs

@@ -14,7 +14,7 @@ use std::{
 
 use neqo_common::qlog::Qlog;
 
-use crate::{recovery::sent, rtt::RttEstimate, Error, Pmtud};
+use crate::{recovery::sent, rtt::RttEstimate, stats::CongestionControlStats, Error, Pmtud};
 
 mod classic_cc;
 mod cubic;
@@ -56,6 +56,7 @@ pub trait CongestionControl: Display + Debug {
         acked_pkts: &[sent::Packet],
         rtt_est: &RttEstimate,
         now: Instant,
+        cc_stats: &mut CongestionControlStats,
     );
 
     /// Returns true if the congestion window was reduced.
@@ -66,10 +67,16 @@ pub trait CongestionControl: Display + Debug {
         pto: Duration,
         lost_packets: &[sent::Packet],
         now: Instant,
+        cc_stats: &mut CongestionControlStats,
     ) -> bool;
 
     /// Returns true if the congestion window was reduced.
-    fn on_ecn_ce_received(&mut self, largest_acked_pkt: &sent::Packet, now: Instant) -> bool;
+    fn on_ecn_ce_received(
+        &mut self,
+        largest_acked_pkt: &sent::Packet,
+        now: Instant,
+        cc_stats: &mut CongestionControlStats,
+    ) -> bool;
 
     #[must_use]
     fn recovery_packet(&self) -> bool;

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

@@ -28,6 +28,7 @@ use crate::{
     pmtud::Pmtud,
     recovery::{self, sent},
     rtt::RttEstimate,
+    stats::CongestionControlStats,
 };
 
 const fn cwnd_after_loss(cwnd: usize) -> usize {
@@ -54,7 +55,12 @@ fn fill_cwnd(cc: &mut ClassicCongestionControl<Cubic>, mut next_pn: u64, now: In
     next_pn
 }
 
-fn ack_packet(cc: &mut ClassicCongestionControl<Cubic>, pn: u64, now: Instant) {
+fn ack_packet(
+    cc: &mut ClassicCongestionControl<Cubic>,
+    pn: u64,
+    now: Instant,
+    cc_stats: &mut CongestionControlStats,
+) {
     let acked = sent::Packet::new(
         packet::Type::Short,
         pn,
@@ -63,10 +69,14 @@ fn ack_packet(cc: &mut ClassicCongestionControl<Cubic>, pn: u64, now: Instant) {
         recovery::Tokens::new(),
         cc.max_datagram_size(),
     );
-    cc.on_packets_acked(&[acked], &RttEstimate::new(RTT), now);
+    cc.on_packets_acked(&[acked], &RttEstimate::new(RTT), now, cc_stats);
 }
 
-fn packet_lost(cc: &mut ClassicCongestionControl<Cubic>, pn: u64) {
+fn packet_lost(
+    cc: &mut ClassicCongestionControl<Cubic>,
+    pn: u64,
+    cc_stats: &mut CongestionControlStats,
+) {
     const PTO: Duration = Duration::from_millis(120);
     let p_lost = sent::Packet::new(
         packet::Type::Short,
@@ -76,7 +86,7 @@ fn packet_lost(cc: &mut ClassicCongestionControl<Cubic>, pn: u64) {
         recovery::Tokens::new(),
         cc.max_datagram_size(),
     );
-    cc.on_packets_lost(None, None, PTO, &[p_lost], now());
+    cc.on_packets_lost(None, None, PTO, &[p_lost], now(), cc_stats);
 }
 
 fn expected_tcp_acks(cwnd_rtt_start: usize, mtu: usize) -> u64 {
@@ -89,6 +99,7 @@ fn expected_tcp_acks(cwnd_rtt_start: usize, mtu: usize) -> u64 {
 #[test]
 fn tcp_phase() {
     let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut cc_stats = CongestionControlStats::default();
 
     // change to congestion avoidance state.
     cubic.set_ssthresh(1);
@@ -121,7 +132,7 @@ fn tcp_phase() {
 
         for _ in 0..acks {
             now += time_increase;
-            ack_packet(&mut cubic, next_pn_ack, now);
+            ack_packet(&mut cubic, next_pn_ack, now, &mut cc_stats);
             next_pn_ack += 1;
             next_pn_send = fill_cwnd(&mut cubic, next_pn_send, now);
         }
@@ -140,7 +151,7 @@ fn tcp_phase() {
     while cwnd_rtt_start == cubic.cwnd() {
         num_acks += 1;
         now += time_increase;
-        ack_packet(&mut cubic, next_pn_ack, now);
+        ack_packet(&mut cubic, next_pn_ack, now, &mut cc_stats);
         next_pn_ack += 1;
         next_pn_send = fill_cwnd(&mut cubic, next_pn_send, now);
     }
@@ -166,7 +177,7 @@ fn tcp_phase() {
     while cwnd_rtt_start_after_tcp == cubic.cwnd() {
         num_acks2 += 1;
         now += time_increase;
-        ack_packet(&mut cubic, next_pn_ack, now);
+        ack_packet(&mut cubic, next_pn_ack, now, &mut cc_stats);
         next_pn_ack += 1;
         next_pn_send = fill_cwnd(&mut cubic, next_pn_send, now);
     }
@@ -196,6 +207,7 @@ fn tcp_phase() {
 #[test]
 fn cubic_phase() {
     let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut cc_stats = CongestionControlStats::default();
     let cwnd_initial_f64 = convert_to_f64(cubic.cwnd_initial());
     // Set w_max to a higher number make sure that cc is the cubic phase (cwnd is calculated
     // by the cubic equation).
@@ -223,7 +235,7 @@ fn cubic_phase() {
         let time_increase = RTT / u32::try_from(acks).unwrap();
         for _ in 0..acks {
             now += time_increase;
-            ack_packet(&mut cubic, next_pn_ack, now);
+            ack_packet(&mut cubic, next_pn_ack, now, &mut cc_stats);
             next_pn_ack += 1;
             next_pn_send = fill_cwnd(&mut cubic, next_pn_send, now);
         }
@@ -251,9 +263,10 @@ fn assert_within<T: Sub<Output = T> + PartialOrd + Copy>(value: T, expected: T,
 #[test]
 fn congestion_event_slow_start() {
     let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut cc_stats = CongestionControlStats::default();
 
     _ = fill_cwnd(&mut cubic, 0, now());
-    ack_packet(&mut cubic, 0, now());
+    ack_packet(&mut cubic, 0, now(), &mut cc_stats);
 
     assert_within(cubic.cc_algorithm().w_max(), 0.0, f64::EPSILON);
 
@@ -264,7 +277,7 @@ fn congestion_event_slow_start() {
     );
 
     // Trigger a congestion_event in slow start phase
-    packet_lost(&mut cubic, 1);
+    packet_lost(&mut cubic, 1, &mut cc_stats);
 
     // w_max is equal to cwnd before decrease.
     let cwnd_initial_f64 = convert_to_f64(cubic.cwnd_initial());
@@ -277,11 +290,13 @@ fn congestion_event_slow_start() {
         cubic.cwnd(),
         cwnd_after_loss_slow_start(cubic.cwnd_initial(), cubic.max_datagram_size())
     );
+    assert_eq!(cc_stats.congestion_events_loss, 1);
 }
 
 #[test]
 fn congestion_event_congestion_avoidance() {
     let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut cc_stats = CongestionControlStats::default();
 
     // Set ssthresh to something small to make sure that cc is in the congection avoidance phase.
     cubic.set_ssthresh(1);
@@ -294,21 +309,23 @@ fn congestion_event_congestion_avoidance() {
         .set_w_max(3.0 * max_datagram_size_f64);
 
     _ = fill_cwnd(&mut cubic, 0, now());
-    ack_packet(&mut cubic, 0, now());
+    ack_packet(&mut cubic, 0, now(), &mut cc_stats);
 
     assert_eq!(cubic.cwnd(), cubic.cwnd_initial());
 
-    // Trigger a congestion_event in slow start phase
-    packet_lost(&mut cubic, 1);
+    // Trigger a congestion_event in congestion avoidance phase.
+    packet_lost(&mut cubic, 1, &mut cc_stats);
 
     let cwnd_initial_f64 = convert_to_f64(cubic.cwnd_initial());
     assert_within(cubic.cc_algorithm().w_max(), cwnd_initial_f64, f64::EPSILON);
     assert_eq!(cubic.cwnd(), cwnd_after_loss(cubic.cwnd_initial()));
+    assert_eq!(cc_stats.congestion_events_loss, 1);
 }
 
 #[test]
 fn congestion_event_congestion_avoidance_fast_convergence() {
     let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut cc_stats = CongestionControlStats::default();
 
     // Set ssthresh to something small to make sure that cc is in the congection avoidance phase.
     cubic.set_ssthresh(1);
@@ -318,7 +335,7 @@ fn congestion_event_congestion_avoidance_fast_convergence() {
     cubic.cc_algorithm_mut().set_w_max(cwnd_initial_f64 * 10.0);
 
     _ = fill_cwnd(&mut cubic, 0, now());
-    ack_packet(&mut cubic, 0, now());
+    ack_packet(&mut cubic, 0, now(), &mut cc_stats);
 
     assert_within(
         cubic.cc_algorithm().w_max(),
@@ -328,20 +345,22 @@ fn congestion_event_congestion_avoidance_fast_convergence() {
     assert_eq!(cubic.cwnd(), cubic.cwnd_initial());
 
     // Trigger a congestion_event.
-    packet_lost(&mut cubic, 1);
+    packet_lost(&mut cubic, 1, &mut cc_stats);
 
     assert_within(
         cubic.cc_algorithm().w_max(),
         cwnd_initial_f64 * Cubic::FAST_CONVERGENCE_FACTOR,
         f64::EPSILON,
     );
     assert_eq!(cubic.cwnd(), cwnd_after_loss(cubic.cwnd_initial()));
+    assert_eq!(cc_stats.congestion_events_loss, 1);
 }
 
 #[test]
 fn congestion_event_congestion_avoidance_no_overflow() {
     const PTO: Duration = Duration::from_millis(120);
     let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut cc_stats = CongestionControlStats::default();
 
     // Set ssthresh to something small to make sure that cc is in the congection avoidance phase.
     cubic.set_ssthresh(1);
@@ -351,7 +370,7 @@ fn congestion_event_congestion_avoidance_no_overflow() {
     cubic.cc_algorithm_mut().set_w_max(cwnd_initial_f64 * 10.0);
 
     _ = fill_cwnd(&mut cubic, 0, now());
-    ack_packet(&mut cubic, 1, now());
+    ack_packet(&mut cubic, 1, now(), &mut cc_stats);
 
     assert_within(
         cubic.cc_algorithm().w_max(),
@@ -361,5 +380,10 @@ fn congestion_event_congestion_avoidance_no_overflow() {
     assert_eq!(cubic.cwnd(), cubic.cwnd_initial());
 
     // Now ack packet that was send earlier.
-    ack_packet(&mut cubic, 0, now().checked_sub(PTO).unwrap());
+    ack_packet(
+        &mut cubic,
+        0,
+        now().checked_sub(PTO).unwrap(),
+        &mut cc_stats,
+    );
 }

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

@@ -6,6 +6,11 @@
 
 // Congestion control
 
+#![expect(
+    clippy::too_many_lines,
+    reason = "A lot of multiline function calls due to formatting"
+)]
+
 use std::time::Duration;
 
 use test_fixture::now;
@@ -17,6 +22,7 @@ use crate::{
     pmtud::Pmtud,
     recovery::{self, sent},
     rtt::RttEstimate,
+    stats::CongestionControlStats,
 };
 
 const PTO: Duration = RTT;
@@ -34,6 +40,7 @@ fn cwnd_is_halved(cc: &ClassicCongestionControl<NewReno>) {
 #[test]
 fn issue_876() {
     let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut cc_stats = CongestionControlStats::default();
     let now = now();
     let before = now.checked_sub(Duration::from_millis(100)).unwrap();
     let after = now + Duration::from_millis(150);
@@ -105,7 +112,14 @@ fn issue_876() {
     cwnd_is_default(&cc);
     assert_eq!(cc.bytes_in_flight(), 6 * cc.max_datagram_size() - 3);
 
-    cc.on_packets_lost(Some(now), None, PTO, &sent_packets[0..1], now);
+    cc.on_packets_lost(
+        Some(now),
+        None,
+        PTO,
+        &sent_packets[0..1],
+        now,
+        &mut cc_stats,
+    );
 
     // We are now in recovery
     assert!(cc.recovery_packet());
@@ -125,13 +139,21 @@ fn issue_876() {
         &sent_packets[6..],
         &RttEstimate::new(crate::DEFAULT_INITIAL_RTT),
         now,
+        &mut cc_stats,
     );
     assert_eq!(cc.acked_bytes(), sent_packets[6].len());
     cwnd_is_halved(&cc);
     assert_eq!(cc.bytes_in_flight(), 5 * cc.max_datagram_size() - 2);
 
     // Packet from before is lost. Should not hurt cwnd.
-    cc.on_packets_lost(Some(now), None, PTO, &sent_packets[1..2], now);
+    cc.on_packets_lost(
+        Some(now),
+        None,
+        PTO,
+        &sent_packets[1..2],
+        now,
+        &mut cc_stats,
+    );
     assert!(!cc.recovery_packet());
     assert_eq!(cc.acked_bytes(), sent_packets[6].len());
     cwnd_is_halved(&cc);
@@ -142,6 +164,7 @@ fn issue_876() {
 // https://github.com/mozilla/neqo/pull/1465
 fn issue_1465() {
     let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU));
+    let mut cc_stats = CongestionControlStats::default();
     let mut pn = 0;
     let mut now = now();
     let max_datagram_size = cc.max_datagram_size();
@@ -174,7 +197,7 @@ fn issue_1465() {
     // advance one rtt to detect lost packet there this simplifies the timers, because
     // on_packet_loss would only be called after RTO, but that is not relevant to the problem
     now += RTT;
-    cc.on_packets_lost(Some(now), None, PTO, &[p1], now);
+    cc.on_packets_lost(Some(now), None, PTO, &[p1], now, &mut cc_stats);
 
     // We are now in recovery
     assert!(cc.recovery_packet());
@@ -183,30 +206,40 @@ fn issue_1465() {
     assert_eq!(cc.bytes_in_flight(), 2 * cc.max_datagram_size());
 
     // Don't reduce the cwnd again on second packet loss
-    cc.on_packets_lost(Some(now), None, PTO, &[p3], now);
+    cc.on_packets_lost(Some(now), None, PTO, &[p3], now, &mut cc_stats);
     assert_eq!(cc.acked_bytes(), 0);
     cwnd_is_halved(&cc); // still the same as after first packet loss
     assert_eq!(cc.bytes_in_flight(), cc.max_datagram_size());
 
     // the acked packets before on_packet_sent were the cause of
     // https://github.com/mozilla/neqo/pull/1465
-    cc.on_packets_acked(&[p2], &RttEstimate::new(crate::DEFAULT_INITIAL_RTT), now);
+    cc.on_packets_acked(
+        &[p2],
+        &RttEstimate::new(crate::DEFAULT_INITIAL_RTT),
+        now,
+        &mut cc_stats,
+    );
 
     assert_eq!(cc.bytes_in_flight(), 0);
 
     // send out recovery packet and get it acked to get out of recovery state
     let p4 = send_next(&mut cc, now);
     cc.on_packet_sent(&p4, now);
     now += RTT;
-    cc.on_packets_acked(&[p4], &RttEstimate::new(crate::DEFAULT_INITIAL_RTT), now);
+    cc.on_packets_acked(
+        &[p4],
+        &RttEstimate::new(crate::DEFAULT_INITIAL_RTT),
+        now,
+        &mut cc_stats,
+    );
 
     // do the same as in the first rtt but now the bug appears
     let p5 = send_next(&mut cc, now);
     let p6 = send_next(&mut cc, now);
     now += RTT;
 
     let cur_cwnd = cc.cwnd();
-    cc.on_packets_lost(Some(now), None, PTO, &[p5], now);
+    cc.on_packets_lost(Some(now), None, PTO, &[p5], now, &mut cc_stats);
 
     // go back into recovery
     assert!(cc.recovery_packet());
@@ -215,6 +248,6 @@ fn issue_1465() {
     assert_eq!(cc.bytes_in_flight(), 2 * cc.max_datagram_size());
 
     // this shouldn't introduce further cwnd reduction, but it did before https://github.com/mozilla/neqo/pull/1465
-    cc.on_packets_lost(Some(now), None, PTO, &[p6], now);
+    cc.on_packets_lost(Some(now), None, PTO, &[p6], now, &mut cc_stats);
     assert_eq!(cc.cwnd(), cur_cwnd / 2);
 }

neqo-transport/src/connection/tests/ecn.rs

@@ -217,6 +217,7 @@ fn debug() {
         "stats for\u{0020}
   rx: 0 drop 0 dup 0 saved 0
   tx: 0 lost 0 lateack 0 ptoack 0 unackdrop 0
+  cc: ce_loss 0 ce_ecn 0 ce_spurious 0
   pmtud: 0 sent 0 acked 0 lost 0 change 0 iface_mtu 0 pmtu
   resumed: false
   frames rx: