diff --git a/src/node/dataplane/rx_loop.rs b/src/node/dataplane/rx_loop.rs index 7e81268d..922afab3 100644 --- a/src/node/dataplane/rx_loop.rs +++ b/src/node/dataplane/rx_loop.rs @@ -387,6 +387,28 @@ impl Node { // binds can be the narrow one, and one that detaches // can be the reason the clamp was tight. self.refresh_tun_mss_ceiling(); + + // A peer reachable only through an interface that has + // gone is not reachable. Withdraw it now rather than + // leaving the liveness reaper to notice up to + // `link_dead_timeout_secs` later, during which this + // node both drops transit traffic in silence and keeps + // advertising reachability it does not have. + // + // Not policy-filtered: whether an interface's absence + // is normal is a statement about node *health*, not + // about whether the routes over it still work. + if !edge.present { + let reaped = + self.reap_peers_on_transport(edge.transport_id).await; + if reaped > 0 { + info!( + transport_id = %edge.transport_id, + peers = reaped, + "Withdrew peers whose interface went away" + ); + } + } } } Some(ipv6_packet) = tun_outbound_rx.recv() => { diff --git a/src/node/handlers/mmp.rs b/src/node/handlers/mmp.rs index 1f7b3b9d..ab8afc06 100644 --- a/src/node/handlers/mmp.rs +++ b/src/node/handlers/mmp.rs @@ -547,6 +547,65 @@ impl Node { /// `now_ms` is the sweep's hoisted wall-clock ms (the same value the old reap /// fed `note_link_dead`); it flows to the executor `ReportLost` arm via /// `ambient.now_ms`. + /// Reap every active peer reachable only through `transport_id`. + /// + /// Called on a transport's detach edge. Until this existed, losing an + /// interface withdrew nothing: the peers stayed in the registry, the + /// routes through them stayed selectable, and this node kept advertising + /// reachability it no longer had — so transit traffic was dropped in + /// silence, and other nodes kept routing toward us for those destinations, + /// until the liveness reaper noticed up to `link_dead_timeout_secs` later. + /// Measured on real hardware that was 27 seconds of routing through a link + /// that had already gone, with four alternative peers available the whole + /// time. + /// + /// The detach edge is both earlier and more certain than inactivity, so it + /// is the better trigger. This routes through the same + /// [`Self::route_link_dead`] the liveness reaper uses rather than + /// open-coding a second teardown — every consequence of losing a peer + /// (sessions, path MTU, session indices, the link, the control machine, + /// tree cleanup and re-announce, bloom withdrawal) already hangs off that + /// one path, and a parallel one would drift from it. + /// + /// Deliberately undamped. A flapping interface cannot drive a reap storm + /// through here, because `ChurnGuard` stops publishing presence edges + /// after three short-lived bindings and does not resume until one lasts — + /// so the edges this reacts to are already rate-limited at the source, and + /// a second damper here would only add a way for the two to disagree. + /// + /// Returns how many peers were reaped. + pub(in crate::node) async fn reap_peers_on_transport( + &mut self, + transport_id: TransportId, + ) -> usize { + let doomed: Vec = self + .peers + .iter() + .filter(|(_, peer)| peer.transport_id() == Some(transport_id)) + .map(|(node_addr, _)| *node_addr) + .collect(); + + if doomed.is_empty() { + return 0; + } + + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); + + let reaped = doomed.len(); + for node_addr in doomed { + debug!( + peer = %self.peer_display_name(&node_addr), + %transport_id, + "Removing peer: its interface went away" + ); + self.route_link_dead(node_addr, now_ms).await; + } + reaped + } + async fn route_link_dead(&mut self, node_addr: NodeAddr, now_ms: u64) { let link = match self.peers.get(&node_addr) { Some(peer) => peer.link_id(), diff --git a/src/node/tests/heartbeat.rs b/src/node/tests/heartbeat.rs index b5ff6e3e..85c8c9d9 100644 --- a/src/node/tests/heartbeat.rs +++ b/src/node/tests/heartbeat.rs @@ -123,3 +123,74 @@ async fn heartbeat_unaffected_without_rekey() { cleanup_nodes(&mut nodes).await; } + +// --------------------------------------------------------------------------- +// Reaping peers when their interface goes away +// +// The detach edge is both earlier and more certain than inactivity, so it is +// the better trigger for withdrawing what the interface carried. These drive +// the same real two-node peering the liveness tests use, because a peer only +// reaches the established context the reap acts on by actually peering. +// --------------------------------------------------------------------------- + +/// A peer reachable only through an interface that has gone is withdrawn on +/// the detach edge, without waiting out `link_dead_timeout_secs`. +/// +/// Note what is *not* set here: the link-dead timeout keeps its default, and +/// no time is advanced. The peer is live by every liveness measure and is +/// still withdrawn, because the transport under it is gone — which is the +/// whole distinction this adds. +#[tokio::test] +async fn a_detached_transport_withdraws_the_peers_that_needed_it() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + verify_tree_convergence(&nodes); + + let addr_1 = *nodes[1].node.node_addr(); + let transport_id = nodes[0] + .node + .get_peer(&addr_1) + .expect("peer present") + .transport_id() + .expect("an established peer names its transport"); + + let reaped = nodes[0].node.reap_peers_on_transport(transport_id).await; + + assert_eq!(reaped, 1); + assert!( + nodes[0].node.get_peer(&addr_1).is_none(), + "a peer must not outlive the interface it was reachable through" + ); + + cleanup_nodes(&mut nodes).await; +} + +/// The reap is scoped to the transport that detached. +/// +/// The failure this guards is the one that would make the feature worse than +/// the defect: an interface going away must not withdraw the peers that were +/// never reachable through it, which on a mesh router is most of them. +#[tokio::test] +async fn a_detached_transport_leaves_other_transports_peers_alone() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + verify_tree_convergence(&nodes); + + let addr_1 = *nodes[1].node.node_addr(); + let peer_transport = nodes[0] + .node + .get_peer(&addr_1) + .expect("peer present") + .transport_id() + .expect("an established peer names its transport"); + + // A transport this peer was never reachable through. + let unrelated = TransportId::new(peer_transport.as_u32() + 100); + let reaped = nodes[0].node.reap_peers_on_transport(unrelated).await; + + assert_eq!(reaped, 0, "an unrelated transport withdraws nothing"); + assert!( + nodes[0].node.get_peer(&addr_1).is_some(), + "a peer on a healthy transport must survive another one detaching" + ); + + cleanup_nodes(&mut nodes).await; +} diff --git a/testing/iface-binding/test.sh b/testing/iface-binding/test.sh index eb0e6603..19a15bde 100755 --- a/testing/iface-binding/test.sh +++ b/testing/iface-binding/test.sh @@ -200,6 +200,21 @@ wait_for() { return 1 } +# Poll until the command prints a value no greater than `want`. +wait_for_at_most() { + local timeout="$1" want="$2"; shift 2 + local i got + for i in $(seq 1 "$timeout"); do + got="$("$@" || true)" + if [ -n "$got" ] && [ "$got" -le "$want" ] 2>/dev/null; then + return 0 + fi + sleep 1 + done + echo " (last value: '${got:-}', wanted <= '$want')" >&2 + return 1 +} + # Poll until the command prints a value that is at least `want`. wait_for_at_least() { local timeout="$1" want="$2"; shift 2 @@ -387,6 +402,20 @@ else echo " which is inside the deadline's reach)" fi +# The peers that interface carried must go with it, and go *now*. +# +# `link_dead_timeout_secs` is at its 30 s default here, so a withdrawal inside +# 15 s can only have come from the detach edge and not from the liveness +# reaper. That gap is the whole point: until the edge drove the teardown, this +# node kept the peer, kept selecting routes through it, and kept advertising +# reachability it no longer had — dropping transit traffic in silence for the +# whole timeout, with alternative paths sitting unused. +if ! wait_for_at_most 15 0 peer_count "$NODE_A"; then + fail "(c) node-a kept a peer that was only reachable over the downed \ +interface; the detach edge did not withdraw it" +fi +pass "(c) the peers the interface carried were withdrawn on the detach edge" + log "Bringing $LAB_IFACE back up on node-a" docker exec "$NODE_A" ip link set "$LAB_IFACE" up @@ -401,6 +430,13 @@ if ! wait_for_at_least 45 2 iface_field "$NODE_A" lab binds; then fi pass "(c) the interface flapped and the daemon followed it both ways" +# And the withdrawal is not a one-way door: the peer comes back over the +# rebound interface on its own, by beacon, with no operator action. +if ! wait_for_at_least 60 1 peer_count "$NODE_A"; then + fail "(c) node-a did not re-peer after its interface came back" +fi +pass "(c) peering re-established over the rebound interface" + # ── (d) destroy and recreate ───────────────────────────────────────────── # # Deleting the netdev outright is the case the old ENXIO beacon-socket reopen