diff --git a/crates/bench/benches/transit.rs b/crates/bench/benches/transit.rs index 278e00d..239166f 100644 --- a/crates/bench/benches/transit.rs +++ b/crates/bench/benches/transit.rs @@ -42,6 +42,7 @@ fn run( let server = TestServer::start_with(4_200_000_000, |config| { config.export = Arc::new(ExportPolicy { learned_routes: !matches!(passing, Passing::Not), + all_paths: false, marks: match passing { Passing::Dressed => Marks::parse(&[], &["64601:7".to_string()], &[]).unwrap(), _ => Marks::default(), diff --git a/crates/integration-tests/src/test_server.rs b/crates/integration-tests/src/test_server.rs index 58d9528..405ba58 100644 --- a/crates/integration-tests/src/test_server.rs +++ b/crates/integration-tests/src/test_server.rs @@ -29,6 +29,9 @@ pub struct TestServer { handle: JoinHandle>, /// The single IPv4 loopback address the server is bound to. pub addr: SocketAddr, + /// Every address it is bound to, in the order configured: `addr` + /// first. + pub addrs: Vec, /// The server's routing table. pub rib: Arc, /// Handle to the server's locally originated routes. @@ -85,11 +88,8 @@ impl TestServer { customize(&mut config); let bound = BgpServer::bind(config).await?; - let addr = bound - .local_addrs() - .into_iter() - .next() - .expect("at least one listener"); + let addrs = bound.local_addrs(); + let addr = *addrs.first().expect("at least one listener"); let rib = Arc::clone(&bound.rib); let local_rib = bound.local_rib.clone(); let registry = bound.registry.clone(); @@ -102,6 +102,7 @@ impl TestServer { Ok(Self { handle, addr, + addrs, rib, local_rib, registry, diff --git a/crates/integration-tests/tests/add_path.rs b/crates/integration-tests/tests/add_path.rs index 55b12bb..7c96002 100644 --- a/crates/integration-tests/tests/add_path.rs +++ b/crates/integration-tests/tests/add_path.rs @@ -1,7 +1,9 @@ //! End-to-end tests of ADD-PATH (RFC 7911): a peer sending several routes //! for one prefix, told apart by path identifiers. -use std::net::{Ipv4Addr, SocketAddr}; +use std::collections::BTreeMap; +use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; +use std::sync::Arc; use std::time::Duration; use bgp_integration_tests::announcement::{Announcement, withdrawal}; @@ -15,7 +17,7 @@ use bgp_primitives::ip_prefix::Nlri; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::config::PeerConfig; use bgp_server::family::SUPPORTED; -use bgp_server::policy::ImportPolicy; +use bgp_server::policy::{ExportPolicy, ImportPolicy}; use eyre::Result; use serde_json::Value; @@ -242,3 +244,148 @@ async fn test_prefixes_are_sent_with_path_identifiers_when_agreed() -> Result<() assert_eq!(held(&server), [(0, vec![PEER_ASN], true)]); Ok(()) } + +const COLLECTOR_ASN: u32 = 65100; + +/// What a collector holds: for each prefix and path identifier, the +/// route's AS path. +type Held = BTreeMap<(String, u32), Vec>; + +/// Reads what the collector is told, into `held`, until `done` says so. +async fn told( + collector: &mut TestPeer, + held: &mut Held, + done: impl Fn(&Held) -> bool, +) -> Result<()> { + while !done(held) { + let update = collector.recv_update().await?; + let path = update + .path_attributes + .iter() + .find_map(|attribute| match attribute { + PathAttribute::AsPath(path) => Some(path.segments[0].path.clone()), + _ => None, + }) + .unwrap_or_default(); + for nlri in &update.withdrawn_routes { + held.remove(&(nlri.prefix.to_string(), nlri.path_id)); + } + for nlri in &update.announced_routes { + held.insert((nlri.prefix.to_string(), nlri.path_id), path.clone()); + } + } + Ok(()) +} + +/// The routes held for 203.0.113.0/24, by path identifier. +fn paths(held: &Held) -> Vec<(u32, Vec)> { + held.iter() + .map(|((prefix, id), path)| { + assert_eq!(prefix, "203.0.113.0/24"); + (*id, path.clone()) + }) + .collect() +} + +/// A collector that agreed to be sent several routes for a prefix, and +/// whose export policy says to send it every route, is told of every +/// peer's route for a prefix, each with a path identifier of its own that +/// stays with the route; a route that goes is withdrawn by its identifier. +#[tokio::test] +async fn test_every_route_goes_to_a_collector_told_apart_by_path_identifiers() -> Result<()> { + let server = TestServer::start_with(LOCAL_ASN, |c| { + // The collector is the configured peer at the IPv4 loopback + // address; the feeders come over IPv6 loopback as peers of no + // account. + c.listen_endpoints + .push(SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 0)); + c.peers = vec![PeerConfig { + add_path: Some(SendReceive::Send), + import: ImportPolicy::permissive(), + export: Arc::new(ExportPolicy { + learned_routes: true, + all_paths: true, + ..ExportPolicy::default() + }), + ..peer_config( + "collector", + SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 1), + COLLECTOR_ASN, + ) + }]; + c.connect_retry = Duration::from_secs(3600); + }) + .await?; + let route = |path: &[u32]| Announcement::of([prefix("203.0.113.0/24").into()], path); + let feeder = async |n: u32| -> Result { + let mut peer = TestPeer::connect(server.addrs[1]).await?; + peer.handshake(PEER_ASN + n, Ipv4Addr::new(10, 0, 0, n as u8 + 1), 90) + .await?; + Ok(peer) + }; + let mut a = feeder(0).await?; + let mut b = feeder(1).await?; + server.wait_for_sessions(2).await?; + a.announce(route(&[PEER_ASN, 64510])).await?; + b.announce(route(&[PEER_ASN + 1, 64510])).await?; + server.wait_for_routes(2).await?; + + let mut capabilities = vec![Capability::FourByteAsn { asn: COLLECTOR_ASN }]; + capabilities.extend(SUPPORTED.into_iter().map(Capability::multiprotocol)); + capabilities.push(Capability::AddPath { + entries: vec![AddPathEntry { + afi: AfiSafi::IPV4_UNICAST.afi, + safi: AfiSafi::IPV4_UNICAST.safi, + send_recv: SendReceive::Receive, + }], + }); + let mut collector = TestPeer::connect(server.addr).await?; + collector + .handshake_with( + COLLECTOR_ASN, + Ipv4Addr::new(10, 0, 0, 100), + 90, + capabilities, + ) + .await?; + collector.ctx.add_path = vec![AfiSafi::IPV4_UNICAST]; + + // Both routes, told apart. + let mut held = Held::new(); + told(&mut collector, &mut held, |held| held.len() == 2).await?; + assert_eq!( + paths(&held), + [ + (1, vec![LOCAL_ASN, PEER_ASN, 64510]), + (2, vec![LOCAL_ASN, PEER_ASN + 1, 64510]), + ] + ); + + // The second peer's route goes: withdrawn by its identifier, the + // first's untouched. + b.announce(withdrawal([prefix("203.0.113.0/24").into()])) + .await?; + told(&mut collector, &mut held, |held| held.len() == 1).await?; + assert_eq!(paths(&held), [(1, vec![LOCAL_ASN, PEER_ASN, 64510])]); + + // The first peer's route changes: it goes out again with the same + // identifier. A third peer's route gets the lowest free one. + a.announce(route(&[PEER_ASN, 64511, 64510])).await?; + let mut c = feeder(2).await?; + server.wait_for_sessions(4).await?; + c.announce(route(&[PEER_ASN + 2, 64510])).await?; + let changed = |held: &Held| { + held.len() == 2 + && held.get(&("203.0.113.0/24".to_string(), 1)) + == Some(&vec![LOCAL_ASN, PEER_ASN, 64511, 64510]) + }; + told(&mut collector, &mut held, changed).await?; + assert_eq!( + paths(&held), + [ + (1, vec![LOCAL_ASN, PEER_ASN, 64511, 64510]), + (2, vec![LOCAL_ASN, PEER_ASN + 2, 64510]), + ] + ); + Ok(()) +} diff --git a/crates/integration-tests/tests/transit.rs b/crates/integration-tests/tests/transit.rs index 5e0f82f..6728f59 100644 --- a/crates/integration-tests/tests/transit.rs +++ b/crates/integration-tests/tests/transit.rs @@ -27,6 +27,7 @@ async fn transit(customize: impl FnOnce(&mut bgp_server::config::Config)) -> Res TestServer::start_with(LOCAL_ASN, |config| { config.export = Arc::new(ExportPolicy { learned_routes: true, + all_paths: false, ..ExportPolicy::default() }); customize(config); @@ -264,6 +265,7 @@ async fn test_communities_say_who_is_told() -> Result<()> { }); config.export = Arc::new(ExportPolicy { learned_routes: true, + all_paths: false, marks: Marks::parse(&[], &["64601:7".to_string()], &[]).unwrap(), prepend: 1, }); diff --git a/crates/server/proto/config.proto b/crates/server/proto/config.proto index 4c655a5..d87826d 100644 --- a/crates/server/proto/config.proto +++ b/crates/server/proto/config.proto @@ -442,6 +442,14 @@ message ExportPolicy { // Pass on the best routes learned from other peers. bool learned_routes = 1; + // Pass on every route learned from other peers, not only the best of + // each prefix, each with a path identifier (ADD-PATH, RFC 7911). For a + // route collector, which wants to see every path. Needs learned_routes, + // and the peer's add_path to offer sending; for a family the peer did + // not agree to receive several routes for, the best goes as usual. The + // export policy and the limits apply to every route alike. + bool all_paths = 6; + // Communities put on every route announced to the peer: which router // it went through, say. "64601:100", or a well-known name. repeated string add_communities = 2; diff --git a/crates/server/src/advertise.rs b/crates/server/src/advertise.rs index 930c24a..1eec721 100644 --- a/crates/server/src/advertise.rs +++ b/crates/server/src/advertise.rs @@ -44,17 +44,19 @@ use bgp_primitives::parser::ParserContext; use bgp_rib::table::{AttrSet, DEFAULT_LOCAL_PREF, Event, PeerIndex, Reader, RibTable}; use bgp_rib::{LocalRib, LocalRibEvent, LocalRoute}; use bytes::BytesMut; +use prometheus::IntGauge; use tokio::sync::broadcast::Receiver; use tokio::sync::broadcast::error::RecvError; use tokio::sync::{Notify, OwnedSemaphorePermit, Semaphore, watch}; use tokio::task::JoinHandle; use tracing::{debug, error, info, warn}; +use crate::address::PeerAddress; use crate::error::SessionError; use crate::export::{self, Egress, Outgoing, Reflection, Source}; use crate::family::FamilySet; use crate::marks::Control; -use crate::metrics::ROUTES_EXPORTED; +use crate::metrics::{PEER_ADVERTISED, ROUTES_EXPORTED}; use crate::policy::PeerRole; use crate::session::{PeerId, SessionClosed, SessionEvent, SessionHandle, SessionRegistry}; @@ -154,11 +156,12 @@ impl Advertiser { changes: self.local_rib.subscribe(), local_rib: self.local_rib.clone(), // A follower of the best routes, if the peer is to be told - // of them: it is given the whole table first. - learned: session - .export - .learned_routes - .then(|| Learned::new(self.rib.reader())), + // of them, or of every route if it is to have them all and + // agreed to tell them apart: it is given the whole table first. + learned: session.export.learned_routes.then(|| { + let all_paths = session.export.all_paths && !session.add_path_send.is_empty(); + Learned::new(self.learned_reader(all_paths), all_paths) + }), rib: Arc::clone(&self.rib), resyncs: Arc::clone(&self.resyncs), control: self.control, @@ -172,6 +175,12 @@ impl Advertiser { .as_ref() .map(|_| session.families.clone()), converged: Some(self.converged.clone()).filter(|converged| !*converged.borrow()), + // The peer goes by its configured name, or by its address, as + // in its other metrics. + advertised_metric: PEER_ADVERTISED.with_label_values(&[&session + .name + .clone() + .unwrap_or_else(|| PeerAddress::from(session.peer.addr).to_string())]), }; let running = Running { session: session.clone(), @@ -183,6 +192,16 @@ impl Advertiser { self.exporters.insert(session.peer, running); } + /// A new follower of the table for an exporter: of every route, or of + /// the best ones. + fn learned_reader(&self, all_paths: bool) -> Reader { + if all_paths { + self.rib.reader_of_all_routes() + } else { + self.rib.reader() + } + } + /// Makes the exporters match the sessions there are, after events were /// missed: sessions may have come and gone unheard, and peers may have /// asked for their routes again. @@ -225,6 +244,8 @@ struct Exporter { /// Word of every peer having sent its routes since the start, until it /// comes. converged: Option>, + /// How many routes the peer holds from us, for whoever asks. + advertised_metric: IntGauge, } /// How many changes of best route are taken from the table at a time. @@ -259,6 +280,13 @@ enum News { attrs: Arc, }, Withdrawn(IpPrefix), + /// All the routes of `prefix`, for a session that is told of every + /// route: who each is from, that peer's identifier for it, its + /// attributes, and whether it is the best. + Routes { + prefix: IpPrefix, + routes: Vec<(PeerIndex, u32, Arc, bool)>, + }, } /// What a learned route goes out as depends on: the attributes it came @@ -272,9 +300,115 @@ struct Dressed { octets: usize, } -/// What the peer is to end up with for each prefix heard of: the route as -/// it goes out, or `None` for a prefix it is not to have. -type Verdicts = HashMap>>; +/// A route of a prefix as a session that sends every route tells it +/// apart: the peer it is from, and that peer's identifier for it. +type PathKey = (PeerIndex, u32); + +/// A route announced to a peer that is sent every route, and not +/// withdrawn since. +struct Sent { + key: PathKey, + /// The path identifier it went out with (ADD-PATH, RFC 7911), which + /// stays its own for as long as it is announced. + id: u32, + /// As it went out, to tell whether it has to go out again. + dressed: Arc, +} + +/// What the peer is to end up with for a prefix. +enum Verdict { + /// The best route as it goes out, or `None` for a prefix the peer is + /// not to have. + Best(Option>), + /// Every route that goes out, with what tells it apart; empty for a + /// prefix the peer is not to have. + All(Vec<(PathKey, Arc)>), +} + +/// What the peer is to end up with for each prefix heard of. +type Verdicts = HashMap; + +/// The learned routes announced to the peer: our Adj-RIB-Out for it, as +/// far as it needs keeping. +enum Announced { + /// The prefixes, each with its best route. + Best(HashSet), + /// For each prefix, every route announced, and how many in all. + All { + routes: HashMap>, + count: usize, + }, +} + +impl Announced { + fn new(all_paths: bool) -> Self { + if all_paths { + Self::All { + routes: HashMap::new(), + count: 0, + } + } else { + Self::Best(HashSet::new()) + } + } + + /// How many routes the peer holds. + fn routes(&self) -> usize { + match self { + Self::Best(prefixes) => prefixes.len(), + Self::All { count, .. } => *count, + } + } + + /// Takes everything out, leaving this empty, as what the peer holds + /// in doubt until it is heard of again. + fn take(&mut self) -> Self { + match self { + Self::Best(prefixes) => Self::Best(std::mem::take(prefixes)), + Self::All { routes, count } => Self::All { + routes: std::mem::take(routes), + count: std::mem::take(count), + }, + } + } + + /// Adds what `other` holds. + fn absorb(&mut self, other: Self) { + match (self, other) { + (Self::Best(prefixes), Self::Best(more)) => prefixes.extend(more), + (Self::All { routes, count }, Self::All { routes: more, .. }) => { + for (prefix, sent) in more { + *count += sent.len(); + routes.insert(prefix, sent); + } + } + // Both are of one session, which is one or the other. + _ => {} + } + } + + /// Everything, as withdrawals, by IP version. + fn withdrawals(self) -> [Vec; 2] { + let mut withdrawn: [Vec; 2] = Default::default(); + match self { + Self::Best(prefixes) => { + for prefix in prefixes { + withdrawn[version_index(prefix.address_family())].push(prefix.into()); + } + } + Self::All { routes, .. } => { + for (prefix, sent) in routes { + let withdrawn = &mut withdrawn[version_index(prefix.address_family())]; + withdrawn.extend(sent.iter().map(|sent| Nlri { + prefix, + path_id: sent.id, + })); + } + } + } + withdrawn + } +} /// What an exporter knows of the learned routes it has passed on. struct Learned { @@ -283,13 +417,18 @@ struct Learned { /// until the table is all there; and the leave to keep them. gathered: Option, permit: Option, - /// The prefixes announced to the peer and not withdrawn since. - announced: HashSet, + /// Whether every route goes, and not only the best of each prefix: + /// the reader is then one of all routes. Only for a session on which + /// routes are told apart by path identifiers for at least one family; + /// for a family they are not, the best goes all the same. + all_paths: bool, + /// What is announced to the peer and not withdrawn since. + announced: Announced, /// Whether the peer has been told of the whole table once. synced: bool, /// While the table is being gone through anew: what was announced /// before and has not been heard of again yet. - stale: Option>, + stale: Option, /// Routes as they go out, by the attributes they came with, who they /// came from and their IP version: a table's worth of routes share a /// few sets of attributes, and each is worked out once. The key holds @@ -301,7 +440,16 @@ impl Learned { /// How many learned routes the peer holds from us: those announced, /// and those not heard of again yet since the table was reset. fn held_by_peer(&self) -> usize { - self.announced.len() + self.stale.as_ref().map_or(0, HashSet::len) + self.announced.routes() + self.stale.as_ref().map_or(0, Announced::routes) + } + + /// The verdict that the peer is to have nothing for a prefix. + fn nothing(&self) -> Verdict { + if self.all_paths { + Verdict::All(Vec::new()) + } else { + Verdict::Best(None) + } } /// Takes the next changes of best route from the table: all of what @@ -325,8 +473,20 @@ impl Learned { attrs: Arc::clone(attrs), }, Event::Withdrawn { prefix } => News::Withdrawn(prefix), - // Not given to a follower of the best routes. - Event::Routes { .. } => return, + Event::Routes { prefix, routes } => News::Routes { + prefix, + routes: routes + .iter() + .map(|route| { + ( + route.peer, + route.path_id, + Arc::clone(route.attrs), + route.best, + ) + }) + .collect(), + }, }); }); if progress.caught_up || progress.events == 0 { @@ -336,12 +496,13 @@ impl Learned { news } - fn new(reader: Reader) -> Self { + fn new(reader: Reader, all_paths: bool) -> Self { Self { reader, gathered: None, permit: None, - announced: HashSet::new(), + all_paths, + announced: Announced::new(all_paths), synced: false, stale: None, dressed: HashMap::new(), @@ -394,6 +555,24 @@ impl Learned { } } +/// Who a route from `peer` is from, as the export policy sees it, asked +/// of the table once for each peer and remembered in `sources`. `None` if +/// the peer has gone, and its routes with it. +fn source_of( + rib: &RibTable, + sources: &mut HashMap>, + peer: PeerIndex, +) -> Option { + *sources.entry(peer).or_insert_with(|| { + rib.peer(peer).map(|info| Source { + peer, + ibgp: info.ibgp, + reflector_client: info.reflector_client, + router_id: info.router_id, + }) + }) +} + /// Waits until every peer has sent its routes since the start; for ever if /// that is not to be waited for. async fn converged(converged: &mut Option>) { @@ -455,7 +634,11 @@ impl Exporter { // Everything, again: a new follower starts with the // whole table. if let Some(learned) = &mut self.learned { - learned.reader = self.rib.reader(); + learned.reader = if learned.all_paths { + self.rib.reader_of_all_routes() + } else { + self.rib.reader() + }; } } () = changed(&mut self.learned) => { @@ -521,6 +704,15 @@ impl Exporter { } } + /// Notes how many routes the peer now holds from us, for the status + /// page and the metrics. + fn note_advertised(&self) { + let advertised = + self.own_announced.len() + self.learned.as_ref().map_or(0, Learned::held_by_peer); + self.session.counters.set_advertised(advertised); + self.advertised_metric.set(advertised as i64); + } + /// The session as far as it decides what goes out on it and how. fn towards(&self) -> Towards { let session = &self.session; @@ -565,13 +757,24 @@ impl Exporter { let mut synced = false; // Who the routes are from, asked of the table once for each. let mut sources: HashMap> = HashMap::new(); + // Whether a learned route for `prefix` may go to the peer at all: + // what we originate ourselves is announced as that, and not as + // someone else's route for it. + let may_go = |prefix: &IpPrefix| { + session.families.carries(prefix) + && !self.own_announced.contains(prefix) + && limits.is_none_or(|limits| limits.allows(prefix)) + }; for item in news { match item { News::Reset => { // Whatever was announced and is not heard of again by // the time the table is through is withdrawn then. - let known = std::mem::take(&mut learned.announced); - learned.stale.get_or_insert_default().extend(known); + let known = learned.announced.take(); + learned + .stale + .get_or_insert_with(|| Announced::new(learned.all_paths)) + .absorb(known); verdicts.clear(); synced = false; // Gathering a table takes memory: only so many sessions @@ -585,35 +788,45 @@ impl Exporter { learned.permit = None; } News::Withdrawn(prefix) => { - verdicts.insert(prefix, None); + verdicts.insert(prefix, learned.nothing()); } News::Best { prefix, peer, attrs, } => { - let source = *sources.entry(peer).or_insert_with(|| { - self.rib.peer(peer).map(|info| Source { - peer, - ibgp: info.ibgp, - reflector_client: info.reflector_client, - router_id: info.router_id, - }) - }); - let dressed = match source { + let dressed = match source_of(&self.rib, &mut sources, peer) { // What we originate ourselves is announced as // that, and not as someone else's route for it. - Some(source) - if session.families.carries(&prefix) - && !self.own_announced.contains(&prefix) - && limits.is_none_or(|limits| limits.allows(&prefix)) => - { + Some(source) if may_go(&prefix) => { learned.dressed(&attrs, source, prefix.address_family(), &egress) } // The peer has gone, and its routes with it. _ => None, }; - verdicts.insert(prefix, dressed); + verdicts.insert(prefix, Verdict::Best(dressed)); + } + News::Routes { prefix, routes } => { + let version = prefix.address_family(); + // Where the peer did not agree to tell the routes of a + // prefix apart, only the best goes, as plain BGP has it. + let told_apart = session.add_path_send.contains(AfiSafi::unicast(version)); + let mut wanted = Vec::new(); + if may_go(&prefix) { + for (peer, path_id, attrs, best) in routes { + if !told_apart && !best { + continue; + } + let Some(source) = source_of(&self.rib, &mut sources, peer) else { + continue; + }; + if let Some(dressed) = learned.dressed(&attrs, source, version, &egress) + { + wanted.push(((peer, path_id), dressed)); + } + } + } + verdicts.insert(prefix, Verdict::All(wanted)); } } } @@ -638,32 +851,90 @@ impl Exporter { let mut batches: HashMap<(usize, IpVersion), (Arc, Vec)> = HashMap::new(); for (prefix, verdict) in verdicts { let version = prefix.address_family(); - // Heard of again: no longer in doubt, whichever way it goes. - let was_stale = learned - .stale - .as_mut() - .is_some_and(|stale| stale.remove(&prefix)); - match verdict { - Some(dressed) => { - learned.announced.insert(prefix); - batches - .entry((Arc::as_ptr(&dressed) as usize, version)) - .or_insert_with(|| (dressed, Vec::new())) - .1 - .push(prefix.into()); - } - None => { - if learned.announced.remove(&prefix) || was_stale { - withdrawn[version_index(version)].push(prefix.into()); + let withdrawn = &mut withdrawn[version_index(version)]; + let mut announce = |dressed: Arc, nlri: Nlri| { + batches + .entry((Arc::as_ptr(&dressed) as usize, version)) + .or_insert_with(|| (dressed, Vec::new())) + .1 + .push(nlri); + }; + match (verdict, &mut learned.announced, &mut learned.stale) { + (Verdict::Best(verdict), Announced::Best(announced), stale) => { + // Heard of again: no longer in doubt, whichever way it + // goes. + let was_stale = match stale { + Some(Announced::Best(stale)) => stale.remove(&prefix), + _ => false, + }; + match verdict { + Some(dressed) => { + announced.insert(prefix); + announce(dressed, prefix.into()); + } + None => { + if announced.remove(&prefix) || was_stale { + withdrawn.push(prefix.into()); + } + } } } + (Verdict::All(wanted), Announced::All { routes, count }, stale) => { + // What the peer has for the prefix: announced, or in + // doubt since the table was reset. + let mut had = routes.remove(&prefix).unwrap_or_default(); + *count -= had.len(); + if let Some(Announced::All { routes, count }) = stale + && let Some(stale) = routes.remove(&prefix) + { + *count -= stale.len(); + had.extend(stale); + } + // A route it had keeps its identifier, and goes out + // again only if it changed; a new one gets the lowest + // identifier not in use for the prefix. + let mut has: Vec = Vec::with_capacity(wanted.len()); + for (key, dressed) in wanted { + let id = match had.iter().position(|sent| sent.key == key) { + Some(at) => { + let sent = had.swap_remove(at); + if Arc::ptr_eq(&sent.dressed, &dressed) { + has.push(sent); + continue; + } + sent.id + } + None => (1..) + .find(|id| !had.iter().chain(&has).any(|sent| sent.id == *id)) + .expect("fewer routes than identifiers"), + }; + announce( + Arc::clone(&dressed), + Nlri { + prefix, + path_id: id, + }, + ); + has.push(Sent { key, id, dressed }); + } + withdrawn.extend(had.iter().map(|sent| Nlri { + prefix, + path_id: sent.id, + })); + *count += has.len(); + if !has.is_empty() { + routes.insert(prefix, has); + } + } + // A verdict is of the kind the session's bookkeeping is. + _ => {} } } if synced { learned.synced = true; if let Some(stale) = learned.stale.take() { - for prefix in stale { - withdrawn[version_index(prefix.address_family())].push(prefix.into()); + for (version, routes) in withdrawn.iter_mut().zip(stale.withdrawals()) { + version.extend(routes); } } } @@ -703,6 +974,7 @@ impl Exporter { session.send(Message::Update(update)).await?; } } + self.note_advertised(); Ok(()) } @@ -775,6 +1047,7 @@ impl Exporter { debug!(peer = %session.peer, prefix = %route.prefix, "Announcing local route"); session.send(Message::Update(update)).await?; self.own_announced.insert(route.prefix); + self.note_advertised(); Ok(()) } @@ -790,6 +1063,7 @@ impl Exporter { self.session.extended_next_hop, ))) .await?; + self.note_advertised(); Ok(()) } } @@ -1568,6 +1842,7 @@ mod tests { ); handle.export = Arc::new(crate::policy::ExportPolicy { learned_routes: true, + all_paths: false, ..crate::policy::ExportPolicy::default() }); handle.reflector_client = true; @@ -1740,6 +2015,7 @@ mod tests { handle.role = Some(PeerRole::Peer); handle.export = Arc::new(ExportPolicy { learned_routes: true, + all_paths: false, ..ExportPolicy::default() }); handle.export_limits = Some(Arc::new(limits)); diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index 3476acd..133d32a 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -428,8 +428,12 @@ fn parse_prepend(prepend: u32) -> Option { } fn parse_export(export: &proto::ExportPolicy) -> Result { + if export.all_paths && !export.learned_routes { + return Err(ExportProblem::AllPathsWithoutLearnedRoutes); + } Ok(ExportPolicy { learned_routes: export.learned_routes, + all_paths: export.all_paths, marks: Marks::parse( &export.remove_communities, &export.add_communities, @@ -1510,6 +1514,19 @@ mod tests { /// What a peer is expected to send is kept for the status page; zero, /// the default, is nothing to compare with. + #[test] + fn all_paths_needs_learned_routes() { + let without = "export { all_paths: true }"; + let error = rejected(&format!("{BASE}\n{without}")); + assert!( + error.to_string().contains("all_paths needs learned_routes"), + "{error}" + ); + let with = "export { learned_routes: true all_paths: true }"; + let config = parse(&format!("{BASE}\n{with}")).expect("in order"); + assert!(config.export.all_paths); + } + #[test] fn expected_routes_are_kept_when_given() { let config = parse(&format!( diff --git a/crates/server/src/connection.rs b/crates/server/src/connection.rs index 4564f47..bf1753a 100644 --- a/crates/server/src/connection.rs +++ b/crates/server/src/connection.rs @@ -43,8 +43,8 @@ use crate::family::FamilySet; use crate::fsm::{Action, Event, Fsm, FsmConfig, Negotiated, Received}; use crate::holddown::{HeldDown, HoldDowns}; use crate::metrics::{ - EXPORT_LIMIT_EXCEEDED, MESSAGES_RECEIVED, MESSAGES_SENT, PEER_ROUTES, PREFIX_LIMIT_EXCEEDED, - SESSIONS_ESTABLISHED_TOTAL, UPDATE_ERRORS, message_type_name, + EXPORT_LIMIT_EXCEEDED, MESSAGES_RECEIVED, MESSAGES_SENT, PEER_ADVERTISED, PEER_ROUTES, + PREFIX_LIMIT_EXCEEDED, SESSIONS_ESTABLISHED_TOTAL, UPDATE_ERRORS, message_type_name, }; use crate::panic::contained; use crate::policy::{self, ExportLimits, ExportPolicy, ImportPolicy, PeerRole}; @@ -315,6 +315,7 @@ async fn release_routes( // The peer has no session to count its routes for. // Fails only if the session never set it, which is nothing to do. let _ = PEER_ROUTES.remove_label_values(&[session.rejections.label()]); + let _ = PEER_ADVERTISED.remove_label_values(&[session.rejections.label()]); if *session.leave.borrow() == Some(Leave::Restart) { // The daemon is about to go, and its routes with it; the kernel // keeps its own. Taking them out first would only hold the diff --git a/crates/server/src/error.rs b/crates/server/src/error.rs index 220fd10..daf5177 100644 --- a/crates/server/src/error.rs +++ b/crates/server/src/error.rs @@ -336,6 +336,8 @@ pub enum ExportProblem { Marks(#[from] crate::marks::MarksProblem), #[error("as_path_prepend of {value} is more than {max}")] Prepend { value: u32, max: u32 }, + #[error("all_paths needs learned_routes")] + AllPathsWithoutLearnedRoutes, } /// What is wrong with one `originate` entry. diff --git a/crates/server/src/export.rs b/crates/server/src/export.rs index ebf48fb..1487802 100644 --- a/crates/server/src/export.rs +++ b/crates/server/src/export.rs @@ -529,6 +529,7 @@ mod tests { fn all_routes() -> ExportPolicy { ExportPolicy { learned_routes: true, + all_paths: false, ..ExportPolicy::default() } } @@ -922,6 +923,7 @@ mod tests { |texts: &[&str]| -> Vec { texts.iter().map(|text| text.to_string()).collect() }; let policy = ExportPolicy { learned_routes: true, + all_paths: false, marks: Marks::parse(&strings(&["64601:900"]), &strings(&["64601:5"]), &[]).unwrap(), prepend: 2, }; diff --git a/crates/server/src/metrics.rs b/crates/server/src/metrics.rs index 924c019..f7e2c06 100644 --- a/crates/server/src/metrics.rs +++ b/crates/server/src/metrics.rs @@ -69,6 +69,15 @@ pub static PEER_ROUTES: LazyLock = LazyLock::new(|| { .expect("metric registration") }); +pub static PEER_ADVERTISED: LazyLock = LazyLock::new(|| { + register_int_gauge_vec!( + "bgp_peer_advertised_routes", + "Routes announced to an established peer and not withdrawn since", + &["peer"] + ) + .expect("metric registration") +}); + pub static PEER_MAX_PREFIXES: LazyLock = LazyLock::new(|| { register_int_gauge_vec!( "bgp_peer_max_prefixes", @@ -228,6 +237,7 @@ pub fn init() { LazyLock::force(&RIB_JOURNAL_ENTRIES); LazyLock::force(&ROUTES_EXPORTED); LazyLock::force(&PEER_UP); + LazyLock::force(&PEER_ADVERTISED); BUILD_INFO .with_label_values(&[env!("CARGO_PKG_VERSION")]) .set(1); diff --git a/crates/server/src/policy.rs b/crates/server/src/policy.rs index fd43aa9..39eaa85 100644 --- a/crates/server/src/policy.rs +++ b/crates/server/src/policy.rs @@ -266,6 +266,11 @@ pub struct ExportPolicy { /// Pass on the best routes learned from other peers, and not only /// what we originate. pub learned_routes: bool, + /// Pass on every route learned from other peers, not only the best + /// of each prefix, told apart by path identifiers (ADD-PATH, RFC + /// 7911), for the families the peer agreed to receive them for; a + /// route collector asks for this. For other families the best goes. + pub all_paths: bool, /// Communities taken off and put on every route announced. pub marks: Marks, /// How many more times our AS goes on the path. diff --git a/crates/server/src/session.rs b/crates/server/src/session.rs index f199307..92425e7 100644 --- a/crates/server/src/session.rs +++ b/crates/server/src/session.rs @@ -93,6 +93,10 @@ pub struct SessionCounters { /// Routes of the peer's kept from its last session, which it has not /// sent again yet (graceful restart). stale: AtomicU64, + /// Routes announced to the peer and not withdrawn since: what it + /// holds from us, every route counted where it is sent several for + /// a prefix. + advertised: AtomicU64, } impl SessionCounters { @@ -112,6 +116,14 @@ impl SessionCounters { self.held.store(held as u64, Ordering::Relaxed); } + pub fn advertised(&self) -> u64 { + self.advertised.load(Ordering::Relaxed) + } + + pub(crate) fn set_advertised(&self, advertised: usize) { + self.advertised.store(advertised as u64, Ordering::Relaxed); + } + pub fn stale(&self) -> u64 { self.stale.load(Ordering::Relaxed) } diff --git a/crates/web/src/api.rs b/crates/web/src/api.rs index 5f9791d..8fdf357 100644 --- a/crates/web/src/api.rs +++ b/crates/web/src/api.rs @@ -89,6 +89,8 @@ pub struct PeerView { /// How many routes the peer announced on this session that its import /// policy refused. pub rejected_routes: Option, + /// Routes announced to the peer and not withdrawn since. + pub advertised_routes: Option, /// How many of the peer's routes are held out of the table as invalid /// by the RPKI's data, to be taken if that changes. pub held_routes: Option, @@ -159,6 +161,7 @@ impl From<&SessionHandle> for PeerView { add_path_send: Some(s.add_path_send.iter().map(|f| f.to_string()).collect()), routes: Some(s.counters.routes()), rejected_routes: Some(s.counters.rejected()), + advertised_routes: Some(s.counters.advertised()), held_routes: Some(s.counters.held()), expected_routes: None, role: s.role.map(PeerRole::as_str), @@ -198,6 +201,7 @@ impl PeerView { add_path_send: None, routes: None, rejected_routes: None, + advertised_routes: None, held_routes: None, expected_routes: peer.expected_routes, role: peer.role.map(PeerRole::as_str), diff --git a/crates/web/static/statusz.html b/crates/web/static/statusz.html index 8ab610d..818761b 100644 --- a/crates/web/static/statusz.html +++ b/crates/web/static/statusz.html @@ -181,7 +181,7 @@ JSON: status, peers, Glyphs: v4v6 address families · GR graceful restart · ENH IPv4 with IPv6 next hops · AP ADD-PATH · 2B two-octet AS numbers only · ↑ provider ↓ customer ↔ peer RS route server RSC its client iBGP our AS. Point at one for its meaning; click a peer's name to look at its routes. - +
namepeerstateup forour address (next hop)sessionroutesrejected · helddetail
namepeerstateup forour address (next hop)sessionroutessentrejected · helddetail
@@ -557,6 +557,7 @@ from ${up ? esc(p.local_addr) : DASH} ${up ? sessionGlyphs(p) : DASH} ${up ? trendGlyph(p) + fmtCount(p.routes) + expected + stale : DASH} + ${up ? fmtCount(p.advertised_routes) : DASH} ${rejected}${held} ${peerDetail(p)}`; }; diff --git a/docs/content/docs/development/testing.md b/docs/content/docs/development/testing.md index f13f761..0f54b5e 100644 --- a/docs/content/docs/development/testing.md +++ b/docs/content/docs/development/testing.md @@ -99,6 +99,7 @@ test phases. | `hijacks` | FRR advertises a local prefix, a route with BIRD as third-party next hop, and the exchange LAN prefix; all are rejected | | `nexthops` | A preferred route from FRR with an unresolvable next hop is held, and the route from BIRD is selected and installed; the selection changes when the next hop becomes resolvable, and reverts when it becomes unresolvable | | `transit` | FRR as customer and BIRD as provider: FRR's routes are advertised to BIRD with the local AS prepended; BIRD's routes are advertised to FRR with the OTC attribute; a control community prevents the advertisement of one FRR route to BIRD; traffic is forwarded between their networks | +| `collector` | FRR as a route collector with ADD-PATH and `all_paths`: BIRD peers twice, FRR receives both of BIRD's routes for a prefix with distinct path identifiers, and one is withdrawn by its identifier when its session is disabled | | `v6only` | Exchange LAN without IPv4 addresses: one IPv6 session with each router for both address families; IPv4 routes with IPv6 next hops (RFC 8950) in BGP and in the kernel; IPv4 forwarding verified with `ping` | | `linklocal` | Exchange LAN with link-local addresses only, with the same addresses on a second interface: sessions with FRR (by interface, with radvd) and BIRD at `fe80::…%ix0`; both address families in BGP and in the kernel; forwarding verified with `ping`; interface disabled and enabled | diff --git a/docs/content/docs/guides/passing-routes-on.md b/docs/content/docs/guides/passing-routes-on.md index 6aa3237..76ceca5 100644 --- a/docs/content/docs/guides/passing-routes-on.md +++ b/docs/content/docs/guides/passing-routes-on.md @@ -398,12 +398,48 @@ Ferrix advertised (`announced`) and withdrew (`withdrawn`). Ferrix does not provide a view of the routes advertised to a neighbor (Adj-RIB-Out); to display them, use the neighbor's own tools. +## Feeding a Route Collector + +A route collector, such as a looking glass or a measurement project, wants +every route that a router has accepted, not only the best path for each +prefix. ADD-PATH (RFC 7911) lets one session carry several routes for a +prefix, each with a path identifier. To advertise every accepted route to +a neighbor, configure ADD-PATH in the sending direction and enable +`all_paths` in the neighbor's export policy: + +```textproto +peers { + name: "collector" + endpoint { address: "203.0.113.10" } + remote_asn: 64496 + role: PEER + add_path: ADD_PATH_SEND + export { + learned_routes: true + all_paths: true + } + export_limits {} +} +``` + +Ferrix advertises every route for a prefix only for the address families +for which the neighbor negotiated ADD-PATH receive. For other families it +advertises the best path, as without `all_paths`. Each route is advertised +with a path identifier that stays with the route for as long as it is +advertised; a route that is withdrawn is withdrawn by its identifier, and +a route whose attributes change is advertised again with the same +identifier. The export policy, the BGP role rules, the well-known +communities and the export limits apply to every route alike, and +`max_prefixes` in `export_limits` counts every route advertised. + +A session with `all_paths` keeps a record of every route advertised and +its identifier, which takes about 50 bytes for each route. For a full +Internet table from several neighbors, this is some tens of megabytes. + ## Limitations - Per-client best path selection for route servers is not supported. Every client receives the same best path. -- ADD-PATH is not supported for advertisement; only the best path is - advertised. - Best path selection does not use ORIGINATOR_ID and CLUSTER_LIST as tie breakers (RFC 4456 Section 9). - UPDATE messages are generated separately for each session, even when diff --git a/docs/content/docs/guides/peers.md b/docs/content/docs/guides/peers.md index 0f1cad5..4d3bcca 100644 --- a/docs/content/docs/guides/peers.md +++ b/docs/content/docs/guides/peers.md @@ -193,8 +193,11 @@ add_path: ADD_PATH_RECEIVE ``` ADD-PATH is enabled for each address family and direction only if the -neighbor also supports it. Ferrix negotiates `ADD_PATH_SEND` and -`ADD_PATH_BOTH`, but it advertises only one path for each prefix. +neighbor also supports it. With `ADD_PATH_SEND` or `ADD_PATH_BOTH`, +Ferrix advertises routes with path identifiers. By default it still +advertises one path for each prefix; to advertise every accepted route +for a prefix, enable `all_paths` in the neighbor's export policy. See +[Feeding a Route Collector](@/docs/guides/passing-routes-on.md#feeding-a-route-collector). ## Hold Timer diff --git a/docs/content/docs/operations/monitoring.md b/docs/content/docs/operations/monitoring.md index 662d1b6..8edc754 100644 --- a/docs/content/docs/operations/monitoring.md +++ b/docs/content/docs/operations/monitoring.md @@ -37,8 +37,9 @@ Below the status line, the page displays the following information: - The configuration that was read at startup. - Each neighbor, with its session state, time established, next hop, - session properties, number of accepted routes, rejected announcements, - and held routes, and its last error. Neighbors that need attention are + session properties, number of accepted routes, number of routes + advertised to it (**sent**), rejected announcements, and held routes, + and its last error. Neighbors that need attention are listed first; the others are grouped by role. The IPv4 and IPv6 sessions of one neighbor, named `NAME-v4` and `NAME-v6`, are listed together, IPv6 first, and joined by a bar. The **problems only** option hides the neighbors that @@ -238,6 +239,7 @@ Table 3 lists queries for a monitoring dashboard. |---|---| | Established sessions | `sum(bgp_peer_up)`, of `count(bgp_peer_up)` | | Routes by neighbor | `bgp_peer_routes` | +| Routes advertised to each neighbor | `bgp_peer_advertised_routes` | | Prefixes with a best path | `bgp_rib_prefixes` | | Rejected routes by reason | `sum by (reason) (rate(bgp_routes_rejected_total[5m]))` | | UPDATE messages received and sent | `rate(bgp_messages_received_total{type="update"}[5m])`, `rate(bgp_messages_sent_total{type="update"}[5m])` | diff --git a/docs/content/docs/reference/configuration.md b/docs/content/docs/reference/configuration.md index e4f7ce1..131e031 100644 --- a/docs/content/docs/reference/configuration.md +++ b/docs/content/docs/reference/configuration.md @@ -154,6 +154,7 @@ Statements that determine which routes are advertised to a neighbor. See | Statement | Default | Description | |---|---|---| | `learned_routes` bool | `false` | Enables the advertisement of learned routes, subject to BGP roles and communities. | +| `all_paths` bool | `false` | Advertises every accepted route for a prefix, not only the best path, each with a path identifier (ADD-PATH, RFC 7911). Requires `learned_routes`, and takes effect for an address family only if the neighbor negotiated ADD-PATH receive for it; otherwise the best path is advertised. Export policy and export limits apply to every route. See [Feeding a Route Collector](@/docs/guides/passing-routes-on.md#feeding-a-route-collector). | | `add_communities` string, repeated | None | Specifies standard communities added to advertised routes. | | `add_large_communities` string, repeated | None | Specifies large communities added to advertised routes. | | `remove_communities` string, repeated | None | Specifies patterns of communities removed from advertised routes. Applied before communities are added. | diff --git a/docs/content/docs/reference/http-api.md b/docs/content/docs/reference/http-api.md index d7b4ae3..70bf96b 100644 --- a/docs/content/docs/reference/http-api.md +++ b/docs/content/docs/reference/http-api.md @@ -61,6 +61,7 @@ neighbor: "add_path_send": [], "routes": 1, "rejected_routes": 1, + "advertised_routes": 2, "held_routes": 0, "expected_routes": null, "role": null, @@ -84,6 +85,7 @@ neighbor: | `direction` | `inbound` if the neighbor initiated the connection, `outbound` otherwise. | | `routes` | Number of the neighbor's routes in the routing table. | | `rejected_routes` | Number of the neighbor's routes rejected by its import policy. | +| `advertised_routes` | Number of routes advertised to the neighbor and not withdrawn since. With ADD-PATH and `all_paths`, every route advertised counts. | | `held_routes` | Number of held routes: routes rejected for a reason that can change without a new UPDATE, such as RPKI validity or next hop resolution. | | `expected_routes` | The `expected_routes` value from the configuration, or `null` if it is not set. | | `role_confirmed` | Whether the neighbor advertised a BGP role that matches the configured role. | diff --git a/docs/content/docs/reference/metrics.md b/docs/content/docs/reference/metrics.md index 4088f8d..5592706 100644 --- a/docs/content/docs/reference/metrics.md +++ b/docs/content/docs/reference/metrics.md @@ -29,6 +29,7 @@ alerting rules based on these metrics, see | Metric | Type | Description | |---|---|---| | `bgp_peer_routes{peer}` | Gauge | Number of the neighbor's routes in the routing table. | +| `bgp_peer_advertised_routes{peer}` | Gauge | Number of routes advertised to the neighbor and not withdrawn since. | | `bgp_peer_max_prefixes{peer}` | Gauge | Prefix limit of the neighbor (`import.max_prefixes`), if configured. | | `bgp_prefix_limit_exceeded_total{peer}` | Counter | Number of sessions torn down because the neighbor exceeded its prefix limit. | | `bgp_export_limit_exceeded_total{peer}` | Counter | Number of sessions torn down because the routes to be advertised to the neighbor exceeded its export limit (`export_limits.max_prefixes`). | diff --git a/docs/content/docs/reference/rfcs.md b/docs/content/docs/reference/rfcs.md index b66cc53..d7446bc 100644 --- a/docs/content/docs/reference/rfcs.md +++ b/docs/content/docs/reference/rfcs.md @@ -306,7 +306,7 @@ Version 2 is implemented from version 23 of the draft. |---------|--------| | Capability negotiation per family and direction | ✅ | | Receiving several routes for a prefix | ✅ | -| Sending several routes for a prefix | ❌ (path identifiers are sent; one route each) | +| Sending several routes for a prefix | ✅ with `all_paths` in the export policy; the best path only otherwise | ### RFC 4271 §9.1.2 / RFC 4486 / operational practice — import policy diff --git a/scripts/interop/lab.sh b/scripts/interop/lab.sh index 13a9681..54f2ce2 100755 --- a/scripts/interop/lab.sh +++ b/scripts/interop/lab.sh @@ -29,7 +29,7 @@ cd "$(dirname "$0")/../.." bgp="${CARGO_TARGET_DIR:-target}/release/bgp" work=/tmp/ferrix-lab pathspace=ferrixlab -all_phases="basics roles rpki refresh flaps restart graceful scale limits hijacks nexthops transit leaks v6only linklocal" +all_phases="basics roles rpki refresh flaps restart graceful scale limits hijacks nexthops transit collector leaks v6only linklocal" phases="${PHASES:-$all_phases}" scale_routes="${SCALE_ROUTES:-200000}" @@ -319,9 +319,9 @@ frr_down() { # --- BIRD ----------------------------------------------------------------------- -# bird_config [extra-static-routes-file] +# bird_config [extra-static-routes-file] [more-configuration] bird_config() { - local extra=${1:-} + local extra=${1:-} more=${2:-} cat > "$work/bird.conf" </dev/null + frr_configure "router bgp 64602" "neighbor 10.9.0.1 local-role customer" + dut_up + wait_for "five sessions" established 5 + wait_for "BIRD's route from both its sessions" bash -c "sudo ip netns exec lab-dut curl -sf 'http://127.0.0.1:8080/api/routes/received?prefix=10.3.0.0/24' | jq -e '.routes | length == 2'" + peer_is frr4 '.add_path_send | index("IPv4 unicast") != null' >/dev/null \ + || fail "ADD-PATH was not agreed with FRR: $(peer_is frr4 .add_path_send)" + wait_for "both routes at FRR, told apart" bash -c "sudo ip netns exec lab-frr vtysh -N $pathspace -c 'show bgp ipv4 unicast 10.3.0.0/24 json' 2>/dev/null | jq -e '.paths | length == 2 and all(.aspath.string == \"64601 4200000003\")'" + # Its two prefixes twice, and our own route: what the page and the + # metrics say it holds from us. + wait_for "the count of routes the collector holds" peer_is frr4 '.advertised_routes == 5' + ok "the collector is told both of BIRD's routes for a prefix, with path identifiers" + + bird_ctl disable dut4b >/dev/null + wait_for "one route left at FRR" bash -c "sudo ip netns exec lab-frr vtysh -N $pathspace -c 'show bgp ipv4 unicast 10.3.0.0/24 json' 2>/dev/null | jq -e '.paths | length == 1'" + wait_for "the count of routes the collector holds" peer_is frr4 '.advertised_routes == 3' + ok "a route that goes is withdrawn from the collector by its identifier, the other stays" + + frr_configure "router bgp 64602" "neighbor 10.9.0.1 local-role peer" + bird_config + bird_ctl configure >/dev/null + in_ns bird ip addr del 10.9.0.4/24 dev ix0 + dut_down TERM + dut_config + dut_up + converged + ok "back to normal" +} + # A customer leaks, and the export limits hold. FRR, now our customer, # announces a route outside its address space and then more routes than # BIRD, our provider, is to be told of. A customer's routes go to