feat: Send and process ACK-ECN

Most of the remaining bits from mozilla#1495

Depends on mozilla#1604

neqo-common/src/datagram.rs

@@ -90,7 +90,6 @@ fn fmt_datagram() {
     let d = datagram([0; 1].to_vec());
     assert_eq!(
         format!("{d:?}"),
-        "Datagram IpTos(Cs0, NotEct) TTL Some(128) [fe80::1]:443->[fe80::1]:443: [1]: 00"
-            .to_string()
+        "Datagram IpTos(Cs0, Ect0) TTL Some(128) [fe80::1]:443->[fe80::1]:443: [1]: 00".to_string()
     );
 }

neqo-transport/Cargo.toml

@@ -10,6 +10,7 @@ license.workspace = true
 
 [dependencies]
 # Sync with https://searchfox.org/mozilla-central/source/Cargo.lock 2024-02-08
+enum-map = "2.7"
 indexmap = "1.9"
 log = { version = "0.4", default-features = false }
 neqo-common = { path = "../neqo-common" }
@@ -19,7 +20,6 @@ smallvec = "1.11"
 
 [dev-dependencies]
 criterion = { version = "0.5", features = ["html_reports"] }
-enum-map = "2.7"
 test-fixture = { path = "../test-fixture" }
 
 [features]

neqo-transport/src/connection/mod.rs

@@ -462,7 +462,7 @@ impl Connection {
     }
 
     /// # Errors
-    /// When the operation fails.    
+    /// When the operation fails.
     pub fn client_enable_ech(&mut self, ech_config_list: impl AsRef<[u8]>) -> Res<()> {
         self.crypto.client_enable_ech(ech_config_list)
     }
@@ -1502,7 +1502,16 @@ impl Connection {
                         self.stats.borrow_mut().dups_rx += 1;
                     } else {
                         match self.process_packet(path, &payload, now) {
-                            Ok(migrate) => self.postprocess_packet(path, d, &packet, migrate, now),
+                            Ok(migrate) => {
+                                // Since we processed frames from this IP packet now,
+                                // update the ECN counts (RFC9000, Section 13.4.1).
+                                if let Some(space) = self.acks.get_mut(space) {
+                                    space.inc_ecn_count(d.tos().into(), 1);
+                                } else {
+                                    qdebug!("Not tracking ECN for dropped packet number space");
+                                }
+                                self.postprocess_packet(path, d, &packet, migrate, now);
+                            }
                             Err(e) => {
                                 self.ensure_error_path(path, &packet, now);
                                 return Err(e);
@@ -2706,10 +2715,13 @@ impl Connection {
                 ack_delay,
                 first_ack_range,
                 ack_ranges,
+                ecn_count,
             } => {
                 let ranges =
                     Frame::decode_ack_frame(largest_acknowledged, first_ack_range, &ack_ranges)?;
                 self.handle_ack(space, largest_acknowledged, ranges, ack_delay, now);
+                // TODO: Handle incoming ECN info.
+                qdebug!("input_frame {:?}", ecn_count);
             }
             Frame::Crypto { offset, data } => {
                 qtrace!(

neqo-transport/src/frame.rs

@@ -8,12 +8,14 @@
 
 use std::{convert::TryFrom, ops::RangeInclusive};
 
-use neqo_common::{qtrace, Decoder};
+use enum_map::{enum_map, EnumMap};
+use neqo_common::{qtrace, Decoder, IpTosEcn};
 
 use crate::{
     cid::MAX_CONNECTION_ID_LEN,
     packet::PacketType,
     stream_id::{StreamId, StreamType},
+    tracking::EcnCount,
     AppError, ConnectionError, Error, Res, TransportError,
 };
 
@@ -23,7 +25,7 @@ pub type FrameType = u64;
 const FRAME_TYPE_PADDING: FrameType = 0x0;
 pub const FRAME_TYPE_PING: FrameType = 0x1;
 pub const FRAME_TYPE_ACK: FrameType = 0x2;
-const FRAME_TYPE_ACK_ECN: FrameType = 0x3;
+pub const FRAME_TYPE_ACK_ECN: FrameType = 0x3;
 pub const FRAME_TYPE_RESET_STREAM: FrameType = 0x4;
 pub const FRAME_TYPE_STOP_SENDING: FrameType = 0x5;
 pub const FRAME_TYPE_CRYPTO: FrameType = 0x6;
@@ -110,6 +112,7 @@ pub enum Frame<'a> {
         ack_delay: u64,
         first_ack_range: u64,
         ack_ranges: Vec<AckRange>,
+        ecn_count: EcnCount,
     },
     ResetStream {
         stream_id: StreamId,
@@ -217,7 +220,7 @@ impl<'a> Frame<'a> {
         match self {
             Self::Padding => FRAME_TYPE_PADDING,
             Self::Ping => FRAME_TYPE_PING,
-            Self::Ack { .. } => FRAME_TYPE_ACK, // We don't do ACK ECN.
+            Self::Ack { .. } => FRAME_TYPE_ACK,
             Self::ResetStream { .. } => FRAME_TYPE_RESET_STREAM,
             Self::StopSending { .. } => FRAME_TYPE_STOP_SENDING,
             Self::Crypto { .. } => FRAME_TYPE_CRYPTO,
@@ -442,17 +445,25 @@ impl<'a> Frame<'a> {
                 }
 
                 // Now check for the values for ACK_ECN.
-                if t == FRAME_TYPE_ACK_ECN {
-                    dv(dec)?;
-                    dv(dec)?;
-                    dv(dec)?;
-                }
+                let ecn_count: EcnCount = match t {
+                    FRAME_TYPE_ACK_ECN => {
+                        let (ect0, ect1, ce) = (dv(dec)?, dv(dec)?, dv(dec)?);
+                        enum_map! {
+                                IpTosEcn::NotEct => 0,
+                                IpTosEcn::Ect0 => ect0,
+                                IpTosEcn::Ect1 => ect1,
+                                IpTosEcn::Ce => ce,
+                        }
+                    }
+                    _ => EnumMap::default(),
+                };
 
                 Ok(Self::Ack {
                     largest_acknowledged: la,
                     ack_delay: ad,
                     first_ack_range: fa,
                     ack_ranges: arr,
+                    ecn_count,
                 })
             }
             FRAME_TYPE_STOP_SENDING => Ok(Self::StopSending {
@@ -645,7 +656,8 @@ mod tests {
             largest_acknowledged: 0x1234,
             ack_delay: 0x1235,
             first_ack_range: 0x1236,
-            ack_ranges: ar,
+            ack_ranges: ar.clone(),
+            ecn_count: EnumMap::default(),
         };
 
         just_dec(&f, "025234523502523601020304");
@@ -655,10 +667,23 @@ mod tests {
         let mut dec = enc.as_decoder();
         assert_eq!(Frame::decode(&mut dec).unwrap_err(), Error::NoMoreData);
 
-        // Try to parse ACK_ECN without ECN values
+        // Try to parse ACK_ECN with ECN values
+        let ecn_count = enum_map! {
+            IpTosEcn::NotEct => 0,
+            IpTosEcn::Ect0 => 1,
+            IpTosEcn::Ect1 => 2,
+            IpTosEcn::Ce => 3,
+        };
+        let fe = Frame::Ack {
+            largest_acknowledged: 0x1234,
+            ack_delay: 0x1235,
+            first_ack_range: 0x1236,
+            ack_ranges: ar,
+            ecn_count,
+        };
         let enc = Encoder::from_hex("035234523502523601020304010203");
         let mut dec = enc.as_decoder();
-        assert_eq!(Frame::decode(&mut dec).unwrap(), f);
+        assert_eq!(Frame::decode(&mut dec).unwrap(), fe);
     }
 
     #[test]

neqo-transport/src/path.rs

@@ -17,12 +17,12 @@ use std::{
     time::{Duration, Instant},
 };
 
-use neqo_common::{hex, qdebug, qinfo, qlog::NeqoQlog, qtrace, Datagram, Encoder, IpTos};
+use neqo_common::{hex, qdebug, qinfo, qlog::NeqoQlog, qtrace, Datagram, Encoder, IpTos, IpTosEcn};
 use neqo_crypto::random;
 
 use crate::{
     ackrate::{AckRate, PeerAckDelay},
-    cc::CongestionControlAlgorithm,
+    cc::{CongestionControlAlgorithm, MAX_DATAGRAM_SIZE},
     cid::{ConnectionId, ConnectionIdRef, ConnectionIdStore, RemoteConnectionIdEntry},
     frame::{FRAME_TYPE_PATH_CHALLENGE, FRAME_TYPE_PATH_RESPONSE, FRAME_TYPE_RETIRE_CONNECTION_ID},
     packet::PacketBuilder,
@@ -545,6 +545,8 @@ pub struct Path {
     received_bytes: usize,
     /// The number of bytes sent on this path.
     sent_bytes: usize,
+    /// The number of ECN-marked bytes sent on this path that were declared lost.
+    lost_ecn_bytes: usize,
 
     /// For logging of events.
     qlog: NeqoQlog,
@@ -574,10 +576,11 @@ impl Path {
             challenge: None,
             rtt: RttEstimate::default(),
             sender,
-            tos: IpTos::default(), // TODO: Default to Ect0 when ECN is supported.
-            ttl: 64,               // This is the default TTL on many OSes.
+            tos: IpTosEcn::Ect0.into(),
+            ttl: 64, // This is the default TTL on many OSes.
             received_bytes: 0,
             sent_bytes: 0,
+            lost_ecn_bytes: 0,
             qlog,
         }
     }
@@ -980,6 +983,25 @@ impl Path {
             self.rtt.pto(space), // Important: the base PTO, not adjusted.
             lost_packets,
         );
+
+        if self.tos == IpTosEcn::Ect0.into() {
+            // If the path is currently marking outgoing packets as ECT(0),
+            // update the count of lost ECN-marked bytes.
+            self.lost_ecn_bytes += lost_packets.iter().map(|p| p.size).sum::<usize>();
+
+            // If we lost more than 3 MTUs worth of ECN-marked bytes, then
+            // disable ECN on this path. See RFC 9000, Section 13.4.2.
+            // This doesn't quite implement the algorithm given in RFC 9000,
+            // Appendix A.4, but it should be OK. (It might be worthwhile caching
+            // destination IP addresses for paths on which we had to disable ECN,
+            // in order to not persitently delay connection establishment to
+            // those destinations.)
+            if self.lost_ecn_bytes > MAX_DATAGRAM_SIZE * 3 {
+                qinfo!([self], "Disabling ECN on path due to excessive loss");
+                self.tos = IpTosEcn::NotEct.into();
+            }
+        }
+
         if cwnd_reduced {
             self.rtt.update_ack_delay(self.sender.cwnd(), self.mtu());
         }

neqo-transport/src/qlog.rs

@@ -13,7 +13,7 @@ use std::{
     time::Duration,
 };
 
-use neqo_common::{hex, qinfo, qlog::NeqoQlog, Decoder};
+use neqo_common::{hex, qinfo, qlog::NeqoQlog, Decoder, IpTosEcn};
 use qlog::events::{
     connectivity::{ConnectionStarted, ConnectionState, ConnectionStateUpdated},
     quic::{
@@ -404,6 +404,7 @@ fn frame_to_qlogframe(frame: &Frame) -> QuicFrame {
             ack_delay,
             first_ack_range,
             ack_ranges,
+            ecn_count: ecn_counts,
         } => {
             let ranges =
                 Frame::decode_ack_frame(*largest_acknowledged, *first_ack_range, ack_ranges).ok();
@@ -419,9 +420,9 @@ fn frame_to_qlogframe(frame: &Frame) -> QuicFrame {
             QuicFrame::Ack {
                 ack_delay: Some(*ack_delay as f32 / 1000.0),
                 acked_ranges,
-                ect1: None,
-                ect0: None,
-                ce: None,
+                ect1: Some(ecn_counts[IpTosEcn::Ect1]),
+                ect0: Some(ecn_counts[IpTosEcn::Ect0]),
+                ce: Some(ecn_counts[IpTosEcn::Ce]),
             }
         }
         Frame::ResetStream {

neqo-transport/src/tracking.rs

@@ -16,11 +16,13 @@ use std::{
     time::{Duration, Instant},
 };
 
-use neqo_common::{qdebug, qinfo, qtrace, qwarn};
+use enum_map::EnumMap;
+use neqo_common::{qdebug, qinfo, qtrace, qwarn, IpTosEcn};
 use neqo_crypto::{Epoch, TLS_EPOCH_HANDSHAKE, TLS_EPOCH_INITIAL};
 use smallvec::{smallvec, SmallVec};
 
 use crate::{
+    frame::{FRAME_TYPE_ACK, FRAME_TYPE_ACK_ECN},
     packet::{PacketBuilder, PacketNumber, PacketType},
     recovery::RecoveryToken,
     stats::FrameStats,
@@ -352,6 +354,9 @@ pub struct AckToken {
     ranges: Vec<PacketRange>,
 }
 
+/// The counts for different ECN marks.
+pub type EcnCount = EnumMap<IpTosEcn, u64>;
+
 /// A structure that tracks what packets have been received,
 /// and what needs acknowledgement for a packet number space.
 #[derive(Debug)]
@@ -380,6 +385,8 @@ pub struct RecvdPackets {
     /// Whether we are ignoring packets that arrive out of order
     /// for the purposes of generating immediate acknowledgment.
     ignore_order: bool,
+    /// The counts of different ECN marks that have been received.
+    ecn_count: EcnCount,
 }
 
 impl RecvdPackets {
@@ -397,9 +404,15 @@ impl RecvdPackets {
             unacknowledged_count: 0,
             unacknowledged_tolerance: DEFAULT_ACK_PACKET_TOLERANCE,
             ignore_order: false,
+            ecn_count: EcnCount::default(),
         }
     }
 
+    /// Increase the ECN count for the mark given by `ecn` by `n`.
+    pub fn inc_ecn_count(&mut self, ecn: IpTosEcn, n: u64) {
+        self.ecn_count[ecn] += n;
+    }
+
     /// Get the time at which the next ACK should be sent.
     pub fn ack_time(&self) -> Option<Instant> {
         self.ack_time
@@ -596,7 +609,15 @@ impl RecvdPackets {
             .cloned()
             .collect::<Vec<_>>();
 
-        builder.encode_varint(crate::frame::FRAME_TYPE_ACK);
+        let have_ecn_counts = self.ecn_count[IpTosEcn::Ect0] > 0
+            || self.ecn_count[IpTosEcn::Ect1] > 0
+            || self.ecn_count[IpTosEcn::Ce] > 0;
+
+        builder.encode_varint(if have_ecn_counts {
+            FRAME_TYPE_ACK_ECN
+        } else {
+            FRAME_TYPE_ACK
+        });
         let mut iter = ranges.iter();
         let Some(first) = iter.next() else { return };
         builder.encode_varint(first.largest);
@@ -620,6 +641,12 @@ impl RecvdPackets {
             last = r.smallest;
         }
 
+        if have_ecn_counts {
+            builder.encode_varint(self.ecn_count[IpTosEcn::Ect0]);
+            builder.encode_varint(self.ecn_count[IpTosEcn::Ect1]);
+            builder.encode_varint(self.ecn_count[IpTosEcn::Ce]);
+        }
+
         // We've sent an ACK, reset the timer.
         self.ack_time = None;
         self.last_ack_time = Some(now);

neqo-transport/tests/connection.rs

@@ -62,8 +62,8 @@ fn truncate_long_packet() {
 /// Test that reordering parts of the server Initial doesn't change things.
 #[test]
 fn reorder_server_initial() {
-    // A simple ACK frame for a single packet with packet number 0.
-    const ACK_FRAME: &[u8] = &[0x02, 0x00, 0x00, 0x00, 0x00];
+    // A simple ACK_ECN frame for a single packet with packet number 0 with a single ECT(0) mark.
+    const ACK_FRAME: &[u8] = &[0x03, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00];
 
     let mut client = new_client(
         ConnectionParameters::default().versions(Version::Version1, vec![Version::Version1]),

test-fixture/src/lib.rs

@@ -24,7 +24,7 @@ use neqo_common::{
     event::Provider,
     hex,
     qlog::{new_trace, NeqoQlog},
-    qtrace, Datagram, Decoder, IpTos, Role,
+    qtrace, Datagram, Decoder, IpTosEcn, Role,
 };
 use neqo_crypto::{init_db, random, AllowZeroRtt, AntiReplay, AuthenticationStatus};
 use neqo_http3::{Http3Client, Http3Parameters, Http3Server};
@@ -96,7 +96,7 @@ pub fn datagram(data: Vec<u8>) -> Datagram {
     Datagram::new(
         DEFAULT_ADDR,
         DEFAULT_ADDR,
-        IpTos::default(),
+        IpTosEcn::Ect0.into(),
         Some(128),
         data,
     )