diff --git a/Cargo.lock b/Cargo.lock index 2c19edc92..d3f759fcf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1910,6 +1910,7 @@ dependencies = [ "rand 0.10.3", "serde", "serde_json", + "smallvec", "thiserror", "time", "tokio", diff --git a/bpa/Cargo.toml b/bpa/Cargo.toml index 8a78199c8..02879e63b 100644 --- a/bpa/Cargo.toml +++ b/bpa/Cargo.toml @@ -50,6 +50,7 @@ trace-err = "1" rand = { version = "0.10", default-features = false, features = ["alloc", "sys_rng"] } hashbrown = "0.17" foldhash = { version = "0.2", default-features = false } +smallvec = "1" lru = { version = "0.18", default-features = false } portable-atomic = { version = "1", default-features = false, features = ["fallback"] } thiserror = { version = "2", default-features = false } diff --git a/bpa/src/routing/rib.rs b/bpa/src/routing/rib.rs index 3bc08f343..51e4de79f 100644 --- a/bpa/src/routing/rib.rs +++ b/bpa/src/routing/rib.rs @@ -25,7 +25,7 @@ use super::{ Error, Result, RoutingAgent, TableId, action::{Action, InternalAction, RouteAction}, agent::sink::Sink, - table::{Entry, LookupResult, RouteTable}, + table::{Entry, LookupResult, RouteTable, Trail}, }; use crate::{ Arc, HashMap, HashSet, @@ -303,7 +303,7 @@ impl Rib { let result = table.find_recurse( key.lookup_eid(&bundle.primary().destination), true, - &mut HashSet::new(), + &mut Trail::new(), )?; let previous; @@ -311,7 +311,7 @@ impl Rib { previous = bundle .previous_node() .unwrap_or_else(|| bundle.id().source.clone()); - table.find_recurse(&previous, false, &mut HashSet::new())? + table.find_recurse(&previous, false, &mut Trail::new())? } else { result }; diff --git a/bpa/src/routing/table.rs b/bpa/src/routing/table.rs index ca7479449..88d376483 100644 --- a/bpa/src/routing/table.rs +++ b/bpa/src/routing/table.rs @@ -2,6 +2,7 @@ use core::cmp::Ordering; use hardy_bpv7::{eid::Eid, status_report::ReasonCode}; use hardy_eid_patterns::EidPattern; +use smallvec::SmallVec; use tracing::trace; #[cfg(feature = "instrument")] @@ -190,11 +191,11 @@ impl RouteTable { &'a self, to: &'a Eid, reflect: bool, - trail: &mut HashSet<&'a Eid>, + trail: &mut Trail<'a>, ) -> Option> { trace!("Looking for route for {to}"); - let mut peers: Vec<(u32, &'a Eid)> = Vec::new(); + let mut peers: PeerVec<'a> = SmallVec::new(); for entries in self.routes.values() { for (pattern, actions) in entries { if pattern.matches(to) { @@ -211,13 +212,14 @@ impl RouteTable { } } Action::Route(RouteAction::Via(via)) => { - if !trail.insert(to) { + if trail.contains(&to) { trace!("Skipping recursive route for {to}"); continue; } + trail.push(to); let sub_result = self.find_recurse(via, reflect, trail); - trail.remove(&to); + trail.pop(); // Carry each peer's resolved next-hop up unchanged: it is the // adjacent neighbour EID recorded at the Forward base case, which @@ -255,7 +257,7 @@ impl RouteTable { let (peer, next_hop) = peers.remove(0); return Some(LookupResult::Forward(peer, next_hop)); } - _ => return Some(LookupResult::ForwardEcmp(peers)), + _ => return Some(LookupResult::ForwardEcmp(peers.into_vec())), } } } @@ -264,7 +266,7 @@ impl RouteTable { } pub(super) fn find_peers(&self, to: &Eid) -> Option> { - match self.find_recurse(to, false, &mut HashSet::new()) { + match self.find_recurse(to, false, &mut Trail::new()) { Some(LookupResult::Forward(peer, _)) => Some([peer].into()), Some(LookupResult::ForwardEcmp(peers)) => { Some(peers.into_iter().map(|(peer, _)| peer).collect()) @@ -289,7 +291,17 @@ impl RouteTable { } } -fn sorted_insert<'a>(peers: &mut Vec<(u32, &'a Eid)>, peer: u32, next_hop: &'a Eid) { +/// Cycle-detection trail for the recursive via walk. Via chains are short, +/// so the inline capacity keeps the common lookup allocation-free; it is used +/// only as a set (membership, push on descent, pop on return). +pub(super) type Trail<'a> = SmallVec<[&'a Eid; 4]>; + +/// Candidate next-hop peers accumulated during a lookup. The non-ECMP case +/// resolves to zero or one peer, which the inline capacity holds without +/// allocating. +type PeerVec<'a> = SmallVec<[(u32, &'a Eid); 2]>; + +fn sorted_insert<'a>(peers: &mut PeerVec<'a>, peer: u32, next_hop: &'a Eid) { if let Err(idx) = peers.binary_search_by_key(&peer, |(p, _)| *p) { peers.insert(idx, (peer, next_hop)); } @@ -501,14 +513,14 @@ mod tests { // ipn:0.2.5 matches both routes; the union's matching member is the // most specific pattern in the table and must win. let to: Eid = "ipn:0.2.5".parse().unwrap(); - match table.find_recurse(&to, false, &mut HashSet::new()) { + match table.find_recurse(&to, false, &mut Trail::new()) { Some(LookupResult::Forward(peer, _)) => assert_eq!(peer, 1), other => panic!("unexpected lookup result: {other:?}"), } // The union's broad member routes independently. let elsewhere: Eid = "ipn:7.7.7".parse().unwrap(); - match table.find_recurse(&elsewhere, false, &mut HashSet::new()) { + match table.find_recurse(&elsewhere, false, &mut Trail::new()) { Some(LookupResult::Forward(peer, _)) => assert_eq!(peer, 1), other => panic!("unexpected lookup result: {other:?}"), } @@ -517,10 +529,10 @@ mod tests { assert!(table.remove(&union, &union_entry, 10)); assert!( table - .find_recurse(&elsewhere, false, &mut HashSet::new()) + .find_recurse(&elsewhere, false, &mut Trail::new()) .is_none() ); - match table.find_recurse(&to, false, &mut HashSet::new()) { + match table.find_recurse(&to, false, &mut Trail::new()) { Some(LookupResult::Forward(peer, _)) => assert_eq!(peer, 2), other => panic!("unexpected lookup result: {other:?}"), }