session: mirror proactive PathMtuNotification into path_mtu_lookup

The TUN-side TCP MSS clamp consults `path_mtu_lookup` (FipsAddress-
keyed) when sizing outbound TCP flows. Until now, only the reactive
`MtuExceeded` handler mirrored the bottleneck MTU into that store;
the proactive end-to-end `PathMtuNotification` echoed by the
destination updated only `MmpSessionState.path_mtu`, leaving the TUN
mirror stale.

On stable long-lived paths, the proactive echo can tighten the
session-canonical MTU well before any transit router fires a
`MtuExceeded` for those flows (since all current traffic is already
sized by the tighter session value). New TCP flows opened during
that window get clamped by the discovery-time value rather than the
session-canonical one, leading to PMTU-D loss until the reactive
path eventually fires.

Mirror the post-apply MTU into `path_mtu_lookup` whenever
`apply_notification` returns true, with the same tighter-only
semantics as the reactive mirror — never loosen the clamp. Gated on
the bool return so spurious writes don't happen on rejected
increases or no-op same-value notifications.

Four new unit tests exercise the empty-lookup write, tighten-
existing, keep-tighter-existing, and no-session-no-op paths,
parallel to the existing reactive-mirror test trio.
This commit is contained in:
Johnathan Corgan
2026-05-06 14:39:22 +00:00
parent a78f670a6a
commit 7fc890b7a2
3 changed files with 187 additions and 9 deletions
+11
View File
@@ -392,6 +392,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
the TUN-side `path_mtu_lookup` so later flows pick up forward-path
bottlenecks without re-discovery. Windows TUN reader receives the
same per-destination plumbing.
- Proactive end-to-end `PathMtuNotification` now mirrors into the
TUN-side `path_mtu_lookup` (TCP MSS clamp store), parallel to the
reactive `MtuExceeded` mirror that already existed. Previously the
proactive handler only updated the session-canonical
`MmpSessionState.path_mtu`; on stable long-lived paths where the
destination's echo had tightened the session MTU but no transit
router had emitted a fresh `MtuExceeded` (because all current
traffic was already sized by the tighter session value), new TCP
flows opened in that window kept getting clamped by the staler
discovery-time value. The proactive mirror closes that gap with
the same tighter-only semantics — never loosens the clamp.
- Auto-connect peers now reconnect after a graceful `Disconnect`
notification from the remote side. `handle_disconnect` previously
removed the peer without scheduling a reconnect, orphaning the
+56 -9
View File
@@ -949,7 +949,11 @@ impl Node {
///
/// The destination is telling us the path MTU has changed.
/// Apply source-side rules (decrease immediate, increase validated).
fn handle_session_path_mtu_notification(&mut self, src_addr: &NodeAddr, body: &[u8]) {
pub(in crate::node) fn handle_session_path_mtu_notification(
&mut self,
src_addr: &NodeAddr,
body: &[u8],
) {
let notif = match PathMtuNotification::decode(body) {
Ok(n) => n,
Err(e) => {
@@ -973,16 +977,59 @@ impl Node {
let old_mtu = mmp.path_mtu.current_mtu();
let now = std::time::Instant::now();
mmp.path_mtu.apply_notification(notif.path_mtu, now);
let changed = mmp.path_mtu.apply_notification(notif.path_mtu, now);
let new_mtu = mmp.path_mtu.current_mtu();
if new_mtu != old_mtu {
debug!(
src = %peer_name,
old_mtu,
new_mtu,
"Path MTU changed via notification"
);
if !changed {
return;
}
debug!(
src = %peer_name,
old_mtu,
new_mtu,
"Path MTU changed via notification"
);
// Mirror the new effective MTU into the FipsAddress-keyed lookup used
// by the TUN reader/writer at TCP MSS clamp time. Without this, new
// TCP flows opened on a path the proactive end-to-end echo has
// already tightened keep getting clamped by the staler discovery-
// time value until a reactive MtuExceeded happens to fire. Keep the
// tighter of existing-or-new — never loosen the clamp.
let fips_addr = crate::FipsAddress::from_node_addr(src_addr);
match self.path_mtu_lookup.write() {
Ok(mut map) => match map.get(&fips_addr).copied() {
Some(existing) if existing <= new_mtu => {
debug!(
dest = %peer_name,
fips_addr = %fips_addr,
new_mtu,
existing,
"PathMtuNotification: keeping tighter existing path_mtu_lookup value"
);
}
other => {
map.insert(fips_addr, new_mtu);
debug!(
dest = %peer_name,
fips_addr = %fips_addr,
new_mtu,
prior = ?other,
map_len = map.len(),
"PathMtuNotification: tightened path_mtu_lookup"
);
}
},
Err(e) => {
warn!(
dest = %peer_name,
fips_addr = %fips_addr,
new_mtu,
error = %e,
"path_mtu_lookup write lock poisoned; PathMtuNotification not reflected"
);
}
}
}
+120
View File
@@ -2207,3 +2207,123 @@ async fn test_handle_mtu_exceeded_keeps_tighter_existing_path_mtu_lookup() {
"MtuExceeded with looser bottleneck must not loosen a tighter existing value"
);
}
// ============================================================================
// Proactive PathMtuNotification → path_mtu_lookup focused unit tests
//
// These exercise the receive-side write path that mirrors the proactive
// end-to-end echo into `path_mtu_lookup`. Without this mirror, new TCP
// flows opened on a path the proactive notification has tightened keep
// getting clamped by the staler discovery-time value until a reactive
// MtuExceeded fires for those flows — long-lived stable paths can sit
// in the gap indefinitely.
// ============================================================================
/// Build a PathMtuNotification body (2 bytes: path_mtu LE).
fn build_path_mtu_notification_body(mtu: u16) -> Vec<u8> {
mtu.to_le_bytes().to_vec()
}
/// Insert an Established session with MMP initialized so the proactive
/// PathMtuNotification handler can apply notifications.
fn install_established_session_with_mmp(node: &mut Node, remote: &Identity) {
let session = make_noise_session(node.identity(), remote);
let remote_addr = *remote.node_addr();
let mut entry = crate::node::session::SessionEntry::new(
remote_addr,
remote.pubkey_full(),
EndToEndState::Established(session),
1000,
true,
);
entry.init_mmp(&node.config.node.session_mmp);
node.sessions.insert(remote_addr, entry);
}
#[test]
fn test_handle_path_mtu_notification_writes_path_mtu_lookup_when_empty() {
let mut node = make_node();
let remote = Identity::generate();
let remote_addr = *remote.node_addr();
let remote_fips = crate::FipsAddress::from_node_addr(&remote_addr);
install_established_session_with_mmp(&mut node, &remote);
assert!(
node.path_mtu_lookup_get(&remote_fips).is_none(),
"lookup should start empty for this destination"
);
let body = build_path_mtu_notification_body(1280);
node.handle_session_path_mtu_notification(&remote_addr, &body);
assert_eq!(
node.path_mtu_lookup_get(&remote_fips),
Some(1280),
"PathMtuNotification should populate path_mtu_lookup with the reported MTU"
);
}
#[test]
fn test_handle_path_mtu_notification_tightens_existing_path_mtu_lookup() {
let mut node = make_node();
let remote = Identity::generate();
let remote_addr = *remote.node_addr();
let remote_fips = crate::FipsAddress::from_node_addr(&remote_addr);
install_established_session_with_mmp(&mut node, &remote);
// Pre-seed with a generous value (e.g., from the discovery seed at link
// promotion time, before the destination's proactive echo arrived).
node.path_mtu_lookup_insert(remote_fips, 1500);
let body = build_path_mtu_notification_body(1280);
node.handle_session_path_mtu_notification(&remote_addr, &body);
assert_eq!(
node.path_mtu_lookup_get(&remote_fips),
Some(1280),
"PathMtuNotification with smaller MTU must tighten the lookup"
);
}
#[test]
fn test_handle_path_mtu_notification_keeps_tighter_existing_path_mtu_lookup() {
let mut node = make_node();
let remote = Identity::generate();
let remote_addr = *remote.node_addr();
let remote_fips = crate::FipsAddress::from_node_addr(&remote_addr);
install_established_session_with_mmp(&mut node, &remote);
// Pre-seed with a tighter value than what the proactive notification
// reports (e.g., from a prior reactive MtuExceeded on a narrower hop).
// The mirror must never loosen the clamp.
node.path_mtu_lookup_insert(remote_fips, 1200);
let body = build_path_mtu_notification_body(1400);
node.handle_session_path_mtu_notification(&remote_addr, &body);
assert_eq!(
node.path_mtu_lookup_get(&remote_fips),
Some(1200),
"PathMtuNotification with looser MTU must not loosen a tighter existing value"
);
}
#[test]
fn test_handle_path_mtu_notification_no_session_no_op() {
let mut node = make_node();
let remote = Identity::generate();
let remote_addr = *remote.node_addr();
let remote_fips = crate::FipsAddress::from_node_addr(&remote_addr);
// No session installed. The handler should drop the notification entirely.
let body = build_path_mtu_notification_body(1280);
node.handle_session_path_mtu_notification(&remote_addr, &body);
assert!(
node.path_mtu_lookup_get(&remote_fips).is_none(),
"PathMtuNotification with no session must not touch path_mtu_lookup"
);
}