Merge branch 'master' into next

Forward-merges the squashed per-destination TCP MSS clamping work
(master `ae60743`) into next.

Auto-merge of source files clean. Two pre-existing test assertions
on the next-side discovery tests were updated as part of merge
resolution to reflect the target-edge MTU fold introduced by the
merged-in commit:

- test_transit_forwards_when_mtu_sufficient: 1400 → 1280 = min(target-edge 1280, transit 1400)
- test_response_path_mtu_four_node_chain: 1350 → 1280 = min(target-edge 1280, transits 1350+1500)

Resulting next tree is bit-for-bit identical to the pre-squash next
state previously verified by full local CI sweep (29 + 1 next-only
suites pass on `6a8b519` in 23m 41s).
This commit is contained in:
Johnathan Corgan
2026-05-02 17:39:26 +00:00
7 changed files with 586 additions and 50 deletions
+123 -11
View File
@@ -7,6 +7,7 @@
use crate::node::{Node, RecentRequest};
use crate::protocol::{LookupRequest, LookupResponse};
use crate::transport::{TransportAddr, TransportId};
use crate::{NodeAddr, PeerIdentity};
use tracing::{debug, info, trace, warn};
@@ -138,16 +139,7 @@ impl Node {
self.stats_mut().discovery.resp_forwarded += 1;
// Apply path_mtu min() from the outgoing link's transport MTU
if let Some(peer) = self.peers.get(&from_peer)
&& let Some(tid) = peer.transport_id()
&& let Some(transport) = self.transports.get(&tid)
{
if let Some(addr) = peer.current_addr() {
response.path_mtu = response.path_mtu.min(transport.link_mtu(addr));
} else {
response.path_mtu = response.path_mtu.min(transport.mtu());
}
}
self.apply_outgoing_link_mtu_to_response(&mut response, &from_peer);
debug!(
request_id = response.request_id,
@@ -217,6 +209,32 @@ impl Node {
self.coord_cache
.insert_with_path_mtu(target, response.target_coords, now_ms, path_mtu);
// Mirror path_mtu into the FipsAddress-keyed read-only lookup
// map used by the TUN reader/writer at TCP MSS clamp time.
let fips_addr = crate::FipsAddress::from_node_addr(&target);
match self.path_mtu_lookup.write() {
Ok(mut map) => {
let prior = map.insert(fips_addr, path_mtu);
debug!(
target = %self.peer_display_name(&target),
fips_addr = %fips_addr,
path_mtu = path_mtu,
prior = ?prior,
map_len = map.len(),
"Wrote path_mtu_lookup from discovery LookupResponse"
);
}
Err(e) => {
warn!(
target = %self.peer_display_name(&target),
fips_addr = %fips_addr,
path_mtu = path_mtu,
error = %e,
"path_mtu_lookup write lock poisoned; clamp will not see this update"
);
}
}
// Clean up pending lookup tracking
self.pending_lookups.remove(&target);
@@ -256,7 +274,8 @@ impl Node {
LookupResponse::proof_bytes(request.request_id, &request.target, &our_coords);
let proof = self.identity().sign(&proof_data);
let response = LookupResponse::new(request.request_id, request.target, our_coords, proof);
let mut response =
LookupResponse::new(request.request_id, request.target, our_coords, proof);
// Route toward origin via reverse path.
let next_hop_addr = if let Some(recent) = self.recent_requests.get(&request.request_id) {
@@ -275,10 +294,17 @@ impl Node {
}
};
// Fold our outgoing-link MTU into path_mtu so the target-edge link
// appears in the bottleneck calculation. Without this, the response
// leaves the target with path_mtu = u16::MAX and only intermediate
// transits min-fold; the target's first reverse-path hop is missed.
self.apply_outgoing_link_mtu_to_response(&mut response, &next_hop_addr);
debug!(
request_id = request.request_id,
origin = %self.peer_display_name(&request.origin),
next_hop = %self.peer_display_name(&next_hop_addr),
path_mtu = response.path_mtu,
"Sending LookupResponse"
);
@@ -616,6 +642,92 @@ impl Node {
self.recent_requests
.retain(|_, entry| !entry.is_expired(current_time_ms, expiry_ms));
}
/// Min-fold our outgoing-link MTU into a LookupResponse's `path_mtu`.
///
/// Used at both transit-side reverse-path forward and at the target's
/// own send_lookup_response. The link MTU we apply is the MTU of the
/// transport+addr we'll use to deliver the response toward `next_hop`.
/// No-op when `next_hop` is not a directly-connected peer or its
/// transport is not registered.
pub(in crate::node) fn apply_outgoing_link_mtu_to_response(
&self,
response: &mut LookupResponse,
next_hop: &NodeAddr,
) {
if let Some(peer) = self.peers.get(next_hop)
&& let Some(tid) = peer.transport_id()
&& let Some(transport) = self.transports.get(&tid)
{
let link_mtu = if let Some(addr) = peer.current_addr() {
transport.link_mtu(addr)
} else {
transport.mtu()
};
response.path_mtu = response.path_mtu.min(link_mtu);
}
}
/// Seed `path_mtu_lookup` for a directly-connected peer.
///
/// Called when an FMP link-layer peer is promoted to active. The seed
/// value is the local outgoing-link MTU on the peer's transport, which
/// is the actual link constraint for direct-link traffic. Stored only
/// when no tighter value exists: discovery's reverse-path bottleneck
/// or MMP `MtuExceeded` reactive learning take precedence when smaller.
///
/// Without this seed, configured/auto-connect peers (which establish
/// sessions without going through the discovery Lookup flow) leave
/// `path_mtu_lookup` empty for their FipsAddress, causing
/// `per_flow_max_mss` to fall back to the global ceiling and the
/// SYN-time TCP MSS clamp to over-estimate the effective path.
pub(in crate::node) fn seed_path_mtu_for_link_peer(
&self,
peer_addr: &NodeAddr,
transport_id: TransportId,
addr: &TransportAddr,
) {
let Some(transport) = self.transports.get(&transport_id) else {
debug!(
peer = %self.peer_display_name(peer_addr),
transport_id = %transport_id,
"seed_path_mtu_for_link_peer: transport not registered, skipping seed"
);
return;
};
let link_mtu = transport.link_mtu(addr);
let fips_addr = crate::FipsAddress::from_node_addr(peer_addr);
let Ok(mut map) = self.path_mtu_lookup.write() else {
warn!(
peer = %self.peer_display_name(peer_addr),
"seed_path_mtu_for_link_peer: path_mtu_lookup write lock poisoned"
);
return;
};
match map.get(&fips_addr).copied() {
Some(existing) if existing <= link_mtu => {
// Keep the tighter learned value; never loosen the clamp.
debug!(
peer = %self.peer_display_name(peer_addr),
fips_addr = %fips_addr,
link_mtu = link_mtu,
existing = existing,
"seed_path_mtu_for_link_peer: keeping tighter existing value"
);
}
other => {
map.insert(fips_addr, link_mtu);
debug!(
peer = %self.peer_display_name(peer_addr),
fips_addr = %fips_addr,
link_mtu = link_mtu,
prior = ?other,
map_len = map.len(),
"seed_path_mtu_for_link_peer: wrote link MTU"
);
}
}
}
}
/// Tracks a pending discovery lookup with retry state.
+4
View File
@@ -1401,6 +1401,8 @@ impl Node {
let _ = self.index_allocator.free(old_idx);
}
self.seed_path_mtu_for_link_peer(&peer_node_addr, transport_id, &current_addr);
let mut new_peer = ActivePeer::with_session(
verified_identity,
link_id,
@@ -1504,6 +1506,8 @@ impl Node {
.get(&peer_node_addr)
.map(|p| p.last_tree_announce_sent_ms());
self.seed_path_mtu_for_link_peer(&peer_node_addr, transport_id, &current_addr);
let mut new_peer = ActivePeer::with_session(
verified_identity,
link_id,
+8 -2
View File
@@ -667,8 +667,11 @@ impl Node {
(fds[0], fds[1])
};
// Create writer (dups the fd for independent write access)
let (writer, tun_tx) = device.create_writer(max_mss)?;
// Create writer (dups the fd for independent write access).
// Pass path_mtu_lookup so inbound SYN-ACK clamp can read
// per-destination path MTU learned via discovery.
let (writer, tun_tx) =
device.create_writer(max_mss, self.path_mtu_lookup.clone())?;
// Spawn writer thread
let writer_handle = thread::spawn(move || {
@@ -684,6 +687,7 @@ impl Node {
// Spawn reader thread
let transport_mtu = self.transport_mtu();
let path_mtu_lookup = self.path_mtu_lookup.clone();
#[cfg(target_os = "macos")]
let reader_handle = thread::spawn(move || {
run_tun_reader(
@@ -693,6 +697,7 @@ impl Node {
reader_tun_tx,
outbound_tx,
transport_mtu,
path_mtu_lookup,
shutdown_read_fd,
);
});
@@ -705,6 +710,7 @@ impl Node {
reader_tun_tx,
outbound_tx,
transport_mtu,
path_mtu_lookup,
);
});
+8
View File
@@ -306,6 +306,12 @@ pub struct Node {
/// Recent discovery requests (dedup + reverse-path forwarding).
/// Maps request_id → RecentRequest.
recent_requests: HashMap<u64, RecentRequest>,
/// Per-destination path MTU lookup, keyed by FipsAddress (mirrors
/// `coord_cache.entries[*].path_mtu`). Sync read-only access from
/// the TUN reader/writer threads at TCP MSS clamp time so the
/// SYN/SYN-ACK clamp can use the smaller of the local-egress floor
/// and the learned per-destination path MTU.
path_mtu_lookup: Arc<std::sync::RwLock<HashMap<crate::FipsAddress, u16>>>,
// === Transports & Links ===
/// Active transports (owned by Node).
@@ -628,6 +634,7 @@ impl Node {
peer_aliases: HashMap::new(),
peer_acl,
host_map,
path_mtu_lookup: Arc::new(std::sync::RwLock::new(HashMap::new())),
})
}
@@ -759,6 +766,7 @@ impl Node {
peer_aliases: HashMap::new(),
peer_acl,
host_map,
path_mtu_lookup: Arc::new(std::sync::RwLock::new(HashMap::new())),
})
}
+49 -26
View File
@@ -702,14 +702,13 @@ async fn test_discovery_100_nodes() {
#[tokio::test]
async fn test_response_path_mtu_two_node() {
// Two-node topology: node0 — node1
// Node0 initiates lookup for node1. The response should carry path_mtu
// reflecting the transport MTU (1280 in tests) clamped by transit.
// In a two-node setup: node1 (target) initializes path_mtu=u16::MAX,
// then the response is sent directly to node0. Since node1 is the
// target and sends directly, the transit logic does not apply for the
// first hop (the target sends directly). But node0 is the originator
// and doesn't apply transit MTU. So path_mtu should be u16::MAX in
// this simple case (no transit nodes to clamp it).
// Node0 initiates lookup for node1. node1 is the target and generates
// the response: send_lookup_response folds in node1's own outgoing-link
// MTU before sending, so path_mtu reflects the target-edge link
// constraint (the test transport MTU, 1280) even with no transit hops.
// Without that target-edge fold, a 2-node lookup would leave path_mtu
// at u16::MAX since no transit min-fold runs — that's the gap closed
// alongside the configured-peer seed in the B3 follow-up.
let edges = vec![(0, 1)];
let mut nodes = run_tree_test(2, &edges, false).await;
@@ -732,21 +731,40 @@ async fn test_response_path_mtu_two_node() {
"Node 0 should have cached node 1's route"
);
// Check that path_mtu was stored in the cache entry
let entry = nodes[0].node.coord_cache().get_entry(&node1_addr).unwrap();
let path_mtu = entry
.path_mtu()
.expect("path_mtu should be set from discovery");
// In a 2-node setup, no transit node applies the min() so path_mtu stays u16::MAX
assert_eq!(
path_mtu,
u16::MAX,
"Two-node path_mtu should be u16::MAX (no transit nodes to clamp)"
path_mtu, 1280,
"Two-node path_mtu should be the target-edge link MTU (1280 in tests)"
);
cleanup_nodes(&mut nodes).await;
}
#[tokio::test]
async fn test_apply_outgoing_link_mtu_to_response_unknown_peer_noop() {
// When next_hop is not a directly-connected peer (no entry in
// self.peers), apply_outgoing_link_mtu_to_response is a no-op and the
// response's path_mtu is left unchanged. Pins the early-return path.
let node = make_node();
let unknown = make_node_addr(0x99);
let coords = TreeCoordinate::from_addrs(vec![unknown, make_node_addr(0)]).unwrap();
let identity = Identity::generate();
let proof_data = LookupResponse::proof_bytes(1, &unknown, &coords);
let proof = identity.sign(&proof_data);
let mut response = LookupResponse::new(1, unknown, coords, proof);
response.path_mtu = 1500;
node.apply_outgoing_link_mtu_to_response(&mut response, &unknown);
assert_eq!(
response.path_mtu, 1500,
"Unknown next_hop must leave path_mtu untouched"
);
}
#[tokio::test]
async fn test_response_path_mtu_three_node_chain() {
// Topology: node0 — node1 — node2
@@ -928,7 +946,11 @@ async fn test_transit_forwards_when_mtu_sufficient() {
// Topology: node0(1280) — node1(1400) — node2(1280)
// Node0 initiates lookup for node2 with min_mtu=1280 (default TUN MTU).
// Node1's transport MTU is 1400 >= 1280, so the request passes through.
// Node1 annotates path_mtu = min(u16::MAX, 1400) = 1400 on response.
// Bottleneck min-fold accumulates contributions from BOTH the target's
// own outgoing-link MTU (the target-edge fold added with the
// direct-link/target-edge gap fix) and each transit node's outgoing-
// link MTU. With node2 (target) at 1280 and node1 (transit) at 1400,
// the bottleneck is min(1280, 1400) = 1280.
let mtus = [1280, 1400, 1280];
let edges = vec![(0, 1), (1, 2)];
let mut nodes = run_tree_test_with_mtus(&mtus, &edges).await;
@@ -957,8 +979,8 @@ async fn test_transit_forwards_when_mtu_sufficient() {
let entry = nodes[0].node.coord_cache().get_entry(&node2_addr).unwrap();
let path_mtu = entry.path_mtu().expect("path_mtu should be set");
assert_eq!(
path_mtu, 1400,
"path_mtu should reflect transit node's transport MTU (1400)"
path_mtu, 1280,
"path_mtu should be min(target-edge 1280, transit 1400) = 1280"
);
cleanup_nodes(&mut nodes).await;
@@ -966,16 +988,17 @@ async fn test_transit_forwards_when_mtu_sufficient() {
#[tokio::test]
async fn test_response_path_mtu_four_node_chain() {
// Topology: node0(1280) — node1(1400) — node2(900) — node3(1280)
// Topology: node0(1280) — node1(1500) — node2(1350) — node3(1280)
// Node0 initiates lookup for node3. Response travels node3→node2→node1→node0.
// Transit nodes apply min(): node2 sees min(u16::MAX, 900) = 900,
// node1 sees min(900, 1400) = 900.
// Final path_mtu at node0 should be 900 (bottleneck at node2).
// The bottleneck min-fold now accumulates contributions from the target's
// own outgoing link MTU (target-edge fold added with the direct-link gap
// fix) AND each transit node's outgoing link MTU on the reverse path.
// node3 (target, 1280) → 1280; node2 (transit, 1350) → min(1280, 1350) =
// 1280; node1 (transit, 1500) → min(1280, 1500) = 1280. Result: 1280.
//
// Note: min_mtu=1280 from TUN config. Node2's MTU (900) < 1280 would prune
// the forward request at node2, so node3 would never be reached. To test
// path_mtu annotation we need all transit links to pass the min_mtu check.
// Use MTUs above 1280 to avoid pruning but with different values to verify min().
// Note: min_mtu=1280 from TUN config. All transit MTUs ≥ 1280 so the
// forward request is not pruned; the test exercises the response-side
// min-fold accumulation explicitly.
let mtus = [1280, 1500, 1350, 1280];
let edges = vec![(0, 1), (1, 2), (2, 3)];
let mut nodes = run_tree_test_with_mtus(&mtus, &edges).await;
@@ -1004,8 +1027,8 @@ async fn test_response_path_mtu_four_node_chain() {
let entry = nodes[0].node.coord_cache().get_entry(&node3_addr).unwrap();
let path_mtu = entry.path_mtu().expect("path_mtu should be set");
assert_eq!(
path_mtu, 1350,
"Four-node chain path_mtu should be min of transit MTUs (1350)"
path_mtu, 1280,
"Four-node chain path_mtu = min(target-edge 1280, transits 1350+1500) = 1280"
);
cleanup_nodes(&mut nodes).await;
+135
View File
@@ -1033,3 +1033,138 @@ async fn test_transport_mtu_min_with_single_operational() {
transport.stop().await.ok();
}
}
// path_mtu_lookup seeding for direct-link (configured) peers — closes the
// B3 coverage gap where configured/auto-connect peers never go through the
// discovery Lookup flow and so their FipsAddress was missing from
// path_mtu_lookup, causing the SYN-time TCP MSS clamp to fall back to the
// global ceiling.
#[tokio::test]
async fn test_seed_path_mtu_inserts_when_empty() {
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);
let peer_addr = make_node_addr(0xAA);
let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr);
let transport_addr = TransportAddr::from_string("10.0.0.2:2121");
node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr);
let stored = node
.path_mtu_lookup
.read()
.unwrap()
.get(&fips_addr)
.copied();
assert_eq!(
stored,
Some(1452),
"Empty lookup should be seeded with the link MTU"
);
for transport in node.transports.values_mut() {
transport.stop().await.ok();
}
}
#[tokio::test]
async fn test_seed_path_mtu_keeps_tighter_existing_value() {
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);
let peer_addr = make_node_addr(0xBB);
let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr);
let transport_addr = TransportAddr::from_string("10.0.0.3:2121");
// Pre-populate with a tighter value, e.g. learned from discovery's
// reverse-path bottleneck.
node.path_mtu_lookup
.write()
.unwrap()
.insert(fips_addr, 1280);
node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr);
let stored = node
.path_mtu_lookup
.read()
.unwrap()
.get(&fips_addr)
.copied();
assert_eq!(
stored,
Some(1280),
"Existing tighter value (1280) must not be loosened by direct-link seed (1452)"
);
for transport in node.transports.values_mut() {
transport.stop().await.ok();
}
}
#[tokio::test]
async fn test_seed_path_mtu_tightens_looser_existing_value() {
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, 1280).await;
node.transports.insert(TransportId::new(1), udp);
let peer_addr = make_node_addr(0xCC);
let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr);
let transport_addr = TransportAddr::from_string("10.0.0.4:2121");
// Pre-populate with a looser stale value.
node.path_mtu_lookup
.write()
.unwrap()
.insert(fips_addr, 1452);
node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr);
let stored = node
.path_mtu_lookup
.read()
.unwrap()
.get(&fips_addr)
.copied();
assert_eq!(
stored,
Some(1280),
"Direct-link seed (1280) must overwrite looser existing value (1452)"
);
for transport in node.transports.values_mut() {
transport.stop().await.ok();
}
}
#[tokio::test]
async fn test_seed_path_mtu_noop_for_unknown_transport() {
let node = make_node();
let peer_addr = make_node_addr(0xDD);
let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr);
let transport_addr = TransportAddr::from_string("10.0.0.5:2121");
// No transport registered — call must be a no-op, not panic.
node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(99), &transport_addr);
let map = node.path_mtu_lookup.read().unwrap();
assert!(
map.get(&fips_addr).is_none(),
"Seed must be a no-op when transport_id is not registered"
);
}