From 6011d233c143b73f6f28e7da90e4a05688e30cd1 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 12 Jul 2026 15:32:04 +0000 Subject: [PATCH] discovery: keep tighter path_mtu when applying a LookupResponse An originator handling a LookupResponse unconditionally overwrote the cached path_mtu_lookup entry, so a looser (larger) estimate in a later response could clobber a tighter value already learned from a reactive MtuExceeded or PathMtuNotification. Read-and-compare before writing and keep the minimum, so a looser discovery estimate no longer loosens the clamp. Add a regression test. --- src/node/handlers/discovery.rs | 37 ++++++++++++++++++++++---------- src/node/tests/discovery.rs | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/node/handlers/discovery.rs b/src/node/handlers/discovery.rs index e3d8c7b..cf688c5 100644 --- a/src/node/handlers/discovery.rs +++ b/src/node/handlers/discovery.rs @@ -240,17 +240,32 @@ impl Node { // 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" - ); - } + Ok(mut map) => match map.get(&fips_addr).copied() { + Some(existing) if existing <= path_mtu => { + // Keep the tighter learned value; never loosen the + // clamp. A reactive MtuExceeded or PathMtuNotification + // tighten takes precedence over a looser discovery + // estimate (cross-carrier keep-tighter). + debug!( + target = %self.peer_display_name(&target), + fips_addr = %fips_addr, + path_mtu = path_mtu, + existing = existing, + "LookupResponse: keeping tighter existing path_mtu_lookup value" + ); + } + other => { + map.insert(fips_addr, path_mtu); + debug!( + target = %self.peer_display_name(&target), + fips_addr = %fips_addr, + path_mtu = path_mtu, + prior = ?other, + map_len = map.len(), + "Wrote path_mtu_lookup from discovery LookupResponse" + ); + } + }, Err(e) => { warn!( target = %self.peer_display_name(&target), diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index 671e911..3503f72 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -912,6 +912,45 @@ async fn test_originator_stores_path_mtu_in_cache() { ); } +#[tokio::test] +async fn test_originator_lookup_response_keeps_tighter_path_mtu_lookup() { + // Regression: a LookupResponse carrying a looser (larger) path_mtu must + // NOT clobber a tighter (smaller) value already in path_mtu_lookup that a + // reactive MtuExceeded or PathMtuNotification learned. Cross-carrier + // keep-tighter: the clamp must never loosen. + let mut node = make_node(); + let from = make_node_addr(0xAA); + + let target_identity = Identity::generate(); + let target = *target_identity.node_addr(); + let root = make_node_addr(0xF0); + let coords = TreeCoordinate::from_addrs(vec![target, root]).unwrap(); + + node.register_identity(target, target_identity.pubkey_full()); + + // Pre-seed a tighter value, as if a reactive signal already narrowed it. + let target_fips = crate::FipsAddress::from_node_addr(&target); + node.path_mtu_lookup_insert(target_fips, 1280); + + let proof_data = LookupResponse::proof_bytes(800, &target, &coords); + let proof = target_identity.sign(&proof_data); + + let mut response = LookupResponse::new(800, target, coords.clone(), proof); + // Looser discovery estimate that must be rejected in favor of the tighter + // existing entry. + response.path_mtu = 1500; + + let payload = &response.encode()[1..]; + + node.handle_lookup_response(&from, payload).await; + + assert_eq!( + node.path_mtu_lookup_get(&target_fips), + Some(1280), + "LookupResponse must not loosen a tighter existing path_mtu_lookup value" + ); +} + // ============================================================================ // Open-Discovery Sweep — cache-injection unit test // ============================================================================