feat(transport): add ecn metrics to Stats (#2072)

* feat(transport): add ecn metrics to Stats

* clippy

* Document NotEct 0

* clippy

* fix doc links

neqo-transport/src/connection/mod.rs

@@ -1460,7 +1460,9 @@ impl Connection {
     ) {
         let space = PacketNumberSpace::from(packet.packet_type());
         if let Some(space) = self.acks.get_mut(space) {
-            *space.ecn_marks() += d.tos().into();
+            let space_ecn_marks = space.ecn_marks();
+            *space_ecn_marks += d.tos().into();
+            self.stats.borrow_mut().ecn_rx = *space_ecn_marks;
         } else {
             qtrace!("Not tracking ECN for dropped packet number space");
         }
@@ -2427,7 +2429,10 @@ impl Connection {
                 self.loss_recovery.on_packet_sent(path, initial);
             }
             path.borrow_mut().add_sent(packets.len());
-            Ok(SendOption::Yes(path.borrow_mut().datagram(packets)))
+            Ok(SendOption::Yes(
+                path.borrow_mut()
+                    .datagram(packets, &mut self.stats.borrow_mut()),
+            ))
         }
     }
 

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

@@ -12,11 +12,11 @@ use test_fixture::{
     fixture_init, now, DEFAULT_ADDR_V4,
 };
 
-use super::{send_something_with_modifier, send_with_modifier_and_receive, DEFAULT_RTT};
 use crate::{
     connection::tests::{
         connect_force_idle, connect_force_idle_with_modifier, default_client, default_server,
-        handshake_with_modifier, migration::get_cid, new_client, new_server, send_something,
+        handshake_with_modifier, migration::get_cid, new_client, new_server, send_and_receive,
+        send_something, send_something_with_modifier, send_with_modifier_and_receive, DEFAULT_RTT,
     },
     ecn::ECN_TEST_COUNT,
     ConnectionId, ConnectionParameters, StreamType,
@@ -91,6 +91,37 @@ fn handshake_delay_with_ecn_blackhole() {
     );
 }
 
+#[test]
+fn stats() {
+    let now = now();
+    let mut client = default_client();
+    let mut server = default_server();
+    connect_force_idle(&mut client, &mut server);
+
+    for _ in 0..ECN_TEST_COUNT {
+        let ack = send_and_receive(&mut client, &mut server, now);
+        client.process_input(&ack.unwrap(), now);
+    }
+
+    for _ in 0..ECN_TEST_COUNT {
+        let ack = send_and_receive(&mut server, &mut client, now);
+        server.process_input(&ack.unwrap(), now);
+    }
+
+    for stats in [client.stats(), server.stats()] {
+        assert_eq!(stats.ecn_paths_capable, 1);
+        assert_eq!(stats.ecn_paths_not_capable, 0);
+
+        for codepoint in [IpTosEcn::Ect1, IpTosEcn::Ce] {
+            assert_eq!(stats.ecn_tx[codepoint], 0);
+            assert_eq!(stats.ecn_rx[codepoint], 0);
+        }
+    }
+
+    assert!(client.stats().ecn_tx[IpTosEcn::Ect0] <= server.stats().ecn_rx[IpTosEcn::Ect0]);
+    assert!(server.stats().ecn_tx[IpTosEcn::Ect0] <= client.stats().ecn_rx[IpTosEcn::Ect0]);
+}
+
 #[test]
 fn disables_on_loss() {
     let now = now();

neqo-transport/src/ecn.rs

@@ -12,6 +12,7 @@ use neqo_common::{qdebug, qinfo, qwarn, IpTosEcn};
 use crate::{
     packet::{PacketNumber, PacketType},
     recovery::SentPacket,
+    Stats,
 };
 
 /// The number of packets to use for testing a path for ECN capability.
@@ -25,7 +26,7 @@ const ECN_TEST_COUNT_INITIAL_PHASE: usize = 3;
 
 /// The state information related to testing a path for ECN capability.
 /// See RFC9000, Appendix A.4.
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 enum EcnValidationState {
     /// The path is currently being tested for ECN capability, with the number of probes sent so
     /// far on the path during the ECN validation.
@@ -50,7 +51,32 @@ impl Default for EcnValidationState {
     }
 }
 
+impl EcnValidationState {
+    fn set(&mut self, new: Self, stats: &mut Stats) {
+        let old = std::mem::replace(self, new);
+
+        match old {
+            Self::Testing { .. } | Self::Unknown => {}
+            Self::Failed => debug_assert!(false, "Failed is a terminal state"),
+            Self::Capable => stats.ecn_paths_capable -= 1,
+        }
+        match new {
+            Self::Testing { .. } | Self::Unknown => {}
+            Self::Failed => stats.ecn_paths_not_capable += 1,
+            Self::Capable => stats.ecn_paths_capable += 1,
+        }
+    }
+}
+
 /// The counts for different ECN marks.
+///
+/// Note: [`EcnCount`] is used both for outgoing UDP datagrams, returned by
+/// remote through QUIC ACKs and for incoming UDP datagrams, read from IP TOS
+/// header. In the former case, given that QUIC ACKs only carry
+/// [`IpTosEcn::Ect0`], [`IpTosEcn::Ect1`] and [`IpTosEcn::Ce`], but never
+/// [`IpTosEcn::NotEct`], the [`IpTosEcn::NotEct`] value will always be 0.
+///
+/// See also <https://www.rfc-editor.org/rfc/rfc9000.html#section-19.3.2>.
 #[derive(PartialEq, Eq, Debug, Clone, Copy, Default)]
 pub struct EcnCount(EnumMap<IpTosEcn, u64>);
 
@@ -126,13 +152,13 @@ impl EcnInfo {
     /// Exit ECN validation if the number of packets sent exceeds `ECN_TEST_COUNT`.
     /// We do not implement the part of the RFC that says to exit ECN validation if the time since
     /// the start of ECN validation exceeds 3 * PTO, since this seems to happen much too quickly.
-    pub fn on_packet_sent(&mut self) {
+    pub fn on_packet_sent(&mut self, stats: &mut Stats) {
         if let EcnValidationState::Testing { probes_sent, .. } = &mut self.state {
             *probes_sent += 1;
             qdebug!("ECN probing: sent {} probes", probes_sent);
             if *probes_sent == ECN_TEST_COUNT {
                 qdebug!("ECN probing concluded with {} probes sent", probes_sent);
-                self.state = EcnValidationState::Unknown;
+                self.state.set(EcnValidationState::Unknown, stats);
             }
         }
     }
@@ -144,16 +170,17 @@ impl EcnInfo {
         &mut self,
         acked_packets: &[SentPacket],
         ack_ecn: Option<EcnCount>,
+        stats: &mut Stats,
     ) -> bool {
         let prev_baseline = self.baseline;
 
-        self.validate_ack_ecn_and_update(acked_packets, ack_ecn);
+        self.validate_ack_ecn_and_update(acked_packets, ack_ecn, stats);
 
         matches!(self.state, EcnValidationState::Capable)
             && (self.baseline - prev_baseline)[IpTosEcn::Ce] > 0
     }
 
-    pub fn on_packets_lost(&mut self, lost_packets: &[SentPacket]) {
+    pub fn on_packets_lost(&mut self, lost_packets: &[SentPacket], stats: &mut Stats) {
         if let EcnValidationState::Testing {
             probes_sent,
             initial_probes_lost: probes_lost,
@@ -170,7 +197,7 @@ impl EcnInfo {
                     "ECN validation failed, all {} initial marked packets were lost",
                     probes_lost
                 );
-                self.state = EcnValidationState::Failed;
+                self.state.set(EcnValidationState::Failed, stats);
             }
         }
     }
@@ -180,6 +207,7 @@ impl EcnInfo {
         &mut self,
         acked_packets: &[SentPacket],
         ack_ecn: Option<EcnCount>,
+        stats: &mut Stats,
     ) {
         // RFC 9000, Appendix A.4:
         //
@@ -212,7 +240,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.state = EcnValidationState::Failed;
+            self.state.set(EcnValidationState::Failed, stats);
             return;
         };
 
@@ -229,7 +257,7 @@ impl EcnInfo {
             .unwrap();
         if newly_acked_sent_with_ect0 == 0 {
             qwarn!("ECN validation failed, no ECT(0) packets were newly acked");
-            self.state = EcnValidationState::Failed;
+            self.state.set(EcnValidationState::Failed, stats);
             return;
         }
         let ecn_diff = ack_ecn - self.baseline;
@@ -240,15 +268,16 @@ impl EcnInfo {
                 sum_inc,
                 newly_acked_sent_with_ect0
             );
-            self.state = EcnValidationState::Failed;
+            self.state.set(EcnValidationState::Failed, stats);
         } else if ecn_diff[IpTosEcn::Ect1] > 0 {
             qwarn!("ECN validation failed, ACK counted ECT(1) marks that were never sent");
-            self.state = EcnValidationState::Failed;
+            self.state.set(EcnValidationState::Failed, stats);
         } else if self.state != EcnValidationState::Capable {
             qinfo!("ECN validation succeeded, path is capable");
-            self.state = EcnValidationState::Capable;
+            self.state.set(EcnValidationState::Capable, stats);
         }
         self.baseline = ack_ecn;
+        stats.ecn_tx = ack_ecn;
         self.largest_acked = largest_acked;
     }
 

neqo-transport/src/path.rs

@@ -698,12 +698,12 @@ impl Path {
     }
 
     /// Make a datagram.
-    pub fn datagram<V: Into<Vec<u8>>>(&mut self, payload: V) -> Datagram {
+    pub fn datagram<V: Into<Vec<u8>>>(&mut self, payload: V, stats: &mut Stats) -> Datagram {
         // Make sure to use the TOS value from before calling EcnInfo::on_packet_sent, which may
         // update the ECN state and can hence change it - this packet should still be sent
         // with the current value.
         let tos = self.tos();
-        self.ecn_info.on_packet_sent();
+        self.ecn_info.on_packet_sent(stats);
         Datagram::new(self.local, self.remote, tos, payload)
     }
 
@@ -980,7 +980,7 @@ impl Path {
     ) {
         debug_assert!(self.is_primary());
 
-        let ecn_ce_received = self.ecn_info.on_packets_acked(acked_pkts, ack_ecn);
+        let ecn_ce_received = self.ecn_info.on_packets_acked(acked_pkts, ack_ecn, stats);
         if ecn_ce_received {
             let cwnd_reduced = self
                 .sender
@@ -1004,7 +1004,7 @@ impl Path {
         now: Instant,
     ) {
         debug_assert!(self.is_primary());
-        self.ecn_info.on_packets_lost(lost_packets);
+        self.ecn_info.on_packets_lost(lost_packets, stats);
         let cwnd_reduced = self.sender.on_packets_lost(
             self.rtt.first_sample_time(),
             prev_largest_acked_sent,

neqo-transport/src/stats.rs

@@ -16,7 +16,7 @@ use std::{
 
 use neqo_common::qwarn;
 
-use crate::packet::PacketNumber;
+use crate::{ecn::EcnCount, packet::PacketNumber};
 
 pub const MAX_PTO_COUNTS: usize = 16;
 
@@ -166,6 +166,25 @@ pub struct Stats {
     pub incoming_datagram_dropped: usize,
 
     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 counts for outgoing UDP datagrams, returned by remote through QUIC ACKs.
+    ///
+    /// Note: Given that QUIC ACKs only carry [`Ect0`], [`Ect1`] and [`Ce`], but
+    /// never [`NotEct`], the [`NotEct`] value will always be 0.
+    ///
+    /// See also <https://www.rfc-editor.org/rfc/rfc9000.html#section-19.3.2>.
+    ///
+    /// [`Ect0`]: neqo_common::tos::IpTosEcn::Ect0
+    /// [`Ect1`]: neqo_common::tos::IpTosEcn::Ect1
+    /// [`Ce`]: neqo_common::tos::IpTosEcn::Ce
+    /// [`NotEct`]: neqo_common::tos::IpTosEcn::NotEct
+    pub ecn_tx: EcnCount,
+    /// ECN counts for incoming UDP datagrams, read from IP TOS header.
+    pub ecn_rx: EcnCount,
 }
 
 impl Stats {
@@ -222,7 +241,12 @@ impl Debug for Stats {
         writeln!(f, "  frames rx:")?;
         self.frame_rx.fmt(f)?;
         writeln!(f, "  frames tx:")?;
-        self.frame_tx.fmt(f)
+        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
+        )
     }
 }