From 9b3ccb9fc0cca6604e9da1fb121b1ee2735339fe Mon Sep 17 00:00:00 2001 From: ali-molajani Date: Sat, 6 Dec 2025 19:51:00 +0330 Subject: [PATCH 1/2] Integrate Mozilla's mtu crate to constrain MTU discovery This change layers interface MTU detection on top of user configuration rather than replacing it. When creating client connections, we query the interface MTU for the destination IP using Mozilla's mtu crate and apply it as an additional constraint on the MTU discovery upper bound. Key changes: - Add mtu crate dependency to quinn - Query interface MTU when creating client connections in quinn layer - Pass interface MTU constraint through to quinn-proto layer - Apply constraint in MTU discovery SearchState, respecting user config and peer limits (effective_upper_bound = min(user_bound, interface_mtu, peer_limit)) - Add tests for interface MTU constraint behavior The constraint is applied as an additional limit, so user configuration takes precedence if it's more restrictive, maintaining backward compatibility. --- Cargo.lock | 146 ++++++++++++++++++++++++++-- quinn-proto/src/connection/mod.rs | 4 +- quinn-proto/src/connection/mtud.rs | 71 +++++++++++--- quinn-proto/src/connection/paths.rs | 2 + quinn-proto/src/endpoint.rs | 5 + quinn/Cargo.toml | 1 + quinn/src/endpoint.rs | 15 ++- 7 files changed, 220 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a6e1d21b71..f1253f45f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -242,7 +242,7 @@ version = "0.13.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57900537c00a0565a35b63c4c281b372edfc9744b072fd4a3b414350a8f5ed48" dependencies = [ - "bindgen", + "bindgen 0.72.1", "cc", "cmake", "dunce", @@ -350,6 +350,26 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7dfdb4953a096c551ce9ace855a604d702e6e62d77fac690575ae347571717f5" +[[package]] +name = "bindgen" +version = "0.69.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "271383c67ccabffb7381723dea0672a673f292304fcb45c01cc648c7a8d58088" +dependencies = [ + "bitflags", + "cexpr", + "clang-sys", + "itertools 0.12.1", + "lazy_static", + "lazycell", + "proc-macro2", + "quote", + "regex", + "rustc-hash 1.1.0", + "shlex", + "syn", +] + [[package]] name = "bindgen" version = "0.72.1" @@ -365,7 +385,7 @@ dependencies = [ "proc-macro2", "quote", "regex", - "rustc-hash", + "rustc-hash 2.1.1", "shlex", "syn", ] @@ -1299,6 +1319,15 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" +[[package]] +name = "itertools" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba291022dbbd398a455acf126c1e341954079855bc60dfdda641363bd6922569" +dependencies = [ + "either", +] + [[package]] name = "itertools" version = "0.13.0" @@ -1371,6 +1400,12 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" +[[package]] +name = "lazycell" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" + [[package]] name = "libc" version = "0.2.177" @@ -1394,7 +1429,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" dependencies = [ "cfg-if", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -1510,6 +1545,19 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "mtu" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3e3124fb16267c5a77ce8cd015bcb5bb3f8aab28ec32d89d65f6a31c5c1306a0" +dependencies = [ + "bindgen 0.69.5", + "cfg_aliases", + "libc", + "static_assertions", + "windows", +] + [[package]] name = "nom" version = "7.1.3" @@ -1604,7 +1652,7 @@ dependencies = [ "libc", "redox_syscall", "smallvec", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -1801,12 +1849,13 @@ dependencies = [ "crc", "directories-next", "futures-io", + "mtu", "pin-project-lite", "quinn-proto", "quinn-udp", "rand", "rcgen", - "rustc-hash", + "rustc-hash 2.1.1", "rustls", "smol", "socket2", @@ -1836,7 +1885,7 @@ dependencies = [ "rand_pcg", "rcgen", "ring", - "rustc-hash", + "rustc-hash 2.1.1", "rustls", "rustls-pki-types", "rustls-platform-verifier", @@ -1992,6 +2041,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rustc-hash" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" + [[package]] name = "rustc-hash" version = "2.1.1" @@ -2297,6 +2352,12 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" +[[package]] +name = "static_assertions" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" + [[package]] name = "strsim" version = "0.11.1" @@ -2870,12 +2931,81 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.59.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f919aee0a93304be7f62e8e5027811bbba96bcb1de84d6618be56e43f8a32a1" +dependencies = [ + "windows-core", + "windows-targets 0.53.5", +] + +[[package]] +name = "windows-core" +version = "0.59.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "810ce18ed2112484b0d4e15d022e5f598113e220c53e373fb31e67e21670c1ce" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-result", + "windows-strings", + "windows-targets 0.53.5", +] + +[[package]] +name = "windows-implement" +version = "0.59.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "83577b051e2f49a058c308f17f273b570a6a758386fc291b5f6a934dd84e48c1" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "windows-interface" +version = "0.59.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "windows-link" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e6ad25900d524eaabdbbb96d20b4311e1e7ae1699af4fb28c17ae66c80d798a" + [[package]] name = "windows-link" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-result" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56f42bd332cc6c8eac5af113fc0c1fd6a8fd2aa08a0119358686e5160d0586c6" +dependencies = [ + "windows-link 0.1.3", +] + +[[package]] +name = "windows-strings" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87fa48cc5d406560701792be122a10132491cff9d0aeb23583cc2dcafc847319" +dependencies = [ + "windows-link 0.1.3", +] + [[package]] name = "windows-sys" version = "0.45.0" @@ -2909,7 +3039,7 @@ version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" dependencies = [ - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -2949,7 +3079,7 @@ version = "0.53.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4945f9f551b88e0d65f3db0bc25c33b8acea4d9e41163edf90dcd0b19f9069f3" dependencies = [ - "windows-link", + "windows-link 0.2.1", "windows_aarch64_gnullvm 0.53.1", "windows_aarch64_msvc 0.53.1", "windows_i686_gnu 0.53.1", diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 4053c78ab3..3fd16bdfc7 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -257,6 +257,7 @@ impl Connection { allow_mtud: bool, rng_seed: [u8; 32], side_args: SideArgs, + interface_mtu_constraint: Option, ) -> Self { let pref_addr_cid = side_args.pref_addr_cid(); let path_validated = side_args.path_validated(); @@ -283,7 +284,7 @@ impl Connection { now, if pref_addr_cid.is_some() { 2 } else { 1 }, ), - path: PathData::new(remote, allow_mtud, None, 0, now, &config), + path: PathData::new(remote, allow_mtud, None, 0, now, &config, interface_mtu_constraint), path_counter: 0, allow_mtud, local_ip, @@ -3122,6 +3123,7 @@ impl Connection { self.path_counter, now, &self.config, + None, // Interface MTU constraint not available for path migration ) }; new_path.challenge = Some(self.rng.random()); diff --git a/quinn-proto/src/connection/mtud.rs b/quinn-proto/src/connection/mtud.rs index 7e6fc1d8ae..12a0c65b53 100644 --- a/quinn-proto/src/connection/mtud.rs +++ b/quinn-proto/src/connection/mtud.rs @@ -21,6 +21,7 @@ impl MtuDiscovery { min_mtu: u16, peer_max_udp_payload_size: Option, config: MtuDiscoveryConfig, + interface_mtu_constraint: Option, ) -> Self { debug_assert!( initial_plpmtu >= min_mtu, @@ -30,7 +31,7 @@ impl MtuDiscovery { let mut mtud = Self::with_state( initial_plpmtu, min_mtu, - Some(EnabledMtuDiscovery::new(config)), + Some(EnabledMtuDiscovery::new(config, interface_mtu_constraint)), ); // We might be migrating an existing connection to a new path, in which case the transport @@ -59,7 +60,7 @@ impl MtuDiscovery { pub(super) fn reset(&mut self, current_mtu: u16, min_mtu: u16) { self.current_mtu = current_mtu; if let Some(state) = self.state.take() { - self.state = Some(EnabledMtuDiscovery::new(state.config)); + self.state = Some(EnabledMtuDiscovery::new(state.config, state.interface_mtu_constraint)); self.on_peer_max_udp_payload_size_received(state.peer_max_udp_payload_size); } self.black_hole_detector = BlackHoleDetector::new(min_mtu); @@ -173,14 +174,16 @@ struct EnabledMtuDiscovery { phase: Phase, peer_max_udp_payload_size: u16, config: MtuDiscoveryConfig, + interface_mtu_constraint: Option, } impl EnabledMtuDiscovery { - fn new(config: MtuDiscoveryConfig) -> Self { + fn new(config: MtuDiscoveryConfig, interface_mtu_constraint: Option) -> Self { Self { phase: Phase::Initial, peer_max_udp_payload_size: MAX_UDP_PAYLOAD, config, + interface_mtu_constraint, } } @@ -192,6 +195,7 @@ impl EnabledMtuDiscovery { current_mtu, self.peer_max_udp_payload_size, &self.config, + self.interface_mtu_constraint, )); } else if let Phase::Complete(next_mtud_activation) = &self.phase { if now < *next_mtud_activation { @@ -203,6 +207,7 @@ impl EnabledMtuDiscovery { current_mtu, self.peer_max_udp_payload_size, &self.config, + self.interface_mtu_constraint, )); } @@ -304,11 +309,16 @@ impl SearchState { mut lower_bound: u16, peer_max_udp_payload_size: u16, config: &MtuDiscoveryConfig, + interface_mtu_constraint: Option, ) -> Self { lower_bound = lower_bound.min(peer_max_udp_payload_size); - let upper_bound = config + // Apply interface MTU constraint as an additional limit on the upper bound + // This layers on top of user configuration rather than replacing it + let effective_upper_bound = config .upper_bound - .clamp(lower_bound, peer_max_udp_payload_size); + .min(interface_mtu_constraint.unwrap_or(u16::MAX)) + .min(peer_max_udp_payload_size); + let upper_bound = effective_upper_bound.clamp(lower_bound, peer_max_udp_payload_size); Self { in_flight_probe: None, @@ -530,7 +540,7 @@ mod tests { fn default_mtud() -> MtuDiscovery { let config = MtuDiscoveryConfig::default(); - MtuDiscovery::new(1_200, 1_200, None, config) + MtuDiscovery::new(1_200, 1_200, None, config, None) } fn completed(mtud: &MtuDiscovery) -> bool { @@ -655,7 +665,7 @@ mod tests { fn mtu_discovery_after_complete_reactivates_when_interval_elapsed() { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(9_000); - let mut mtud = MtuDiscovery::new(1_200, 1_200, None, config); + let mut mtud = MtuDiscovery::new(1_200, 1_200, None, config, None); let now = Instant::now(); drive_to_completion(&mut mtud, now, 1_500); @@ -724,7 +734,7 @@ mod tests { #[test] fn mtu_discovery_with_previous_peer_max_udp_payload_size_clamps_upper_bound() { - let mut mtud = MtuDiscovery::new(1500, 1_200, Some(1400), MtuDiscoveryConfig::default()); + let mut mtud = MtuDiscovery::new(1500, 1_200, Some(1400), MtuDiscoveryConfig::default(), None); assert_eq!(mtud.current_mtu, 1400); assert_eq!(mtud.state.as_ref().unwrap().peer_max_udp_payload_size, 1400); @@ -765,7 +775,7 @@ mod tests { fn mtu_discovery_with_1500_limit_and_10000_upper_bound() { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(10_000); - let mut mtud = MtuDiscovery::new(1_200, 1_200, None, config); + let mut mtud = MtuDiscovery::new(1_200, 1_200, None, config, None); let probed_sizes = drive_to_completion(&mut mtud, Instant::now(), 1500); @@ -782,7 +792,7 @@ mod tests { fn mtu_discovery_no_lost_probes_finds_maximum_udp_payload() { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(MAX_UDP_PAYLOAD); - let mut mtud = MtuDiscovery::new(1200, 1200, None, config); + let mut mtud = MtuDiscovery::new(1200, 1200, None, config, None); drive_to_completion(&mut mtud, Instant::now(), u16::MAX); @@ -794,7 +804,7 @@ mod tests { fn mtu_discovery_lost_half_of_probes_finds_maximum_udp_payload() { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(MAX_UDP_PAYLOAD); - let mut mtud = MtuDiscovery::new(1200, 1200, None, config); + let mut mtud = MtuDiscovery::new(1200, 1200, None, config, None); let now = Instant::now(); let mut iterations = 0; @@ -842,7 +852,7 @@ mod tests { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(1400); - let state = SearchState::new(1500, u16::MAX, &config); + let state = SearchState::new(1500, u16::MAX, &config, None); assert_eq!(state.lower_bound, 1500); assert_eq!(state.upper_bound, 1500); } @@ -852,7 +862,7 @@ mod tests { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(9000); - let state = SearchState::new(1500, 1300, &config); + let state = SearchState::new(1500, 1300, &config, None); assert_eq!(state.lower_bound, 1300); assert_eq!(state.upper_bound, 1300); } @@ -862,11 +872,44 @@ mod tests { let mut config = MtuDiscoveryConfig::default(); config.upper_bound(9000); - let state = SearchState::new(1200, 1450, &config); + let state = SearchState::new(1200, 1450, &config, None); assert_eq!(state.lower_bound, 1200); assert_eq!(state.upper_bound, 1450); } + #[test] + fn search_state_interface_mtu_constraint_clamps_upper_bound() { + let mut config = MtuDiscoveryConfig::default(); + config.upper_bound(9000); + + // Interface MTU constraint should limit the upper bound even if config allows higher + let state = SearchState::new(1200, u16::MAX, &config, Some(1300)); + assert_eq!(state.lower_bound, 1200); + assert_eq!(state.upper_bound, 1300); + } + + #[test] + fn search_state_interface_mtu_constraint_respects_user_config() { + let mut config = MtuDiscoveryConfig::default(); + config.upper_bound(1400); + + // User config should take precedence if it's more restrictive + let state = SearchState::new(1200, u16::MAX, &config, Some(1500)); + assert_eq!(state.lower_bound, 1200); + assert_eq!(state.upper_bound, 1400); + } + + #[test] + fn search_state_interface_mtu_constraint_respects_peer_limit() { + let mut config = MtuDiscoveryConfig::default(); + config.upper_bound(9000); + + // Peer limit should take precedence if it's most restrictive + let state = SearchState::new(1200, 1250, &config, Some(1500)); + assert_eq!(state.lower_bound, 1200); + assert_eq!(state.upper_bound, 1250); + } + // Loss of packets larger than have been acknowledged should indicate a black hole #[test] fn simple_black_hole_detection() { diff --git a/quinn-proto/src/connection/paths.rs b/quinn-proto/src/connection/paths.rs index 70582dfa2a..3cd44753bc 100644 --- a/quinn-proto/src/connection/paths.rs +++ b/quinn-proto/src/connection/paths.rs @@ -62,6 +62,7 @@ impl PathData { generation: u64, now: Instant, config: &TransportConfig, + interface_mtu_constraint: Option, ) -> Self { let congestion = config .congestion_controller_factory @@ -95,6 +96,7 @@ impl PathData { config.min_mtu, peer_max_udp_payload_size, mtud_config.clone(), + interface_mtu_constraint, ) }, ), diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs index 668e3c2f2f..af92a0cf77 100644 --- a/quinn-proto/src/endpoint.rs +++ b/quinn-proto/src/endpoint.rs @@ -326,6 +326,7 @@ impl Endpoint { config: ClientConfig, remote: SocketAddr, server_name: &str, + interface_mtu_constraint: Option, ) -> Result<(ConnectionHandle, Connection), ConnectError> { if self.cids_exhausted() { return Err(ConnectError::CidsExhausted); @@ -371,6 +372,7 @@ impl Endpoint { token_store: config.token_store, server_name: server_name.into(), }, + interface_mtu_constraint, ); Ok((ch, conn)) } @@ -630,6 +632,7 @@ impl Endpoint { pref_addr_cid, path_validated: remote_address_validated, }, + None, // Interface MTU constraint not available for server connections ); self.index.insert_initial(dst_cid, ch); @@ -794,6 +797,7 @@ impl Endpoint { tls: Box, transport_config: Arc, side_args: SideArgs, + interface_mtu_constraint: Option, ) -> Connection { let mut rng_seed = [0; 32]; self.rng.fill_bytes(&mut rng_seed); @@ -814,6 +818,7 @@ impl Endpoint { self.allow_mtud, rng_seed, side_args, + interface_mtu_constraint, ); let mut cids_issued = 0; diff --git a/quinn/Cargo.toml b/quinn/Cargo.toml index 42a3d4917c..620e0e34cb 100644 --- a/quinn/Cargo.toml +++ b/quinn/Cargo.toml @@ -57,6 +57,7 @@ bytes = { workspace = true } futures-io = { workspace = true, optional = true } rustc-hash = { workspace = true } pin-project-lite = { workspace = true } +mtu = "0.2" proto = { package = "quinn-proto", path = "../quinn-proto", version = "0.12.0", default-features = false } rustls = { workspace = true, optional = true } smol = { workspace = true, optional = true } diff --git a/quinn/src/endpoint.rs b/quinn/src/endpoint.rs index 442608cc5c..7d3b64210e 100644 --- a/quinn/src/endpoint.rs +++ b/quinn/src/endpoint.rs @@ -236,9 +236,22 @@ impl Endpoint { addr }; + // Query interface MTU for the destination IP as an additional constraint + let interface_mtu_constraint = mtu::interface_and_mtu(addr.ip()) + .ok() + .and_then(|(_interface, interface_mtu)| { + // Convert interface MTU to UDP payload size (subtract IP + UDP headers) + // IPv4: 20 bytes IP header + 8 bytes UDP header = 28 bytes + // IPv6: 40 bytes IP header + 8 bytes UDP header = 48 bytes + let header_overhead = if addr.is_ipv6() { 48 } else { 28 }; + let udp_payload_size = interface_mtu.saturating_sub(header_overhead); + // Convert usize to u16, clamping to u16::MAX if too large + u16::try_from(udp_payload_size).ok() + }); + let (ch, conn) = endpoint .inner - .connect(self.runtime.now(), config, addr, server_name)?; + .connect(self.runtime.now(), config, addr, server_name, interface_mtu_constraint)?; let sender = endpoint.socket.create_sender(); endpoint.stats.outgoing_handshakes += 1; From 9a4b27ef5cc97fd39b3e40559a729aa2b2c8da2f Mon Sep 17 00:00:00 2001 From: ali-molajani Date: Mon, 8 Dec 2025 09:48:44 +0330 Subject: [PATCH 2/2] There was potential to return None, solved --- quinn/src/endpoint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quinn/src/endpoint.rs b/quinn/src/endpoint.rs index 7d3b64210e..ef7aba8de9 100644 --- a/quinn/src/endpoint.rs +++ b/quinn/src/endpoint.rs @@ -246,7 +246,7 @@ impl Endpoint { let header_overhead = if addr.is_ipv6() { 48 } else { 28 }; let udp_payload_size = interface_mtu.saturating_sub(header_overhead); // Convert usize to u16, clamping to u16::MAX if too large - u16::try_from(udp_payload_size).ok() + Some((udp_payload_size.min(u16::MAX as usize)) as u16) }); let (ch, conn) = endpoint