diff --git a/src/node/mod.rs b/src/node/mod.rs index 4da248d..34c5f12 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -1024,18 +1024,31 @@ impl Node { crate::upper::icmp::effective_ipv6_mtu(self.transport_mtu()) } - /// Get the transport MTU for a specific transport. + /// Get the transport MTU governing the global TUN-boundary MSS clamp. /// - /// When called without a specific transport context, returns the MTU - /// of the first operational transport, or 1280 (IPv6 minimum) as - /// fallback. This is used for initial TUN configuration where a - /// specific transport isn't yet known. + /// Returns the **minimum** MTU across all operational transports, or + /// 1280 (IPv6 minimum) as fallback. Used for initial TUN configuration + /// where a specific egress transport isn't yet known: the resulting + /// `effective_ipv6_mtu` (transport_mtu - 77) and `max_mss` + /// (effective_mtu - 60) form a conservative ceiling that fits ANY + /// configured-transport's egress, eliminating PMTU-D black holes that + /// would otherwise occur when a flow's actual egress is smaller than + /// the clamp ceiling assumed at TUN init. + /// + /// Returning the smallest (rather than the first-iterated, which used + /// to vary across HashMap iteration order + async-startup race) makes + /// the clamp deterministic across daemon restarts. + /// + /// See `ISSUE-2026-0011` for the empirical investigation. pub fn transport_mtu(&self) -> u16 { - // Prefer the MTU from the first operational transport - for handle in self.transports.values() { - if handle.is_operational() { - return handle.mtu(); - } + let min_operational = self + .transports + .values() + .filter(|h| h.is_operational()) + .map(|h| h.mtu()) + .min(); + if let Some(mtu) = min_operational { + return mtu; } // Fallback to config: try UDP first, then Ethernet if let Some((_, cfg)) = self.config.transports.udp.iter().next() { diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index 69a839e..ce90b2a 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -951,3 +951,82 @@ fn test_promote_clears_retry_pending() { "retry_pending should be cleared on successful promotion" ); } + +// ============================================================================ +// transport_mtu() — ISSUE-2026-0011 regression coverage +// ============================================================================ + +/// Helper: spawn a UdpTransport with the given mtu, started and operational. +async fn make_udp_transport_with_mtu(id: u32, mtu: u16) -> TransportHandle { + let (packet_tx, _packet_rx) = packet_channel(64); + let transport_id = TransportId::new(id); + let mut udp = UdpTransport::new( + transport_id, + Some(format!("udp{}", id)), + crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + mtu: Some(mtu), + ..Default::default() + }, + packet_tx, + ); + udp.start_async().await.unwrap(); + TransportHandle::Udp(udp) +} + +#[tokio::test] +async fn test_transport_mtu_returns_min_across_operational() { + // Multiple operational transports with varied MTUs. The picker must + // return the smallest, deterministically, regardless of HashMap + // iteration order. This is the core ISSUE-2026-0011 regression test. + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp1 = make_udp_transport_with_mtu(1, 1497).await; + let udp2 = make_udp_transport_with_mtu(2, 1280).await; + let udp3 = make_udp_transport_with_mtu(3, 1400).await; + + node.transports.insert(TransportId::new(1), udp1); + node.transports.insert(TransportId::new(2), udp2); + node.transports.insert(TransportId::new(3), udp3); + + // Expect the smallest (UDP-1280), not whichever HashMap iterates first. + assert_eq!(node.transport_mtu(), 1280); + + // effective_ipv6_mtu = 1280 - 77 = 1203, max_mss = 1203 - 60 = 1143 + // (verifies the downstream clamp value). + assert_eq!(node.effective_ipv6_mtu(), 1203); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + +#[tokio::test] +async fn test_transport_mtu_fallback_when_no_operational_transports() { + // No transports configured at all → falls back to 1280 (IPv6 minimum). + let node = make_node(); + assert_eq!(node.transport_mtu(), 1280); +} + +#[tokio::test] +async fn test_transport_mtu_min_with_single_operational() { + // Single transport: trivially returns its MTU. Pins the picker doesn't + // accidentally drop down to a smaller fallback when one transport is + // operational. + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp = make_udp_transport_with_mtu(1, 1452).await; + node.transports.insert(TransportId::new(1), udp); + + assert_eq!(node.transport_mtu(), 1452); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +}