bgp: give AddressFamily an associated Prefix key type - #132
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add an AddressFamily::Prefix associated type for NLRI/RIB keys while keeping policy, northbound, and IBus boundaries on normal IP network values. IPv4 and IPv6 unicast use identity conversions, preserving current behavior. Switch BGP RIB prefix storage from PrefixMap to BTreeMap so future address families are not constrained by prefix_trie's primitive-integer key representation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| } | ||
|
|
||
| #[cfg(feature = "testing")] | ||
| pub fn spawn_protocol_task_with_test<P>( |
There was a problem hiding this comment.
This would work just fine with the old spawn_protocol_task function signature. I don't think there is a need to separate it into 3 different functions.
The feature flag will handle that.
| impl TimeoutTask { | ||
| /// Returns an inert timeout handle for deterministic protocol tests. | ||
| #[cfg(feature = "testing")] | ||
| pub fn new<F, Fut>(_timeout: Duration, _cb: F) -> TimeoutTask |
There was a problem hiding this comment.
Descent proposition but this is handled by default in each protocols' task.rs file. e.g
Line 409 in c52e1aa
We may need to add some lines of code before the TimeourTask {} is called in future for some protocols, and this limits that.
|
|
||
| #[derive(Debug)] | ||
| pub struct RoutingTable<A: AddressFamily> { | ||
| pub prefixes: PrefixMap<A::IpNetwork, Destination>, |
There was a problem hiding this comment.
Not sure what I think about nuking the PrefixMap for BtreeMap.
It works particularly well in this context where we need a Map that is more flexible than the traditional Ipv4 & Ipv6 Unicast addresses...also way better for insertions when the number of prefixes grow.
However, once the tables REALLY grow in size, PrefixMap becomes really useful in terms of searching through the existing prefixes.
We can give this some more thought ?
I may be wrong of course, but I think we can do a compromise...
Maybe having a PrefixMap for the IPV4 and IPV6 Addresses. Then for the rest (like VPNv6), we could use a BTreeMap. Throwing a thought out there, I'd like to know yours.
| // Get prefix RIB entry. | ||
| let rib = &mut instance.state.rib; | ||
| let table = A::table(&mut rib.tables); | ||
| let prefix = A::prefix_from_ip_network(prefix.into()).unwrap(); |
There was a problem hiding this comment.
We can use a default here instead of unwrap....
2 participants
Preparatory refactor for multiprotocol RIBs (VPN, labeled-unicast, EVPN). Adds an associated
Prefixtype to theAddressFamilytrait, separating the RIB/NLRI key from the policy/wire IP-network type, with conversion hooks. IPv4/IPv6 unicast keepPrefix = IpNetwork; behavior is unchanged.The BGP RIB container changes from
prefix_trie::PrefixMapto an exact-key ordered map, because BGP uses exact lookup/iteration (not LPM); this also avoids forcing wider NLRI keys (e.g. VPNv6 = RD + IPv6) into a single primitive-integer representation.Includes a small test-infrastructure commit so workspace tests build under the testing feature.
🤖 Generated with Claude Code