feat(ecn): expose ECN path validation outcome in Stats (#2270)

Expose the ECN path validation outcome via `neqo_transport::Stats`.

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

@@ -19,7 +19,7 @@ use crate::{
         new_server, send_and_receive, send_something, send_something_with_modifier,
         send_with_modifier_and_receive, DEFAULT_RTT,
     },
-    ecn::ECN_TEST_COUNT,
+    ecn::{EcnValidationOutcome, ECN_TEST_COUNT},
     path::MAX_PATH_PROBES,
     ConnectionId, ConnectionParameters, StreamType,
 };
@@ -154,8 +154,12 @@ fn stats() {
     }
 
     for stats in [client.stats(), server.stats()] {
-        assert_eq!(stats.ecn_paths_capable, 1);
-        assert_eq!(stats.ecn_paths_not_capable, 0);
+        for (outcome, count) in stats.ecn_path_validation.iter() {
+            match outcome {
+                EcnValidationOutcome::Capable => assert_eq!(*count, 1),
+                EcnValidationOutcome::NotCapable(_) => assert_eq!(*count, 0),
+            }
+        }
 
         for codepoint in [IpTosEcn::Ect1, IpTosEcn::Ce] {
             assert_eq!(stats.ecn_tx[codepoint], 0);

neqo-transport/src/ecn.rs

@@ -6,7 +6,7 @@
 
 use std::ops::{AddAssign, Deref, DerefMut, Sub};
 
-use enum_map::EnumMap;
+use enum_map::{Enum, EnumMap};
 use neqo_common::{qdebug, qinfo, qwarn, IpTosEcn};
 
 use crate::{
@@ -37,7 +37,7 @@ enum EcnValidationState {
     /// The validation test has concluded but the path's ECN capability is not yet known.
     Unknown,
     /// The path is known to **not** be ECN capable.
-    Failed,
+    Failed(EcnValidationError),
     /// The path is known to be ECN capable.
     Capable,
 }
@@ -57,13 +57,15 @@ impl EcnValidationState {
 
         match old {
             Self::Testing { .. } | Self::Unknown => {}
-            Self::Failed => debug_assert!(false, "Failed is a terminal state"),
-            Self::Capable => stats.ecn_paths_capable -= 1,
+            Self::Failed(_) => debug_assert!(false, "Failed is a terminal state"),
+            Self::Capable => stats.ecn_path_validation[EcnValidationOutcome::Capable] -= 1,
         }
         match new {
             Self::Testing { .. } | Self::Unknown => {}
-            Self::Failed => stats.ecn_paths_not_capable += 1,
-            Self::Capable => stats.ecn_paths_capable += 1,
+            Self::Failed(error) => {
+                stats.ecn_path_validation[EcnValidationOutcome::NotCapable(error)] += 1;
+            }
+            Self::Capable => stats.ecn_path_validation[EcnValidationOutcome::Capable] += 1,
         }
     }
 }
@@ -125,6 +127,36 @@ impl AddAssign<IpTosEcn> for EcnCount {
     }
 }
 
+#[derive(PartialEq, Eq, Debug, Clone, Copy, Default)]
+pub struct EcnValidationCount(EnumMap<EcnValidationOutcome, u64>);
+
+impl Deref for EcnValidationCount {
+    type Target = EnumMap<EcnValidationOutcome, u64>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl DerefMut for EcnValidationCount {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+#[derive(Debug, Clone, Copy, Enum, PartialEq, Eq)]
+pub enum EcnValidationError {
+    BlackHole,
+    Bleaching,
+    ReceivedUnsentECT1,
+}
+
+#[derive(Debug, Clone, Copy, Enum, PartialEq, Eq)]
+pub enum EcnValidationOutcome {
+    Capable,
+    NotCapable(EcnValidationError),
+}
+
 #[derive(Debug, Default)]
 pub struct EcnInfo {
     /// The current state of ECN validation on this path.
@@ -164,8 +196,8 @@ impl EcnInfo {
     }
 
     /// Disable ECN.
-    pub fn disable_ecn(&mut self, stats: &mut Stats) {
-        self.state.set(EcnValidationState::Failed, stats);
+    pub fn disable_ecn(&mut self, stats: &mut Stats, reason: EcnValidationError) {
+        self.state.set(EcnValidationState::Failed(reason), stats);
     }
 
     /// Process ECN counts from an ACK frame.
@@ -202,7 +234,7 @@ impl EcnInfo {
                     "ECN validation failed, all {} initial marked packets were lost",
                     probes_lost
                 );
-                self.disable_ecn(stats);
+                self.disable_ecn(stats, EcnValidationError::BlackHole);
             }
         }
     }
@@ -220,7 +252,7 @@ impl EcnInfo {
         // > (see Section 13.4.2.1) causes the ECN state for the path to become "capable", unless
         // > no marked packet has been acknowledged.
         match self.state {
-            EcnValidationState::Testing { .. } | EcnValidationState::Failed => return,
+            EcnValidationState::Testing { .. } | EcnValidationState::Failed(_) => return,
             EcnValidationState::Unknown | EcnValidationState::Capable => {}
         }
 
@@ -245,7 +277,7 @@ impl EcnInfo {
         // > corresponding ECN counts are not present in the ACK frame.
         let Some(ack_ecn) = ack_ecn else {
             qwarn!("ECN validation failed, no ECN counts in ACK frame");
-            self.disable_ecn(stats);
+            self.disable_ecn(stats, EcnValidationError::Bleaching);
             return;
         };
 
@@ -262,7 +294,7 @@ impl EcnInfo {
             .unwrap();
         if newly_acked_sent_with_ect0 == 0 {
             qwarn!("ECN validation failed, no ECT(0) packets were newly acked");
-            self.disable_ecn(stats);
+            self.disable_ecn(stats, EcnValidationError::Bleaching);
             return;
         }
         let ecn_diff = ack_ecn - self.baseline;
@@ -273,10 +305,10 @@ impl EcnInfo {
                 sum_inc,
                 newly_acked_sent_with_ect0
             );
-            self.disable_ecn(stats);
+            self.disable_ecn(stats, EcnValidationError::Bleaching);
         } else if ecn_diff[IpTosEcn::Ect1] > 0 {
             qwarn!("ECN validation failed, ACK counted ECT(1) marks that were never sent");
-            self.disable_ecn(stats);
+            self.disable_ecn(stats, EcnValidationError::ReceivedUnsentECT1);
         } else if self.state != EcnValidationState::Capable {
             qinfo!("ECN validation succeeded, path is capable");
             self.state.set(EcnValidationState::Capable, stats);
@@ -290,7 +322,7 @@ impl EcnInfo {
     pub const fn ecn_mark(&self) -> IpTosEcn {
         match self.state {
             EcnValidationState::Testing { .. } | EcnValidationState::Capable => IpTosEcn::Ect0,
-            EcnValidationState::Failed | EcnValidationState::Unknown => IpTosEcn::NotEct,
+            EcnValidationState::Failed(_) | EcnValidationState::Unknown => IpTosEcn::NotEct,
         }
     }
 }

neqo-transport/src/path.rs

@@ -733,7 +733,8 @@ impl Path {
                     [self],
                     "Possible ECN blackhole, disabling ECN and re-probing path"
                 );
-                self.ecn_info.disable_ecn(stats);
+                self.ecn_info
+                    .disable_ecn(stats, crate::ecn::EcnValidationError::BlackHole);
                 ProbeState::ProbeNeeded { probe_count: 0 }
             } else {
                 qinfo!([self], "Probing failed");

neqo-transport/src/stats.rs

@@ -16,7 +16,10 @@ use std::{
 
 use neqo_common::qwarn;
 
-use crate::{ecn::EcnCount, packet::PacketNumber};
+use crate::{
+    ecn::{EcnCount, EcnValidationCount},
+    packet::PacketNumber,
+};
 
 pub const MAX_PTO_COUNTS: usize = 16;
 
@@ -194,10 +197,8 @@ pub struct Stats {
 
     pub datagram_tx: DatagramStats,
 
-    /// Number of paths known to be ECN capable.
-    pub ecn_paths_capable: usize,
-    /// Number of paths known to be ECN incapable.
-    pub ecn_paths_not_capable: usize,
+    /// ECN path validation count, indexed by validation outcome.
+    pub ecn_path_validation: EcnValidationCount,
     /// ECN counts for outgoing UDP datagrams, returned by remote through QUIC ACKs.
     ///
     /// Note: Given that QUIC ACKs only carry [`Ect0`], [`Ect1`] and [`Ce`], but
@@ -271,8 +272,8 @@ impl Debug for Stats {
         self.frame_tx.fmt(f)?;
         writeln!(
             f,
-            "  ecn: {:?} for tx {:?} for rx {} capable paths {} not capable paths",
-            self.ecn_tx, self.ecn_rx, self.ecn_paths_capable, self.ecn_paths_not_capable
+            "  ecn: {:?} for tx {:?} for rx {:?} path validation outcomes",
+            self.ecn_tx, self.ecn_rx, self.ecn_path_validation,
         )
     }
 }