Per-destination TCP MSS clamping at the TUN boundary

Adds source-side TCP MSS clamping informed by per-destination path
MTU learned via discovery, with a conservative IPv6-minimum-derived
ceiling for cold flows where discovery has not yet completed. Closes
the multi-hop default-config TCP wedges observed in production where
a sender's local-floor MSS exceeds what some intermediate forwarder
hop is willing to carry: silent drops, no PTB feedback through the
userspace TUN to the kernel TCP stack, retransmits at the same too-
large MSS, application connection times out.

## Architecture

A new `Arc<RwLock<HashMap<FipsAddress, u16>>>` field
`path_mtu_lookup` on Node mirrors the per-destination path MTU in a
form accessible from sync TUN reader/writer threads. A new
`per_flow_max_mss` helper in `src/upper/tun.rs` reads the lookup at
SYN-clamp time and returns the appropriate ceiling for the flow.

Three write sites populate `path_mtu_lookup`:

1. **Discovery originator branch** of `handle_lookup_response`:
   the path MTU bottleneck accumulated through the reverse path
   lands here when a LookupResponse arrives at the originator.
   Same value also lands in `coord_cache` per the existing
   `insert_with_path_mtu` API.

2. **FMP peer-promotion seed** (`seed_path_mtu_for_link_peer`):
   when an FMP link-layer peer is promoted to active, the local
   outgoing-link MTU on the peer's transport seeds the lookup.
   Tighter existing values (learned via discovery) are preserved;
   the seed only writes when no entry exists or the existing
   value is looser than the link MTU. Without this seed,
   directly-configured peers (auto_connect / static peer config)
   would leave `path_mtu_lookup` empty for their FipsAddress
   because the FSP session establishes without ever issuing a
   LookupRequest.

3. **Target-edge fold at `send_lookup_response`**: when a node is
   the discovery target, it folds its own outgoing-link MTU to
   the response's next-hop into `path_mtu` before sending.
   Without this fold, the response leaves the target with
   `path_mtu = u16::MAX` and only intermediate transits min-fold;
   the target's first reverse-path hop is never represented in
   the bottleneck calculation. Refactored the existing transit-
   side min-fold into a shared `apply_outgoing_link_mtu_to_response`
   helper called from both sites.

## Read-side: per_flow_max_mss

Two TUN call sites consume the lookup:

- Outbound `handle_tun_packet` clamps SYN MSS using packet[24..40]
  (IPv6 destination) as the lookup key.
- Inbound `TunWriter::run` clamps SYN-ACK MSS using packet[8..24]
  (IPv6 source).

When the lookup contains a learned value, the helper computes
`min(global_max_mss, effective_ipv6_mtu(path_mtu) - 60)` where 60
is IPv6 (40) + TCP (20) headers and `effective_ipv6_mtu` accounts
for the FIPS encapsulation overhead.

When the lookup is empty for a destination — the cold-flow case —
the helper returns `min(global_max_mss, IPv6-minimum-derived
ceiling)`. RFC 8200 mandates every IPv6 path accept ≥1280-byte
packets, so the IPv6-minimum-derived MSS (1280 - 77 - 60 = 1143)
fits any compliant path. Without this conservative ceiling, the
first SYN to a destination with no learned path MTU exits the TUN
at the kernel-natural MSS (TUN MTU - 60), and the application
connection wedges silently before discovery completes for a
corrected second SYN to fire. The fix is provably safe: the
ceiling is taken with `min` against the local global so operators
with even tighter local floors are never loosened upward.

Subsequent flows pick up the actual learned per-destination value
once discovery (or the FMP-promotion seed for direct peers)
populates the lookup.

## Diagnostic logging

All write and read sites emit instrumentation suitable for
operators bisecting a wedged path:

- `debug!` log on every `path_mtu_lookup` write (discovery
  originator path and FMP-promotion seed path), showing the
  FipsAddress, written value, prior value, and post-write map
  size. `warn!` on poisoned-lock failure path.
- `trace!` log per `per_flow_max_mss` call covering every
  fall-through branch (wrong addr_bytes length, non-fd::/8
  prefix, lookup poisoned, no entry for destination, empty-lookup
  conservative ceiling) and the success path. trace level filters
  out under normal log settings; capture with
  `RUST_LOG=info,fips::node::handlers::discovery=debug,fips::upper::tun=trace`.

## Tests

15 new unit tests across 3 files:

- `per_flow_max_mss` (8 tests in `src/upper/tun.rs::tests`):
  empty-lookup conservative ceiling, empty-lookup global-smaller
  floor, learned-value-overrides-conservative, per-destination
  smaller, per-destination larger capped by global, non-fips
  addr, short addr slice, per-destination independence.
- `seed_path_mtu_for_link_peer` (4 tests in
  `src/node/tests/unit.rs`): seed when empty, keep tighter
  existing, tighten looser existing, no-op for unknown
  transport.
- Discovery integration (3 tests in `src/node/tests/discovery.rs`):
  apply_outgoing_link_mtu_to_response on unknown peer no-op,
  two-node target-edge fold (path_mtu reflects target-edge link),
  three-node chain transit min-fold (existing test, updated for
  target-edge inclusion).

Two pre-existing discovery tests had assertions updated to
account for the target-edge fold:

- `test_response_path_mtu_two_node`: previously asserted
  `u16::MAX` (no transit to min-fold); now asserts 1280 (the
  test transport MTU, folded in by send_lookup_response).
- `test_response_path_mtu_four_node_chain`: previously asserted
  1350 (transit MTUs only); now asserts 1280 (target-edge MTU
  is the bottleneck).
- `test_transit_forwards_when_mtu_sufficient`: previously
  asserted 1400 (transit MTU only); now asserts 1280 (target-
  edge MTU is the bottleneck).

## Verification

Local CI on this commit: 29/29 suites pass, 1105 lib tests pass,
clippy --all-targets --all-features -D warnings clean, cargo fmt
clean. Production deploy verified via trace capture across the
managed fleet: cold-flow conservative ceiling branch fires on
first SYN, learned-lookup branch takes over once discovery
completes, both behaviors observable end-to-end at the SYN MSS
on the wire.

No wire-format change. No config-format change.
This commit is contained in:
Johnathan Corgan
2026-05-02 17:37:07 +00:00
parent 996a591001
commit ae607431eb
7 changed files with 568 additions and 37 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"
);
@@ -573,6 +599,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
@@ -1012,6 +1012,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,
@@ -1107,6 +1109,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
@@ -624,8 +624,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 || {
@@ -641,6 +644,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(
@@ -650,6 +654,7 @@ impl Node {
reader_tun_tx,
outbound_tx,
transport_mtu,
path_mtu_lookup,
shutdown_read_fd,
);
});
@@ -662,6 +667,7 @@ impl Node {
reader_tun_tx,
outbound_tx,
transport_mtu,
path_mtu_lookup,
);
});
+8
View File
@@ -301,6 +301,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).
@@ -616,6 +622,7 @@ impl Node {
peer_aliases: HashMap::new(),
peer_acl,
host_map,
path_mtu_lookup: Arc::new(std::sync::RwLock::new(HashMap::new())),
})
}
@@ -745,6 +752,7 @@ impl Node {
peer_aliases: HashMap::new(),
peer_acl,
host_map,
path_mtu_lookup: Arc::new(std::sync::RwLock::new(HashMap::new())),
})
}
+31 -13
View File
@@ -710,14 +710,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;
@@ -740,21 +739,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
+135
View File
@@ -1030,3 +1030,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"
);
}