From d5191fb976ef4a6e3a7bd3ef2cf21a6d435d3c39 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 26 Sep 2024 15:37:52 +0300 Subject: [PATCH 01/26] feat: Stop the PMTUD search at the interface MTU WIP Should we optimistically *start* the search at the interface MTU, and only start from 1280 when that fails? --- neqo-transport/Cargo.toml | 1 + neqo-transport/src/cc/classic_cc.rs | 47 +++++++++++++------------ neqo-transport/src/cc/tests/cubic.rs | 13 +++---- neqo-transport/src/cc/tests/new_reno.rs | 5 +-- neqo-transport/src/path.rs | 4 ++- neqo-transport/src/pmtud.rs | 19 ++++++---- 6 files changed, 50 insertions(+), 39 deletions(-) diff --git a/neqo-transport/Cargo.toml b/neqo-transport/Cargo.toml index 24a0926db7..3db4053d7d 100644 --- a/neqo-transport/Cargo.toml +++ b/neqo-transport/Cargo.toml @@ -22,6 +22,7 @@ indexmap = { version = "2.2", default-features = false } # See https://github.co log = { workspace = true } neqo-common = { path = "../neqo-common" } neqo-crypto = { path = "../neqo-crypto" } +mtu = { version = "0.1.3", default-features = false } qlog = { workspace = true } smallvec = { version = "1.11", default-features = false } static_assertions = { version = "1.1", default-features = false } diff --git a/neqo-transport/src/cc/classic_cc.rs b/neqo-transport/src/cc/classic_cc.rs index 1130178bc0..c255aad0fb 100644 --- a/neqo-transport/src/cc/classic_cc.rs +++ b/neqo-transport/src/cc/classic_cc.rs @@ -614,6 +614,7 @@ mod tests { }; const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)); + const MTU: usize = 1_500; const PTO: Duration = Duration::from_millis(100); const RTT: Duration = Duration::from_millis(98); const RTT_ESTIMATE: RttEstimate = RttEstimate::from_duration(Duration::from_millis(98)); @@ -652,11 +653,11 @@ mod tests { match cc { CongestionControlAlgorithm::NewReno => Box::new(ClassicCongestionControl::new( NewReno::default(), - Pmtud::new(IP_ADDR), + Pmtud::new(IP_ADDR, MTU), )), CongestionControlAlgorithm::Cubic => Box::new(ClassicCongestionControl::new( Cubic::default(), - Pmtud::new(IP_ADDR), + Pmtud::new(IP_ADDR, MTU), )), } } @@ -894,13 +895,13 @@ mod tests { fn persistent_congestion_no_lost() { let lost = make_lost(&[]); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost )); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost @@ -912,13 +913,13 @@ mod tests { fn persistent_congestion_one_lost() { let lost = make_lost(&[1]); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost )); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost @@ -932,37 +933,37 @@ mod tests { // sample are not considered. So 0 is ignored. let lost = make_lost(&[0, PERSISTENT_CONG_THRESH + 1, PERSISTENT_CONG_THRESH + 2]); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)), 1, 1, &lost )); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)), 0, 1, &lost )); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)), 1, 0, &lost )); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)), 1, 1, &lost )); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)), 0, 1, &lost )); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)), 1, 0, &lost @@ -983,13 +984,13 @@ mod tests { lost[0].len(), ); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost )); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost @@ -1003,13 +1004,13 @@ mod tests { fn persistent_congestion_min() { let lost = make_lost(&[1, PERSISTENT_CONG_THRESH + 2]); assert!(persistent_congestion_by_pto( - ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost )); assert!(persistent_congestion_by_pto( - ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost @@ -1022,7 +1023,7 @@ mod tests { #[test] fn persistent_congestion_no_prev_ack_newreno() { let lost = make_lost(&[1, PERSISTENT_CONG_THRESH + 2]); - let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)); + let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)); cc.detect_persistent_congestion(Some(by_pto(0)), None, PTO, lost.iter()); assert_eq!(cc.cwnd(), cc.cwnd_min()); } @@ -1030,7 +1031,7 @@ mod tests { #[test] fn persistent_congestion_no_prev_ack_cubic() { let lost = make_lost(&[1, PERSISTENT_CONG_THRESH + 2]); - let mut cc = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)); + let mut cc = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); cc.detect_persistent_congestion(Some(by_pto(0)), None, PTO, lost.iter()); assert_eq!(cc.cwnd(), cc.cwnd_min()); } @@ -1041,7 +1042,7 @@ mod tests { fn persistent_congestion_unsorted_newreno() { let lost = make_lost(&[PERSISTENT_CONG_THRESH + 2, 1]); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost @@ -1054,7 +1055,7 @@ mod tests { fn persistent_congestion_unsorted_cubic() { let lost = make_lost(&[PERSISTENT_CONG_THRESH + 2, 1]); assert!(!persistent_congestion_by_pto( - ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)), + ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)), 0, 0, &lost @@ -1065,7 +1066,7 @@ mod tests { fn app_limited_slow_start() { const BELOW_APP_LIMIT_PKTS: usize = 5; const ABOVE_APP_LIMIT_PKTS: usize = BELOW_APP_LIMIT_PKTS + 1; - let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)); + let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)); let cwnd = cc.congestion_window; let mut now = now(); let mut next_pn = 0; @@ -1149,7 +1150,7 @@ mod tests { const BELOW_APP_LIMIT_PKTS: usize = CWND_PKTS_CA - 2; const ABOVE_APP_LIMIT_PKTS: usize = BELOW_APP_LIMIT_PKTS + 1; - let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)); + let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)); let mut now = now(); // Change state to congestion avoidance by introducing loss. @@ -1264,7 +1265,7 @@ mod tests { #[test] fn ecn_ce() { - let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)); + let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)); let p_ce = SentPacket::new( PacketType::Short, 1, diff --git a/neqo-transport/src/cc/tests/cubic.rs b/neqo-transport/src/cc/tests/cubic.rs index 54c8c2c3c8..045a9b202d 100644 --- a/neqo-transport/src/cc/tests/cubic.rs +++ b/neqo-transport/src/cc/tests/cubic.rs @@ -32,6 +32,7 @@ use crate::{ }; const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)); +const MTU: usize = 1_500; const RTT: Duration = Duration::from_millis(100); const fn cwnd_after_loss(cwnd: usize) -> usize { @@ -95,7 +96,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)); + let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); // change to congestion avoidance state. cubic.set_ssthresh(1); @@ -202,7 +203,7 @@ fn tcp_phase() { #[test] fn cubic_phase() { - let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)); + let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); let cwnd_initial_f64: f64 = convert_to_f64(cubic.cwnd_initial()); // Set last_max_cwnd to a higher number make sure that cc is the cubic phase (cwnd is calculated // by the cubic equation). @@ -257,7 +258,7 @@ fn assert_within + PartialOrd + Copy>(value: T, expected: T, #[test] fn congestion_event_slow_start() { - let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)); + let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); _ = fill_cwnd(&mut cubic, 0, now()); ack_packet(&mut cubic, 0, now()); @@ -288,7 +289,7 @@ fn congestion_event_slow_start() { #[test] fn congestion_event_congestion_avoidance() { - let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)); + let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); // Set ssthresh to something small to make sure that cc is in the congection avoidance phase. cubic.set_ssthresh(1); @@ -312,7 +313,7 @@ fn congestion_event_congestion_avoidance() { #[test] fn congestion_event_congestion_avoidance_2() { - let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR)); + let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); // Set ssthresh to something small to make sure that cc is in the congection avoidance phase. cubic.set_ssthresh(1); @@ -341,7 +342,7 @@ fn congestion_event_congestion_avoidance_2() { #[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)); + let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); // Set ssthresh to something small to make sure that cc is in the congection avoidance phase. cubic.set_ssthresh(1); diff --git a/neqo-transport/src/cc/tests/new_reno.rs b/neqo-transport/src/cc/tests/new_reno.rs index 1ee8c74f67..4dca9427b8 100644 --- a/neqo-transport/src/cc/tests/new_reno.rs +++ b/neqo-transport/src/cc/tests/new_reno.rs @@ -23,6 +23,7 @@ use crate::{ }; const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)); +const MTU: usize = 1_500; const PTO: Duration = Duration::from_millis(100); const RTT: Duration = Duration::from_millis(98); const RTT_ESTIMATE: RttEstimate = RttEstimate::from_duration(RTT); @@ -39,7 +40,7 @@ fn cwnd_is_halved(cc: &ClassicCongestionControl) { #[test] fn issue_876() { - let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)); + let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)); let time_now = now(); let time_before = time_now.checked_sub(Duration::from_millis(100)).unwrap(); let time_after = time_now + Duration::from_millis(150); @@ -150,7 +151,7 @@ fn issue_876() { #[test] // https://github.com/mozilla/neqo/pull/1465 fn issue_1465() { - let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR)); + let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)); let mut pn = 0; let mut now = now(); let max_datagram_size = cc.max_datagram_size(); diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs index 49c289f60b..f53c03bd87 100644 --- a/neqo-transport/src/path.rs +++ b/neqo-transport/src/path.rs @@ -569,7 +569,9 @@ impl Path { qlog: NeqoQlog, now: Instant, ) -> Self { - let mut sender = PacketSender::new(cc, pacing, Pmtud::new(remote.ip()), now); + let iface_mtu = + mtu::interface_and_mtu(&(local, remote)).map_or_else(|_| usize::MAX, |(_, m)| m); + let mut sender = PacketSender::new(cc, pacing, Pmtud::new(remote.ip(), iface_mtu), now); sender.set_qlog(qlog.clone()); Self { local, diff --git a/neqo-transport/src/pmtud.rs b/neqo-transport/src/pmtud.rs index 8a2179e41d..928bd44fde 100644 --- a/neqo-transport/src/pmtud.rs +++ b/neqo-transport/src/pmtud.rs @@ -46,6 +46,7 @@ pub struct Pmtud { search_table: &'static [usize], header_size: usize, mtu: usize, + iface_mtu: usize, probe_index: usize, probe_count: usize, probe_state: Probe, @@ -71,13 +72,14 @@ impl Pmtud { } #[must_use] - pub const fn new(remote_ip: IpAddr) -> Self { + pub const fn new(remote_ip: IpAddr, iface_mtu: usize) -> Self { let search_table = Self::search_table(remote_ip); let probe_index = 0; Self { search_table, header_size: Self::header_size(remote_ip), mtu: search_table[probe_index], + iface_mtu, probe_index, probe_count: 0, probe_state: Probe::NotNeeded, @@ -303,7 +305,10 @@ impl Pmtud { /// Starts the next upward PMTUD probe. pub fn start(&mut self) { - if self.probe_index < SEARCH_TABLE_LEN - 1 { + if self.probe_index < SEARCH_TABLE_LEN - 1 // Not at the end of the search table + // Next size is <= iface MTU + && self.search_table[self.probe_index + 1] <= self.iface_mtu + { self.probe_state = Probe::Needed; // We need to send a probe self.probe_count = 0; // For the first time self.probe_index += 1; // At this size @@ -407,7 +412,7 @@ mod tests { fn find_pmtu(addr: IpAddr, mtu: usize) { fixture_init(); let now = now(); - let mut pmtud = Pmtud::new(addr); + let mut pmtud = Pmtud::new(addr, mtu); let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); @@ -445,7 +450,7 @@ mod tests { fixture_init(); let now = now(); - let mut pmtud = Pmtud::new(addr); + let mut pmtud = Pmtud::new(addr, mtu); let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); @@ -498,7 +503,7 @@ mod tests { fixture_init(); let now = now(); - let mut pmtud = Pmtud::new(addr); + let mut pmtud = Pmtud::new(addr, larger_mtu); let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); @@ -570,7 +575,7 @@ mod tests { #[test] fn pmtud_on_packets_lost() { let now = now(); - let mut pmtud = Pmtud::new(V4); + let mut pmtud = Pmtud::new(V4, 1500); let mut stats = Stats::default(); // No packets lost, nothing should change. @@ -638,7 +643,7 @@ mod tests { #[test] fn pmtud_on_packets_lost_and_acked() { let now = now(); - let mut pmtud = Pmtud::new(V4); + let mut pmtud = Pmtud::new(V4, 1500); let mut stats = Stats::default(); // A packet of size 100 was ACKed, which is smaller than all probe sizes. From ead6c3cbec57344e1daba98515c4fde2b7cbc132 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 26 Sep 2024 16:07:09 +0300 Subject: [PATCH 02/26] Remove unused function, while I'm here. --- neqo-transport/src/pmtud.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/neqo-transport/src/pmtud.rs b/neqo-transport/src/pmtud.rs index 928bd44fde..d2ab6e82ab 100644 --- a/neqo-transport/src/pmtud.rs +++ b/neqo-transport/src/pmtud.rs @@ -110,12 +110,6 @@ impl Pmtud { self.probe_state == Probe::Needed } - /// Returns true if a PMTUD probe was sent. - #[must_use] - pub fn probe_sent(&self) -> bool { - self.probe_state == Probe::Sent - } - /// Returns the size of the current PMTUD probe. #[must_use] pub const fn probe_size(&self) -> usize { From 3f83c055f2a6e6584415c1799dabc22785668361 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 26 Sep 2024 16:16:52 +0300 Subject: [PATCH 03/26] Try and fix Firefox CI build --- .github/workflows/firefox.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/firefox.yml b/.github/workflows/firefox.yml index a55354fa6d..5cba7b3dc5 100644 --- a/.github/workflows/firefox.yml +++ b/.github/workflows/firefox.yml @@ -86,9 +86,10 @@ jobs: - name: Plumb in Neqo run: | - # Get qlog version used by neqo + # Get qlog and mtu version used by neqo cargo generate-lockfile QLOG_VERSION=$(cargo pkgid qlog | cut -d@ -f2) + MTU_VERSION=$(cargo pkgid mtu | cut -d@ -f2) rm Cargo.lock cd mozilla-unified { @@ -96,6 +97,10 @@ jobs: echo 'who = "CI"' echo 'criteria = "safe-to-deploy"' echo "version = \"$QLOG_VERSION\"" + echo '[[audits.mtu]]' + echo 'who = "CI"' + echo 'criteria = "safe-to-deploy"' + echo "version = \"$MTU_VERSION\"" } >> supply-chain/audits.toml sed -i'' -e "s/qlog =.*/qlog = \"$QLOG_VERSION\"/" netwerk/socket/neqo_glue/Cargo.toml { From 5f2e4b5c6088e8641cfaf98b806c1cd78c8d4cc0 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 27 Sep 2024 15:20:18 +0300 Subject: [PATCH 04/26] Use `stop` when we hit the iface MTU --- neqo-transport/src/connection/mod.rs | 8 +++--- neqo-transport/src/pmtud.rs | 39 ++++++++++++++-------------- neqo-transport/src/sender.rs | 2 +- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 0ccf854c91..cdf6dc4638 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -2791,7 +2791,7 @@ impl Connection { Ok(()) } - fn set_confirmed(&mut self) -> Res<()> { + fn set_confirmed(&mut self, now: Instant) -> Res<()> { self.set_state(State::Confirmed); if self.conn_params.pmtud_enabled() { self.paths @@ -2799,7 +2799,7 @@ impl Connection { .ok_or(Error::InternalError)? .borrow_mut() .pmtud_mut() - .start(); + .start(now); } Ok(()) } @@ -2963,7 +2963,7 @@ impl Connection { if self.role == Role::Server || !self.state.connected() { return Err(Error::ProtocolViolation); } - self.set_confirmed()?; + self.set_confirmed(now)?; self.discard_keys(PacketNumberSpace::Handshake, now); self.migrate_to_preferred_address(now)?; } @@ -3155,7 +3155,7 @@ impl Connection { .resumed(); if self.role == Role::Server { self.state_signaling.handshake_done(); - self.set_confirmed()?; + self.set_confirmed(now)?; } qinfo!([self], "Connection established"); Ok(()) diff --git a/neqo-transport/src/pmtud.rs b/neqo-transport/src/pmtud.rs index d2ab6e82ab..cbab0123ba 100644 --- a/neqo-transport/src/pmtud.rs +++ b/neqo-transport/src/pmtud.rs @@ -93,7 +93,7 @@ impl Pmtud { if self.probe_state == Probe::NotNeeded && self.raise_timer.map_or(false, |t| now >= t) { qdebug!("PMTUD raise timer fired"); self.raise_timer = None; - self.start(); + self.start(now); } } @@ -156,7 +156,7 @@ impl Pmtud { /// Checks whether a PMTUD probe has been acknowledged, and if so, updates the PMTUD state. /// May also initiate a new probe process for a larger MTU. - pub fn on_packets_acked(&mut self, acked_pkts: &[SentPacket], stats: &mut Stats) { + pub fn on_packets_acked(&mut self, acked_pkts: &[SentPacket], now: Instant, stats: &mut Stats) { // Reset the loss counts for all packets sizes <= the size of the largest ACKed packet. let max_len = acked_pkts.iter().map(SentPacket::len).max().unwrap_or(0); if max_len == 0 { @@ -183,7 +183,7 @@ impl Pmtud { stats.pmtud_ack += acked; self.mtu = self.search_table[self.probe_index]; qdebug!("PMTUD probe of size {} succeeded", self.mtu); - self.start(); + self.start(now); } /// Stops the PMTUD process, setting the MTU to the largest successful probe size. @@ -279,7 +279,9 @@ impl Pmtud { // TODO: If we are declaring losses, that means that we're getting packets through. // The size of those will put a floor on the MTU. We're currently conservative and // start from scratch, but we don't strictly need to do that. - self.restart(stats); + self.reset(stats); + qdebug!("PMTUD reset and restarting, PLPMTU is now {}", self.mtu); + self.start(now); } else { // We saw multiple losses of packets > the current MTU during PMTU discovery, so // we're done. @@ -287,18 +289,17 @@ impl Pmtud { } } - fn restart(&mut self, stats: &mut Stats) { + /// Resets the PMTUD process, starting from the smallest probe size. + fn reset(&mut self, stats: &mut Stats) { self.probe_index = 0; self.mtu = self.search_table[self.probe_index]; self.loss_counts.fill(0); self.raise_timer = None; stats.pmtud_change += 1; - qdebug!("PMTUD restarted, PLPMTU is now {}", self.mtu); - self.start(); } /// Starts the next upward PMTUD probe. - pub fn start(&mut self) { + pub fn start(&mut self, now: Instant) { if self.probe_index < SEARCH_TABLE_LEN - 1 // Not at the end of the search table // Next size is <= iface MTU && self.search_table[self.probe_index + 1] <= self.iface_mtu @@ -311,8 +312,8 @@ impl Pmtud { self.search_table[self.probe_index], ); } else { - // If we're at the end of the search table, we're done. - self.probe_state = Probe::NotNeeded; + // If we're at the end of the search table or hit the local interface MTU, we're done. + self.stop(self.probe_index, now); } } @@ -395,7 +396,7 @@ mod tests { let packet = make_sentpacket(pn, now, encoder.len()); if encoder.len() + Pmtud::header_size(addr) <= mtu { - pmtud.on_packets_acked(&[packet], stats); + pmtud.on_packets_acked(&[packet], now, stats); assert_eq!(stats_before.pmtud_ack + 1, stats.pmtud_ack); } else { pmtud.on_packets_lost(&[packet], stats, now); @@ -410,7 +411,7 @@ mod tests { let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); - pmtud.start(); + pmtud.start(now); assert!(pmtud.needs_probe()); while pmtud.needs_probe() { @@ -449,7 +450,7 @@ mod tests { let mut prot = CryptoDxState::test_default(); assert!(smaller_mtu >= pmtud.search_table[0]); - pmtud.start(); + pmtud.start(now); assert!(pmtud.needs_probe()); while pmtud.needs_probe() { @@ -502,7 +503,7 @@ mod tests { let mut prot = CryptoDxState::test_default(); assert!(larger_mtu >= pmtud.search_table[0]); - pmtud.start(); + pmtud.start(now); assert!(pmtud.needs_probe()); while pmtud.needs_probe() { @@ -642,18 +643,18 @@ mod tests { // A packet of size 100 was ACKed, which is smaller than all probe sizes. // Loss counts should be unchanged. - pmtud.on_packets_acked(&[make_sentpacket(0, now, 100)], &mut stats); + pmtud.on_packets_acked(&[make_sentpacket(0, now, 100)], now, &mut stats); assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); // A packet of size 100_000 was ACKed, which is larger than all probe sizes. // Loss counts should be unchanged. - pmtud.on_packets_acked(&[make_sentpacket(0, now, 100_000)], &mut stats); + pmtud.on_packets_acked(&[make_sentpacket(0, now, 100_000)], now, &mut stats); assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); pmtud.loss_counts.fill(0); // Reset the loss counts. // No packets ACKed, nothing should change. - pmtud.on_packets_acked(&[], &mut stats); + pmtud.on_packets_acked(&[], now, &mut stats); assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); // One packet of size 4000 was lost, which should increase loss counts >= 4000 by one. @@ -662,7 +663,7 @@ mod tests { assert_eq!(expected_lc, pmtud.loss_counts); // Now a packet of size 5000 is ACKed, which should reset all loss counts <= 5000. - pmtud.on_packets_acked(&[make_sentpacket(0, now, 5000)], &mut stats); + pmtud.on_packets_acked(&[make_sentpacket(0, now, 5000)], now, &mut stats); let expected_lc = search_table_zero(&pmtud, &pmtud.loss_counts, 5000); assert_eq!(expected_lc, pmtud.loss_counts); @@ -673,7 +674,7 @@ mod tests { assert_eq!(expected_lc, pmtud.loss_counts); // Now a packet of size 8000 is ACKed, which should reset all loss counts <= 8000. - pmtud.on_packets_acked(&[make_sentpacket(0, now, 8000)], &mut stats); + pmtud.on_packets_acked(&[make_sentpacket(0, now, 8000)], now, &mut stats); let expected_lc = search_table_zero(&pmtud, &pmtud.loss_counts, 8000); assert_eq!(expected_lc, pmtud.loss_counts); diff --git a/neqo-transport/src/sender.rs b/neqo-transport/src/sender.rs index a9ead627aa..9ffe4f4a0a 100644 --- a/neqo-transport/src/sender.rs +++ b/neqo-transport/src/sender.rs @@ -109,7 +109,7 @@ impl PacketSender { stats: &mut Stats, ) { self.cc.on_packets_acked(acked_pkts, rtt_est, now); - self.pmtud_mut().on_packets_acked(acked_pkts, stats); + self.pmtud_mut().on_packets_acked(acked_pkts, now, stats); self.maybe_update_pacer_mtu(); } From 5f03837fb4d97906be80109b6b8501f2f31b3aa8 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Mon, 30 Sep 2024 08:39:29 +0300 Subject: [PATCH 05/26] Log --- neqo-transport/Cargo.toml | 2 +- neqo-transport/src/path.rs | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/neqo-transport/Cargo.toml b/neqo-transport/Cargo.toml index 3db4053d7d..86050b31a8 100644 --- a/neqo-transport/Cargo.toml +++ b/neqo-transport/Cargo.toml @@ -22,7 +22,7 @@ indexmap = { version = "2.2", default-features = false } # See https://github.co log = { workspace = true } neqo-common = { path = "../neqo-common" } neqo-crypto = { path = "../neqo-crypto" } -mtu = { version = "0.1.3", default-features = false } +mtu = { path = "../../mtu", default-features = false } qlog = { workspace = true } smallvec = { version = "1.11", default-features = false } static_assertions = { version = "1.1", default-features = false } diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs index f53c03bd87..caed4d2940 100644 --- a/neqo-transport/src/path.rs +++ b/neqo-transport/src/path.rs @@ -15,7 +15,9 @@ use std::{ time::{Duration, Instant}, }; -use neqo_common::{hex, qdebug, qinfo, qlog::NeqoQlog, qtrace, Datagram, Encoder, IpTos, IpTosEcn}; +use neqo_common::{ + hex, qdebug, qinfo, qlog::NeqoQlog, qtrace, qwarn, Datagram, Encoder, IpTos, IpTosEcn, +}; use neqo_crypto::random; use crate::{ @@ -569,8 +571,19 @@ impl Path { qlog: NeqoQlog, now: Instant, ) -> Self { - let iface_mtu = - mtu::interface_and_mtu(&(local, remote)).map_or_else(|_| usize::MAX, |(_, m)| m); + // We're passing `None` into `mtu::interface_and_mtu` here, which force the lookup to only + // take the local address into account. Taking the remote address into account would require + // a call to `connect`, which Firefox blocklists for Rust dependencies. + let iface_mtu = match mtu::interface_and_mtu(&(local.ip(), remote)) { + Ok((name, mtu)) => { + qdebug!("Outbound interface {name} has MTU {mtu}"); + mtu + } + Err(e) => { + qwarn!("Failed to determine outbound interface: {e}"); + usize::MAX + } + }; let mut sender = PacketSender::new(cc, pacing, Pmtud::new(remote.ip(), iface_mtu), now); sender.set_qlog(qlog.clone()); Self { From cfb24b76401aa807349bad1d60d454929e1c06ae Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Tue, 8 Oct 2024 15:44:07 +0300 Subject: [PATCH 06/26] Try new PR for MTU --- neqo-transport/Cargo.toml | 4 ++-- neqo-transport/src/cc/classic_cc.rs | 10 +++++----- neqo-transport/src/cc/tests/cubic.rs | 6 +----- neqo-transport/src/cc/tests/mod.rs | 9 +++++++++ neqo-transport/src/cc/tests/new_reno.rs | 11 +++-------- neqo-transport/src/path.rs | 2 +- neqo-transport/src/pmtud.rs | 4 ++-- 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/neqo-transport/Cargo.toml b/neqo-transport/Cargo.toml index 86050b31a8..070c3a1295 100644 --- a/neqo-transport/Cargo.toml +++ b/neqo-transport/Cargo.toml @@ -18,11 +18,11 @@ workspace = true [dependencies] # Sync with https://searchfox.org/mozilla-central/source/Cargo.lock 2024-02-08 enum-map = { version = "2.7", default-features = false } -indexmap = { version = "2.2", default-features = false } # See https://github.com/mozilla/neqo/issues/1858 +indexmap = { version = "2.2", default-features = false } # See https://github.com/mozilla/neqo/issues/1858 log = { workspace = true } neqo-common = { path = "../neqo-common" } neqo-crypto = { path = "../neqo-crypto" } -mtu = { path = "../../mtu", default-features = false } +mtu = { git = "https://github.com/mozilla/mtu.git", branch = "feat-from-routing", default-features = false } # TODO: Point to the crate on crates.io qlog = { workspace = true } smallvec = { version = "1.11", default-features = false } static_assertions = { version = "1.1", default-features = false } diff --git a/neqo-transport/src/cc/classic_cc.rs b/neqo-transport/src/cc/classic_cc.rs index c255aad0fb..565b68f3fa 100644 --- a/neqo-transport/src/cc/classic_cc.rs +++ b/neqo-transport/src/cc/classic_cc.rs @@ -12,6 +12,9 @@ use std::{ time::{Duration, Instant}, }; +use ::qlog::events::{quic::CongestionStateUpdated, EventData}; +use neqo_common::{const_max, const_min, qdebug, qinfo, qlog::NeqoQlog, qtrace}; + use super::CongestionControl; use crate::{ packet::PacketNumber, @@ -21,9 +24,6 @@ use crate::{ sender::PACING_BURST_SIZE, Pmtud, }; -#[rustfmt::skip] // to keep `::` and thus prevent conflict with `crate::qlog` -use ::qlog::events::{quic::CongestionStateUpdated, EventData}; -use neqo_common::{const_max, const_min, qdebug, qinfo, qlog::NeqoQlog, qtrace}; pub const CWND_INITIAL_PKTS: usize = 10; const PERSISTENT_CONG_THRESH: u32 = 3; @@ -613,11 +613,11 @@ mod tests { Pmtud, }; - const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)); + const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED); const MTU: usize = 1_500; const PTO: Duration = Duration::from_millis(100); const RTT: Duration = Duration::from_millis(98); - const RTT_ESTIMATE: RttEstimate = RttEstimate::from_duration(Duration::from_millis(98)); + const RTT_ESTIMATE: RttEstimate = RttEstimate::from_duration(RTT); const ZERO: Duration = Duration::from_secs(0); const EPSILON: Duration = Duration::from_nanos(1); const GAP: Duration = Duration::from_secs(1); diff --git a/neqo-transport/src/cc/tests/cubic.rs b/neqo-transport/src/cc/tests/cubic.rs index 045a9b202d..912be96ecf 100644 --- a/neqo-transport/src/cc/tests/cubic.rs +++ b/neqo-transport/src/cc/tests/cubic.rs @@ -8,7 +8,6 @@ #![allow(clippy::cast_sign_loss)] use std::{ - net::{IpAddr, Ipv4Addr}, ops::Sub, time::{Duration, Instant}, }; @@ -16,6 +15,7 @@ use std::{ use neqo_common::IpTosEcn; use test_fixture::now; +use super::{IP_ADDR, MTU, RTT}; use crate::{ cc::{ classic_cc::ClassicCongestionControl, @@ -31,10 +31,6 @@ use crate::{ rtt::RttEstimate, }; -const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)); -const MTU: usize = 1_500; -const RTT: Duration = Duration::from_millis(100); - const fn cwnd_after_loss(cwnd: usize) -> usize { cwnd * CUBIC_BETA_USIZE_DIVIDEND / CUBIC_BETA_USIZE_DIVISOR } diff --git a/neqo-transport/src/cc/tests/mod.rs b/neqo-transport/src/cc/tests/mod.rs index 879693fb24..fc9d10e3a8 100644 --- a/neqo-transport/src/cc/tests/mod.rs +++ b/neqo-transport/src/cc/tests/mod.rs @@ -4,5 +4,14 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::{ + net::{IpAddr, Ipv4Addr}, + time::Duration, +}; + mod cubic; mod new_reno; + +const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED); +const MTU: usize = 1_500; +const RTT: Duration = Duration::from_millis(100); diff --git a/neqo-transport/src/cc/tests/new_reno.rs b/neqo-transport/src/cc/tests/new_reno.rs index 4dca9427b8..92004cb793 100644 --- a/neqo-transport/src/cc/tests/new_reno.rs +++ b/neqo-transport/src/cc/tests/new_reno.rs @@ -6,14 +6,12 @@ // Congestion control -use std::{ - net::{IpAddr, Ipv4Addr}, - time::Duration, -}; +use std::time::Duration; use neqo_common::IpTosEcn; use test_fixture::now; +use super::{IP_ADDR, MTU, RTT}; use crate::{ cc::{new_reno::NewReno, ClassicCongestionControl, CongestionControl}, packet::PacketType, @@ -22,10 +20,7 @@ use crate::{ rtt::RttEstimate, }; -const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)); -const MTU: usize = 1_500; -const PTO: Duration = Duration::from_millis(100); -const RTT: Duration = Duration::from_millis(98); +const PTO: Duration = RTT; const RTT_ESTIMATE: RttEstimate = RttEstimate::from_duration(RTT); fn cwnd_is_default(cc: &ClassicCongestionControl) { diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs index c57d5a0874..2eb9a552e9 100644 --- a/neqo-transport/src/path.rs +++ b/neqo-transport/src/path.rs @@ -573,7 +573,7 @@ impl Path { // We're passing `None` into `mtu::interface_and_mtu` here, which force the lookup to only // take the local address into account. Taking the remote address into account would require // a call to `connect`, which Firefox blocklists for Rust dependencies. - let iface_mtu = match mtu::interface_and_mtu(&(local.ip(), remote)) { + let iface_mtu = match mtu::interface_and_mtu(remote.ip()) { Ok((name, mtu)) => { qdebug!("Outbound interface {name} has MTU {mtu}"); mtu diff --git a/neqo-transport/src/pmtud.rs b/neqo-transport/src/pmtud.rs index cbab0123ba..3140d2dfd7 100644 --- a/neqo-transport/src/pmtud.rs +++ b/neqo-transport/src/pmtud.rs @@ -344,8 +344,8 @@ mod tests { Pmtud, Stats, }; - const V4: IpAddr = IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)); - const V6: IpAddr = IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0)); + const V4: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED); + const V6: IpAddr = IpAddr::V6(Ipv6Addr::UNSPECIFIED); fn make_sentpacket(pn: u64, now: Instant, len: usize) -> SentPacket { SentPacket::new( From e485804413c9996184402abffd376d5b64e3c2be Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Tue, 8 Oct 2024 15:47:38 +0300 Subject: [PATCH 07/26] Remove stale comment --- neqo-transport/src/path.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs index 2eb9a552e9..1eeb0d5a84 100644 --- a/neqo-transport/src/path.rs +++ b/neqo-transport/src/path.rs @@ -570,9 +570,6 @@ impl Path { qlog: NeqoQlog, now: Instant, ) -> Self { - // We're passing `None` into `mtu::interface_and_mtu` here, which force the lookup to only - // take the local address into account. Taking the remote address into account would require - // a call to `connect`, which Firefox blocklists for Rust dependencies. let iface_mtu = match mtu::interface_and_mtu(remote.ip()) { Ok((name, mtu)) => { qdebug!("Outbound interface {name} has MTU {mtu}"); From d35f7e4af71f23013d6b5dd54fcb520c47eb734b Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Tue, 8 Oct 2024 16:10:01 +0300 Subject: [PATCH 08/26] Set policy --- .github/workflows/firefox.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/firefox.yml b/.github/workflows/firefox.yml index 370cec7e30..1255889b60 100644 --- a/.github/workflows/firefox.yml +++ b/.github/workflows/firefox.yml @@ -102,6 +102,12 @@ jobs: echo 'criteria = "safe-to-deploy"' echo "version = \"$MTU_VERSION\"" } >> supply-chain/audits.toml + { + echo '[policy.qlog]' + echo 'audit-as-crates-io = true' + echo '[policy.mtu]' + echo 'audit-as-crates-io = true' + } >> supply-chain/config.toml sed -i'' -e "s/qlog =.*/qlog = \"$QLOG_VERSION\"/" netwerk/socket/neqo_glue/Cargo.toml { echo '[patch."https://github.com/mozilla/neqo"]' From 56a7a4bd64112c37d067ac864336c04d9b303ff8 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Tue, 8 Oct 2024 16:26:03 +0300 Subject: [PATCH 09/26] Again --- .github/workflows/firefox.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/firefox.yml b/.github/workflows/firefox.yml index 1255889b60..04fb445af0 100644 --- a/.github/workflows/firefox.yml +++ b/.github/workflows/firefox.yml @@ -103,8 +103,6 @@ jobs: echo "version = \"$MTU_VERSION\"" } >> supply-chain/audits.toml { - echo '[policy.qlog]' - echo 'audit-as-crates-io = true' echo '[policy.mtu]' echo 'audit-as-crates-io = true' } >> supply-chain/config.toml From 5ff1a7d6db97a265e976b791ddba4b9a563fdb33 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Tue, 8 Oct 2024 16:37:12 +0300 Subject: [PATCH 10/26] Again --- .github/workflows/firefox.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/firefox.yml b/.github/workflows/firefox.yml index 04fb445af0..5f0c971de3 100644 --- a/.github/workflows/firefox.yml +++ b/.github/workflows/firefox.yml @@ -101,6 +101,10 @@ jobs: echo 'who = "CI"' echo 'criteria = "safe-to-deploy"' echo "version = \"$MTU_VERSION\"" + echo '[[audits.mtu]]' + echo 'who = "CI"' + echo 'criteria = "safe-to-deploy"' + echo "version = \"0.2.0@git:9a8b72d3eebf04cc5accbddbbef3e3603dcf943f\"" } >> supply-chain/audits.toml { echo '[policy.mtu]' From 1f18511c01465cc2d76835c009daaaf233746a85 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 22 Nov 2024 14:26:52 +0200 Subject: [PATCH 11/26] mtu 0.2.0 --- neqo-transport/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/neqo-transport/Cargo.toml b/neqo-transport/Cargo.toml index e3d84fbf53..57716d4954 100644 --- a/neqo-transport/Cargo.toml +++ b/neqo-transport/Cargo.toml @@ -22,7 +22,7 @@ indexmap = { version = "2.2", default-features = false } # See https://github.co log = { workspace = true } neqo-common = { path = "../neqo-common" } neqo-crypto = { path = "../neqo-crypto" } -mtu = { git = "https://github.com/mozilla/mtu.git", branch = "feat-from-routing", default-features = false } # TODO: Point to the crate on crates.io +mtu = { version = "0.2", default-features = false } qlog = { workspace = true } smallvec = { version = "1.13", default-features = false } static_assertions = { workspace = true } From 0822ea67fd3f7e6705d9420470e9112846bb4f39 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 28 Nov 2024 15:36:05 +0200 Subject: [PATCH 12/26] Add stats for interface and path MTU --- neqo-transport/src/connection/mod.rs | 5 +++- neqo-transport/src/connection/tests/mod.rs | 1 + neqo-transport/src/path.rs | 10 ++++++-- neqo-transport/src/pmtud.rs | 27 ++++++++++++---------- neqo-transport/src/recovery/mod.rs | 4 +++- neqo-transport/src/stats.rs | 13 +++++++++-- 6 files changed, 42 insertions(+), 18 deletions(-) diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index e41750a62c..f32131293b 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -337,6 +337,7 @@ impl Connection { c.conn_params.pacing_enabled(), NeqoQlog::default(), now, + &mut c.stats.borrow_mut(), ); c.setup_handshake_path(&Rc::new(RefCell::new(path)), now); Ok(c) @@ -1553,6 +1554,7 @@ impl Connection { self.conn_params.get_cc_algorithm(), self.conn_params.pacing_enabled(), now, + &mut self.stats.borrow_mut(), ); path.borrow_mut().add_received(d.len()); let res = self.input_path(&path, d, received); @@ -1924,6 +1926,7 @@ impl Connection { self.conn_params.get_cc_algorithm(), self.conn_params.pacing_enabled(), now, + &mut self.stats.borrow_mut(), ); self.ensure_permanent(&path, now)?; qinfo!( @@ -2884,7 +2887,7 @@ impl Connection { .ok_or(Error::InternalError)? .borrow_mut() .pmtud_mut() - .start(now); + .start(now, &mut self.stats.borrow_mut()); } Ok(()) } diff --git a/neqo-transport/src/connection/tests/mod.rs b/neqo-transport/src/connection/tests/mod.rs index e897b7082b..5accfab36d 100644 --- a/neqo-transport/src/connection/tests/mod.rs +++ b/neqo-transport/src/connection/tests/mod.rs @@ -690,6 +690,7 @@ fn assert_path_challenge_min_len(c: &Connection, d: &Datagram, now: Instant) { c.conn_params.get_cc_algorithm(), c.conn_params.pacing_enabled(), now, + &mut c.stats.borrow_mut(), ); if path.borrow().amplification_limit() < path.borrow().plpmtu() { // If the amplification limit is less than the PLPMTU, then the path diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs index f9e1e13039..1bb747d3b9 100644 --- a/neqo-transport/src/path.rs +++ b/neqo-transport/src/path.rs @@ -81,6 +81,7 @@ impl Paths { cc: CongestionControlAlgorithm, pacing: bool, now: Instant, + stats: &mut Stats, ) -> PathRef { self.paths .iter() @@ -92,7 +93,8 @@ impl Paths { } }) .unwrap_or_else(|| { - let mut p = Path::temporary(local, remote, cc, pacing, self.qlog.clone(), now); + let mut p = + Path::temporary(local, remote, cc, pacing, self.qlog.clone(), now, stats); if let Some(primary) = self.primary.as_ref() { p.prime_rtt(primary.borrow().rtt()); } @@ -255,7 +257,9 @@ impl Paths { } else { // See if the PMTUD raise timer wants to fire. if let Some(path) = self.primary() { - path.borrow_mut().pmtud_mut().maybe_fire_raise_timer(now); + path.borrow_mut() + .pmtud_mut() + .maybe_fire_raise_timer(now, stats); } true } @@ -530,10 +534,12 @@ impl Path { pacing: bool, qlog: NeqoQlog, now: Instant, + stats: &mut Stats, ) -> Self { let iface_mtu = match mtu::interface_and_mtu(remote.ip()) { Ok((name, mtu)) => { qdebug!("Outbound interface {name} has MTU {mtu}"); + stats.pmtud_iface_mtu = mtu; mtu } Err(e) => { diff --git a/neqo-transport/src/pmtud.rs b/neqo-transport/src/pmtud.rs index 633ead05d2..606fa6130d 100644 --- a/neqo-transport/src/pmtud.rs +++ b/neqo-transport/src/pmtud.rs @@ -89,11 +89,11 @@ impl Pmtud { } /// Checks whether the PMTUD raise timer should be fired, and does so if needed. - pub fn maybe_fire_raise_timer(&mut self, now: Instant) { + pub fn maybe_fire_raise_timer(&mut self, now: Instant, stats: &mut Stats) { if self.probe_state == Probe::NotNeeded && self.raise_timer.is_some_and(|t| now >= t) { qdebug!("PMTUD raise timer fired"); self.raise_timer = None; - self.start(now); + self.start(now, stats); } } @@ -182,15 +182,17 @@ impl Pmtud { // total number of successful probes. stats.pmtud_ack += acked; self.mtu = self.search_table[self.probe_index]; + stats.pmtud_pmtu = self.mtu; qdebug!("PMTUD probe of size {} succeeded", self.mtu); - self.start(now); + self.start(now, stats); } /// Stops the PMTUD process, setting the MTU to the largest successful probe size. - fn stop(&mut self, idx: usize, now: Instant) { + fn stop(&mut self, idx: usize, now: Instant, stats: &mut Stats) { self.probe_state = Probe::NotNeeded; // We don't need to send any more probes self.probe_index = idx; // Index of the last successful probe self.mtu = self.search_table[idx]; // Leading to this MTU + stats.pmtud_pmtu = self.mtu; self.probe_count = 0; // Reset the count self.loss_counts.fill(0); // Reset the loss counts self.raise_timer = Some(now + PMTU_RAISE_TIMER); @@ -281,11 +283,11 @@ impl Pmtud { // start from scratch, but we don't strictly need to do that. self.reset(stats); qdebug!("PMTUD reset and restarting, PLPMTU is now {}", self.mtu); - self.start(now); + self.start(now, stats); } else { // We saw multiple losses of packets > the current MTU during PMTU discovery, so // we're done. - self.stop(last_ok, now); + self.stop(last_ok, now, stats); } } @@ -293,13 +295,14 @@ impl Pmtud { fn reset(&mut self, stats: &mut Stats) { self.probe_index = 0; self.mtu = self.search_table[self.probe_index]; + stats.pmtud_pmtu = self.mtu; self.loss_counts.fill(0); self.raise_timer = None; stats.pmtud_change += 1; } /// Starts the next upward PMTUD probe. - pub fn start(&mut self, now: Instant) { + pub fn start(&mut self, now: Instant, stats: &mut Stats) { if self.probe_index < SEARCH_TABLE_LEN - 1 // Not at the end of the search table // Next size is <= iface MTU && self.search_table[self.probe_index + 1] <= self.iface_mtu @@ -313,7 +316,7 @@ impl Pmtud { ); } else { // If we're at the end of the search table or hit the local interface MTU, we're done. - self.stop(self.probe_index, now); + self.stop(self.probe_index, now, stats); } } @@ -411,7 +414,7 @@ mod tests { let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); - pmtud.start(now); + pmtud.start(now, &mut stats); assert!(pmtud.needs_probe()); while pmtud.needs_probe() { @@ -450,7 +453,7 @@ mod tests { let mut prot = CryptoDxState::test_default(); assert!(smaller_mtu >= pmtud.search_table[0]); - pmtud.start(now); + pmtud.start(now, &mut stats); assert!(pmtud.needs_probe()); while pmtud.needs_probe() { @@ -503,7 +506,7 @@ mod tests { let mut prot = CryptoDxState::test_default(); assert!(larger_mtu >= pmtud.search_table[0]); - pmtud.start(now); + pmtud.start(now, &mut stats); assert!(pmtud.needs_probe()); while pmtud.needs_probe() { @@ -513,7 +516,7 @@ mod tests { qdebug!("Increasing MTU to {}", larger_mtu); let now = now + PMTU_RAISE_TIMER; - pmtud.maybe_fire_raise_timer(now); + pmtud.maybe_fire_raise_timer(now, &mut stats); while pmtud.needs_probe() { pmtud_step(&mut pmtud, &mut stats, &mut prot, addr, larger_mtu, now); } diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs index 8f89b1cfcd..4879750a70 100644 --- a/neqo-transport/src/recovery/mod.rs +++ b/neqo-transport/src/recovery/mod.rs @@ -1050,6 +1050,7 @@ mod tests { impl Default for Fixture { fn default() -> Self { const CC: CongestionControlAlgorithm = CongestionControlAlgorithm::NewReno; + let stats = StatsCell::default(); let mut path = Path::temporary( DEFAULT_ADDR, DEFAULT_ADDR, @@ -1057,6 +1058,7 @@ mod tests { true, NeqoQlog::default(), now(), + &mut stats.borrow_mut(), ); path.make_permanent( None, @@ -1065,7 +1067,7 @@ mod tests { path.set_primary(true, now()); path.rtt_mut().set_initial(TEST_RTT); Self { - lr: LossRecovery::new(StatsCell::default(), FAST_PTO_SCALE), + lr: LossRecovery::new(stats, FAST_PTO_SCALE), path: Rc::new(RefCell::new(path)), } } diff --git a/neqo-transport/src/stats.rs b/neqo-transport/src/stats.rs index a8c35c1c66..b88a92abc4 100644 --- a/neqo-transport/src/stats.rs +++ b/neqo-transport/src/stats.rs @@ -168,6 +168,10 @@ pub struct Stats { pub pmtud_lost: usize, /// Number of times a path MTU changed unexpectedly. pub pmtud_change: usize, + /// MTU of the local interface used for the current path. + pub pmtud_iface_mtu: usize, + /// Probed PMTU of the current path. + pub pmtud_pmtu: usize, /// Whether the connection was resumed successfully. pub resumed: bool, @@ -261,8 +265,13 @@ impl Debug for Stats { )?; writeln!( f, - " pmtud: {} sent {} acked {} lost {} change", - self.pmtud_tx, self.pmtud_ack, self.pmtud_lost, self.pmtud_change + " pmtud: {} sent {} acked {} lost {} change {} iface_mtu {} pmtu", + self.pmtud_tx, + self.pmtud_ack, + self.pmtud_lost, + self.pmtud_change, + self.pmtud_iface_mtu, + self.pmtud_pmtu )?; writeln!(f, " resumed: {}", self.resumed)?; writeln!(f, " frames rx:")?; From 8900893c27d1053c1f22d58c685520b7622bebd9 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 28 Nov 2024 15:39:51 +0200 Subject: [PATCH 13/26] Cargo.lock --- Cargo.lock | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 146 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e619126696..95d7939d31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -83,12 +83,15 @@ dependencies = [ "itertools", "lazy_static", "lazycell", + "log", + "prettyplease", "proc-macro2", "quote", "regex", "rustc-hash", "shlex", "syn", + "which", ] [[package]] @@ -257,6 +260,31 @@ dependencies = [ "itertools", ] +[[package]] +name = "crossbeam-deque" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "613f8cc01fe9cf1a3eb3d7f488fd2fa8388403e97039e2f73692932e291a770d" +dependencies = [ + "crossbeam-epoch", + "crossbeam-utils", +] + +[[package]] +name = "crossbeam-epoch" +version = "0.9.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +dependencies = [ + "crossbeam-utils", +] + +[[package]] +name = "crossbeam-utils" +version = "0.8.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22ec99545bb0ed0ea7bb9b8e1e9122ea386ff8a48c0922e43f36d45ab09e0e80" + [[package]] name = "crunchy" version = "0.2.2" @@ -377,6 +405,16 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" +[[package]] +name = "errno" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" +dependencies = [ + "libc", + "windows-sys", +] + [[package]] name = "fnv" version = "1.0.7" @@ -516,6 +554,15 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "home" +version = "0.5.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3d1354bf6b7235cb4a0576c2619fd4ed18183f689b12b006a0ee7329eeff9a5" +dependencies = [ + "windows-sys", +] + [[package]] name = "icu_collections" version = "1.5.0" @@ -720,9 +767,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.158" +version = "0.2.166" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8adc4bb1803a324070e64a98ae98f38934d91957a99cfb3a43dcbc01bc56439" +checksum = "c2ccc108bbc0b1331bd061864e7cd823c0cab660bbe6970e66e2c0614decde36" [[package]] name = "libfuzzer-sys" @@ -744,6 +791,12 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "linux-raw-sys" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" + [[package]] name = "litemap" version = "0.7.3" @@ -795,6 +848,21 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "903970ae2f248d7275214cf8f387f8ba0c4ea7e3d87a320e85493db60ce28616" +[[package]] +name = "mtu" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20fc0487be022a57be172b2ce106f21c07e3a9bdce268158f7b08745799b1518" +dependencies = [ + "bindgen", + "cfg_aliases", + "libc", + "static_assertions", + "windows-bindgen", + "windows-core", + "windows-targets", +] + [[package]] name = "neqo-bin" version = "0.11.0" @@ -882,6 +950,7 @@ dependencies = [ "enum-map", "indexmap", "log", + "mtu", "neqo-common", "neqo-crypto", "qlog", @@ -957,6 +1026,16 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "prettyplease" +version = "0.2.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "64d1ec885c64d0457d564db4ec299b2dae3f9c02808b8ad9c3a089c591b18033" +dependencies = [ + "proc-macro2", + "syn", +] + [[package]] name = "proc-macro2" version = "1.0.86" @@ -1002,6 +1081,26 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rayon" +version = "1.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b418a60154510ca1a002a752ca9714984e21e4241e804d32555251faf8b78ffa" +dependencies = [ + "either", + "rayon-core", +] + +[[package]] +name = "rayon-core" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1465873a3dfdaa8ae7cb14b4383657caab0b3e8a0aa9ae8e04b044854c8dfce2" +dependencies = [ + "crossbeam-deque", + "crossbeam-utils", +] + [[package]] name = "regex" version = "1.9.4" @@ -1053,6 +1152,19 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" +[[package]] +name = "rustix" +version = "0.38.41" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7f649912bc1495e167a6edee79151c84b1bad49748cb4f1f1167f459f6224f6" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys", +] + [[package]] name = "ryu" version = "1.0.12" @@ -1317,6 +1429,18 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +[[package]] +name = "which" +version = "4.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87ba24419a2078cd2b0f2ede2691b6c66d8e47836da3b6db8265ebad47afbfc7" +dependencies = [ + "either", + "home", + "once_cell", + "rustix", +] + [[package]] name = "winapi" version = "0.3.9" @@ -1358,6 +1482,20 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "windows-bindgen" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91cd28d93c692351f3a6e5615567c56756e330bee1c99c6bdd57bfc5ab15f589" +dependencies = [ + "proc-macro2", + "rayon", + "serde", + "serde_json", + "syn", + "windows-metadata", +] + [[package]] name = "windows-core" version = "0.58.0" @@ -1393,6 +1531,12 @@ dependencies = [ "syn", ] +[[package]] +name = "windows-metadata" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e837f3c3012cfe9e7086302a93f441a7999439be1ad4c530d55d2f6d2921809" + [[package]] name = "windows-result" version = "0.2.0" From 8e8e81b65ca690eb827bb2a05caa9d0d871ac200 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 29 Nov 2024 10:14:25 +0200 Subject: [PATCH 14/26] Not needed now that `mtu@0.2` is published --- .github/workflows/firefox.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/firefox.yml b/.github/workflows/firefox.yml index a2200a8a09..c7b7507f09 100644 --- a/.github/workflows/firefox.yml +++ b/.github/workflows/firefox.yml @@ -102,15 +102,7 @@ jobs: echo 'who = "CI"' echo 'criteria = "safe-to-deploy"' echo "version = \"$MTU_VERSION\"" - echo '[[audits.mtu]]' - echo 'who = "CI"' - echo 'criteria = "safe-to-deploy"' - echo "version = \"0.2.0@git:9a8b72d3eebf04cc5accbddbbef3e3603dcf943f\"" } >> supply-chain/audits.toml - { - echo '[policy.mtu]' - echo 'audit-as-crates-io = true' - } >> supply-chain/config.toml sed -i'' -e "s/qlog =.*/qlog = \"$QLOG_VERSION\"/" netwerk/socket/neqo_glue/Cargo.toml { echo '[patch."https://github.com/mozilla/neqo"]' From 126812e7035e8f24f000331df35bd3c1280f0ce3 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 29 Nov 2024 10:33:12 +0200 Subject: [PATCH 15/26] Bump `mtu` crate --- Cargo.lock | 67 ++------------------------------------- neqo-transport/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 95d7939d31..e5c5245180 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -83,15 +83,12 @@ dependencies = [ "itertools", "lazy_static", "lazycell", - "log", - "prettyplease", "proc-macro2", "quote", "regex", "rustc-hash", "shlex", "syn", - "which", ] [[package]] @@ -405,16 +402,6 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" -[[package]] -name = "errno" -version = "0.3.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" -dependencies = [ - "libc", - "windows-sys", -] - [[package]] name = "fnv" version = "1.0.7" @@ -554,15 +541,6 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" -[[package]] -name = "home" -version = "0.5.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e3d1354bf6b7235cb4a0576c2619fd4ed18183f689b12b006a0ee7329eeff9a5" -dependencies = [ - "windows-sys", -] - [[package]] name = "icu_collections" version = "1.5.0" @@ -791,12 +769,6 @@ dependencies = [ "windows-targets", ] -[[package]] -name = "linux-raw-sys" -version = "0.4.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" - [[package]] name = "litemap" version = "0.7.3" @@ -850,9 +822,9 @@ checksum = "903970ae2f248d7275214cf8f387f8ba0c4ea7e3d87a320e85493db60ce28616" [[package]] name = "mtu" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "20fc0487be022a57be172b2ce106f21c07e3a9bdce268158f7b08745799b1518" +checksum = "66d84f795628ce972acc04d13f58727df10e93486b3d4916ee71b34c1f6430d9" dependencies = [ "bindgen", "cfg_aliases", @@ -1026,16 +998,6 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" -[[package]] -name = "prettyplease" -version = "0.2.25" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "64d1ec885c64d0457d564db4ec299b2dae3f9c02808b8ad9c3a089c591b18033" -dependencies = [ - "proc-macro2", - "syn", -] - [[package]] name = "proc-macro2" version = "1.0.86" @@ -1152,19 +1114,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" -[[package]] -name = "rustix" -version = "0.38.41" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7f649912bc1495e167a6edee79151c84b1bad49748cb4f1f1167f459f6224f6" -dependencies = [ - "bitflags", - "errno", - "libc", - "linux-raw-sys", - "windows-sys", -] - [[package]] name = "ryu" version = "1.0.12" @@ -1429,18 +1378,6 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" -[[package]] -name = "which" -version = "4.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87ba24419a2078cd2b0f2ede2691b6c66d8e47836da3b6db8265ebad47afbfc7" -dependencies = [ - "either", - "home", - "once_cell", - "rustix", -] - [[package]] name = "winapi" version = "0.3.9" diff --git a/neqo-transport/Cargo.toml b/neqo-transport/Cargo.toml index 367062f108..da125c074a 100644 --- a/neqo-transport/Cargo.toml +++ b/neqo-transport/Cargo.toml @@ -22,7 +22,7 @@ indexmap = { version = "2.2", default-features = false } # See https://github.co log = { workspace = true } neqo-common = { path = "../neqo-common" } neqo-crypto = { path = "../neqo-crypto" } -mtu = { version = "0.2", default-features = false } +mtu = { version = "0.2.1", default-features = false } # neqo is only user currently, can bump freely qlog = { workspace = true } smallvec = { version = "1.13", default-features = false } static_assertions = { workspace = true } From ba79ce19d3387f5eb7caa760aee4f80bf5a28507 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 29 Nov 2024 11:12:00 +0200 Subject: [PATCH 16/26] Trust windows-bindgen --- .github/workflows/firefox.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/firefox.yml b/.github/workflows/firefox.yml index c7b7507f09..c1d58a372a 100644 --- a/.github/workflows/firefox.yml +++ b/.github/workflows/firefox.yml @@ -112,6 +112,13 @@ jobs: echo 'neqo-qpack = { path = "../neqo-qpack" }' echo 'neqo-crypto = { path = "../neqo-crypto" }' } >> Cargo.toml + { + echo '[[trusted.windows-bindgen]]' + echo 'criteria = "safe-to-deploy"' + echo 'user-id = 64539 # Kenny Kerr (kennykerr)' + echo 'start = "2021-11-15"' + echo 'end = "2024-09-12"' + } >> supply-chain/audits.toml cargo update neqo-http3 neqo-transport neqo-common neqo-qpack neqo-crypto ./mach vendor rust --ignore-modified From 20c1838d2f4dee49ce726a30588c8868e1e0fd0b Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 29 Nov 2024 11:21:22 +0200 Subject: [PATCH 17/26] `windows-metadata` --- .github/workflows/firefox.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/firefox.yml b/.github/workflows/firefox.yml index c1d58a372a..a5ef3247f2 100644 --- a/.github/workflows/firefox.yml +++ b/.github/workflows/firefox.yml @@ -118,6 +118,11 @@ jobs: echo 'user-id = 64539 # Kenny Kerr (kennykerr)' echo 'start = "2021-11-15"' echo 'end = "2024-09-12"' + echo '[[trusted.windows-metadata]]' + echo 'criteria = "safe-to-deploy"' + echo 'user-id = 64539 # Kenny Kerr (kennykerr)' + echo 'start = "2021-11-15"' + echo 'end = "2024-09-12"' } >> supply-chain/audits.toml cargo update neqo-http3 neqo-transport neqo-common neqo-qpack neqo-crypto ./mach vendor rust --ignore-modified From 988459cf2a62eeddfe018843e4cbd81a5af63ac7 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 29 Nov 2024 12:40:21 +0200 Subject: [PATCH 18/26] mtu@0.2.2 --- Cargo.lock | 4 ++-- neqo-transport/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5c5245180..c971a7e2d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -822,9 +822,9 @@ checksum = "903970ae2f248d7275214cf8f387f8ba0c4ea7e3d87a320e85493db60ce28616" [[package]] name = "mtu" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66d84f795628ce972acc04d13f58727df10e93486b3d4916ee71b34c1f6430d9" +checksum = "921122c46b7e9b0d37dbfa97c2dfc2913ba8ef368fb228b67f5d813cf438f6e8" dependencies = [ "bindgen", "cfg_aliases", diff --git a/neqo-transport/Cargo.toml b/neqo-transport/Cargo.toml index da125c074a..265c411f67 100644 --- a/neqo-transport/Cargo.toml +++ b/neqo-transport/Cargo.toml @@ -22,7 +22,7 @@ indexmap = { version = "2.2", default-features = false } # See https://github.co log = { workspace = true } neqo-common = { path = "../neqo-common" } neqo-crypto = { path = "../neqo-crypto" } -mtu = { version = "0.2.1", default-features = false } # neqo is only user currently, can bump freely +mtu = { version = "0.2.2", default-features = false } # neqo is only user currently, can bump freely qlog = { workspace = true } smallvec = { version = "1.13", default-features = false } static_assertions = { workspace = true } From ede84e3ed6f13bf26b83ab80e73f2ea2623a422e Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Mon, 2 Dec 2024 09:14:24 +0200 Subject: [PATCH 19/26] mtu@0.2.3 --- Cargo.lock | 5 ++--- neqo-transport/Cargo.toml | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c971a7e2d5..1babd7027c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -822,13 +822,12 @@ checksum = "903970ae2f248d7275214cf8f387f8ba0c4ea7e3d87a320e85493db60ce28616" [[package]] name = "mtu" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "921122c46b7e9b0d37dbfa97c2dfc2913ba8ef368fb228b67f5d813cf438f6e8" +version = "0.2.3" dependencies = [ "bindgen", "cfg_aliases", "libc", + "mozbuild", "static_assertions", "windows-bindgen", "windows-core", diff --git a/neqo-transport/Cargo.toml b/neqo-transport/Cargo.toml index 265c411f67..f49f776b12 100644 --- a/neqo-transport/Cargo.toml +++ b/neqo-transport/Cargo.toml @@ -22,7 +22,7 @@ indexmap = { version = "2.2", default-features = false } # See https://github.co log = { workspace = true } neqo-common = { path = "../neqo-common" } neqo-crypto = { path = "../neqo-crypto" } -mtu = { version = "0.2.2", default-features = false } # neqo is only user currently, can bump freely +mtu = { version = "0.2.3", default-features = false } # neqo is only user currently, can bump freely qlog = { workspace = true } smallvec = { version = "1.13", default-features = false } static_assertions = { workspace = true } @@ -40,6 +40,7 @@ build-fuzzing-corpus = [ "test-fixture/disable-random", ] disable-encryption = ["neqo-crypto/disable-encryption"] +gecko = ["mtu/gecko"] [lib] # See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options From 74d08b71c2031f14b176ca4bf843c263377bdd8e Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Tue, 17 Dec 2024 13:16:32 +0200 Subject: [PATCH 20/26] Deps --- Cargo.lock | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 37abf0f0b8..927ed66a73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -745,9 +745,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.166" +version = "0.2.158" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c2ccc108bbc0b1331bd061864e7cd823c0cab660bbe6970e66e2c0614decde36" +checksum = "d8adc4bb1803a324070e64a98ae98f38934d91957a99cfb3a43dcbc01bc56439" [[package]] name = "libfuzzer-sys" @@ -823,6 +823,8 @@ checksum = "903970ae2f248d7275214cf8f387f8ba0c4ea7e3d87a320e85493db60ce28616" [[package]] name = "mtu" version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8e76af2be1c20282d2b7290590552a58bcfad00937fa8ac6f240202e76a0dae" dependencies = [ "bindgen", "cfg_aliases", From 03c4684df1fcfb31540c644f5fa149fc51ea21d1 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 18 Dec 2024 13:16:39 +0200 Subject: [PATCH 21/26] Update Cargo.lock Co-authored-by: Max Inden Signed-off-by: Lars Eggert --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 05eb89bdfa..1b28a27f1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -259,7 +259,7 @@ dependencies = [ [[package]] name = "crossbeam-deque" -version = "0.8.5" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "613f8cc01fe9cf1a3eb3d7f488fd2fa8388403e97039e2f73692932e291a770d" dependencies = [ From 8004d36f9d5cb6c14ae48228617e90449d035392 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 18 Dec 2024 13:16:46 +0200 Subject: [PATCH 22/26] Update Cargo.lock Co-authored-by: Max Inden Signed-off-by: Lars Eggert --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 1b28a27f1e..4a8d94581d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -269,7 +269,7 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.18" +version = "0.9.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" dependencies = [ From 639586e8f064768cc921e15265f89f0fd7addf23 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 18 Dec 2024 13:17:07 +0200 Subject: [PATCH 23/26] Update neqo-transport/src/stats.rs Co-authored-by: Max Inden Signed-off-by: Lars Eggert --- neqo-transport/src/stats.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/neqo-transport/src/stats.rs b/neqo-transport/src/stats.rs index 5922c355a6..33fdc10c2e 100644 --- a/neqo-transport/src/stats.rs +++ b/neqo-transport/src/stats.rs @@ -171,7 +171,7 @@ pub struct Stats { pub pmtud_lost: usize, /// Number of times a path MTU changed unexpectedly. pub pmtud_change: usize, - /// MTU of the local interface used for the current path. + /// MTU of the local interface used for the most recent path. pub pmtud_iface_mtu: usize, /// Probed PMTU of the current path. pub pmtud_pmtu: usize, From aa164dbcb499f7de406e7b91b4d1be138a1154f7 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 18 Dec 2024 16:04:41 +0200 Subject: [PATCH 24/26] Fixes --- neqo-transport/src/cc/classic_cc.rs | 11 +++-------- neqo-transport/src/cc/tests/mod.rs | 6 +++--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/neqo-transport/src/cc/classic_cc.rs b/neqo-transport/src/cc/classic_cc.rs index e9b29550f6..57e7a2b1f4 100644 --- a/neqo-transport/src/cc/classic_cc.rs +++ b/neqo-transport/src/cc/classic_cc.rs @@ -604,10 +604,7 @@ impl ClassicCongestionControl { #[cfg(test)] mod tests { - use std::{ - net::{IpAddr, Ipv4Addr}, - time::{Duration, Instant}, - }; + use std::time::{Duration, Instant}; use neqo_common::{qinfo, IpTosEcn}; use test_fixture::now; @@ -618,6 +615,7 @@ mod tests { classic_cc::State, cubic::{Cubic, CUBIC_BETA_USIZE_DIVIDEND, CUBIC_BETA_USIZE_DIVISOR}, new_reno::NewReno, + tests::{IP_ADDR, MTU, RTT}, CongestionControl, CongestionControlAlgorithm, CWND_INITIAL_PKTS, }, packet::{PacketNumber, PacketType}, @@ -626,10 +624,7 @@ mod tests { Pmtud, }; - const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED); - const MTU: usize = 1_500; - const PTO: Duration = Duration::from_millis(100); - const RTT: Duration = Duration::from_millis(98); + const PTO: Duration = RTT; const RTT_ESTIMATE: RttEstimate = RttEstimate::from_duration(RTT); const ZERO: Duration = Duration::from_secs(0); const EPSILON: Duration = Duration::from_nanos(1); diff --git a/neqo-transport/src/cc/tests/mod.rs b/neqo-transport/src/cc/tests/mod.rs index fc9d10e3a8..7abcb5190c 100644 --- a/neqo-transport/src/cc/tests/mod.rs +++ b/neqo-transport/src/cc/tests/mod.rs @@ -12,6 +12,6 @@ use std::{ mod cubic; mod new_reno; -const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED); -const MTU: usize = 1_500; -const RTT: Duration = Duration::from_millis(100); +pub const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED); +pub const MTU: usize = 1_500; +pub const RTT: Duration = Duration::from_millis(100); From fdebdb1a61b88c0a5cdb773d12dad722bcd586b9 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 18 Dec 2024 16:17:13 +0200 Subject: [PATCH 25/26] Cargo.lock --- Cargo.lock | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4a8d94581d..c104e7a109 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -261,8 +261,9 @@ dependencies = [ name = "crossbeam-deque" version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "613f8cc01fe9cf1a3eb3d7f488fd2fa8388403e97039e2f73692932e291a770d" +checksum = "715e8152b692bba2d374b53d4875445368fdf21a94751410af607a5ac677d1fc" dependencies = [ + "cfg-if", "crossbeam-epoch", "crossbeam-utils", ] @@ -271,9 +272,13 @@ dependencies = [ name = "crossbeam-epoch" version = "0.9.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +checksum = "46bd5f3f85273295a9d14aedfb86f6aadbff6d8f5295c4a9edb08e819dcf5695" dependencies = [ + "autocfg", + "cfg-if", "crossbeam-utils", + "memoffset", + "scopeguard", ] [[package]] @@ -787,6 +792,15 @@ version = "2.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" +[[package]] +name = "memoffset" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d61c719bcfbcf5d62b3a09efa6088de8c54bc0bfcd3ea7ae39fcc186108b8de1" +dependencies = [ + "autocfg", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -1135,6 +1149,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "scopeguard" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" + [[package]] name = "semver" version = "1.0.16" From 2b85cfb71e02460d99c13c51e5c65fc9a302bddb Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 18 Dec 2024 16:38:31 +0200 Subject: [PATCH 26/26] Address more code review comments --- neqo-transport/src/cc/tests/mod.rs | 2 +- neqo-transport/src/path.rs | 4 ++-- neqo-transport/src/pmtud.rs | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/neqo-transport/src/cc/tests/mod.rs b/neqo-transport/src/cc/tests/mod.rs index 7abcb5190c..45bed8fd88 100644 --- a/neqo-transport/src/cc/tests/mod.rs +++ b/neqo-transport/src/cc/tests/mod.rs @@ -13,5 +13,5 @@ mod cubic; mod new_reno; pub const IP_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED); -pub const MTU: usize = 1_500; +pub const MTU: Option = Some(1_500); pub const RTT: Duration = Duration::from_millis(100); diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs index ed8430cf08..c6fe207375 100644 --- a/neqo-transport/src/path.rs +++ b/neqo-transport/src/path.rs @@ -540,11 +540,11 @@ impl Path { Ok((name, mtu)) => { qdebug!("Outbound interface {name} has MTU {mtu}"); stats.pmtud_iface_mtu = mtu; - mtu + Some(mtu) } Err(e) => { qwarn!("Failed to determine outbound interface: {e}"); - usize::MAX + None } }; let mut sender = PacketSender::new(cc, pacing, Pmtud::new(remote.ip(), iface_mtu), now); diff --git a/neqo-transport/src/pmtud.rs b/neqo-transport/src/pmtud.rs index 606fa6130d..fd40f92699 100644 --- a/neqo-transport/src/pmtud.rs +++ b/neqo-transport/src/pmtud.rs @@ -72,14 +72,14 @@ impl Pmtud { } #[must_use] - pub const fn new(remote_ip: IpAddr, iface_mtu: usize) -> Self { + pub fn new(remote_ip: IpAddr, iface_mtu: Option) -> Self { let search_table = Self::search_table(remote_ip); let probe_index = 0; Self { search_table, header_size: Self::header_size(remote_ip), mtu: search_table[probe_index], - iface_mtu, + iface_mtu: iface_mtu.unwrap_or(usize::MAX), probe_index, probe_count: 0, probe_state: Probe::NotNeeded, @@ -410,7 +410,7 @@ mod tests { fn find_pmtu(addr: IpAddr, mtu: usize) { fixture_init(); let now = now(); - let mut pmtud = Pmtud::new(addr, mtu); + let mut pmtud = Pmtud::new(addr, Some(mtu)); let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); @@ -448,7 +448,7 @@ mod tests { fixture_init(); let now = now(); - let mut pmtud = Pmtud::new(addr, mtu); + let mut pmtud = Pmtud::new(addr, Some(mtu)); let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); @@ -501,7 +501,7 @@ mod tests { fixture_init(); let now = now(); - let mut pmtud = Pmtud::new(addr, larger_mtu); + let mut pmtud = Pmtud::new(addr, Some(larger_mtu)); let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); @@ -573,7 +573,7 @@ mod tests { #[test] fn pmtud_on_packets_lost() { let now = now(); - let mut pmtud = Pmtud::new(V4, 1500); + let mut pmtud = Pmtud::new(V4, Some(1500)); let mut stats = Stats::default(); // No packets lost, nothing should change. @@ -641,7 +641,7 @@ mod tests { #[test] fn pmtud_on_packets_lost_and_acked() { let now = now(); - let mut pmtud = Pmtud::new(V4, 1500); + let mut pmtud = Pmtud::new(V4, Some(1500)); let mut stats = Stats::default(); // A packet of size 100 was ACKed, which is smaller than all probe sizes.