mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-12 09:33:23 +00:00
Merge branch 'master' into next
This commit is contained in:
@@ -162,6 +162,34 @@ impl Node {
|
||||
transport_id = %packet.transport_id,
|
||||
"Unknown FMP version, dropping"
|
||||
);
|
||||
|
||||
// If the packet arrived on an adopted Nostr-NAT bootstrap
|
||||
// transport, the originating peer is necessarily on a
|
||||
// different FMP-protocol version than us — the discovery
|
||||
// sweep would otherwise re-traverse them every cycle even
|
||||
// though no msg1/msg2 exchange can ever succeed. Bump the
|
||||
// discovery-layer cooldown to the long protocol-mismatch
|
||||
// window and emit a single WARN per fresh observation.
|
||||
if self.bootstrap_transports.contains(&packet.transport_id)
|
||||
&& let Some(npub) = self
|
||||
.bootstrap_transport_npubs
|
||||
.get(&packet.transport_id)
|
||||
.cloned()
|
||||
&& let Some(handle) = self.nostr_discovery_handle()
|
||||
{
|
||||
let now_ms = Self::now_ms();
|
||||
let cooldown_secs = handle.protocol_mismatch_cooldown_secs();
|
||||
if handle.record_protocol_mismatch(&npub, now_ms) {
|
||||
warn!(
|
||||
peer_npub = %npub,
|
||||
transport_id = %packet.transport_id,
|
||||
peer_version = prefix.version,
|
||||
our_version = FMP_VERSION,
|
||||
cooldown_secs,
|
||||
"Nostr-discovered peer speaks a different FMP version; suppressing retraversal"
|
||||
);
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -1029,7 +1029,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) => {
|
||||
@@ -1053,16 +1057,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"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1895,6 +1895,8 @@ impl Node {
|
||||
crate::transport::TransportHandle::Udp(transport),
|
||||
);
|
||||
self.bootstrap_transports.insert(transport_id);
|
||||
self.bootstrap_transport_npubs
|
||||
.insert(transport_id, traversal.peer_npub.clone());
|
||||
|
||||
let remote_addr = TransportAddr::from_string(&traversal.remote_addr.to_string());
|
||||
if let Err(err) = self
|
||||
@@ -1902,6 +1904,7 @@ impl Node {
|
||||
.await
|
||||
{
|
||||
self.bootstrap_transports.remove(&transport_id);
|
||||
self.bootstrap_transport_npubs.remove(&transport_id);
|
||||
if let Some(mut handle) = self.transports.remove(&transport_id) {
|
||||
let _ = handle.stop().await;
|
||||
}
|
||||
|
||||
@@ -458,6 +458,13 @@ pub struct Node {
|
||||
startup_open_discovery_sweep_done: bool,
|
||||
/// Per-peer UDP transports adopted from NAT traversal handoff.
|
||||
bootstrap_transports: HashSet<TransportId>,
|
||||
/// Originating peer npub (bech32) for each adopted bootstrap
|
||||
/// transport, captured at `adopt_established_traversal` time.
|
||||
/// Populated alongside `bootstrap_transports`; cleared in
|
||||
/// `cleanup_bootstrap_transport_if_unused`. Used by the rx loop to
|
||||
/// route fatal-protocol-mismatch observations back to the
|
||||
/// Nostr-discovery `failure_state` for long cooldown application.
|
||||
bootstrap_transport_npubs: HashMap<TransportId, String>,
|
||||
|
||||
// === Periodic Parent Re-evaluation ===
|
||||
/// Timestamp of last periodic parent re-evaluation (for pacing).
|
||||
@@ -626,6 +633,7 @@ impl Node {
|
||||
nostr_discovery_started_at_ms: None,
|
||||
startup_open_discovery_sweep_done: false,
|
||||
bootstrap_transports: HashSet::new(),
|
||||
bootstrap_transport_npubs: HashMap::new(),
|
||||
last_parent_reeval: None,
|
||||
last_congestion_log: None,
|
||||
estimated_mesh_size: None,
|
||||
@@ -758,6 +766,7 @@ impl Node {
|
||||
nostr_discovery_started_at_ms: None,
|
||||
startup_open_discovery_sweep_done: false,
|
||||
bootstrap_transports: HashSet::new(),
|
||||
bootstrap_transport_npubs: HashMap::new(),
|
||||
last_parent_reeval: None,
|
||||
last_congestion_log: None,
|
||||
estimated_mesh_size: None,
|
||||
@@ -1496,6 +1505,7 @@ impl Node {
|
||||
);
|
||||
|
||||
self.bootstrap_transports.remove(&transport_id);
|
||||
self.bootstrap_transport_npubs.remove(&transport_id);
|
||||
self.transport_drops.remove(&transport_id);
|
||||
self.transports.remove(&transport_id);
|
||||
}
|
||||
|
||||
@@ -2206,3 +2206,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"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user