mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-09 08:14:42 +00:00
node: inherit primary UDP config when adopting NAT-traversal sockets
`Node::adopt_established_traversal` was constructing the adopted UDP transport with `UdpConfig::default()` — MTU 1280, default recv/send buffer sizes, default accept/advertise flags. If the operator had configured a higher MTU on the primary `[transports.udp]` listener (e.g. 1500 on a path where larger frames are known viable), full-sized tunnel datagrams sent over the NAT-traversed link would exceed the adopted socket's MTU and get dropped at the socket layer with no visibility into why throughput collapsed. Inherit the primary UDP config (MTU + recv/send buffer sizes + accept / advertise flags) and clear the bind / external-address fields since the adopted socket is already bound. Lookup tries `transport_name` first so operators with multiple named `[transports.udp.<name>]` listeners pick up inheritance from the matching listener, and falls back to the unnamed `Single` listener so single-instance configs work unchanged. The previous default of MTU 1280 was deliberately the IPv6 minimum, the only value guaranteed to survive arbitrary middlebox paths. With this change, operators who set their primary listener higher (based on known-clean LAN topology) will have NAT-traversed flows initially attempting that higher MTU and possibly black-holing on tighter paths until reactive `MtuExceeded` recovery kicks in. Documented in the adoption call-site comment so future readers understand why the conservative default went away. Discovered in a downstream consumer where a `MESH_TUNNEL_MTU=1320` / encrypted wire ~1426B produced silent packet drop on every session that had been promoted onto a NAT-traversed link. Adds two sibling tests in `src/node/tests/bootstrap.rs` pinning the new behaviour for the `Single` and `Named` config variants.
This commit is contained in:
committed by
Johnathan Corgan
parent
e471807239
commit
f0bb29ff6e
+30
-8
@@ -1824,17 +1824,39 @@ impl Node {
|
||||
self.register_identity(peer_node_addr, peer_identity.pubkey_full());
|
||||
|
||||
let transport_id = self.allocate_transport_id();
|
||||
// Adopted ephemeral UDP transports use UdpConfig::default() when the
|
||||
// bootstrap runtime doesn't pass an override. Default MTU resolves to
|
||||
// 1280 (IPv6 minimum), which is the only value guaranteed to survive
|
||||
// arbitrary NAT-traversal middlebox paths. Inheriting from the named
|
||||
// [transports.udp] config (Option 3 in ISSUE-2026-0013) would track
|
||||
// operator config more closely but risks regressions on hostile paths;
|
||||
// accepted as-is until a concrete use case justifies the change.
|
||||
// Adopted ephemeral UDP transports inherit MTU + socket-buffer sizing
|
||||
// (and accept_connections / advertise flags) from the operator's
|
||||
// configured [transports.udp] when the bootstrap runtime doesn't
|
||||
// pass an explicit override. Lookup tries `transport_name` first
|
||||
// (covers the `Named` multi-listener variant) and falls back to the
|
||||
// unnamed `Single` listener, so single- and named-listener configs
|
||||
// both inherit cleanly.
|
||||
//
|
||||
// Tradeoff: `UdpConfig::default()` sets MTU 1280 (IPv6 minimum), the
|
||||
// only value guaranteed to survive arbitrary middlebox paths.
|
||||
// Inheriting a higher operator-chosen MTU means NAT-traversed flows
|
||||
// initially attempt that MTU and may black-hole on tighter paths
|
||||
// until reactive `MtuExceeded` recovery kicks in. Operators who
|
||||
// raise the primary MTU based on known-clean topology accept that
|
||||
// tradeoff; the silent drop on a too-low default was strictly
|
||||
// worse for the common case where the primary MTU is reachable.
|
||||
//
|
||||
// Bind / external address fields are cleared since the socket is
|
||||
// already bound.
|
||||
let inherited_config = traversal.transport_config.clone().unwrap_or_else(|| {
|
||||
let mut cfg = self
|
||||
.lookup_udp_config(traversal.transport_name.as_deref())
|
||||
.or_else(|| self.lookup_udp_config(None))
|
||||
.cloned()
|
||||
.unwrap_or_default();
|
||||
cfg.bind_addr = None;
|
||||
cfg.external_addr = None;
|
||||
cfg
|
||||
});
|
||||
let mut transport = crate::transport::udp::UdpTransport::new(
|
||||
transport_id,
|
||||
traversal.transport_name.clone(),
|
||||
traversal.transport_config.clone().unwrap_or_default(),
|
||||
inherited_config,
|
||||
packet_tx,
|
||||
);
|
||||
|
||||
|
||||
@@ -2,10 +2,11 @@
|
||||
|
||||
use super::*;
|
||||
use crate::EstablishedTraversal;
|
||||
use crate::config::UdpConfig;
|
||||
use crate::config::{TransportInstances, UdpConfig};
|
||||
use crate::node::wire::{PHASE_MSG1, PHASE_MSG2};
|
||||
use crate::transport::udp::UdpTransport;
|
||||
use crate::utils::index::IndexAllocator;
|
||||
use std::collections::HashMap;
|
||||
use tokio::time::{Duration, timeout, timeout_at};
|
||||
|
||||
#[tokio::test]
|
||||
@@ -243,3 +244,90 @@ async fn test_third_peer_can_handshake_via_adopted_transport_socket() {
|
||||
transport.stop().await.ok();
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_adopted_udp_inherits_mtu_from_single_primary_config() {
|
||||
let mut node = make_node();
|
||||
node.config.transports.udp = TransportInstances::Single(UdpConfig {
|
||||
mtu: Some(1500),
|
||||
..Default::default()
|
||||
});
|
||||
|
||||
let (packet_tx, packet_rx) = packet_channel(64);
|
||||
node.packet_tx = Some(packet_tx);
|
||||
node.packet_rx = Some(packet_rx);
|
||||
node.state = NodeState::Running;
|
||||
|
||||
let peer = make_node();
|
||||
let adopted_socket = std::net::UdpSocket::bind("127.0.0.1:0").unwrap();
|
||||
let handoff = EstablishedTraversal::new(
|
||||
"sess-inherit-single",
|
||||
peer.npub(),
|
||||
"127.0.0.1:9".parse().unwrap(),
|
||||
adopted_socket,
|
||||
);
|
||||
|
||||
let result = node.adopt_established_traversal(handoff).await.unwrap();
|
||||
let adopted = node
|
||||
.get_transport(&result.transport_id)
|
||||
.expect("adopted transport present");
|
||||
assert_eq!(
|
||||
adopted.mtu(),
|
||||
1500,
|
||||
"adopted UDP transport should inherit MTU from the primary [transports.udp] config",
|
||||
);
|
||||
|
||||
for (_, transport) in node.transports.iter_mut() {
|
||||
transport.stop().await.ok();
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_adopted_udp_inherits_mtu_from_named_primary_config() {
|
||||
let mut node = make_node();
|
||||
let mut named = HashMap::new();
|
||||
named.insert(
|
||||
"primary".to_string(),
|
||||
UdpConfig {
|
||||
mtu: Some(1500),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
named.insert(
|
||||
"secondary".to_string(),
|
||||
UdpConfig {
|
||||
mtu: Some(1280),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
node.config.transports.udp = TransportInstances::Named(named);
|
||||
|
||||
let (packet_tx, packet_rx) = packet_channel(64);
|
||||
node.packet_tx = Some(packet_tx);
|
||||
node.packet_rx = Some(packet_rx);
|
||||
node.state = NodeState::Running;
|
||||
|
||||
let peer = make_node();
|
||||
let adopted_socket = std::net::UdpSocket::bind("127.0.0.1:0").unwrap();
|
||||
let handoff = EstablishedTraversal::new(
|
||||
"sess-inherit-named",
|
||||
peer.npub(),
|
||||
"127.0.0.1:9".parse().unwrap(),
|
||||
adopted_socket,
|
||||
)
|
||||
.with_transport_name("primary");
|
||||
|
||||
let result = node.adopt_established_traversal(handoff).await.unwrap();
|
||||
let adopted = node
|
||||
.get_transport(&result.transport_id)
|
||||
.expect("adopted transport present");
|
||||
assert_eq!(
|
||||
adopted.mtu(),
|
||||
1500,
|
||||
"adopted UDP transport should inherit MTU from the named [transports.udp.<name>] config matching transport_name",
|
||||
);
|
||||
|
||||
for (_, transport) in node.transports.iter_mut() {
|
||||
transport.stop().await.ok();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user