mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-22 07:48:26 +00:00
node: re-push TreeAnnounce when a peer advertises a worse root
A node with a single tree peer has its periodic parent re-evaluation disabled (it needs at least two peers for a meaningful comparison), so it depends entirely on its peer pushing a TreeAnnounce for it to attach. That push happens once at promotion time plus on the parent's slow periodic no-change re-broadcast. If the one-shot attaching announce is lost, the single-uplink node falls back to self-root and cannot recover until the next periodic re-broadcast (reeval_interval_secs later), stranding it out of the tree and unreachable end-to-end in the interim. Make tree-position exchange self-healing on the receive path: when an accepted TreeAnnounce advertises a root strictly worse (higher NodeAddr, since election is smallest-wins) than our own, echo our current declaration back to that peer. A stranded self-root node's announce now provokes its better-rooted peer to re-push its real position immediately, so the node re-attaches within a round-trip instead of waiting for the periodic cadence. Echo only in that one direction. If the peer's root is lower (better) than ours, we are the stale side: the peer would ignore our worse root anyway and we converge via the parent re-evaluation that follows, so echoing back is pure waste and would double announce traffic in the learning direction during a root change or partition merge. Equal roots are already converged. The echo is bounded by the existing per-peer 500 ms tree-announce rate limiter and is a no-op once the peer adopts our root, so it adds no traffic in a converged mesh. Add a spanning-tree unit test that drives a converged child back to self-root with its peer-ancestry view of the root cleared (modelling the lost attaching announce), and asserts the root re-pushes on the resulting root disagreement and the child re-attaches.
This commit is contained in:
@@ -852,3 +852,103 @@ async fn test_rejects_tree_announce_with_inconsistent_root() {
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
/// A peer whose TreeAnnounce advertises a root worse (higher NodeAddr) than
|
||||
/// ours must be answered with our own current TreeAnnounce, so a node that has
|
||||
/// fallen back to self-root (e.g. because its parent's attaching announce was
|
||||
/// lost) can re-attach without waiting for the parent's slow periodic
|
||||
/// re-broadcast.
|
||||
///
|
||||
/// This reproduces, at the unit level, the single-uplink-leaf convergence
|
||||
/// wedge: a node with exactly one peer has its periodic parent re-evaluation
|
||||
/// disabled, so if it misses its parent's attaching announce it is stranded as
|
||||
/// a self-root until the parent's next periodic re-broadcast. The receive-path
|
||||
/// re-push makes recovery event-driven: the stranded node's self-root announce
|
||||
/// provokes its better-rooted parent to echo its real position straight back.
|
||||
#[tokio::test]
|
||||
async fn test_tree_announce_repushed_on_root_disagreement() {
|
||||
// Converge a 2-node tree: one node is root, the other its child.
|
||||
let mut nodes = run_tree_test(2, &[(0, 1)], false).await;
|
||||
|
||||
// Identify which index is the root (smallest NodeAddr) and which is the
|
||||
// attached child — the topology is address-order dependent.
|
||||
let root_idx = if nodes[0].node.tree_state().is_root() {
|
||||
0
|
||||
} else {
|
||||
1
|
||||
};
|
||||
let child_idx = 1 - root_idx;
|
||||
|
||||
let root_addr = *nodes[root_idx].node.node_addr();
|
||||
let child_addr = *nodes[child_idx].node.node_addr();
|
||||
|
||||
// Sanity: before the disturbance the child is attached to the root.
|
||||
assert_eq!(
|
||||
nodes[child_idx].node.tree_state().root(),
|
||||
&root_addr,
|
||||
"child should start attached to the root"
|
||||
);
|
||||
assert!(
|
||||
!nodes[child_idx].node.tree_state().is_root(),
|
||||
"child should not start as its own root"
|
||||
);
|
||||
|
||||
// Simulate the lost attaching announce: force the child back to self-root
|
||||
// AND drop its cached view of the root's tree position, as it would be if
|
||||
// it had never processed the root's attaching announce. The child's
|
||||
// advertised root is now itself, disagreeing with the root's view, and its
|
||||
// own periodic re-evaluation cannot recover it (single peer).
|
||||
{
|
||||
// Field-split borrow (tree_state mut + identity immut) mirrors the
|
||||
// production re-sign sites; Identity is not Clone here.
|
||||
let n = &mut nodes[child_idx].node;
|
||||
n.tree_state.become_root();
|
||||
n.tree_state.remove_peer(&root_addr);
|
||||
n.tree_state.sign_declaration(&n.identity).unwrap();
|
||||
}
|
||||
assert!(nodes[child_idx].node.tree_state().is_root());
|
||||
|
||||
// Drain any packets the disturbance may have queued so the counters below
|
||||
// isolate the re-push.
|
||||
let _ = drain_all_packets(&mut nodes, false).await;
|
||||
|
||||
// The stranded child announces its (self-root) position to its only peer,
|
||||
// the root. The root's advertised root differs from the child's, so the
|
||||
// root must re-push its current announce back.
|
||||
let root_sent_before = nodes[root_idx].node.stats().tree.sent;
|
||||
nodes[child_idx]
|
||||
.node
|
||||
.send_tree_announce_to_peer(&root_addr)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Deliver the child's announce to the root and let the resulting re-push
|
||||
// (and the child's processing of it) settle.
|
||||
let _ = drain_all_packets(&mut nodes, false).await;
|
||||
|
||||
// The root emitted at least one TreeAnnounce in response to the child's
|
||||
// root-disagreeing announce (the receive-path re-push).
|
||||
let root_sent_after = nodes[root_idx].node.stats().tree.sent;
|
||||
assert!(
|
||||
root_sent_after > root_sent_before,
|
||||
"root should re-push its TreeAnnounce when a peer advertises a worse root \
|
||||
(sent before={}, after={})",
|
||||
root_sent_before,
|
||||
root_sent_after
|
||||
);
|
||||
|
||||
// End state: the child has re-attached to the root rather than remaining a
|
||||
// stranded self-root.
|
||||
assert!(
|
||||
!nodes[child_idx].node.tree_state().is_root(),
|
||||
"child should have re-attached, not remained a self-root"
|
||||
);
|
||||
assert_eq!(
|
||||
nodes[child_idx].node.tree_state().root(),
|
||||
&root_addr,
|
||||
"child should have re-converged to the root"
|
||||
);
|
||||
let _ = child_addr;
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
@@ -216,6 +216,41 @@ impl Node {
|
||||
"Processed TreeAnnounce"
|
||||
);
|
||||
|
||||
// Re-push our current position when the announcing peer advertises a
|
||||
// strictly worse (higher NodeAddr) root than ours. Root election is
|
||||
// smallest-NodeAddr-wins, so a peer on a higher root has a stale or
|
||||
// pre-attachment view and can attach to (or re-attach through) us;
|
||||
// reply with our current declaration so it does so without waiting
|
||||
// for the next periodic re-broadcast cadence.
|
||||
//
|
||||
// Only the better-rooted side echoes. If the peer's root is lower
|
||||
// (better) than ours, WE are the stale side: the peer would ignore
|
||||
// our worse root anyway, and we converge via the parent re-evaluation
|
||||
// below, so echoing back is pure waste — and during a root change or
|
||||
// partition merge it would double announce traffic in the learning
|
||||
// direction. Equal roots are already converged. Restricting to `>`
|
||||
// keeps the echo to the one direction that helps.
|
||||
//
|
||||
// This closes a convergence wedge on a single-uplink node: its only
|
||||
// peer pushes the attaching announce once at promotion time, and if
|
||||
// that datagram is lost the single-uplink node cannot self-correct
|
||||
// (its own periodic parent re-evaluation is disabled below two peers)
|
||||
// and is stranded as a self-root until the parent's next periodic
|
||||
// re-broadcast (~reeval_interval_secs later). Echoing on root
|
||||
// disagreement makes tree-position exchange self-healing on the
|
||||
// receive path and is naturally bounded by the per-peer 500 ms
|
||||
// tree-announce rate limiter, so it does not storm during normal
|
||||
// convergence (it stops as soon as the peer adopts our root).
|
||||
if *announce.ancestry.root_id() > *self.tree_state.root()
|
||||
&& let Err(e) = self.send_tree_announce_to_peer(from).await
|
||||
{
|
||||
debug!(
|
||||
peer = %self.peer_display_name(from),
|
||||
error = %e,
|
||||
"Failed to re-push TreeAnnounce on root disagreement"
|
||||
);
|
||||
}
|
||||
|
||||
// Bloom filter exchange initiation is handled at handshake completion
|
||||
// ([handshake.rs] mark_update_needed on the new peer) and on actual
|
||||
// content changes via [bloom.rs::handle_filter_announce]'s
|
||||
|
||||
Reference in New Issue
Block a user