Skip to content

Commit

Permalink
bgp: introduce the LocalRoute structure
Browse files Browse the repository at this point in the history 
Local routes selected by the best-path algorithm require storing
different data compared to Adj-RIB routes. This commit creates a
dedicated data structure for local routes, improving both memory
efficiency and code readability. This also open room for further
memory optimizations in the future.

Signed-off-by: Renato Westphal <[email protected]>
  • Loading branch information
@rwestphal
rwestphal committed Feb 24, 2024
1 parent 0e72b51 commit 4ed5fbd
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 40 deletions.
2 changes: 1 addition & 1 deletion holo-bgp/src/events.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ fn process_nbr_route_refresh_af<A>(
let table = A::table(&mut instance.state.rib.tables);
let update_queue = A::update_queue(&mut nbr.update_queues);
for (prefix, dest) in &table.prefixes {
let (route, _) = dest.local.as_ref().unwrap();
let route = dest.local.as_ref().unwrap();
let attrs = route.attrs.get();
update_queue.reach.entry(attrs).or_default().insert(*prefix);
}
Expand Down
14 changes: 12 additions & 2 deletions holo-bgp/src/neighbor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::packet::message::{
Capability, DecodeCxt, EncodeCxt, FourOctetAsNumber, KeepaliveMsg, Message,
NotificationMsg, OpenMsg,
};
use crate::rib::Rib;
use crate::rib::{Rib, Route};
use crate::tasks::messages::input::{NbrRxMsg, NbrTimerMsg, TcpConnectMsg};
use crate::tasks::messages::output::NbrTxMsg;
#[cfg(feature = "testing")]
Expand Down Expand Up @@ -883,7 +883,17 @@ impl Neighbor {
.prefixes
.iter()
.filter_map(|(prefix, dest)| {
dest.local.as_ref().map(|route| (*prefix, route.0.clone()))
dest.local.as_ref().map(|route| {
let route = Route {
origin: route.origin,
attrs: route.attrs.clone(),
route_type: route.route_type,
last_modified: route.last_modified,
ineligible_reason: None,
reject_reason: None,
};
(*prefix, Box::new(route))
})
})
.collect::<Vec<_>>();

Expand Down
40 changes: 17 additions & 23 deletions holo-bgp/src/northbound/state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::packet::attribute::{
};
use crate::packet::consts::{Afi, AttrFlags, Safi};
use crate::packet::message::{AddPathTuple, Capability};
use crate::rib::{AttrSet, Route};
use crate::rib::{AttrSet, LocalRoute, Route};

pub static CALLBACKS: Lazy<Callbacks<Instance>> = Lazy::new(load_callbacks);

Expand Down Expand Up @@ -53,8 +53,8 @@ pub enum ListEntry<'a> {
RibAsPathSegmentMember(u32),
RibClusterList(Ipv4Addr),
RibNeighbor(AfiSafi, &'a Neighbor),
RibV4LocRoute(&'a Ipv4Network, &'a Route),
RibV6LocRoute(&'a Ipv6Network, &'a Route),
RibV4LocRoute(&'a Ipv4Network, &'a LocalRoute),
RibV6LocRoute(&'a Ipv6Network, &'a LocalRoute),
RibV4AdjInPreRoute(&'a Ipv4Network, &'a Route),
RibV6AdjInPreRoute(&'a Ipv6Network, &'a Route),
RibV4AdjInPostRoute(&'a Ipv4Network, &'a Route),
Expand Down Expand Up @@ -815,7 +815,7 @@ fn load_callbacks() -> Callbacks<Instance> {
&& let Some(state) = &instance.state {
let iter = state.rib.tables.ipv4_unicast.prefixes.iter().filter_map(
|(prefix, dest)| {
dest.local.as_ref().map(|(route, _)| {
dest.local.as_ref().map(|route| {
ListEntry::RibV4LocRoute(prefix, route)
})
},
Expand Down Expand Up @@ -852,14 +852,12 @@ fn load_callbacks() -> Callbacks<Instance> {
Some(route.last_modified)
})
.path(bgp::rib::afi_safis::afi_safi::ipv4_unicast::loc_rib::routes::route::eligible_route::PATH)
.get_element_bool(|_instance, args| {
let (_, route) = args.list_entry.as_rib_v4_loc_route().unwrap();
Some(route.is_eligible())
.get_element_bool(|_instance, _args| {
None
})
.path(bgp::rib::afi_safis::afi_safi::ipv4_unicast::loc_rib::routes::route::ineligible_reason::PATH)
.get_element_string(|_instance, args| {
let (_, route) = args.list_entry.as_rib_v4_loc_route().unwrap();
route.ineligible_reason.as_ref().map(|r| r.to_yang().into())
.get_element_string(|_instance, _args| {
None
})
.path(bgp::rib::afi_safis::afi_safi::ipv4_unicast::loc_rib::routes::route::unknown_attributes::unknown_attribute::PATH)
.get_iterate(|_instance, args| {
Expand Down Expand Up @@ -898,9 +896,8 @@ fn load_callbacks() -> Callbacks<Instance> {
Some(attr.value.to_vec())
})
.path(bgp::rib::afi_safis::afi_safi::ipv4_unicast::loc_rib::routes::route::reject_reason::PATH)
.get_element_string(|_instance, args| {
let (_, route) = args.list_entry.as_rib_v4_loc_route().unwrap();
route.reject_reason.as_ref().map(|r| r.to_yang().into())
.get_element_string(|_instance, _args| {
None
})
.path(bgp::rib::afi_safis::afi_safi::ipv4_unicast::neighbors::neighbor::PATH)
.get_iterate(|instance, args| {
Expand Down Expand Up @@ -1316,7 +1313,7 @@ fn load_callbacks() -> Callbacks<Instance> {
&& let Some(state) = &instance.state {
let iter = state.rib.tables.ipv6_unicast.prefixes.iter().filter_map(
|(prefix, dest)| {
dest.local.as_ref().map(|(route, _)| {
dest.local.as_ref().map(|route| {
ListEntry::RibV6LocRoute(prefix, route)
})
},
Expand Down Expand Up @@ -1353,14 +1350,12 @@ fn load_callbacks() -> Callbacks<Instance> {
Some(route.last_modified)
})
.path(bgp::rib::afi_safis::afi_safi::ipv6_unicast::loc_rib::routes::route::eligible_route::PATH)
.get_element_bool(|_instance, args| {
let (_, route) = args.list_entry.as_rib_v6_loc_route().unwrap();
Some(route.is_eligible())
.get_element_bool(|_instance, _args| {
None
})
.path(bgp::rib::afi_safis::afi_safi::ipv6_unicast::loc_rib::routes::route::ineligible_reason::PATH)
.get_element_string(|_instance, args| {
let (_, route) = args.list_entry.as_rib_v6_loc_route().unwrap();
route.ineligible_reason.as_ref().map(|r| r.to_yang().into())
.get_element_string(|_instance, _args| {
None
})
.path(bgp::rib::afi_safis::afi_safi::ipv6_unicast::loc_rib::routes::route::unknown_attributes::unknown_attribute::PATH)
.get_iterate(|_instance, args| {
Expand Down Expand Up @@ -1399,9 +1394,8 @@ fn load_callbacks() -> Callbacks<Instance> {
Some(attr.value.to_vec())
})
.path(bgp::rib::afi_safis::afi_safi::ipv6_unicast::loc_rib::routes::route::reject_reason::PATH)
.get_element_string(|_instance, args| {
let (_, route) = args.list_entry.as_rib_v6_loc_route().unwrap();
route.reject_reason.as_ref().map(|r| r.to_yang().into())
.get_element_string(|_instance, _args| {
None
})
.path(bgp::rib::afi_safis::afi_safi::ipv6_unicast::neighbors::neighbor::PATH)
.get_iterate(|instance, args| {
Expand Down
37 changes: 28 additions & 9 deletions holo-bgp/src/rib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub struct RoutingTable<A: AddressFamily> {

#[derive(Debug, Default)]
pub struct Destination {
pub local: Option<(Box<Route>, BTreeSet<IpAddr>)>,
pub local: Option<Box<LocalRoute>>,
pub adj_rib: BTreeMap<IpAddr, AdjRib>,
}

Expand All @@ -64,6 +64,15 @@ pub struct AdjRib {
pub out_post: Option<Box<Route>>,
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct LocalRoute {
pub origin: RouteOrigin,
pub attrs: RouteAttrs,
pub route_type: RouteType,
pub last_modified: Instant,
pub nexthops: BTreeSet<IpAddr>,
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Route {
pub origin: RouteOrigin,
Expand Down Expand Up @@ -537,27 +546,37 @@ pub(crate) fn loc_rib_update<A>(
compute_nexthops::<A>(dest, &best_route, selection_cfg, mpath_cfg);

// Return early if no change in Loc-RIB is needed.
if let Some((loc_route, loc_nexthops)) = &dest.local
&& *loc_route == best_route
&& *loc_nexthops == nexthops
if let Some(local_route) = &dest.local
&& local_route.origin == best_route.origin
&& local_route.attrs == best_route.attrs
&& local_route.route_type == best_route.route_type
&& local_route.nexthops == nexthops
{
return;
}

// Install route in the global RIB.
// Create new local route.
let local_route = LocalRoute {
origin: best_route.origin,
attrs: best_route.attrs,
route_type: best_route.route_type,
last_modified: best_route.last_modified,
nexthops,
};

// Install local route in the global RIB.
southbound::tx::route_install(
ibus_tx,
prefix,
&best_route,
&nexthops,
&local_route,
match best_route.route_type {
RouteType::Internal => distance_cfg.internal,
RouteType::External => distance_cfg.external,
},
);

// Insert route into the Loc-RIB.
dest.local = Some((best_route, nexthops));
// Insert local route into the Loc-RIB.
dest.local = Some(Box::new(local_route));
} else {
Debug::BestPathNotFound(prefix.into()).log();

Expand Down
9 changes: 4 additions & 5 deletions holo-bgp/src/southbound/tx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
//

use std::collections::BTreeSet;
use std::net::IpAddr;

use holo_utils::ibus::{IbusMsg, IbusSender};
use holo_utils::protocol::Protocol;
Expand All @@ -14,7 +13,7 @@ use holo_utils::southbound::{
};
use ipnetwork::IpNetwork;

use crate::rib::Route;
use crate::rib::LocalRoute;

// ===== global functions =====

Expand All @@ -25,12 +24,12 @@ pub(crate) fn router_id_query(ibus_tx: &IbusSender) {
pub(crate) fn route_install(
ibus_tx: &IbusSender,
prefix: impl Into<IpNetwork>,
route: &Route,
nexthops: &BTreeSet<IpAddr>,
route: &LocalRoute,
distance: u8,
) {
// Fill-in nexthops.
let nexthops = nexthops
let nexthops = route
.nexthops
.iter()
.map(|nexthop| Nexthop::Address {
// TODO
Expand Down

0 comments on commit 4ed5fbd

Please sign in to comment.