From befc292215ddbd7c41a042c2af42a20625c17cb3 Mon Sep 17 00:00:00 2001 From: Rayhaan Jaufeerally Date: Tue, 6 Oct 2026 10:05:31 +0200 Subject: [PATCH] Tidy the export limits, passive peers and status page code Readability, no change in behaviour, but for one log line: - The routes a peer holds from us were counted twice over, differently, in the exporter: once per learned-route batch and once per originated route. Learned::held_by_peer counts the learned ones in one place, and ExportLimits::exceeded_by returns the limit that was passed instead of a bool, so that ending the session no longer has to dig it out again with a default that could not happen. - The two hold-downs for a limit passed, ours or the peer's, were two near-identical blocks; they are one match. - How a session ended is put into words in one place (SessionEnd::reason), for the dialer's backoff and the passive peer's state alike. - parse_export_limits parses one block; what a missing block means (no limits at the top, the top's limits for a peer) is said where it differs, instead of Option in and Option out. - The startup warnings about export limits are a function of their own, and the peers without limits are named in one line rather than one line each: a router with eighty sessions does not need eighty warnings. The warning about a peer without a role is gone: the configuration now refuses one. - Hex formatting of attribute octets is one function; the text for an unknown attribute is built in three plain steps. - The exporter's test fixture returns a named struct, not a tuple. Co-Authored-By: Claude Fable 5.1 --- crates/server/src/advertise.rs | 98 +++++++------- crates/server/src/config.rs | 28 ++-- crates/server/src/connection.rs | 50 ++++---- crates/server/src/dialer.rs | 6 +- crates/server/src/policy.rs | 14 +- crates/server/src/server.rs | 121 +++++++++--------- crates/web/src/api.rs | 96 +++++++------- crates/web/static/statusz.html | 14 +- docs/content/docs/guides/passing-routes-on.md | 6 +- 9 files changed, 219 insertions(+), 214 deletions(-) diff --git a/crates/server/src/advertise.rs b/crates/server/src/advertise.rs index eb953be..1d31263 100644 --- a/crates/server/src/advertise.rs +++ b/crates/server/src/advertise.rs @@ -54,7 +54,7 @@ use crate::export::{self, Egress, Outgoing, Reflection, Source}; use crate::family::FamilySet; use crate::marks::Control; use crate::metrics::ROUTES_EXPORTED; -use crate::policy::{ExportLimits, PeerRole}; +use crate::policy::PeerRole; use crate::session::{PeerId, SessionClosed, SessionEvent, SessionHandle, SessionRegistry}; /// Starts the advertiser. It hears of every session that comes up after @@ -292,6 +292,12 @@ struct Learned { } 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) + } + fn new(reader: Reader) -> Self { Self { reader, @@ -620,15 +626,11 @@ impl Exporter { withdrawn[version_index(prefix.address_family())].push(prefix.into()); } } - // What the peer would have once this goes out: none of it goes if + // What the peer would hold once this goes out: none of it goes if // that is more than it may. - let routes = self.announced.len() - + learned.announced.len() - + learned.stale.as_ref().map_or(0, HashSet::len); - if let Some(limits) = limits - && limits.exceeded_by(routes) - { - return Err(over_limit(session, limits, routes)); + let routes = self.announced.len() + learned.held_by_peer(); + if let Some(limit) = limits.and_then(|limits| limits.exceeded_by(routes)) { + return Err(over_limit(session, limit, routes)); } for (version, routes) in [IpVersion::Ipv4, IpVersion::Ipv6] .into_iter() @@ -691,24 +693,21 @@ impl Exporter { ); return Ok(()); } - if let Some(limits) = session.export_limits.as_deref() { - if !limits.allows(&route.prefix) { - debug!( - peer = %session.peer, - prefix = %route.prefix, - "Not announcing route: its prefix is not in the peer's export limits" - ); - return Ok(()); - } - let learned = self.learned.as_ref().map_or(0, |learned| { - learned.announced.len() + learned.stale.as_ref().map_or(0, HashSet::len) - }); - let routes = self.announced.len() - + usize::from(!self.announced.contains(&route.prefix)) - + learned; - if limits.exceeded_by(routes) { - return Err(over_limit(session, limits, routes)); - } + let limits = session.export_limits.as_deref(); + if limits.is_some_and(|limits| !limits.allows(&route.prefix)) { + debug!( + peer = %session.peer, + prefix = %route.prefix, + "Not announcing route: its prefix is not in the peer's export limits" + ); + return Ok(()); + } + // What the peer would hold with this route. + let routes = self.announced.len() + + usize::from(!self.announced.contains(&route.prefix)) + + self.learned.as_ref().map_or(0, Learned::held_by_peer); + if let Some(limit) = limits.and_then(|limits| limits.exceeded_by(routes)) { + return Err(over_limit(session, limit, routes)); } let towards = self.towards(); let Some(mut update) = build_announce(route, &towards) else { @@ -765,10 +764,9 @@ impl Exporter { } } -/// Ends `session` because `routes` routes were to be announced on it, more -/// than `limits` allow; none of them has been. -fn over_limit(session: &SessionHandle, limits: &ExportLimits, routes: usize) -> Finished { - let limit = limits.max_prefixes.unwrap_or_default(); +/// Ends `session` because `routes` routes were to be announced on it, over +/// its export limit of `limit`; none of them has been. +fn over_limit(session: &SessionHandle, limit: u32, routes: usize) -> Finished { error!( peer = %session.peer, name = session.name.as_deref().unwrap_or_default(), @@ -1644,11 +1642,7 @@ mod tests { prefixes: &[IpPrefix], originated: &[IpPrefix], limits: ExportLimits, - ) -> ( - mpsc::Receiver, - mpsc::UnboundedReceiver, - SessionGuard, - ) { + ) -> LimitedPeer { let registry = SessionRegistry::new(); let rib = Arc::new(RibTable::new(4)); let local_rib = LocalRib::spawn(); @@ -1720,7 +1714,19 @@ mod tests { }); handle.export_limits = Some(Arc::new(limits)); let ending = handle.listen_for_end(); - (outbound, ending, registry.register(handle)) + LimitedPeer { + outbound, + ending, + _registration: registry.register(handle), + } + } + + /// The far end of the limited peer's session: what it is sent, and + /// what ends it. + struct LimitedPeer { + outbound: mpsc::Receiver, + ending: mpsc::UnboundedReceiver, + _registration: SessionGuard, } /// The routes announced on a session's queue until it has been quiet @@ -1764,17 +1770,17 @@ mod tests { prefixes: allowing(&[nth_prefix(2), nth_prefix(11)]), max_prefixes: None, }; - let (mut outbound, mut ending, _registration) = limited( + let mut peer = limited( &[nth_prefix(1), nth_prefix(2), nth_prefix(3)], &[nth_prefix(10), nth_prefix(11)], limits, ) .await; assert_eq!( - announced(&mut outbound).await, + announced(&mut peer.outbound).await, HashSet::from([nth_prefix(2), nth_prefix(11)]) ); - assert!(ending.try_recv().is_err(), "the session goes on"); + assert!(peer.ending.try_recv().is_err(), "the session goes on"); } /// When more routes would go to a peer than its export limit allows, @@ -1787,8 +1793,8 @@ mod tests { prefixes: None, max_prefixes: Some(3), }; - let (mut outbound, mut ending, _registration) = limited(&prefixes, &[], limits).await; - let why = tokio::time::timeout(Duration::from_secs(10), ending.recv()) + let mut peer = limited(&prefixes, &[], limits).await; + let why = tokio::time::timeout(Duration::from_secs(10), peer.ending.recv()) .await .expect("the session is asked to end") .expect("by the exporter"); @@ -1802,7 +1808,7 @@ mod tests { ), "{why:?}" ); - assert_eq!(announced(&mut outbound).await, HashSet::new()); + assert_eq!(announced(&mut peer.outbound).await, HashSet::new()); } /// Our own routes count too, and none goes past the limit: those that @@ -1814,8 +1820,8 @@ mod tests { prefixes: None, max_prefixes: Some(2), }; - let (mut outbound, mut ending, _registration) = limited(&[], &originated, limits).await; - let why = tokio::time::timeout(Duration::from_secs(10), ending.recv()) + let mut peer = limited(&[], &originated, limits).await; + let why = tokio::time::timeout(Duration::from_secs(10), peer.ending.recv()) .await .expect("the session is asked to end") .expect("by the exporter"); @@ -1829,6 +1835,6 @@ mod tests { ), "{why:?}" ); - assert_eq!(announced(&mut outbound).await.len(), 2); + assert_eq!(announced(&mut peer.outbound).await.len(), 2); } } diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index 23a9fa3..c4d49f3 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -157,9 +157,12 @@ impl Config { Some(export) => parse_export(export)?, None => ExportPolicy::default(), }); - let export_limits = parse_export_limits(proto.export_limits.as_ref()) - .map_err(ConfigError::ExportLimits)? - .map(Arc::new); + let export_limits = match proto.export_limits.as_ref() { + Some(limits) => parse_export_limits(limits) + .map_err(ConfigError::ExportLimits)? + .map(Arc::new), + None => None, + }; let graceful_restart = match proto.graceful_restart.as_ref() { Some(own) => GracefulRestartConfig::default().overridden_by(own)?, None => GracefulRestartConfig::default(), @@ -171,8 +174,8 @@ impl Config { peer: name.clone(), problem, }; - let peer = - parse_peer(peer, &export, &export_limits, graceful_restart).map_err(rejected)?; + let peer = parse_peer(peer, &export, export_limits.as_ref(), graceful_restart) + .map_err(rejected)?; // RFC 9234 is about what networks are to each other. if peer.role.is_some() && peer.remote_asn == proto.local_asn { return Err(rejected(PeerProblem::RoleInOwnAs)); @@ -330,14 +333,11 @@ fn parse_export(export: &proto::ExportPolicy) -> Result, + limits: &proto::ExportLimits, ) -> Result, ImportProblem> { - let Some(limits) = limits else { - return Ok(None); - }; let limits = ExportLimits { prefixes: if limits.prefixes.is_empty() { None @@ -352,7 +352,7 @@ fn parse_export_limits( fn parse_peer( peer: Peer, export: &Arc, - export_limits: &Option>, + export_limits: Option<&Arc>, graceful_restart: GracefulRestartConfig, ) -> Result { let ep = peer.endpoint.as_ref().ok_or(PeerProblem::MissingEndpoint)?; @@ -447,10 +447,10 @@ fn parse_peer( None => Arc::clone(export), }, export_limits: match peer.export_limits.as_ref() { - Some(own) => parse_export_limits(Some(own)) + Some(own) => parse_export_limits(own) .map_err(PeerProblem::ExportLimits)? .map(Arc::new), - None => export_limits.clone(), + None => export_limits.cloned(), }, graceful_restart: match peer.graceful_restart.as_ref() { Some(own) => graceful_restart.overridden_by(own)?, diff --git a/crates/server/src/connection.rs b/crates/server/src/connection.rs index 82f2757..923a3f1 100644 --- a/crates/server/src/connection.rs +++ b/crates/server/src/connection.rs @@ -188,6 +188,19 @@ pub struct SessionEnd { pub result: Result<(), SessionError>, } +/// Why a session ended when nothing went wrong. +pub const CLOSED_AT_OUR_REQUEST: &str = "session closed at our request"; + +impl SessionEnd { + /// Why the session ended, in a sentence. + pub fn reason(&self) -> String { + match &self.result { + Ok(()) => CLOSED_AT_OUR_REQUEST.to_string(), + Err(e) => e.to_string(), + } + } +} + /// Runs the BGP session on `stream` until it ends. `direction` is which /// side opened the connection. pub async fn run_session( @@ -214,28 +227,21 @@ pub async fn run_session( }; let established = session.established.is_some(); let hold_downs = &session.server.hold_downs; - if matches!(result, Err(SessionError::PrefixLimitExceeded { .. })) { - // A peer that sends far more than it should will do so again on - // the next session; it is left alone for a while. - hold_downs.hold( - peer_addr.into(), - session.import.hold_down, - HeldDown::OverLimit, - ); - PREFIX_LIMIT_EXCEEDED - .with_label_values(&[session.refusals.label()]) - .inc(); - } - if matches!(result, Err(SessionError::ExportLimitExceeded { .. })) { - // Whatever made us want to send too much will be there on the - // next session too. Until it is put right, the peer is told of - // nothing rather than of too much. - hold_downs.hold( - peer_addr.into(), - session.import.hold_down, - HeldDown::OverExportLimit, - ); - EXPORT_LIMIT_EXCEEDED + // A peer that sent far more than it should, or that we were to send + // far more than we should, will be so again on the next session; it + // is left alone for a while, until it or we are put right. + let over_limit = match &result { + Err(SessionError::PrefixLimitExceeded { .. }) => { + Some((HeldDown::OverLimit, &*PREFIX_LIMIT_EXCEEDED)) + } + Err(SessionError::ExportLimitExceeded { .. }) => { + Some((HeldDown::OverExportLimit, &*EXPORT_LIMIT_EXCEEDED)) + } + _ => None, + }; + if let Some((why, exceeded)) = over_limit { + hold_downs.hold(peer_addr.into(), session.import.hold_down, why); + exceeded .with_label_values(&[session.refusals.label()]) .inc(); } diff --git a/crates/server/src/dialer.rs b/crates/server/src/dialer.rs index 7d037d9..e6a181c 100644 --- a/crates/server/src/dialer.rs +++ b/crates/server/src/dialer.rs @@ -18,7 +18,7 @@ use tracing::{info, warn}; use crate::collision::Direction; use crate::config::PeerConfig; -use crate::connection::{PeerSettings, ServerContext, run_session}; +use crate::connection::{CLOSED_AT_OUR_REQUEST, PeerSettings, ServerContext, run_session}; use crate::session::DialState; /// Upper bound for exponential backoff between connection attempts. @@ -121,7 +121,7 @@ async fn dial_loop(peer: PeerConfig, server: Arc, connect_retry: if end.established { backoff = connect_retry; } - end.result.map_err(|e| e.to_string()) + end.result.is_ok().then_some(()).ok_or_else(|| end.reason()) } Err(e) => Err(format!("connection lost at once: {e}")), }, @@ -130,7 +130,7 @@ async fn dial_loop(peer: PeerConfig, server: Arc, connect_retry: }; let reason = match ended { - Ok(()) => "session closed at our request".to_string(), + Ok(()) => CLOSED_AT_OUR_REQUEST.to_string(), Err(reason) => { warn!(peer = %peer.name, endpoint = %peer.endpoint, err = %reason, "Session attempt ended"); reason diff --git a/crates/server/src/policy.rs b/crates/server/src/policy.rs index 14ec8de..03fd158 100644 --- a/crates/server/src/policy.rs +++ b/crates/server/src/policy.rs @@ -296,10 +296,10 @@ impl ExportLimits { .is_none_or(|prefixes| prefixes.allows(prefix)) } - /// Whether `routes` routes at a time are more than may be announced. - pub fn exceeded_by(&self, routes: usize) -> bool { - self.max_prefixes - .is_some_and(|limit| routes > limit as usize) + /// The limit that `routes` routes at a time would go over, if they + /// would. + pub fn exceeded_by(&self, routes: usize) -> Option { + self.max_prefixes.filter(|limit| routes > *limit as usize) } } @@ -1391,10 +1391,10 @@ mod tests { prefixes: None, max_prefixes: Some(2), }; - assert!(!limits.exceeded_by(2)); - assert!(limits.exceeded_by(3)); + assert_eq!(limits.exceeded_by(2), None); + assert_eq!(limits.exceeded_by(3), Some(2)); assert!(limits.allows(&IpPrefix::try_from("203.0.113.0/24").unwrap())); - assert!(!ExportLimits::default().exceeded_by(usize::MAX)); + assert_eq!(ExportLimits::default().exceeded_by(usize::MAX), None); } #[test] diff --git a/crates/server/src/server.rs b/crates/server/src/server.rs index 11fccce..2a7eeee 100644 --- a/crates/server/src/server.rs +++ b/crates/server/src/server.rs @@ -21,7 +21,7 @@ use bgp_rpki::Rpki; use crate::address::{PeerAddress, socket_text}; use crate::advertise::spawn_advertiser; use crate::collision::{CollisionTable, Direction}; -use crate::config::Config; +use crate::config::{Config, PeerConfig}; use crate::connection::{PeerSettings, ServerContext, run_session}; use crate::dialer::spawn_dialers; use crate::error::ServerError; @@ -198,56 +198,8 @@ impl BgpServer { "No limit on the routes taken from this peer (import.max_prefixes)" ); } - // Without a role there is no rule about which learned routes - // go to the peer: all of them do. - if peer.export.learned_routes - && peer.role.is_none() - && peer.remote_asn != self.config.local_asn - { - warn!( - peer = %peer.name, - "Every route learned from any peer is passed on to this one: it has no role \ - (export.learned_routes without role)" - ); - } - } - // The export limits that hold for some peer, each once, with the - // peers they hold for. - let mut limited: Vec<(&ExportLimits, Vec<&str>)> = Vec::new(); - for peer in &self.config.peers { - let ibgp = peer.remote_asn == self.config.local_asn; - if !ExportLimits::apply_to(peer.role, ibgp) { - continue; - } - let Some(limits) = peer.export_limits.as_deref() else { - warn!( - peer = %peer.name, - "Nothing limits what this peer is told (export_limits)" - ); - continue; - }; - match limited - .iter_mut() - .find(|(seen, _)| std::ptr::eq(*seen, limits)) - { - Some((_, names)) => names.push(&peer.name), - None => limited.push((limits, vec![&peer.name])), - } - } - // A route of our own that the limits leave out is likely a mistake - // in one or the other. - for route in &self.config.originate { - for (limits, names) in &limited { - if !limits.allows(&route.prefix) { - warn!( - prefix = %route.prefix, - peers = names.join(", "), - "An originated route is not announced to these peers: export_limits.prefixes \ - leaves it out" - ); - } - } } + warn_about_export_limits(&self.config); if self.config.accept_unconfigured_peers { warn!("Taking sessions and routes from any host (accept_unconfigured_peers)"); } @@ -425,6 +377,54 @@ pub async fn close_sessions(registry: &SessionRegistry) { } } +/// Says what the export limits leave undone: the peers they are for that +/// have none, and the routes of our own that they leave out, which is +/// likely a mistake in one or the other. +fn warn_about_export_limits(config: &Config) { + let limited: Vec<&PeerConfig> = config + .peers + .iter() + .filter(|peer| ExportLimits::apply_to(peer.role, peer.remote_asn == config.local_asn)) + .collect(); + let unlimited: Vec<&str> = limited + .iter() + .filter(|peer| peer.export_limits.is_none()) + .map(|peer| peer.name.as_str()) + .collect(); + if !unlimited.is_empty() { + warn!( + peers = unlimited.join(", "), + "Nothing limits what these peers are told (export_limits)" + ); + } + // Each distinct set of limits once, with the peers that have it. + let mut distinct: Vec<(&Arc, Vec<&str>)> = Vec::new(); + for peer in &limited { + let Some(limits) = &peer.export_limits else { + continue; + }; + match distinct + .iter_mut() + .find(|(seen, _)| Arc::ptr_eq(seen, limits)) + { + Some((_, peers)) => peers.push(&peer.name), + None => distinct.push((limits, vec![&peer.name])), + } + } + for route in &config.originate { + for (limits, peers) in &distinct { + if !limits.allows(&route.prefix) { + warn!( + prefix = %route.prefix, + peers = peers.join(", "), + "An originated route is not announced to these peers: export_limits.prefixes \ + leaves it out" + ); + } + } + } +} + /// Opens a listening socket on `addr`. fn listen(addr: SocketAddr) -> io::Result { let socket = match addr { @@ -480,23 +480,22 @@ async fn serve( peer: PeerSettings, _slot: ConnectionSlot, ) { - // Nobody dials a passive peer, so its state is said here. - let passive = peer.name.clone().filter(|_| peer.passive); + // Nobody dials a passive peer, so how its session ended is noted here. + let passive = if peer.passive { + peer.name.clone() + } else { + None + }; let registry = context.registry.clone(); let end = run_session(stream, peer_addr, Direction::Inbound, context, peer).await; if let Err(e) = &end.result { warn!(peer_addr = %socket_text(peer_addr), err = %e, "BGP session terminated with error"); } if let Some(name) = passive { - registry.set_dial_state( - &name, - DialState::Passive { - last_error: Some(match end.result { - Ok(()) => "session closed at our request".to_string(), - Err(e) => e.to_string(), - }), - }, - ); + let state = DialState::Passive { + last_error: Some(end.reason()), + }; + registry.set_dial_state(&name, state); } } diff --git a/crates/web/src/api.rs b/crates/web/src/api.rs index 0830f6c..bd0299e 100644 --- a/crates/web/src/api.rs +++ b/crates/web/src/api.rs @@ -327,6 +327,11 @@ fn as_path_text(path: &AsPathAttribute) -> String { segments.collect::>().join(" ") } +/// Octets as hexadecimal digits: `ab01`. +fn hex(octets: &[u8]) -> String { + octets.iter().map(|o| format!("{o:02x}")).collect() +} + /// An extended community (RFC 4360) as text. Route targets and route /// origins, of the three kinds of global administrator, by name; the rest /// as their eight octets. @@ -335,12 +340,7 @@ fn extended_community_text(community: &ExtendedCommunity) -> String { let kind = match b[1] { 0x02 => "rt", 0x03 => "ro", - _ => { - return format!( - "0x{}", - b.iter().map(|o| format!("{o:02x}")).collect::() - ); - } + _ => return format!("0x{}", hex(&b)), }; let u16_at = |at: usize| u16::from_be_bytes([b[at], b[at + 1]]); let u32_at = |at: usize| u32::from_be_bytes([b[at], b[at + 1], b[at + 2], b[at + 3]]); @@ -348,17 +348,21 @@ fn extended_community_text(community: &ExtendedCommunity) -> String { 0x00 => format!("{kind} {}:{}", u16_at(2), u32_at(4)), 0x01 => format!("{kind} {}:{}", Ipv4Addr::from(u32_at(2)), u16_at(6)), 0x02 => format!("{kind} {}:{}", u32_at(2), u16_at(6)), - _ => format!( - "0x{}", - b.iter().map(|o| format!("{o:02x}")).collect::() - ), + _ => format!("0x{}", hex(&b)), } } /// The name and value of an attribute the summary has no field for, or /// `None` for one it has. fn other_attribute(attribute: &PathAttribute) -> Option { - let (name, value) = match attribute { + let other = |name: &str, value: String| { + Some(OtherAttribute { + name: name.to_string(), + value, + }) + }; + let list = |items: Vec, separator| items.join(separator); + match attribute { PathAttribute::Origin(_) | PathAttribute::AsPath(_) | PathAttribute::NextHop(_) @@ -367,60 +371,50 @@ fn other_attribute(attribute: &PathAttribute) -> Option { | PathAttribute::Communities(_) | PathAttribute::LargeCommunities(_) | PathAttribute::MpReachNlri(_) - | PathAttribute::MpUnreachNlri(_) => return None, - PathAttribute::AtomicAggregate(_) => ("atomic aggregate".to_string(), "set".to_string()), - PathAttribute::Aggregator(a) => ("aggregator".to_string(), format!("AS{} {}", a.asn, a.ip)), - PathAttribute::ExtendedCommunities(e) => ( - "extended communities".to_string(), - e.extended_communities - .iter() - .map(extended_community_text) - .collect::>() - .join(", "), + | PathAttribute::MpUnreachNlri(_) => None, + PathAttribute::AtomicAggregate(_) => other("atomic aggregate", "set".to_string()), + PathAttribute::Aggregator(a) => other("aggregator", format!("AS{} {}", a.asn, a.ip)), + PathAttribute::ExtendedCommunities(e) => other( + "extended communities", + list( + e.extended_communities + .iter() + .map(extended_community_text) + .collect(), + ", ", + ), ), - PathAttribute::As4Path(p) => ("AS4_PATH".to_string(), as_path_text(p)), - PathAttribute::OnlyToCustomer(asn) => ("only to customer".to_string(), format!("AS{asn}")), - PathAttribute::OriginatorId(id) => ("originator ID".to_string(), id.to_string()), - PathAttribute::ClusterList(ids) => ( - "cluster list".to_string(), - ids.iter() - .map(Ipv4Addr::to_string) - .collect::>() - .join(" "), + PathAttribute::As4Path(p) => other("AS4_PATH", as_path_text(p)), + PathAttribute::OnlyToCustomer(asn) => other("only to customer", format!("AS{asn}")), + PathAttribute::OriginatorId(id) => other("originator ID", id.to_string()), + PathAttribute::ClusterList(ids) => other( + "cluster list", + list(ids.iter().map(Ipv4Addr::to_string).collect(), " "), ), PathAttribute::UnknownPathAttribute { flags, type_code, payload, } => { - let kind = [ + // "optional transitive, 2 octets: ab01" + let flags: Vec<&str> = [ (flags.optional(), "optional"), (flags.transitive(), "transitive"), (flags.partial(), "partial"), ] .into_iter() .filter_map(|(set, word)| set.then_some(word)) - .collect::>(); - let octets = payload - .iter() - .map(|o| format!("{o:02x}")) - .collect::(); - ( - format!("attribute {type_code}"), - format!( - "{}{} octets{}{octets}", - if kind.is_empty() { - String::new() - } else { - format!("{}, ", kind.join(" ")) - }, - payload.len(), - if payload.is_empty() { "" } else { ": " }, - ), - ) + .collect(); + let mut value = format!("{} octets", payload.len()); + if !flags.is_empty() { + value = format!("{}, {value}", flags.join(" ")); + } + if !payload.is_empty() { + value = format!("{value}: {}", hex(payload)); + } + other(&format!("attribute {type_code}"), value) } - }; - Some(OtherAttribute { name, value }) + } } pub fn summarize(attributes: &[PathAttribute], next_hop: Option<&NlriNextHop>) -> AttrSummary { diff --git a/crates/web/static/statusz.html b/crates/web/static/statusz.html index 99cb84a..e21c10c 100644 --- a/crates/web/static/statusz.html +++ b/crates/web/static/statusz.html @@ -177,14 +177,14 @@ from if (high === L) return low === L ? 'to every network' : `only to AS${low} (and others named so)`; return ''; } - const [admin, fn, arg] = p; + const [admin, func, arg] = p; if (admin !== L) return ''; - if (fn === 0) return arg ? `not to AS${arg}` : 'to no other network'; - if (fn === 1) return `only to AS${arg} (and others named so)`; - if (fn >= 11 && fn <= 13) return `our AS ${TIMES[fn - 10]} more on the path to ${arg ? `AS${arg}` : 'every network'}`; - if (fn === 20) return `not to group ${arg}`; - if (fn === 21) return `only to group ${arg} (and others named so)`; - if (fn >= 31 && fn <= 33) return `our AS ${TIMES[fn - 30]} more on the path to group ${arg}`; + if (func === 0) return arg ? `not to AS${arg}` : 'to no other network'; + if (func === 1) return `only to AS${arg} (and others named so)`; + if (func >= 11 && func <= 13) return `our AS ${TIMES[func - 10]} more on the path to ${arg ? `AS${arg}` : 'every network'}`; + if (func === 20) return `not to group ${arg}`; + if (func === 21) return `only to group ${arg} (and others named so)`; + if (func >= 31 && func <= 33) return `our AS ${TIMES[func - 30]} more on the path to group ${arg}`; return ''; } diff --git a/docs/content/docs/guides/passing-routes-on.md b/docs/content/docs/guides/passing-routes-on.md index fa05e3d..3973bc2 100644 --- a/docs/content/docs/guides/passing-routes-on.md +++ b/docs/content/docs/guides/passing-routes-on.md @@ -143,9 +143,9 @@ in hold-down for the neighbor's `import.hold_down_seconds` (default: 600 seconds). Set `max_prefixes` somewhat above the number of routes that the local AS and its customers originate. -At startup, Ferrix logs a warning for each neighbor to which export limits -apply but none are configured, and for each originated route that the -`prefixes` of the export limits do not permit. +At startup, Ferrix logs a warning that lists the neighbors to which export +limits apply but none are configured, and a warning for each originated +route that the `prefixes` of the export limits do not permit. ### Verifying Export Limits