diff --git a/crates/bench/benches/full_table.rs b/crates/bench/benches/full_table.rs index b7a13b0..4c55887 100644 --- a/crates/bench/benches/full_table.rs +++ b/crates/bench/benches/full_table.rs @@ -185,7 +185,7 @@ fn into_table(wire: &[Vec], routes: usize, threads: usize) { let wire = decoded.attributes_wire.as_deref().expect("kept"); if !update.announced_routes.is_empty() { let attrs = table.attrs().intern_received(wire, None).expect("valid"); - table.announce_paths(peers[at], &attrs, &update.announced_routes); + table.announce(peers[at], &attrs, &update.announced_routes); } for attribute in &update.path_attributes { if let PathAttribute::MpReachNlri(mp) = attribute { @@ -193,7 +193,7 @@ fn into_table(wire: &[Vec], routes: usize, threads: usize) { .attrs() .intern_received(wire, Some(&mp.next_hop)) .expect("valid"); - table.announce_paths(peers[at], &attrs, &mp.prefixes); + table.announce(peers[at], &attrs, &mp.prefixes); } } } diff --git a/crates/bench/src/lib.rs b/crates/bench/src/lib.rs index bbe4854..ea62f7e 100644 --- a/crates/bench/src/lib.rs +++ b/crates/bench/src/lib.rs @@ -24,7 +24,7 @@ use bgp_packet::message::{ LargeCommunity, Message, MpReachNlriPathAttribute, MultiExitDiscPathAttribute, NextHopPathAttribute, NlriNextHop, OriginPathAttribute, PathAttribute, UpdateMessage, }; -use bgp_primitives::constants::{AddressFamilyId, Afi, Safi}; +use bgp_primitives::constants::{AfiSafi, IpVersion}; use bgp_primitives::ip_prefix::{IpPrefix, Nlri}; use bgp_primitives::parser::ParserContext; use bytes::BytesMut; @@ -133,7 +133,7 @@ impl Prefixes { let length = rng.pick(&v4_lengths); // Unicast space, 1.0.0.0 to 223.255.255.255. let address = 0x0100_0000 + (rng.next_u64() as u32) % 0xdf00_0000; - let prefix = IpPrefix::new(AddressFamilyId::Ipv4, address.to_be_bytes(), length) + let prefix = IpPrefix::new(IpVersion::Ipv4, address.to_be_bytes(), length) .expect("four bytes cover any IPv4 prefix"); if seen.insert(prefix) { ipv4.push(prefix); @@ -144,7 +144,7 @@ impl Prefixes { let length = rng.pick(&v6_lengths); // Global unicast, 2000::/3. let address = (0x2000_u128 << 112) | (u128::from(rng.next_u64()) << 61 >> 3); - let prefix = IpPrefix::new(AddressFamilyId::Ipv6, address.to_be_bytes(), length) + let prefix = IpPrefix::new(IpVersion::Ipv6, address.to_be_bytes(), length) .expect("sixteen bytes cover any IPv6 prefix"); if seen.insert(prefix) { ipv6.push(prefix); @@ -219,8 +219,7 @@ pub fn announcements(prefixes: &Prefixes, shape: Shape, peer: u32) -> Vec Vec { /// peer with four-octet AS numbers. pub fn wire_context() -> ParserContext { ParserContext { - four_octet_asn: Some(true), + four_octet_asn: true, ibgp: false, add_path: Vec::new(), } diff --git a/crates/bench/src/mrt.rs b/crates/bench/src/mrt.rs index f6796a8..586ad5b 100644 --- a/crates/bench/src/mrt.rs +++ b/crates/bench/src/mrt.rs @@ -11,7 +11,7 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::path::Path; use bgp_packet::message::{NlriNextHop, PathAttribute}; -use bgp_primitives::constants::AddressFamilyId; +use bgp_primitives::constants::IpVersion; use bgp_primitives::ip_prefix::IpPrefix; use flate2::read::MultiGzDecoder; @@ -84,9 +84,9 @@ impl Dump { } RIB_IPV4_UNICAST | RIB_IPV6_UNICAST => { let version = if subtype == RIB_IPV4_UNICAST { - AddressFamilyId::Ipv4 + IpVersion::Ipv4 } else { - AddressFamilyId::Ipv6 + IpVersion::Ipv6 }; rib_entries(&mut record, version, &mut dump, &mut interned) .ok_or_else(malformed)?; @@ -162,7 +162,7 @@ fn peer_index_table(record: &mut Cursor<'_>) -> Option> { fn rib_entries( record: &mut Cursor<'_>, - version: AddressFamilyId, + version: IpVersion, dump: &mut Dump, interned: &mut HashMap, u32>, ) -> Option<()> { diff --git a/crates/integration-tests/src/test_peer.rs b/crates/integration-tests/src/test_peer.rs index d17db01..e1f94d0 100644 --- a/crates/integration-tests/src/test_peer.rs +++ b/crates/integration-tests/src/test_peer.rs @@ -69,9 +69,7 @@ impl TestPeer { .read_exact(&mut buf[BGP_HEADER_LEN as usize..]) .await?; - let (_, msg) = Message::from_wire(&self.ctx, &buf) - .map_err(|e| eyre!("Failed to parse BGP message: {e:?}"))?; - Ok(msg) + Message::decode(&self.ctx, &buf).map_err(|e| eyre!("Failed to parse BGP message: {e}")) } /// Read messages until an UPDATE arrives, skipping KEEPALIVEs. Any other @@ -95,7 +93,7 @@ impl TestPeer { matches!(opt, OpenOption::Capabilities(caps) if caps.iter().any(|c| matches!(c, Capability::FourByteAsn { .. }))) }); - self.ctx.four_octet_asn = Some(four_byte && self.offered_four_octet_asn); + self.ctx.four_octet_asn = four_byte && self.offered_four_octet_asn; Ok(o) } other => bail!("Expected OPEN, got {:?}", other), @@ -138,9 +136,9 @@ impl TestPeer { self.offered_four_octet_asn = capabilities .iter() .any(|c| matches!(c, Capability::FourByteAsn { .. })); - if let Some(negotiated) = &mut self.ctx.four_octet_asn { - *negotiated &= self.offered_four_octet_asn; - } + // If the server's OPEN has been read already, what was settled + // then still depends on what is offered now. + self.ctx.four_octet_asn &= self.offered_four_octet_asn; let asn_field = if local_asn > u16::MAX as u32 { 23456 } else { diff --git a/crates/integration-tests/src/test_server.rs b/crates/integration-tests/src/test_server.rs index 5765ff6..a77b987 100644 --- a/crates/integration-tests/src/test_server.rs +++ b/crates/integration-tests/src/test_server.rs @@ -142,7 +142,7 @@ impl TestServer { pub fn received_routes(&self) -> Vec { let mut routes = Vec::new(); self.rib.for_each_route(|route| { - let (attributes, next_hop) = route.attrs.attributes(); + let (attributes, next_hop) = route.attrs.attributes().expect("stored attributes parse"); routes.push(ReceivedRoute { prefix: route.prefix, peer: route.peer, diff --git a/crates/integration-tests/tests/basic_connection.rs b/crates/integration-tests/tests/basic_connection.rs index bce60c0..f11f3ca 100644 --- a/crates/integration-tests/tests/basic_connection.rs +++ b/crates/integration-tests/tests/basic_connection.rs @@ -10,7 +10,7 @@ use bgp_packet::message::{ NextHopPathAttribute, NlriNextHop, NotificationMessage, OriginPathAttribute, PathAttribute, UpdateMessage, }; -use bgp_primitives::constants::{AddressFamilyId, Afi, Safi}; +use bgp_primitives::constants::{AfiSafi, IpVersion}; use bgp_primitives::ip_prefix::IpPrefix; use eyre::Result; @@ -213,7 +213,7 @@ async fn test_message_split_across_a_keepalive_tick_is_still_one_message() -> Re let mut peer = TestPeer::connect(server.addr).await?; peer.handshake(65001, Ipv4Addr::new(10, 0, 0, 1), 3).await?; - let prefix = IpPrefix::new(AddressFamilyId::Ipv4, vec![10], 8)?; + let prefix = IpPrefix::new(IpVersion::Ipv4, vec![10], 8)?; let mut update = bytes::BytesMut::new(); Message::Update(UpdateMessage { withdrawn_routes: vec![], @@ -340,7 +340,7 @@ async fn test_server_accepts_update() -> Result<()> { withdrawn_routes: vec![], path_attributes: mandatory_attributes(), // 10.0.0.0/8 — one byte of prefix for a /8. - announced_routes: vec![IpPrefix::new(AddressFamilyId::Ipv4, vec![10], 8)?.into()], + announced_routes: vec![IpPrefix::new(IpVersion::Ipv4, vec![10], 8)?.into()], }); peer.send(&update).await?; @@ -358,13 +358,12 @@ async fn test_server_accepts_ipv6_update() -> Result<()> { .await?; // 2001:db8::/32 - let prefix = IpPrefix::new(AddressFamilyId::Ipv6, vec![0x20, 0x01, 0x0d, 0xb8], 32)?; + let prefix = IpPrefix::new(IpVersion::Ipv6, vec![0x20, 0x01, 0x0d, 0xb8], 32)?; let update = Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: with_origin_and_as_path(PathAttribute::MpReachNlri( MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, next_hop: NlriNextHop::Ipv6("2001:db8::1".parse().unwrap()), prefixes: vec![prefix.into()], }, @@ -392,7 +391,7 @@ async fn test_rib_records_ipv4_announce() -> Result<()> { peer.handshake(65001, Ipv4Addr::new(10, 0, 0, 1), 90) .await?; - let prefix = IpPrefix::new(AddressFamilyId::Ipv4, vec![10], 8)?; + let prefix = IpPrefix::new(IpVersion::Ipv4, vec![10], 8)?; peer.send(&Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: mandatory_attributes(), @@ -424,7 +423,7 @@ async fn test_rib_records_ipv4_withdraw() -> Result<()> { peer.handshake(65001, Ipv4Addr::new(10, 0, 0, 1), 90) .await?; - let prefix = IpPrefix::new(AddressFamilyId::Ipv4, vec![10], 8)?; + let prefix = IpPrefix::new(IpVersion::Ipv4, vec![10], 8)?; // Announce first. peer.send(&Message::Update(UpdateMessage { @@ -467,13 +466,12 @@ async fn test_rib_records_ipv6_announce() -> Result<()> { peer.handshake(65001, Ipv4Addr::new(10, 0, 0, 1), 90) .await?; - let prefix = IpPrefix::new(AddressFamilyId::Ipv6, vec![0x20, 0x01, 0x0d, 0xb8], 32)?; + let prefix = IpPrefix::new(IpVersion::Ipv6, vec![0x20, 0x01, 0x0d, 0xb8], 32)?; peer.send(&Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: with_origin_and_as_path(PathAttribute::MpReachNlri( MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, next_hop: NlriNextHop::Ipv6("2001:db8::1".parse().unwrap()), prefixes: vec![prefix.into()], }, @@ -506,15 +504,14 @@ async fn test_rib_records_ipv6_withdraw() -> Result<()> { peer.handshake(65001, Ipv4Addr::new(10, 0, 0, 1), 90) .await?; - let prefix = IpPrefix::new(AddressFamilyId::Ipv6, vec![0x20, 0x01, 0x0d, 0xb8], 32)?; + let prefix = IpPrefix::new(IpVersion::Ipv6, vec![0x20, 0x01, 0x0d, 0xb8], 32)?; // Announce first. peer.send(&Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: with_origin_and_as_path(PathAttribute::MpReachNlri( MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, next_hop: NlriNextHop::Ipv6("2001:db8::1".parse().unwrap()), prefixes: vec![prefix.into()], }, @@ -528,8 +525,7 @@ async fn test_rib_records_ipv6_withdraw() -> Result<()> { peer.send(&Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, prefixes: vec![prefix.into()], })], announced_routes: vec![], diff --git a/crates/integration-tests/tests/interop.rs b/crates/integration-tests/tests/interop.rs index 12ef78f..2e9e53c 100644 --- a/crates/integration-tests/tests/interop.rs +++ b/crates/integration-tests/tests/interop.rs @@ -7,7 +7,7 @@ use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::TestServer; use bgp_packet::message::{Message, PathAttribute}; use bgp_packet::open::{Capability, OpenOption}; -use bgp_primitives::constants::{Afi, Safi}; +use bgp_primitives::constants::{Afi, AfiSafi, Safi}; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use eyre::Result; @@ -133,7 +133,7 @@ async fn test_ipv6_prefix_update_shape() -> Result<()> { .path_attributes .iter() .any(|a| matches!(a, PathAttribute::MpReachNlri(m) - if m.afi == Afi::IPV6 && m.prefixes[0].to_string() == "2001:db8:1::/48")) + if m.family == AfiSafi::IPV6_UNICAST && m.prefixes[0].to_string() == "2001:db8:1::/48")) ); let _ = Ipv4Addr::LOCALHOST; Ok(()) diff --git a/crates/integration-tests/tests/negotiation.rs b/crates/integration-tests/tests/negotiation.rs index 092e648..debcead 100644 --- a/crates/integration-tests/tests/negotiation.rs +++ b/crates/integration-tests/tests/negotiation.rs @@ -58,8 +58,7 @@ fn dual_stack_update() -> UpdateMessage { AsPathAttribute::from_asns(vec![PEER_ASN]), PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, next_hop: NlriNextHop::Ipv6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1)), prefixes: vec![prefix("2001:db8:1::/48").into()], }), diff --git a/crates/packet/src/errors.rs b/crates/packet/src/errors.rs index 1412fc7..c2a5354 100644 --- a/crates/packet/src/errors.rs +++ b/crates/packet/src/errors.rs @@ -1,4 +1,4 @@ -use serde::{Deserialize, Serialize}; +use serde::Serialize; use strum::FromRepr; use thiserror::Error; @@ -23,67 +23,57 @@ pub struct DecodeError { pub detail: String, } -#[derive(Debug, Error)] -#[error("Unknown BGP error code {code} subcode {subcode}")] -pub struct UnknownErrorCode { - pub code: u8, - pub subcode: u8, -} - -#[derive(Debug, Clone, PartialEq, Eq, Error, Serialize, Deserialize)] -#[repr(u8)] +/// The error a NOTIFICATION reports: its code and subcode (RFC 4271 §4.5). +#[derive(Debug, Clone, PartialEq, Eq, Error, Serialize)] pub enum BgpError { - #[error("Message header error")] - MsgHeader(MsgHeaderSubcode) = 1, + #[error("Message header error: {0}")] + MsgHeader(MsgHeaderSubcode), - #[error("Open message error")] - OpenMsg(OpenMsgSubcode) = 2, + #[error("Open message error: {0}")] + OpenMsg(OpenMsgSubcode), - #[error("Update message error")] - UpdateMsg(UpdateMsgSubcode) = 3, + #[error("Update message error: {0}")] + UpdateMsg(UpdateMsgSubcode), #[error("Hold timer expired")] - HoldTimer = 4, + HoldTimer, - #[error("Finite state machine error")] - FsmError(FsmSubcode) = 5, + #[error("Finite state machine error: {0}")] + FsmError(FsmSubcode), - #[error("Cease")] - Cease(CeaseSubcode) = 6, + #[error("Cease: {0}")] + Cease(CeaseSubcode), - #[error("Route refresh message error")] - RouteRefresh(RouteRefreshSubcode) = 7, + #[error("Route refresh message error: {0}")] + RouteRefresh(RouteRefreshSubcode), #[error("Send hold timer expired")] - SendHoldTimer = 8, + SendHoldTimer, + + /// A code or subcode this implementation has no name for. A peer may + /// send any: new ones are assigned, and zero ("unspecific") is always + /// allowed. The session ends the same way, and the numbers are kept + /// for the log. + #[error("error code {code} subcode {subcode}")] + Other { code: u8, subcode: u8 }, } impl BgpError { - pub fn from_code_subcode(code: u8, subcode: u8) -> Result { - let err = || UnknownErrorCode { code, subcode }; - match code { - 1 => Ok(Self::MsgHeader( - MsgHeaderSubcode::try_from(subcode).map_err(|_| err())?, - )), - 2 => Ok(Self::OpenMsg( - OpenMsgSubcode::try_from(subcode).map_err(|_| err())?, - )), - 3 => Ok(Self::UpdateMsg( - UpdateMsgSubcode::try_from(subcode).map_err(|_| err())?, - )), - 4 => Ok(Self::HoldTimer), - 5 => Ok(Self::FsmError( - FsmSubcode::try_from(subcode).map_err(|_| err())?, - )), - 6 => Ok(Self::Cease( - CeaseSubcode::try_from(subcode).map_err(|_| err())?, - )), - 7 => Ok(Self::RouteRefresh( - RouteRefreshSubcode::try_from(subcode).map_err(|_| err())?, - )), - 8 => Ok(Self::SendHoldTimer), - _ => Err(err()), - } + /// The error for a code and subcode off the wire. Every pair has one, + /// so that a peer's NOTIFICATION can always be read. + pub fn from_code_subcode(code: u8, subcode: u8) -> Self { + let known = match code { + 1 => MsgHeaderSubcode::from_repr(subcode).map(Self::MsgHeader), + 2 => OpenMsgSubcode::from_repr(subcode).map(Self::OpenMsg), + 3 => UpdateMsgSubcode::from_repr(subcode).map(Self::UpdateMsg), + 4 => (subcode == 0).then_some(Self::HoldTimer), + 5 => FsmSubcode::from_repr(subcode).map(Self::FsmError), + 6 => CeaseSubcode::from_repr(subcode).map(Self::Cease), + 7 => RouteRefreshSubcode::from_repr(subcode).map(Self::RouteRefresh), + 8 => (subcode == 0).then_some(Self::SendHoldTimer), + _ => None, + }; + known.unwrap_or(Self::Other { code, subcode }) } pub fn code_subcode(&self) -> (u8, u8) { @@ -96,11 +86,12 @@ impl BgpError { Self::Cease(s) => (6, *s as u8), Self::RouteRefresh(s) => (7, *s as u8), Self::SendHoldTimer => (8, 0), + Self::Other { code, subcode } => (*code, *subcode), } } } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, Deserialize, FromRepr)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, FromRepr)] #[repr(u8)] pub enum MsgHeaderSubcode { #[error("Connection not synchronized")] @@ -113,14 +104,7 @@ pub enum MsgHeaderSubcode { BadMessageType = 3, } -impl TryFrom for MsgHeaderSubcode { - type Error = (); - fn try_from(v: u8) -> Result { - Self::from_repr(v).ok_or(()) - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, Deserialize, FromRepr)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, FromRepr)] #[repr(u8)] pub enum OpenMsgSubcode { #[error("Unspecific")] @@ -148,14 +132,7 @@ pub enum OpenMsgSubcode { RoleMismatch = 8, } -impl TryFrom for OpenMsgSubcode { - type Error = (); - fn try_from(v: u8) -> Result { - Self::from_repr(v).ok_or(()) - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, Deserialize, FromRepr)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, FromRepr)] #[repr(u8)] pub enum UpdateMsgSubcode { #[error("Malformed attribute list")] @@ -189,14 +166,7 @@ pub enum UpdateMsgSubcode { MalformedAsPath = 11, } -impl TryFrom for UpdateMsgSubcode { - type Error = (); - fn try_from(v: u8) -> Result { - Self::from_repr(v).ok_or(()) - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, Deserialize, FromRepr)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, FromRepr)] #[repr(u8)] pub enum FsmSubcode { #[error("Received an unexpected message in OpenSent state")] @@ -209,14 +179,7 @@ pub enum FsmSubcode { UnexpectedEstablished = 3, } -impl TryFrom for FsmSubcode { - type Error = (); - fn try_from(v: u8) -> Result { - Self::from_repr(v).ok_or(()) - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, Deserialize, FromRepr)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, FromRepr)] #[repr(u8)] pub enum CeaseSubcode { #[error("Maximum number of prefixes reached")] @@ -250,23 +213,9 @@ pub enum CeaseSubcode { BfdDown = 10, } -impl TryFrom for CeaseSubcode { - type Error = (); - fn try_from(v: u8) -> Result { - Self::from_repr(v).ok_or(()) - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, Deserialize, FromRepr)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, FromRepr)] #[repr(u8)] pub enum RouteRefreshSubcode { #[error("Invalid message length")] InvalidLength = 1, } - -impl TryFrom for RouteRefreshSubcode { - type Error = (); - fn try_from(v: u8) -> Result { - Self::from_repr(v).ok_or(()) - } -} diff --git a/crates/packet/src/message.rs b/crates/packet/src/message.rs index 2659ebc..d19e364 100644 --- a/crates/packet/src/message.rs +++ b/crates/packet/src/message.rs @@ -1,5 +1,4 @@ use std::net::{Ipv4Addr, Ipv6Addr}; -use std::ops::Deref; use bitfield::bitfield; use bytes::BufMut; @@ -10,12 +9,11 @@ use nom::Parser; use nom::number::complete::be_u8; use nom::number::complete::be_u16; use nom::number::complete::be_u32; -use serde::{Deserialize, Serialize}; -use serde_repr::{Deserialize_repr, Serialize_repr}; -use strum::EnumDiscriminants; +use serde::Serialize; +use serde_repr::Serialize_repr; use crate::update_decode::UpdateError; -use bgp_primitives::constants::{AddressFamilyId, Afi, AfiSafi, Safi}; +use bgp_primitives::constants::{Afi, AfiSafi, IpVersion, Safi}; use bgp_primitives::ip_prefix::{IpPrefix, Nlri}; use bgp_primitives::parser::BgpParserError; use bgp_primitives::parser::EncodeError; @@ -24,7 +22,6 @@ use bgp_primitives::parser::ProtocolErrorValues; use crate::errors::BgpError; use crate::errors::DecodeError; -use crate::errors::InvalidValue; use crate::errors::MsgHeaderSubcode; use crate::errors::OpenMsgSubcode; use crate::open::OpenMessage; @@ -44,16 +41,32 @@ pub const BGP_MAX_MSG_LEN: u16 = 4096; const BGP_MARKER: [u8; 16] = [0xFF; 16]; +/// BGP message type codes (RFC 4271 §4.1, RFC 2918). +mod msg_type { + pub const OPEN: u8 = 1; + pub const UPDATE: u8 = 2; + pub const NOTIFICATION: u8 = 3; + pub const KEEPALIVE: u8 = 4; + pub const ROUTE_REFRESH: u8 = 5; +} + /// Message represents the top-level messages in the BGP protocol. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, EnumDiscriminants)] -#[repr(u8)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub enum Message { - Open(OpenMessage) = 1, - Update(UpdateMessage) = 2, - Notification(NotificationMessage) = 3, - KeepAlive = 4, + Open(OpenMessage), + Update(UpdateMessage), + Notification(NotificationMessage), + KeepAlive, /// BGP Route Refresh message (RFC 2918). - RouteRefresh(RouteRefreshMessage) = 5, + RouteRefresh(RouteRefreshMessage), +} + +fn header_error(subcode: MsgHeaderSubcode, data: Vec, detail: String) -> DecodeError { + DecodeError { + notification: BgpError::MsgHeader(subcode), + data, + detail, + } } impl Message { @@ -70,17 +83,10 @@ impl Message { Self::decode_with_report(ctx, buf).map(|decoded| decoded.message) } - /// As [`Message::decode`], and also returns the errors in an UPDATE - /// that were handled without resetting the session. The list is empty - /// for a well-formed UPDATE and for every other message. + /// As [`Message::decode`], and also returns what else decoding an + /// UPDATE found out: the errors in it that were handled without + /// resetting the session, and its attributes as they were on the wire. pub fn decode_with_report(ctx: &ParserContext, buf: &[u8]) -> Result { - let mut update_errors = Vec::new(); - let mut attributes_wire = None; - let header_error = |subcode, data: Vec, detail: String| DecodeError { - notification: BgpError::MsgHeader(subcode), - data, - detail, - }; let header_len = usize::from(BGP_HEADER_LEN); if buf.len() < header_len { return Err(header_error( @@ -97,7 +103,7 @@ impl Message { )); } let length = u16::from_be_bytes([buf[16], buf[17]]); - let msg_type = buf[18]; + let type_code = buf[18]; if !(BGP_HEADER_LEN..=BGP_MAX_MSG_LEN).contains(&length) || usize::from(length) > buf.len() { return Err(header_error( @@ -119,135 +125,139 @@ impl Message { ) }; - let message = match msg_type { - 1 => OpenMessage::from_wire(ctx, body) - .map(|(_, open)| Self::Open(open)) - .map_err(|e| { - let subcode = match &e { - nom::Err::Error(BgpParserError::ProtocolError(value)) - | nom::Err::Failure(BgpParserError::ProtocolError(value)) => match value { - ProtocolErrorValues::UnsupportedVersion(_) => { - OpenMsgSubcode::UnsupportedVersion - } - ProtocolErrorValues::UnknownOpenOption(_) => { - OpenMsgSubcode::UnsupportedOptionalParam - } - }, - _ => OpenMsgSubcode::Unspecific, - }; - // RFC 4271 §6.2: the Data field of Unsupported Version - // Number is the largest version we do support. - let data = match subcode { - OpenMsgSubcode::UnsupportedVersion => { - u16::from(BGP4_VERSION).to_be_bytes().to_vec() - } - _ => vec![], - }; - DecodeError { - notification: BgpError::OpenMsg(subcode), - data, - detail: format!("malformed OPEN: {}", describe(&e)), - } - }), - 2 => { + let message = match type_code { + msg_type::OPEN => Self::Open(decode_open(body)?), + msg_type::UPDATE => { let decoded = UpdateMessage::decode(ctx, body)?; - update_errors = decoded.errors; - attributes_wire = decoded.attributes_wire; - Ok(Self::Update(decoded.update)) + return Ok(Decoded { + message: Self::Update(decoded.update), + update_errors: decoded.errors, + attributes_wire: decoded.attributes_wire, + }); } - 3 => NotificationMessage::from_wire(ctx, body) - .map(|(_, notification)| Self::Notification(notification)) - .map_err(|e| bad_length(&format!("malformed NOTIFICATION ({})", describe(&e)))), - 4 if body.is_empty() => Ok(Self::KeepAlive), - 4 => Err(bad_length("KEEPALIVE with a body")), - 5 => match body { - [afi_hi, afi_lo, subtype, safi] => Ok(Self::RouteRefresh(RouteRefreshMessage { + msg_type::NOTIFICATION => match body { + [code, subcode, data @ ..] => Self::Notification(NotificationMessage { + error: BgpError::from_code_subcode(*code, *subcode), + data: data.to_vec(), + }), + _ => return Err(bad_length("NOTIFICATION")), + }, + msg_type::KEEPALIVE if body.is_empty() => Self::KeepAlive, + msg_type::KEEPALIVE => return Err(bad_length("KEEPALIVE with a body")), + msg_type::ROUTE_REFRESH => match body { + [afi_hi, afi_lo, subtype, safi] => Self::RouteRefresh(RouteRefreshMessage { afi: Afi(u16::from_be_bytes([*afi_hi, *afi_lo])), subtype: *subtype, safi: Safi(*safi), - })), - _ => Err(bad_length("ROUTE-REFRESH")), + }), + _ => return Err(bad_length("ROUTE-REFRESH")), }, - other => Err(header_error( - MsgHeaderSubcode::BadMessageType, - vec![other], - format!("unknown BGP message type {other}"), - )), - }?; + other => { + return Err(header_error( + MsgHeaderSubcode::BadMessageType, + vec![other], + format!("unknown BGP message type {other}"), + )); + } + }; Ok(Decoded { message, - update_errors, - attributes_wire, + update_errors: Vec::new(), + attributes_wire: None, }) } - /// Parses a complete BGP message including the 19-byte header from `buf`. - /// Returns the unparsed remainder and the decoded `Message`. - /// - /// This is [`Message::decode`] in the shape of the other parsers; use - /// `decode` when the reason for a failure matters. - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let msg = Self::decode(ctx, buf) - .map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; - // `decode` checked that the length in the header is within `buf`. - let length = usize::from(u16::from_be_bytes([buf[16], buf[17]])); - Ok((&buf[length..], msg)) - } - - /// Serializes the message (header + body) into `out`. + /// Writes the message, header and body, to the end of `out`. If it + /// cannot be encoded, `out` is left as it was. pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - // Serialize the body first so we can compute the total length. - let mut body = BytesMut::new(); - match self { - Self::Open(m) => m.to_wire(ctx, &mut body)?, - Self::Update(m) => m.to_wire(ctx, &mut body)?, - Self::Notification(m) => m.to_wire(ctx, &mut body)?, - Self::RouteRefresh(m) => { - body.put_u16(m.afi.0); - body.put_u8(m.subtype); - body.put_u8(m.safi.0); - } - Self::KeepAlive => {} - } - - let total_len = usize::from(BGP_HEADER_LEN) + body.len(); - let total_len = u16::try_from(total_len) - .ok() - .filter(|&l| l <= BGP_MAX_MSG_LEN) - .ok_or(EncodeError::TooLong { - what: "BGP message", - len: total_len, - max: BGP_MAX_MSG_LEN.into(), - })?; - + let start = out.len(); out.put_slice(&BGP_MARKER); - out.put_u16(total_len); + // The length, filled in once the body is written. + out.put_u16(0); out.put_u8(self.type_code()); - out.put(body); - Ok(()) + let written = self.body_to_wire(ctx, out).and_then(|()| { + let length = out.len() - start; + u16::try_from(length) + .ok() + .filter(|length| *length <= BGP_MAX_MSG_LEN) + .ok_or(EncodeError::TooLong { + what: "BGP message", + len: length, + max: BGP_MAX_MSG_LEN.into(), + }) + }); + match written { + Ok(length) => { + out[start + 16..start + 18].copy_from_slice(&length.to_be_bytes()); + Ok(()) + } + Err(e) => { + out.truncate(start); + Err(e) + } + } } - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - let body_len: u16 = match self { - Self::Open(m) => m.wire_len(ctx)?, - Self::Update(m) => m.wire_len(ctx)?, - Self::Notification(m) => m.wire_len(), - Self::RouteRefresh(_) => 4, - Self::KeepAlive => 0, - }; - Ok(BGP_HEADER_LEN + body_len) + fn body_to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + match self { + Self::Open(m) => m.to_wire(out), + Self::Update(m) => m.to_wire(ctx, out), + Self::Notification(m) => { + m.to_wire(out); + Ok(()) + } + Self::RouteRefresh(m) => { + out.put_u16(m.afi.0); + out.put_u8(m.subtype); + out.put_u8(m.safi.0); + Ok(()) + } + Self::KeepAlive => Ok(()), + } } fn type_code(&self) -> u8 { match self { - Self::Open(_) => 1, - Self::Update(_) => 2, - Self::Notification(_) => 3, - Self::KeepAlive => 4, - Self::RouteRefresh(_) => 5, + Self::Open(_) => msg_type::OPEN, + Self::Update(_) => msg_type::UPDATE, + Self::Notification(_) => msg_type::NOTIFICATION, + Self::KeepAlive => msg_type::KEEPALIVE, + Self::RouteRefresh(_) => msg_type::ROUTE_REFRESH, + } + } +} + +/// Decodes the body of an OPEN, with the NOTIFICATION that reports a +/// failure (RFC 4271 §6.2). +fn decode_open(body: &[u8]) -> Result { + let refused = |subcode, data: Vec, detail: String| DecodeError { + notification: BgpError::OpenMsg(subcode), + data, + detail: format!("malformed OPEN: {detail}"), + }; + match OpenMessage::from_wire(body) { + Ok(([], open)) => Ok(open), + Ok((rest, _)) => Err(refused( + OpenMsgSubcode::Unspecific, + vec![], + format!("{} octets after the optional parameters", rest.len()), + )), + Err(e) => { + let (subcode, data) = match &e { + nom::Err::Error(BgpParserError::ProtocolError(value)) + | nom::Err::Failure(BgpParserError::ProtocolError(value)) => match value { + // The Data field is the largest version we do support. + ProtocolErrorValues::UnsupportedVersion(_) => ( + OpenMsgSubcode::UnsupportedVersion, + u16::from(BGP4_VERSION).to_be_bytes().to_vec(), + ), + ProtocolErrorValues::UnknownOpenOption(_) => { + (OpenMsgSubcode::UnsupportedOptionalParam, vec![]) + } + }, + _ => (OpenMsgSubcode::Unspecific, vec![]), + }; + Err(refused(subcode, data, describe(&e))) } } } @@ -265,7 +275,7 @@ pub struct Decoded { } /// A request to send again every route of one address family (RFC 2918). -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] pub struct RouteRefreshMessage { pub afi: Afi, /// Zero for a plain request. RFC 7313 uses the octet, reserved in @@ -290,7 +300,7 @@ impl RouteRefreshMessage { } /// Represents a BGP Update message. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct UpdateMessage { pub withdrawn_routes: Vec, pub path_attributes: Vec, @@ -298,70 +308,50 @@ pub struct UpdateMessage { } impl UpdateMessage { - /// Parses a BGP UPDATE message body (RFC 4271 §4.3), without the - /// 19-octet header, in the shape of the other parsers. - /// - /// This is [`UpdateMessage::decode`] with the report of handled errors - /// dropped; use `decode` when it matters what was wrong. - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let decoded = Self::decode(ctx, buf) - .map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; - Ok((&[], decoded.update)) - } - + /// Writes the body of the UPDATE (what follows the 19-octet header). pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - // Withdrawn routes. // The prefixes in the body are IPv4 unicast (RFC 4271 §4.3). let with_path_id = ctx.has_add_path(AfiSafi::IPV4_UNICAST); - let withdrawn_len = nlri_wire_len(&self.withdrawn_routes, with_path_id); - let withdrawn_len = u16::try_from(withdrawn_len) - .map_err(|_| too_long("withdrawn routes", withdrawn_len))?; - out.put_u16(withdrawn_len); + + let withdrawn = reserve_length(out); write_nlri(&self.withdrawn_routes, with_path_id, out); + fill_length(out, withdrawn, "withdrawn routes")?; - // Path attributes. - let pa_total: u32 = self - .path_attributes - .iter() - .map(|p| p.wire_len(ctx).map(|v| v as u32)) - .sum::>()?; - let pa_total = - u16::try_from(pa_total).map_err(|_| too_long("path attributes", pa_total))?; - out.put_u16(pa_total); - for pa in &self.path_attributes { - pa.to_wire(ctx, out)?; + let attributes = reserve_length(out); + for attribute in &self.path_attributes { + attribute.to_wire(ctx, out)?; } + fill_length(out, attributes, "path attributes")?; - // NLRI (consumes the remainder of the message). + // The NLRI take up the rest of the message. write_nlri(&self.announced_routes, with_path_id, out); - Ok(()) } - - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - // 2 bytes for the withdrawn length field + 2 bytes for the total path - // attribute length field. - let with_path_id = ctx.has_add_path(AfiSafi::IPV4_UNICAST); - let mut total: u32 = 4; - total += nlri_wire_len(&self.withdrawn_routes, with_path_id); - for pa in &self.path_attributes { - total += pa.wire_len(ctx)? as u32; - } - total += nlri_wire_len(&self.announced_routes, with_path_id); - u16::try_from(total).map_err(|_| too_long("UPDATE message", total)) - } } -/// The length on the wire of a list of prefixes as an UPDATE names them: -/// for each, its path identifier if the session uses them for the family -/// (RFC 7911 §3), its length in bits, and the bytes that length covers. -fn nlri_wire_len(nlri: &[Nlri], with_path_id: bool) -> u32 { - nlri.iter().map(|n| n.wire_len(with_path_id) as u32).sum() +/// Leaves room at the end of `out` for a two-octet length that is known +/// only once what it counts has been written, and returns where. +fn reserve_length(out: &mut BytesMut) -> usize { + let at = out.len(); + out.put_u16(0); + at } +/// Fills in the length reserved at `at`: everything written since. +fn fill_length(out: &mut BytesMut, at: usize, what: &'static str) -> Result<(), EncodeError> { + let length = out.len() - at - 2; + let field = u16::try_from(length).map_err(|_| EncodeError::TooLong { + what, + len: length, + max: u16::MAX.into(), + })?; + out[at..at + 2].copy_from_slice(&field.to_be_bytes()); + Ok(()) +} + +/// Writes prefixes as an UPDATE names them: for each, its path identifier +/// if the session uses them for the family (RFC 7911 §3), its length in +/// bits, and the bytes that length covers. fn write_nlri(nlri: &[Nlri], with_path_id: bool, out: &mut BytesMut) { for n in nlri { if with_path_id { @@ -386,17 +376,8 @@ pub(crate) fn describe(e: &nom::Err>) -> String { } } -/// A length that does not fit the two-octet field that has to carry it. -fn too_long(what: &'static str, len: u32) -> EncodeError { - EncodeError::TooLong { - what, - len: len as usize, - max: u16::MAX.into(), - } -} - bitfield! { - #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] + #[derive(Clone, Debug, Serialize, PartialEq, Eq)] pub struct PathAttributeFlags(u8); impl new; u8; @@ -406,32 +387,45 @@ bitfield! { pub extended_length, set_extended_length: 4; } -impl Deref for PathAttributeFlags { - type Target = u8; - - fn deref(&self) -> &Self::Target { - &self.0 - } +/// Path attribute type codes. +pub mod attr_type { + pub const ORIGIN: u8 = 1; + pub const AS_PATH: u8 = 2; + pub const NEXT_HOP: u8 = 3; + pub const MULTI_EXIT_DISC: u8 = 4; + pub const LOCAL_PREF: u8 = 5; + pub const ATOMIC_AGGREGATE: u8 = 6; + pub const AGGREGATOR: u8 = 7; + pub const COMMUNITIES: u8 = 8; + pub const ORIGINATOR_ID: u8 = 9; + pub const CLUSTER_LIST: u8 = 10; + pub const MP_REACH_NLRI: u8 = 14; + pub const MP_UNREACH_NLRI: u8 = 15; + pub const EXTENDED_COMMUNITIES: u8 = 16; + pub const AS4_PATH: u8 = 17; + pub const AS4_AGGREGATOR: u8 = 18; + pub const LARGE_COMMUNITIES: u8 = 32; } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, EnumDiscriminants)] -#[repr(u8)] +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub enum PathAttribute { - Origin(OriginPathAttribute) = 1, - AsPath(AsPathAttribute) = 2, - NextHop(NextHopPathAttribute) = 3, - MultiExitDisc(MultiExitDiscPathAttribute) = 4, - LocalPref(LocalPrefPathAttribute) = 5, - AtomicAggregate(AtomicAggregatePathAttribute) = 6, - Aggregator(AggregatorPathAttribute) = 7, - Communities(CommunitiesPathAttribute) = 8, - MpReachNlri(MpReachNlriPathAttribute) = 14, - MpUnreachNlri(MpUnreachNlriPathAttribute) = 15, - ExtendedCommunities(ExtendedCommunitiesPathAttribute) = 16, + Origin(OriginPathAttribute), + AsPath(AsPathAttribute), + NextHop(NextHopPathAttribute), + MultiExitDisc(MultiExitDiscPathAttribute), + LocalPref(LocalPrefPathAttribute), + AtomicAggregate(AtomicAggregatePathAttribute), + Aggregator(AggregatorPathAttribute), + Communities(CommunitiesPathAttribute), + MpReachNlri(MpReachNlriPathAttribute), + MpUnreachNlri(MpUnreachNlriPathAttribute), + ExtendedCommunities(ExtendedCommunitiesPathAttribute), /// The AS path with four-octet AS numbers, for a route that crosses /// speakers that only handle two-octet ones (RFC 6793 §3). - As4Path(AsPathAttribute) = 17, - LargeCommunities(LargeCommunitiesPathAttribute) = 32, + As4Path(AsPathAttribute), + LargeCommunities(LargeCommunitiesPathAttribute), + /// An attribute this implementation does not interpret, kept as it + /// came so that it can be passed on. UnknownPathAttribute { flags: PathAttributeFlags, type_code: u8, @@ -439,43 +433,21 @@ pub enum PathAttribute { }, } -impl From for PathAttributeDiscriminants { - fn from(value: u8) -> Self { - match value { - v if v == Self::Origin as u8 => Self::Origin, - v if v == Self::AsPath as u8 => Self::AsPath, - v if v == Self::NextHop as u8 => Self::NextHop, - v if v == Self::MultiExitDisc as u8 => Self::MultiExitDisc, - v if v == Self::LocalPref as u8 => Self::LocalPref, - v if v == Self::AtomicAggregate as u8 => Self::AtomicAggregate, - v if v == Self::Aggregator as u8 => Self::Aggregator, - v if v == Self::Communities as u8 => Self::Communities, - v if v == Self::MpReachNlri as u8 => Self::MpReachNlri, - v if v == Self::MpUnreachNlri as u8 => Self::MpUnreachNlri, - v if v == Self::ExtendedCommunities as u8 => Self::ExtendedCommunities, - v if v == Self::As4Path as u8 => Self::As4Path, - v if v == Self::LargeCommunities as u8 => Self::LargeCommunities, - _ => Self::UnknownPathAttribute, - } - } -} - /// The Optional and Transitive flags that the attribute with this type code /// is defined to have, for the attributes whose definition we know. A /// received attribute with other values is in error (RFC 7606 §3.c). pub(crate) fn defined_flags(type_code: u8) -> Option<(bool, bool)> { + use attr_type::*; const WELL_KNOWN: (bool, bool) = (false, true); const OPTIONAL_TRANSITIVE: (bool, bool) = (true, true); const OPTIONAL_NON_TRANSITIVE: (bool, bool) = (true, false); Some(match type_code { - // ORIGIN, AS_PATH, NEXT_HOP, LOCAL_PREF, ATOMIC_AGGREGATE (RFC 4271). - 1 | 2 | 3 | 5 | 6 => WELL_KNOWN, - // MULTI_EXIT_DISC; ORIGINATOR_ID and CLUSTER_LIST (RFC 4456); - // MP_REACH_NLRI and MP_UNREACH_NLRI (RFC 4760). - 4 | 9 | 10 | 14 | 15 => OPTIONAL_NON_TRANSITIVE, - // AGGREGATOR, COMMUNITIES, EXTENDED COMMUNITIES, AS4_PATH, - // AS4_AGGREGATOR, LARGE_COMMUNITY. - 7 | 8 | 16 | 17 | 18 | 32 => OPTIONAL_TRANSITIVE, + ORIGIN | AS_PATH | NEXT_HOP | LOCAL_PREF | ATOMIC_AGGREGATE => WELL_KNOWN, + MULTI_EXIT_DISC | ORIGINATOR_ID | CLUSTER_LIST | MP_REACH_NLRI | MP_UNREACH_NLRI => { + OPTIONAL_NON_TRANSITIVE + } + AGGREGATOR | COMMUNITIES | EXTENDED_COMMUNITIES | AS4_PATH | AS4_AGGREGATOR + | LARGE_COMMUNITIES => OPTIONAL_TRANSITIVE, _ => return None, }) } @@ -492,283 +464,227 @@ fn whole((rest, parsed): (&[u8], T)) -> Result { + pub flags: PathAttributeFlags, + pub type_code: u8, + pub value: &'a [u8], + /// The whole attribute, header and value. + pub wire: &'a [u8], +} + +/// Why the bytes at the front of a buffer are not an attribute. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NotAnAttribute { + /// Too few octets for flags, type and length. + NoHeader, + /// The length says more than there is. + Truncated { + type_code: u8, + length: usize, + available: usize, + }, +} + +impl std::fmt::Display for NotAnAttribute { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NoHeader => f.write_str("too few octets for an attribute"), + Self::Truncated { + type_code, + length, + available, + } => write!( + f, + "attribute {type_code} has length {length} but only {available} octets remain" + ), + } + } +} + +/// Splits the attribute at the front of `buf` from what follows it. +pub fn split_attribute(buf: &[u8]) -> Result<(RawAttribute<'_>, &[u8]), NotAnAttribute> { + let [flags, type_code, rest @ ..] = buf else { + return Err(NotAnAttribute::NoHeader); + }; + let flags = PathAttributeFlags(*flags); + let (length, rest) = match (flags.extended_length(), rest) { + (true, [hi, lo, rest @ ..]) => (usize::from(u16::from_be_bytes([*hi, *lo])), rest), + (false, [length, rest @ ..]) => (usize::from(*length), rest), + _ => return Err(NotAnAttribute::NoHeader), + }; + let Some((value, rest)) = rest.split_at_checked(length) else { + return Err(NotAnAttribute::Truncated { + type_code: *type_code, + length, + available: rest.len(), + }); + }; + let raw = RawAttribute { + flags, + type_code: *type_code, + value, + wire: &buf[..buf.len() - rest.len()], + }; + Ok((raw, rest)) +} + impl PathAttribute { - fn discriminant(&self) -> u8 { + pub fn type_code(&self) -> u8 { + use attr_type::*; match self { - PathAttribute::Origin(_) => PathAttributeDiscriminants::Origin as u8, - PathAttribute::AsPath(_) => PathAttributeDiscriminants::AsPath as u8, - PathAttribute::NextHop(_) => PathAttributeDiscriminants::NextHop as u8, - PathAttribute::MultiExitDisc(_) => PathAttributeDiscriminants::MultiExitDisc as u8, - PathAttribute::LocalPref(_) => PathAttributeDiscriminants::LocalPref as u8, - PathAttribute::AtomicAggregate(_) => PathAttributeDiscriminants::AtomicAggregate as u8, - PathAttribute::Aggregator(_) => PathAttributeDiscriminants::Aggregator as u8, - PathAttribute::Communities(_) => PathAttributeDiscriminants::Communities as u8, - PathAttribute::MpReachNlri(_) => PathAttributeDiscriminants::MpReachNlri as u8, - PathAttribute::MpUnreachNlri(_) => PathAttributeDiscriminants::MpUnreachNlri as u8, - PathAttribute::ExtendedCommunities(_) => { - PathAttributeDiscriminants::ExtendedCommunities as u8 - } - PathAttribute::As4Path(_) => PathAttributeDiscriminants::As4Path as u8, - PathAttribute::LargeCommunities(_) => { - PathAttributeDiscriminants::LargeCommunities as u8 - } - PathAttribute::UnknownPathAttribute { type_code, .. } => *type_code, + Self::Origin(_) => ORIGIN, + Self::AsPath(_) => AS_PATH, + Self::NextHop(_) => NEXT_HOP, + Self::MultiExitDisc(_) => MULTI_EXIT_DISC, + Self::LocalPref(_) => LOCAL_PREF, + Self::AtomicAggregate(_) => ATOMIC_AGGREGATE, + Self::Aggregator(_) => AGGREGATOR, + Self::Communities(_) => COMMUNITIES, + Self::MpReachNlri(_) => MP_REACH_NLRI, + Self::MpUnreachNlri(_) => MP_UNREACH_NLRI, + Self::ExtendedCommunities(_) => EXTENDED_COMMUNITIES, + Self::As4Path(_) => AS4_PATH, + Self::LargeCommunities(_) => LARGE_COMMUNITIES, + Self::UnknownPathAttribute { type_code, .. } => *type_code, } } - /// Whether the Optional and Transitive flags are set on this attribute. - fn optional_and_transitive(&self) -> (bool, bool) { + /// The flags to send the attribute with, but for Extended Length, which + /// depends on how long the value turns out to be. + fn flags(&self) -> PathAttributeFlags { match self { - PathAttribute::UnknownPathAttribute { flags, .. } => { - (flags.optional(), flags.transitive()) + // As they came: the Partial bit has to survive (RFC 4271 §5). + Self::UnknownPathAttribute { flags, .. } => flags.clone(), + known => { + let (optional, transitive) = defined_flags(known.type_code()) + .expect("every attribute with a variant has defined flags"); + PathAttributeFlags::new(optional, transitive, false, false) } - known => defined_flags(known.discriminant()) - .expect("every attribute with a variant has defined flags"), } } - /// The from_wire parser for `PathAttribute` consumes type and length which it uses to - /// determine how many bytes to take and pass down to the corresponding sub-parser. + /// Parses one attribute, header and value, from the front of `buf`. + /// The flags are not checked against the attribute's definition; + /// [`UpdateMessage::decode`] does that for attributes off a session. pub fn from_wire<'a>( ctx: &ParserContext, buf: &'a [u8], ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, attr_flags) = be_u8(buf).map(|(buf, b)| (buf, PathAttributeFlags(b)))?; - let (buf, type_code) = be_u8(buf)?; - - let (buf, length): (_, u16) = if attr_flags.extended_length() { - be_u16(buf)? - } else { - be_u8(buf).map(|(buf, b)| (buf, b as u16))? - }; - - let discriminant = PathAttributeDiscriminants::from(type_code); - if matches!( - discriminant, - PathAttributeDiscriminants::UnknownPathAttribute - ) { - let (buf, pa_buf) = nom::bytes::take(length).parse(buf)?; - return Ok(( - buf, - PathAttribute::UnknownPathAttribute { - flags: attr_flags, - type_code, - payload: pa_buf.to_vec(), - }, - )); - } - Self::parse_known_path_attribute(ctx, buf, discriminant, length) + let (raw, rest) = + split_attribute(buf).map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; + let attribute = Self::parse_value(ctx, raw.flags, raw.type_code, raw.value)?; + Ok((rest, attribute)) } - fn parse_known_path_attribute<'a>( - ctx: &ParserContext, - buf: &'a [u8], - discriminant: PathAttributeDiscriminants, - length: u16, - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, pa_buf) = nom::bytes::take(length).parse(buf)?; - Ok((buf, Self::parse_value(ctx, discriminant, pa_buf)?)) - } - - /// Parses the value of an attribute with a variant. `pa_buf` is exactly - /// the value, and all of it has to be used. + /// Makes the attribute with this type code from its value. `value` is + /// exactly the value, and all of it has to be used. A type code without + /// a variant of its own is kept as bytes. pub(crate) fn parse_value<'a>( ctx: &ParserContext, - discriminant: PathAttributeDiscriminants, - pa_buf: &'a [u8], + flags: PathAttributeFlags, + type_code: u8, + value: &'a [u8], ) -> Result>> { - let attr: PathAttribute = match discriminant { - PathAttributeDiscriminants::Origin => { - PathAttribute::Origin(whole(OriginPathAttribute::from_wire(ctx, pa_buf)?)?) + use attr_type::*; + Ok(match type_code { + ORIGIN => Self::Origin(whole(OriginPathAttribute::from_wire(value)?)?), + AS_PATH => Self::AsPath(whole(AsPathAttribute::from_wire(ctx, value)?)?), + NEXT_HOP => Self::NextHop(whole(NextHopPathAttribute::from_wire(value)?)?), + MULTI_EXIT_DISC => { + Self::MultiExitDisc(whole(MultiExitDiscPathAttribute::from_wire(value)?)?) } - PathAttributeDiscriminants::AsPath => { - PathAttribute::AsPath(whole(AsPathAttribute::from_wire(ctx, pa_buf)?)?) + LOCAL_PREF => Self::LocalPref(whole(LocalPrefPathAttribute::from_wire(value)?)?), + ATOMIC_AGGREGATE => { + Self::AtomicAggregate(whole((value, AtomicAggregatePathAttribute {}))?) } - PathAttributeDiscriminants::NextHop => { - PathAttribute::NextHop(whole(NextHopPathAttribute::from_wire(ctx, pa_buf)?)?) + AGGREGATOR => Self::Aggregator(whole(AggregatorPathAttribute::from_wire(ctx, value)?)?), + COMMUNITIES => Self::Communities(whole(CommunitiesPathAttribute::from_wire(value)?)?), + MP_REACH_NLRI => { + Self::MpReachNlri(whole(MpReachNlriPathAttribute::from_wire(ctx, value)?)?) } - PathAttributeDiscriminants::MultiExitDisc => PathAttribute::MultiExitDisc(whole( - MultiExitDiscPathAttribute::from_wire(ctx, pa_buf)?, + MP_UNREACH_NLRI => { + Self::MpUnreachNlri(whole(MpUnreachNlriPathAttribute::from_wire(ctx, value)?)?) + } + EXTENDED_COMMUNITIES => Self::ExtendedCommunities(whole( + ExtendedCommunitiesPathAttribute::from_wire(value)?, )?), - PathAttributeDiscriminants::LocalPref => { - PathAttribute::LocalPref(whole(LocalPrefPathAttribute::from_wire(ctx, pa_buf)?)?) + // Always four octets wide, whatever the session's AS_PATH is. + AS4_PATH => Self::As4Path(whole(AsPathAttribute::from_wire_with_width(true, value)?)?), + LARGE_COMMUNITIES => { + Self::LargeCommunities(whole(LargeCommunitiesPathAttribute::from_wire(value)?)?) } - PathAttributeDiscriminants::AtomicAggregate => PathAttribute::AtomicAggregate(whole( - AtomicAggregatePathAttribute::from_wire(ctx, pa_buf)?, - )?), - PathAttributeDiscriminants::Aggregator => { - PathAttribute::Aggregator(whole(AggregatorPathAttribute::from_wire(ctx, pa_buf)?)?) - } - PathAttributeDiscriminants::Communities => PathAttribute::Communities(whole( - CommunitiesPathAttribute::from_wire(ctx, pa_buf)?, - )?), - PathAttributeDiscriminants::MpReachNlri => PathAttribute::MpReachNlri(whole( - MpReachNlriPathAttribute::from_wire(ctx, pa_buf)?, - )?), - PathAttributeDiscriminants::MpUnreachNlri => PathAttribute::MpUnreachNlri(whole( - MpUnreachNlriPathAttribute::from_wire(ctx, pa_buf)?, - )?), - PathAttributeDiscriminants::ExtendedCommunities => PathAttribute::ExtendedCommunities( - whole(ExtendedCommunitiesPathAttribute::from_wire(ctx, pa_buf)?)?, - ), - PathAttributeDiscriminants::As4Path => { - PathAttribute::As4Path(whole(AsPathAttribute::from_wire_with_width(true, pa_buf)?)?) - } - PathAttributeDiscriminants::LargeCommunities => PathAttribute::LargeCommunities(whole( - LargeCommunitiesPathAttribute::from_wire(ctx, pa_buf)?, - )?), - PathAttributeDiscriminants::UnknownPathAttribute => { - return Err(Failure(BgpParserError::Malformed( - "an unknown attribute has no value parser".into(), - ))); - } - }; - Ok(attr) - } - - pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - macro_rules! write_path_attribute { - ($out: expr, $attribute: expr) => { - let wire_len = $attribute.wire_len(ctx)?; - let (optional, transitive) = self.optional_and_transitive(); - let attr_flags = PathAttributeFlags::new( - optional, - transitive, - /*partial=*/ false, - wire_len > 255, - ); - out.put_u8(*attr_flags); - out.put_u8(self.discriminant() as u8); - write_wire_len!(out, wire_len); - $attribute.to_wire(ctx, out)?; - }; - } - - macro_rules! write_wire_len { - ($out: expr, $wire_len: expr) => { - if $wire_len > 255 { - $out.put_u16($wire_len as u16); - } else { - $out.put_u8($wire_len as u8); - } - }; - } - - match self { - PathAttribute::Origin(origin_path_attribute) => { - write_path_attribute!(out, origin_path_attribute); - } - PathAttribute::AsPath(as_path_attribute) => { - write_path_attribute!(out, as_path_attribute); - } - PathAttribute::NextHop(next_hop_path_attribute) => { - write_path_attribute!(out, next_hop_path_attribute); - } - PathAttribute::MultiExitDisc(multi_exit_disc_path_attribute) => { - write_path_attribute!(out, multi_exit_disc_path_attribute); - } - PathAttribute::LocalPref(local_pref_path_attribute) => { - write_path_attribute!(out, local_pref_path_attribute); - } - PathAttribute::AtomicAggregate(atomic_aggregate_path_attribute) => { - write_path_attribute!(out, atomic_aggregate_path_attribute); - } - PathAttribute::Aggregator(aggregator_path_attribute) => { - write_path_attribute!(out, aggregator_path_attribute); - } - PathAttribute::Communities(communities_path_attribute) => { - write_path_attribute!(out, communities_path_attribute); - } - PathAttribute::MpReachNlri(mp_reach_nlri_path_attribute) => { - write_path_attribute!(out, mp_reach_nlri_path_attribute); - } - PathAttribute::MpUnreachNlri(mp_unreach_nlri_path_attribute) => { - write_path_attribute!(out, mp_unreach_nlri_path_attribute); - } - PathAttribute::ExtendedCommunities(extended_communities_path_attribute) => { - write_path_attribute!(out, extended_communities_path_attribute); - } - PathAttribute::LargeCommunities(large_communities_path_attribute) => { - write_path_attribute!(out, large_communities_path_attribute); - } - PathAttribute::As4Path(as4_path) => { - // Always four octets wide, whatever the session's AS_PATH is. - let wire_len = as4_path.wire_len_with_width(true)?; - let attr_flags = - PathAttributeFlags::new(true, true, /*partial=*/ false, wire_len > 255); - out.put_u8(*attr_flags); - out.put_u8(self.discriminant()); - write_wire_len!(out, wire_len); - as4_path.to_wire_with_width(true, out)?; - } - PathAttribute::UnknownPathAttribute { + _ => Self::UnknownPathAttribute { flags, type_code, - payload, - } => { - // The flags are sent as they came, except that a payload too - // long for a one-octet length gets the two-octet one. - let length = u16::try_from(payload.len()).map_err(|_| EncodeError::TooLong { - what: "path attribute", - len: payload.len(), - max: u16::MAX.into(), - })?; - let mut flags = flags.clone(); - if length > 255 { - flags.set_extended_length(true); - } - out.put_u8(flags.0); - out.put_u8(*type_code); - if flags.extended_length() { - out.put_u16(length); - } else { - out.put_u8(length as u8); - } - out.put(payload.as_slice()); - } - } + payload: value.to_vec(), + }, + }) + } + /// Writes the attribute, header and value, to the end of `out`. + pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + // A header with a one-octet length first; whether the value needs + // the two-octet one is known once it is written. + let start = out.len(); + let mut flags = self.flags(); + out.put_u8(0); + out.put_u8(self.type_code()); + out.put_u8(0); + let value_start = out.len(); + self.value_to_wire(ctx, out)?; + let length = out.len() - value_start; + + if let Ok(short) = u8::try_from(length) + && !flags.extended_length() + { + out[start + 2] = short; + } else { + let long = u16::try_from(length).map_err(|_| EncodeError::TooLong { + what: "path attribute", + len: length, + max: u16::MAX.into(), + })?; + flags.set_extended_length(true); + // Move the value up by one octet to make room. + out.put_u8(0); + out.copy_within(value_start..value_start + length, value_start + 1); + out[start + 2..start + 4].copy_from_slice(&long.to_be_bytes()); + } + out[start] = flags.0; Ok(()) } - /// Returns the total on-wire length of this path attribute, including the - /// attribute header (flags + type + length) and the payload. - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - let payload_len: u16 = match self { - PathAttribute::Origin(a) => a.wire_len(ctx)?, - PathAttribute::AsPath(a) => a.wire_len(ctx)?, - PathAttribute::NextHop(a) => a.wire_len(ctx)?, - PathAttribute::MultiExitDisc(a) => a.wire_len(ctx)?, - PathAttribute::LocalPref(a) => a.wire_len(ctx)?, - PathAttribute::AtomicAggregate(a) => a.wire_len(ctx)?, - PathAttribute::Aggregator(a) => a.wire_len(ctx)?, - PathAttribute::Communities(a) => a.wire_len(ctx)?, - PathAttribute::MpReachNlri(a) => a.wire_len(ctx)?, - PathAttribute::MpUnreachNlri(a) => a.wire_len(ctx)?, - PathAttribute::ExtendedCommunities(a) => a.wire_len(ctx)?, - PathAttribute::As4Path(a) => a.wire_len_with_width(true)?, - PathAttribute::LargeCommunities(a) => a.wire_len(ctx)?, - PathAttribute::UnknownPathAttribute { flags, payload, .. } => { - // An unknown attribute keeps the length width it came with, - // unless its payload needs the wider one. - let payload_len = - u16::try_from(payload.len()).map_err(|_| EncodeError::TooLong { - what: "path attribute", - len: payload.len(), - max: u16::MAX.into(), - })?; - let extended = flags.extended_length() || payload_len > 255; - let header_len: u16 = if extended { 4 } else { 3 }; - return Ok(header_len + payload_len); - } - }; - let header_len: u16 = if payload_len > u8::MAX as u16 { 4 } else { 3 }; - Ok(header_len + payload_len) + fn value_to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + match self { + Self::Origin(a) => a.to_wire(out), + Self::AsPath(a) => a.to_wire(ctx, out)?, + Self::NextHop(a) => a.to_wire(out), + Self::MultiExitDisc(a) => a.to_wire(out), + Self::LocalPref(a) => a.to_wire(out), + Self::AtomicAggregate(_) => {} + Self::Aggregator(a) => a.to_wire(ctx, out), + Self::Communities(a) => a.to_wire(out), + Self::MpReachNlri(a) => a.to_wire(ctx, out), + Self::MpUnreachNlri(a) => a.to_wire(ctx, out), + Self::ExtendedCommunities(a) => a.to_wire(out), + Self::As4Path(a) => a.to_wire_with_width(true, out)?, + Self::LargeCommunities(a) => a.to_wire(out), + Self::UnknownPathAttribute { payload, .. } => out.put_slice(payload), + } + Ok(()) } } -/// Origin path attribute is a mandatory attribute defined in RFC4271. -#[derive(Debug, PartialEq, Eq, Copy, Clone, Serialize_repr, Deserialize_repr)] +type Parsed<'a, T> = IResult<&'a [u8], T, BgpParserError<&'a [u8]>>; + +fn malformed<'a>(detail: String) -> nom::Err> { + Failure(BgpParserError::Malformed(detail)) +} + +/// ORIGIN: how the route came into BGP (RFC 4271 §5.1.1). +#[derive(Debug, PartialEq, Eq, Copy, Clone, Serialize_repr)] #[repr(u8)] pub enum OriginPathAttribute { IGP = 0, @@ -777,66 +693,34 @@ pub enum OriginPathAttribute { } impl OriginPathAttribute { - pub fn from_wire<'a>( - _: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { let (buf, byte) = be_u8(buf)?; - let attr = match byte { - b if b == OriginPathAttribute::IGP as u8 => OriginPathAttribute::IGP, - b if b == OriginPathAttribute::EGP as u8 => OriginPathAttribute::EGP, - b if b == OriginPathAttribute::INCOMPLETE as u8 => OriginPathAttribute::INCOMPLETE, - _ => { - return IResult::Err(nom::Err::Failure(BgpParserError::Malformed(format!( - "Unknown Origin type {}", - byte - )))); - } + let origin = match byte { + 0 => Self::IGP, + 1 => Self::EGP, + 2 => Self::INCOMPLETE, + other => return Err(malformed(format!("unknown ORIGIN {other}"))), }; - Ok((buf, attr)) + Ok((buf, origin)) } - pub fn wire_len(&self, _ctx: &ParserContext) -> Result { - Ok(1) - } - - pub fn to_wire(&self, _ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + pub fn to_wire(&self, out: &mut BytesMut) { out.put_u8(*self as u8); - Ok(()) } } -impl TryFrom for OriginPathAttribute { - type Error = InvalidValue; - - fn try_from(value: u8) -> Result { - match value { - 0 => Ok(Self::IGP), - 1 => Ok(Self::EGP), - 2 => Ok(Self::INCOMPLETE), - other => Err(InvalidValue { - what: "ORIGIN", - value: other.into(), - }), - } - } -} - -/// ASPathAttribute is a well-known mandatory attribute that contains a list of TLV encoded path -/// segments. Type is either 1 for AS_SET or 2 for AS_SEQUENCE, length is a 1 octet field -/// containing the number of ASNS and the value contains the ASNs. This is defined in Section 4.3 -/// of RFC4271. -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +/// AS_PATH: the ASes a route has passed through, as a list of segments +/// (RFC 4271 §4.3). Each segment is a type (1 for AS_SET, 2 for +/// AS_SEQUENCE), a count in one octet, and that many AS numbers. +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct AsPathAttribute { pub segments: Vec, } -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct AsPathSegment { - /// ordered is true when representing an AS_SEQUENCE, andd false when - /// representing an AS_SET. + /// True for an AS_SEQUENCE, false for an AS_SET. pub ordered: bool, - /// Path is the list of ASNs. pub path: Vec, } @@ -851,39 +735,26 @@ impl AsPathAttribute { }) } - /// Parses an AS_PATH. How wide its AS numbers are depends on what the - /// session negotiated (RFC 6793), so the context must say. - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let Some(four_octet) = ctx.four_octet_asn else { - return Err(nom::Err::Failure(BgpParserError::Malformed( - "Context must set four_octet_asn before being used".into(), - ))); - }; - Self::from_wire_with_width(four_octet, buf) + /// Parses an AS_PATH with AS numbers as wide as the session negotiated + /// (RFC 6793). + pub fn from_wire<'a>(ctx: &ParserContext, buf: &'a [u8]) -> Parsed<'a, Self> { + Self::from_wire_with_width(ctx.four_octet_asn, buf) } - fn from_wire_with_width( - four_octet: bool, - buf: &[u8], - ) -> IResult<&[u8], Self, BgpParserError<&[u8]>> { - let parse_segment = |buf| -> IResult<&[u8], AsPathSegment, BgpParserError<&[u8]>> { + fn from_wire_with_width(four_octet: bool, buf: &[u8]) -> Parsed<'_, Self> { + let parse_segment = |buf| -> Parsed<'_, AsPathSegment> { let (buf, typ) = be_u8(buf)?; let (buf, len) = be_u8(buf)?; // AS_SET and AS_SEQUENCE. The confederation segment types of // RFC 5065 are an error from a peer outside a confederation, // and this speaker is never inside one. if typ != 1 && typ != 2 { - return Err(Failure(BgpParserError::Malformed(format!( + return Err(malformed(format!( "unrecognised AS path segment type {typ}" - )))); + ))); } if len == 0 { - return Err(Failure(BgpParserError::Malformed( - "AS path segment with no AS numbers".into(), - ))); + return Err(malformed("AS path segment with no AS numbers".into())); } let count = usize::from(len); let (buf, path): (_, Vec) = if four_octet { @@ -919,25 +790,18 @@ impl AsPathAttribute { /// AS_TRANS (RFC 6793 §4.2.2); [`AsPathAttribute::as4_path`] is what /// carries the real numbers alongside. pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - self.to_wire_with_width(width(ctx)?, out) + self.to_wire_with_width(ctx.four_octet_asn, out) } fn to_wire_with_width(&self, four_octet: bool, out: &mut BytesMut) -> Result<(), EncodeError> { for segment in &self.segments { - // Segment type. out.put_u8(if segment.ordered { 2 } else { 1 }); - // Segment AS length. - out.put_u8( - segment - .path - .len() - .try_into() - .map_err(|_| EncodeError::TooLong { - what: "AS path segment", - len: segment.path.len(), - max: u8::MAX.into(), - })?, - ); + let count = u8::try_from(segment.path.len()).map_err(|_| EncodeError::TooLong { + what: "AS path segment", + len: segment.path.len(), + max: u8::MAX.into(), + })?; + out.put_u8(count); for asn in &segment.path { if four_octet { out.put_u32(*asn); @@ -949,24 +813,6 @@ impl AsPathAttribute { Ok(()) } - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - self.wire_len_with_width(width(ctx)?) - } - - fn wire_len_with_width(&self, four_octet: bool) -> Result { - let asn_len = if four_octet { 4 } else { 2 }; - let total: usize = self - .segments - .iter() - .map(|segment| 2 + asn_len * segment.path.len()) - .sum(); - u16::try_from(total).map_err(|_| EncodeError::TooLong { - what: "AS path", - len: total, - max: u16::MAX.into(), - }) - } - /// The AS4_PATH to send with this path on a two-octet session, if one is /// needed: it is when some AS number does not fit in two octets and so /// goes out as AS_TRANS in the AS_PATH itself (RFC 6793 §4.2.2). @@ -979,100 +825,53 @@ impl AsPathAttribute { } } -/// How wide AS numbers are on the session `ctx` describes. -fn width(ctx: &ParserContext) -> Result { - ctx.four_octet_asn.ok_or(EncodeError::Unencodable( - "the AS number width has not been negotiated", - )) -} - -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct NextHopPathAttribute(pub Ipv4Addr); impl NextHopPathAttribute { - pub fn from_wire<'a>( - _: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, ip_u32) = be_u32(buf)?; - Ok((buf, Self(Ipv4Addr::from(ip_u32)))) + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { + let (buf, address) = be_u32(buf)?; + Ok((buf, Self(Ipv4Addr::from(address)))) } - pub fn to_wire(&self, _ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + pub fn to_wire(&self, out: &mut BytesMut) { out.put_u32(self.0.into()); - Ok(()) - } - - pub fn wire_len(&self, _: &ParserContext) -> Result { - Ok(4) } } -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct MultiExitDiscPathAttribute(pub u32); impl MultiExitDiscPathAttribute { - pub fn from_wire<'a>( - _: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, val) = be_u32(buf)?; - Ok((buf, Self(val))) + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { + let (buf, value) = be_u32(buf)?; + Ok((buf, Self(value))) } - pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + pub fn to_wire(&self, out: &mut BytesMut) { out.put_u32(self.0); - Ok(()) - } - - pub fn wire_len(&self, _: &ParserContext) -> Result { - Ok(4) } } -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct LocalPrefPathAttribute(pub u32); impl LocalPrefPathAttribute { - pub fn from_wire<'a>( - _: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, val) = be_u32(buf)?; - Ok((buf, Self(val))) + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { + let (buf, value) = be_u32(buf)?; + Ok((buf, Self(value))) } - pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + pub fn to_wire(&self, out: &mut BytesMut) { out.put_u32(self.0); - Ok(()) - } - - pub fn wire_len(&self, _: &ParserContext) -> Result { - Ok(4) } } -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +/// ATOMIC_AGGREGATE: a marker with no value (RFC 4271 §5.1.6). +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct AtomicAggregatePathAttribute {} -impl AtomicAggregatePathAttribute { - pub fn from_wire<'a>( - _: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - Ok((buf, Self {})) - } - - pub fn to_wire(&self, _: &ParserContext, _: &mut BytesMut) -> Result<(), EncodeError> { - Ok(()) - } - - pub fn wire_len(&self, _: &ParserContext) -> Result { - Ok(0) - } -} - -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct AggregatorPathAttribute { pub asn: u32, pub ip: Ipv4Addr, @@ -1081,18 +880,11 @@ pub struct AggregatorPathAttribute { impl AggregatorPathAttribute { /// The AS number is as wide as the session's AS numbers (RFC 6793), so /// the attribute is six or eight octets. - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, asn) = match ctx.four_octet_asn { - Some(true) => be_u32(buf)?, - Some(false) => be_u16(buf).map(|(buf, asn)| (buf, u32::from(asn)))?, - None => { - return Err(nom::Err::Failure(BgpParserError::Malformed( - "Context must set four_octet_asn before being used".into(), - ))); - } + pub fn from_wire<'a>(ctx: &ParserContext, buf: &'a [u8]) -> Parsed<'a, Self> { + let (buf, asn) = if ctx.four_octet_asn { + be_u32(buf)? + } else { + be_u16(buf).map(|(buf, asn)| (buf, u32::from(asn)))? }; let (buf, ip) = be_u32(buf)?; Ok(( @@ -1106,299 +898,100 @@ impl AggregatorPathAttribute { /// On a two-octet session an AS number that does not fit is written as /// AS_TRANS (RFC 6793 §4.2.2). - pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - if width(ctx)? { + pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) { + if ctx.four_octet_asn { out.put_u32(self.asn); } else { out.put_u16(u16::try_from(self.asn).unwrap_or(AS_TRANS)); } out.put_u32(self.ip.into()); - Ok(()) - } - - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - Ok(if width(ctx)? { 8 } else { 6 }) } } -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct CommunitiesPathAttribute(pub Vec<(u16, u16)>); impl CommunitiesPathAttribute { - pub fn from_wire<'a>( - _: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { // At least one: an empty COMMUNITIES is malformed (RFC 7606 §7.8). let (buf, values) = nom::multi::many1(|i| (be_u16, be_u16).parse(i)).parse(buf)?; Ok((buf, CommunitiesPathAttribute(values))) } - pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + pub fn to_wire(&self, out: &mut BytesMut) { for value in &self.0 { out.put_u16(value.0); out.put_u16(value.1); } - Ok(()) - } - - pub fn wire_len(&self, _: &ParserContext) -> Result { - Ok((self.0.len() * 4) as u16) } } -/// Extended Communities as defined in https://www.rfc-editor.org/rfc/rfc4360.html. -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +/// EXTENDED COMMUNITIES (RFC 4360). +#[derive(Debug, PartialEq, Eq, Clone, Serialize)] pub struct ExtendedCommunitiesPathAttribute { pub extended_communities: Vec, } impl ExtendedCommunitiesPathAttribute { - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, extended_communities) = - nom::multi::many1(|buf| ExtendedCommunity::from_wire(ctx, buf)).parse(buf)?; + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { + let (communities, rest) = buf.as_chunks::<8>(); + if communities.is_empty() || !rest.is_empty() { + return Err(malformed(format!( + "{} octets are not a list of extended communities", + buf.len() + ))); + } Ok(( - buf, + rest, Self { - extended_communities, + extended_communities: communities.iter().copied().map(ExtendedCommunity).collect(), }, )) } - pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - for ec in &self.extended_communities { - ec.to_wire(ctx, out)?; + pub fn to_wire(&self, out: &mut BytesMut) { + for community in &self.extended_communities { + out.put_slice(&community.0); } - Ok(()) - } - - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - self.extended_communities - .iter() - .map(|ec| ec.wire_len(ctx)) - .sum::>() } } -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] -pub enum ExtendedCommunity { - /// AS Specific Extended Community as specified in Section 3.1 of RFC4360. - AsSpecific { - typ: u8, - sub_typ: u8, - global_admin: u16, - local_admin: u32, - }, - /// Ipv4 Address Specific Extended Community as specified in Section 3.2 of RFC4360. - Ipv4AddrSpecific { - typ: u8, - sub_typ: u8, - global_admin: u32, - local_admin: u16, - }, - /// Opaque Extended Community as specified in Section 3.3 of RFC4360. - Opaque { - typ: u8, - sub_typ: u8, - /// The six octets after the type and sub-type. - value: [u8; 6], - }, - /// Route Target Community as specified in Section 4 of RFC4360. - RouteTarget { - typ: u8, - sub_typ: u8, - global_admin: u32, - local_admin: u16, - }, - /// Route Origin Community as specified in Section 5 of RFC4360. - RouteOrigin { - typ: u8, - sub_typ: u8, - global_admin: u16, - local_admin: u32, - }, -} +/// One extended community: eight octets, of which the first is the type +/// and, for most types, the second a sub-type (RFC 4360 §2). How the rest +/// divides into an administrator and a value depends on the type, and is +/// left to whoever acts on a particular community. +#[derive(Debug, PartialEq, Eq, Clone, Copy, Serialize)] +pub struct ExtendedCommunity(pub [u8; 8]); impl ExtendedCommunity { - pub fn from_wire<'a>( - _: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, typ) = be_u8(buf)?; - let (buf, sub_typ) = be_u8(buf)?; - let (buf, parsed) = match (typ, sub_typ) { - // Route Target Extended Community. - (0x00..=0x02, 0x02) => { - let (buf, global_admin) = be_u32(buf)?; - let (buf, local_admin) = be_u16(buf)?; - ( - buf, - Self::RouteTarget { - typ, - sub_typ, - global_admin, - local_admin, - }, - ) - } - // Route Origin Extended Community. - (0x00..=0x02, 0x03) => { - let (buf, global_admin) = be_u16(buf)?; - let (buf, local_admin) = be_u32(buf)?; - ( - buf, - Self::RouteOrigin { - typ, - sub_typ, - global_admin, - local_admin, - }, - ) - } - // AS specific Extended Community. - (0x00 | 0x40, _) => { - let (buf, global_admin) = be_u16(buf)?; - let (buf, local_admin) = be_u32(buf)?; - ( - buf, - Self::AsSpecific { - typ, - sub_typ, - global_admin, - local_admin, - }, - ) - } - // IPv4 Address Specific Extended Community. - (0x01 | 0x41, _) => { - let (buf, global_admin) = be_u32(buf)?; - let (buf, local_admin) = be_u16(buf)?; - ( - buf, - Self::Ipv4AddrSpecific { - typ, - sub_typ, - global_admin, - local_admin, - }, - ) - } - _ => { - let (buf, payload) = nom::bytes::take(6_usize).parse(buf)?; - let value: [u8; 6] = payload.try_into().map_err(|_| { - Failure(BgpParserError::Malformed( - "Expected exactly 6 bytes from the parser".into(), - )) - })?; - ( - buf, - Self::Opaque { - typ, - sub_typ, - value, - }, - ) - } - }; - - Ok((buf, parsed)) + pub fn typ(&self) -> u8 { + self.0[0] } - pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - match self { - ExtendedCommunity::AsSpecific { - typ, - sub_typ, - global_admin, - local_admin, - } => { - out.put_u8(*typ); - out.put_u8(*sub_typ); - out.put_u16(*global_admin); - out.put_u32(*local_admin); - } - ExtendedCommunity::Ipv4AddrSpecific { - typ, - sub_typ, - global_admin, - local_admin, - } => { - out.put_u8(*typ); - out.put_u8(*sub_typ); - out.put_u32(*global_admin); - out.put_u16(*local_admin); - } - ExtendedCommunity::Opaque { - typ, - sub_typ, - value, - } => { - out.put_u8(*typ); - out.put_u8(*sub_typ); - out.put(&value[..]); - } - ExtendedCommunity::RouteTarget { - typ, - sub_typ, - global_admin, - local_admin, - } => { - out.put_u8(*typ); - out.put_u8(*sub_typ); - out.put_u32(*global_admin); - out.put_u16(*local_admin); - } - ExtendedCommunity::RouteOrigin { - typ, - sub_typ, - global_admin, - local_admin, - } => { - out.put_u8(*typ); - out.put_u8(*sub_typ); - out.put_u16(*global_admin); - out.put_u32(*local_admin); - } - } - Ok(()) - } - - pub fn wire_len(&self, _: &ParserContext) -> Result { - Ok(8) + pub fn sub_typ(&self) -> u8 { + self.0[1] } } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct LargeCommunitiesPathAttribute { pub communities: Vec, } impl LargeCommunitiesPathAttribute { - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, communities) = - nom::multi::many1(|buf| LargeCommunity::from_wire(ctx, buf)).parse(buf)?; + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { + let (buf, communities) = nom::multi::many1(LargeCommunity::from_wire).parse(buf)?; Ok((buf, Self { communities })) } - pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + pub fn to_wire(&self, out: &mut BytesMut) { for community in &self.communities { - community.to_wire(ctx, out)?; + community.to_wire(out); } - Ok(()) - } - - pub fn wire_len(&self, _: &ParserContext) -> Result { - Ok(12_u16 * self.communities.len() as u16) } } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct LargeCommunity { pub global_admin: u32, pub data_1: u32, @@ -1406,10 +999,7 @@ pub struct LargeCommunity { } impl LargeCommunity { - pub fn from_wire<'a>( - _: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { let (buf, global_admin) = be_u32(buf)?; let (buf, data_1) = be_u32(buf)?; let (buf, data_2) = be_u32(buf)?; @@ -1423,25 +1013,20 @@ impl LargeCommunity { )) } - pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + pub fn to_wire(&self, out: &mut BytesMut) { out.put_u32(self.global_admin); out.put_u32(self.data_1); out.put_u32(self.data_2); - Ok(()) - } - - pub fn wire_len(&self, _: &ParserContext) -> Result { - Ok(12) } } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +/// The next hop of routes in MP_REACH_NLRI. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub enum NlriNextHop { - /// Represents an IPv4 address nexthop. Ipv4(Ipv4Addr), - /// Represents a global IPv6 nexthop. + /// A global IPv6 address. Ipv6(Ipv6Addr), - /// Represents a IPv6 Link Local and global address pair nexthop. + /// A global IPv6 address and a link-local one (RFC 2545 §3). Ipv6WithLl { global: Ipv6Addr, link_local: Ipv6Addr, @@ -1461,19 +1046,35 @@ impl std::fmt::Display for NlriNextHop { } impl NlriNextHop { - pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - match self { - NlriNextHop::Ipv4(ipv4_addr) => out.put(&ipv4_addr.octets()[..]), - NlriNextHop::Ipv6(ipv6_addr) => out.put(&ipv6_addr.octets()[..]), - NlriNextHop::Ipv6WithLl { global, link_local } => { - out.put(&global.octets()[..]); - out.put(&link_local.octets()[..]) - } + /// Reads a next hop that is all of `bytes`. `None` if they are not the + /// length of one. + pub fn from_bytes(bytes: &[u8]) -> Option { + let v6 = |bytes: &[u8]| <[u8; 16]>::try_from(bytes).ok().map(Ipv6Addr::from); + match bytes.len() { + 4 => <[u8; 4]>::try_from(bytes) + .ok() + .map(|a| Self::Ipv4(a.into())), + 16 => v6(bytes).map(Self::Ipv6), + 32 => Some(Self::Ipv6WithLl { + global: v6(&bytes[..16])?, + link_local: v6(&bytes[16..])?, + }), + _ => None, } - - Ok(()) } + pub fn to_wire(&self, out: &mut BytesMut) { + match self { + NlriNextHop::Ipv4(address) => out.put_slice(&address.octets()), + NlriNextHop::Ipv6(address) => out.put_slice(&address.octets()), + NlriNextHop::Ipv6WithLl { global, link_local } => { + out.put_slice(&global.octets()); + out.put_slice(&link_local.octets()); + } + } + } + + /// How many octets the next hop is on the wire. pub fn wire_len(&self) -> u8 { match self { NlriNextHop::Ipv4(_) => 4, @@ -1485,30 +1086,26 @@ impl NlriNextHop { /// Parses one prefix as an UPDATE names it: with a path identifier in /// front if the session uses them for the family (RFC 7911 §3). -pub(crate) fn parse_nlri( - afi: AddressFamilyId, - with_path_id: bool, - buf: &[u8], -) -> IResult<&[u8], Nlri, BgpParserError<&[u8]>> { +fn parse_nlri(version: IpVersion, with_path_id: bool, buf: &[u8]) -> Parsed<'_, Nlri> { let (buf, path_id) = if with_path_id { be_u32(buf)? } else { (buf, 0) }; let (buf, prefix_len) = be_u8(buf)?; - let byte_len = prefix_len.div_ceil(8); - let (buf, prefix_bytes) = nom::bytes::take(byte_len as usize).parse(buf)?; - let prefix = IpPrefix::new(afi, prefix_bytes, prefix_len) - .map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; + let byte_len = usize::from(prefix_len.div_ceil(8)); + let (buf, prefix_bytes) = nom::bytes::take(byte_len).parse(buf)?; + let prefix = + IpPrefix::new(version, prefix_bytes, prefix_len).map_err(|e| malformed(e.to_string()))?; Ok((buf, Nlri { prefix, path_id })) } /// Parses a field that is nothing but prefixes, to its end. -fn parse_nlri_list( - afi: AddressFamilyId, +pub(crate) fn parse_nlri_list( + version: IpVersion, with_path_id: bool, mut buf: &[u8], -) -> IResult<&[u8], Vec, BgpParserError<&[u8]>> { +) -> Parsed<'_, Vec> { // Not many0: that stops quietly at a prefix it cannot read. let mut nlri = Vec::new(); while !buf.is_empty() { - let (rest, parsed) = parse_nlri(afi, with_path_id, buf)?; + let (rest, parsed) = parse_nlri(version, with_path_id, buf)?; nlri.push(parsed); buf = rest; } @@ -1523,159 +1120,96 @@ fn parse_nlri_list( /// of a family that was not negotiated, so the others do not come up. fn ip_version_of_nlri<'a>( family: AfiSafi, -) -> Result>> { - let unsupported = || { - Failure(BgpParserError::Malformed(format!( - "NLRI of {family} is not supported" - ))) - }; +) -> Result>> { + let unsupported = || malformed(format!("NLRI of {family} is not supported")); if family.safi != Safi::UNICAST && family.safi != Safi::MULTICAST { return Err(unsupported()); } - AddressFamilyId::try_from(family.afi).map_err(|_| unsupported()) + IpVersion::try_from(family.afi).map_err(|_| unsupported()) } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +fn parse_family(buf: &[u8]) -> Parsed<'_, AfiSafi> { + let (buf, afi) = be_u16(buf)?; + let (buf, safi) = be_u8(buf)?; + Ok((buf, AfiSafi::new(Afi(afi), Safi(safi)))) +} + +/// MP_REACH_NLRI: routes of a family other than the one in the message +/// body, with their next hop (RFC 4760 §3). +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct MpReachNlriPathAttribute { - pub afi: Afi, - pub safi: Safi, - /// Next hop address (either IPv4 or IPv6 for now). + pub family: AfiSafi, pub next_hop: NlriNextHop, pub prefixes: Vec, } impl MpReachNlriPathAttribute { - pub fn family(&self) -> AfiSafi { - AfiSafi::new(self.afi, self.safi) - } - - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, afi) = be_u16(buf).map(|(buf, afi)| (buf, Afi(afi)))?; - let (buf, safi) = be_u8(buf).map(|(buf, safi)| (buf, Safi(safi)))?; - let family = AfiSafi::new(afi, safi); + pub fn from_wire<'a>(ctx: &ParserContext, buf: &'a [u8]) -> Parsed<'a, Self> { + let (buf, family) = parse_family(buf)?; let version = ip_version_of_nlri(family)?; - let with_path_id = ctx.has_add_path(family); - let (buf, nh_len) = be_u8(buf)?; - let (buf, nh_bytes) = nom::bytes::take(usize::from(nh_len)).parse(buf)?; - let address = |bytes: &[u8]| { - let mut octets = [0u8; 16]; - octets.copy_from_slice(bytes); - Ipv6Addr::from(octets) - }; - let next_hop = match (version, nh_bytes.len()) { - (AddressFamilyId::Ipv4, 4) => NlriNextHop::Ipv4(Ipv4Addr::new( - nh_bytes[0], - nh_bytes[1], - nh_bytes[2], - nh_bytes[3], - )), - (AddressFamilyId::Ipv4, other) => { - return Err(Failure(BgpParserError::Malformed(format!( - "Got nexthop address length {other} when expected 4 for IPv4 AFI" - )))); - } - // RFC 2545: a global address, optionally followed by a - // link-local one. - (AddressFamilyId::Ipv6, 16) => NlriNextHop::Ipv6(address(nh_bytes)), - (AddressFamilyId::Ipv6, 32) => NlriNextHop::Ipv6WithLl { - global: address(&nh_bytes[..16]), - link_local: address(&nh_bytes[16..]), - }, - (AddressFamilyId::Ipv6, other) => { - return Err(Failure(BgpParserError::Malformed(format!( - "Mismatched IPv6 nexthop length, got {other}, want 16 or 32" - )))); - } - }; + let (buf, next_hop_len) = be_u8(buf)?; + let (buf, next_hop) = nom::bytes::take(usize::from(next_hop_len)).parse(buf)?; + // An IPv4 address for IPv4 routes; for IPv6, a global address, + // optionally followed by a link-local one (RFC 2545). + let next_hop = NlriNextHop::from_bytes(next_hop) + .filter(|next_hop| { + matches!(next_hop, NlriNextHop::Ipv4(_)) == (version == IpVersion::Ipv4) + }) + .ok_or_else(|| { + malformed(format!( + "a next hop of {next_hop_len} octets is not valid for {family}" + )) + })?; - // Reserved byte (formerly the SNPA count), for every family. + // Reserved octet (formerly the SNPA count), for every family. let (buf, _) = be_u8(buf)?; - let (buf, prefixes) = parse_nlri_list(version, with_path_id, buf)?; - + let (buf, prefixes) = parse_nlri_list(version, ctx.has_add_path(family), buf)?; Ok(( buf, Self { - afi, - safi, + family, next_hop, prefixes, }, )) } - pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - out.put_u16(self.afi.0); - out.put_u8(self.safi.0); + pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) { + out.put_u16(self.family.afi.0); + out.put_u8(self.family.safi.0); out.put_u8(self.next_hop.wire_len()); - self.next_hop.to_wire(ctx, out)?; + self.next_hop.to_wire(out); out.put_u8(0); - write_nlri(&self.prefixes, ctx.has_add_path(self.family()), out); - Ok(()) - } - - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - // AFI, SAFI, next hop length, next hop, reserved byte, prefixes. - let total = 4 - + u32::from(self.next_hop.wire_len()) - + 1 - + nlri_wire_len(&self.prefixes, ctx.has_add_path(self.family())); - u16::try_from(total).map_err(|_| too_long("MP_REACH_NLRI", total)) + write_nlri(&self.prefixes, ctx.has_add_path(self.family), out); } } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +/// MP_UNREACH_NLRI: withdrawals of routes of a family other than the one +/// in the message body (RFC 4760 §4). +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct MpUnreachNlriPathAttribute { - pub afi: Afi, - pub safi: Safi, + pub family: AfiSafi, pub prefixes: Vec, } impl MpUnreachNlriPathAttribute { - pub fn family(&self) -> AfiSafi { - AfiSafi::new(self.afi, self.safi) - } - - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, afi) = be_u16(buf).map(|(buf, afi)| (buf, Afi(afi)))?; - let (buf, safi) = be_u8(buf).map(|(buf, safi)| (buf, Safi(safi)))?; - let family = AfiSafi::new(afi, safi); + pub fn from_wire<'a>(ctx: &ParserContext, buf: &'a [u8]) -> Parsed<'a, Self> { + let (buf, family) = parse_family(buf)?; let version = ip_version_of_nlri(family)?; - let with_path_id = ctx.has_add_path(family); - - let (buf, prefixes) = parse_nlri_list(version, with_path_id, buf)?; - - Ok(( - buf, - MpUnreachNlriPathAttribute { - afi, - safi, - prefixes, - }, - )) + let (buf, prefixes) = parse_nlri_list(version, ctx.has_add_path(family), buf)?; + Ok((buf, Self { family, prefixes })) } - pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - out.put_u16(self.afi.0); - out.put_u8(self.safi.0); - write_nlri(&self.prefixes, ctx.has_add_path(self.family()), out); - Ok(()) - } - - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - let total = 3 + nlri_wire_len(&self.prefixes, ctx.has_add_path(self.family())); - u16::try_from(total).map_err(|_| too_long("MP_UNREACH_NLRI", total)) + pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) { + out.put_u16(self.family.afi.0); + out.put_u8(self.family.safi.0); + write_nlri(&self.prefixes, ctx.has_add_path(self.family), out); } } /// Represents a BGP NOTIFICATION message per RFC 4271 §4.5. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct NotificationMessage { pub error: BgpError, /// Optional diagnostic data; may be empty. @@ -1683,37 +1217,12 @@ pub struct NotificationMessage { } impl NotificationMessage { - /// Parses a NOTIFICATION message body (without the 19-byte BGP header). - /// Consumes the entire buffer; any remaining bytes become `data`. - pub fn from_wire<'a>( - _ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, error_code) = be_u8(buf)?; - let (buf, error_subcode) = be_u8(buf)?; - - let error = BgpError::from_code_subcode(error_code, error_subcode) - .map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; - - Ok(( - &[], - Self { - error, - data: buf.to_vec(), - }, - )) - } - - pub fn to_wire(&self, _ctx: &ParserContext, buf: &mut BytesMut) -> Result<(), EncodeError> { + /// Writes the body of the NOTIFICATION. + pub fn to_wire(&self, out: &mut BytesMut) { let (code, subcode) = self.error.code_subcode(); - buf.put_u8(code); - buf.put_u8(subcode); - buf.put_slice(&self.data); - Ok(()) - } - - pub fn wire_len(&self) -> u16 { - 2 + self.data.len() as u16 + out.put_u8(code); + out.put_u8(subcode); + out.put_slice(&self.data); } } @@ -1737,7 +1246,7 @@ mod tests { }; use bgp_primitives::{ - constants::{AddressFamilyId, Afi, AfiSafi, Safi}, + constants::{AfiSafi, IpVersion}, ip_prefix::{IpPrefix, Nlri}, parser::ParserContext, }; @@ -1766,10 +1275,10 @@ mod tests { } /// Creates a default context for evaluating the test cases below. - /// This uses `four_octet_asn` set to true and address_family to IPv6. + /// A session with four-octet AS numbers and an external peer. fn default_v6_context() -> ParserContext { ParserContext { - four_octet_asn: Some(true), + four_octet_asn: true, ibgp: false, add_path: Vec::new(), } @@ -1887,14 +1396,13 @@ mod tests { 0x01, 0x0d, 0xb8 ], PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, next_hop: NlriNextHop::Ipv6WithLl { global: ipv6!("2001:db8::1"), link_local: ipv6!("fe80::12"), }, prefixes: vec![ - IpPrefix::new(AddressFamilyId::Ipv6, [0x20, 0x01, 0x0d, 0xb8], 32) + IpPrefix::new(IpVersion::Ipv6, [0x20, 0x01, 0x0d, 0xb8], 32) .unwrap() .into() ] @@ -1908,10 +1416,9 @@ mod tests { 0x80, 0x0f, 0x08, 0x00, 0x02, 0x01, 0x20, 0x20, 0x01, 0x0d, 0xb8 ], PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, prefixes: vec![ - IpPrefix::new(AddressFamilyId::Ipv6, [0x20, 0x01, 0x0d, 0xb8], 32) + IpPrefix::new(IpVersion::Ipv6, [0x20, 0x01, 0x0d, 0xb8], 32) .unwrap() .into() ], @@ -1927,24 +1434,9 @@ mod tests { ], PathAttribute::ExtendedCommunities(ExtendedCommunitiesPathAttribute { extended_communities: vec![ - ExtendedCommunity::RouteOrigin { - typ: 0, - sub_typ: 3, - global_admin: 65000, - local_admin: 1337 - }, - ExtendedCommunity::RouteOrigin { - typ: 0, - sub_typ: 3, - global_admin: 65001, - local_admin: 1337 - }, - ExtendedCommunity::RouteOrigin { - typ: 2, - sub_typ: 3, - global_admin: 65000, - local_admin: 1337 - }, + ExtendedCommunity([0x00, 0x03, 0xfd, 0xe8, 0x00, 0x00, 0x05, 0x39]), + ExtendedCommunity([0x00, 0x03, 0xfd, 0xe9, 0x00, 0x00, 0x05, 0x39]), + ExtendedCommunity([0x02, 0x03, 0xfd, 0xe8, 0x00, 0x00, 0x05, 0x39]), ] }) ); @@ -1983,21 +1475,23 @@ mod tests { // -- UPDATE message roundtrip tests -- - /// Asserts that an UPDATE message roundtrips byte-for-byte and that - /// wire_len agrees with the length of the encoded output. + /// The UPDATE in `body`, as it is to be acted on. + fn decode_update(ctx: &ParserContext, body: &[u8]) -> Result { + Ok(UpdateMessage::decode(ctx, body)?.update) + } + + /// Asserts that an UPDATE message roundtrips byte-for-byte. macro_rules! test_update_roundtrip { ($name:ident, $input_bytes:expr, $expected:expr) => { #[test] fn $name() -> Result<()> { let ctx = &default_v6_context(); - let (rem, parsed) = UpdateMessage::from_wire(ctx, $input_bytes)?; - assert!(rem.is_empty(), "Expected no trailing bytes, got {:x?}", rem); + let parsed = decode_update(ctx, $input_bytes)?; assert_eq!(parsed, $expected); let mut out = BytesMut::with_capacity(u16::MAX as usize); parsed.to_wire(ctx, &mut out)?; assert_eq!(out.to_vec(), $input_bytes); - assert_eq!(parsed.wire_len(ctx)?, $input_bytes.len() as u16); Ok(()) } }; @@ -2005,7 +1499,7 @@ mod tests { // Wrapper to generate a IPv4 prefix used in tests below. fn v4_prefix(length: u8, bytes: &[u8]) -> IpPrefix { - IpPrefix::new(AddressFamilyId::Ipv4, bytes, length).unwrap() + IpPrefix::new(IpVersion::Ipv4, bytes, length).unwrap() } /// A small set of path attributes that commonly accompany an IPv4 unicast @@ -2142,7 +1636,7 @@ mod tests { 0x00, 0x00, 0x00, 0x07, 0x08, 0x0a, // path 7, 10.0.0.0/8 0x00, 0x00, // no attributes ]; - let (_, update) = UpdateMessage::from_wire(&v4_only, &body).unwrap(); + let update = decode_update(&v4_only, &body).unwrap(); assert_eq!( update.withdrawn_routes, vec![Nlri { @@ -2153,14 +1647,13 @@ mod tests { let mut out = BytesMut::new(); update.to_wire(&v4_only, &mut out).unwrap(); assert_eq!(&out[..], &body[..]); - assert_eq!(usize::from(update.wire_len(&v4_only).unwrap()), body.len()); // The same route on a session without ADD-PATH has no identifier. let mut out = BytesMut::new(); update.to_wire(&plain, &mut out).unwrap(); assert_eq!(&out[..], [0x00, 0x02, 0x08, 0x0a, 0x00, 0x00]); // And ADD-PATH for IPv6 says nothing about the IPv4 prefixes in // the body: read that way, these bytes are not prefixes at all. - assert!(UpdateMessage::from_wire(&v6_only, &body).is_err()); + assert!(decode_update(&v6_only, &body).is_err()); // Two routes for 2001:db8::/32, paths 1 and 2, withdrawn. #[rustfmt::skip] @@ -2181,12 +1674,8 @@ mod tests { ); assert_eq!(parsed.prefixes[0].prefix, parsed.prefixes[1].prefix); let mut out = BytesMut::new(); - parsed.to_wire(&v6_only, &mut out).unwrap(); + parsed.to_wire(&v6_only, &mut out); assert_eq!(&out[..], &unreach[..]); - assert_eq!( - usize::from(parsed.wire_len(&v6_only).unwrap()), - unreach.len() - ); } // Found by the property tests in tests/properties.rs. @@ -2200,7 +1689,7 @@ mod tests { // No withdrawn routes, no attributes, NLRI of length 99. let mut body = vec![0, 0, 0, 0, 99]; body.extend_from_slice(&[0xff; 13]); - assert!(UpdateMessage::from_wire(&ctx, &body).is_err()); + assert!(decode_update(&ctx, &body).is_err()); let mut unreach = vec![0x00, 0x02, 0x01, 129]; unreach.extend_from_slice(&[0xff; 17]); @@ -2216,7 +1705,7 @@ mod tests { // 10.255.0.0/12 withdrawn, with the low four bits of the second // octet set. let body = [0, 3, 12, 10, 0xff, 0, 0]; - let (_, update) = UpdateMessage::from_wire(&ctx, &body).unwrap(); + let update = decode_update(&ctx, &body).unwrap(); assert_eq!(update.withdrawn_routes, vec![v4_prefix(12, &[10, 0xf0])]); assert_eq!(update.withdrawn_routes[0].to_string(), "10.240.0.0/12"); } @@ -2226,33 +1715,29 @@ mod tests { /// step. #[test] fn an_opaque_extended_community_is_eight_octets() { - let ctx = default_v6_context(); #[rustfmt::skip] let wire = [ 0x03, 0x0c, 1, 2, 3, 4, 5, 6, // opaque 0x00, 0x02, 0, 0, 0xfd, 0xe8, 0, 100, // route target 65000:100 ]; - let (rest, parsed) = ExtendedCommunitiesPathAttribute::from_wire(&ctx, &wire).unwrap(); + let (rest, parsed) = ExtendedCommunitiesPathAttribute::from_wire(&wire).unwrap(); assert!(rest.is_empty()); assert_eq!( parsed.extended_communities, vec![ - ExtendedCommunity::Opaque { - typ: 0x03, - sub_typ: 0x0c, - value: [1, 2, 3, 4, 5, 6], - }, - ExtendedCommunity::RouteTarget { - typ: 0x00, - sub_typ: 0x02, - global_admin: 65000, - local_admin: 100, - }, + ExtendedCommunity([0x03, 0x0c, 1, 2, 3, 4, 5, 6]), + ExtendedCommunity([0x00, 0x02, 0, 0, 0xfd, 0xe8, 0, 100]), ] ); + assert_eq!(parsed.extended_communities[0].typ(), 0x03); + assert_eq!(parsed.extended_communities[0].sub_typ(), 0x0c); let mut out = BytesMut::new(); - parsed.to_wire(&ctx, &mut out).unwrap(); + parsed.to_wire(&mut out); assert_eq!(&out[..], &wire[..]); + + // A list that is not a whole number of them is malformed. + assert!(ExtendedCommunitiesPathAttribute::from_wire(&wire[..12]).is_err()); + assert!(ExtendedCommunitiesPathAttribute::from_wire(&[]).is_err()); } /// AGGREGATOR carries an AS number as wide as the session's. It could @@ -2261,7 +1746,7 @@ mod tests { #[test] fn aggregator_is_six_octets_on_a_two_octet_session() { let two_octet = ParserContext { - four_octet_asn: Some(false), + four_octet_asn: false, ibgp: false, add_path: Vec::new(), }; @@ -2276,9 +1761,8 @@ mod tests { } ); let mut out = BytesMut::new(); - parsed.to_wire(&two_octet, &mut out).unwrap(); + parsed.to_wire(&two_octet, &mut out); assert_eq!(&out[..], &wire[..]); - assert_eq!(parsed.wire_len(&two_octet).unwrap(), 6); } /// The reserved octet after the next hop is there for every family. It @@ -2297,7 +1781,7 @@ mod tests { let (rest, parsed) = MpReachNlriPathAttribute::from_wire(&ctx, &wire).unwrap(); assert!(rest.is_empty()); assert_eq!( - parsed.family(), + parsed.family, bgp_primitives::constants::AfiSafi::IPV4_UNICAST ); assert_eq!( @@ -2307,7 +1791,7 @@ mod tests { assert_eq!(parsed.prefixes, vec![v4_prefix(24, &[10, 1, 2])]); let mut out = BytesMut::new(); - parsed.to_wire(&ctx, &mut out).unwrap(); + parsed.to_wire(&ctx, &mut out); assert_eq!(&out[..], &wire[..]); } @@ -2339,19 +1823,18 @@ mod tests { }], }; let two_octet = ParserContext { - four_octet_asn: Some(false), + four_octet_asn: false, ibgp: false, add_path: Vec::new(), }; let four_octet = ParserContext { - four_octet_asn: Some(true), + four_octet_asn: true, ibgp: false, add_path: Vec::new(), }; let wire = |attribute: &PathAttribute, ctx: &ParserContext| { let mut out = BytesMut::new(); attribute.to_wire(ctx, &mut out).unwrap(); - assert_eq!(out.len(), usize::from(attribute.wire_len(ctx).unwrap())); out.to_vec() }; @@ -2388,13 +1871,6 @@ mod tests { }], }; assert_eq!(narrow.as4_path(), None); - - // Without a negotiated width there is nothing right to write. - assert!( - as_path - .to_wire(&ParserContext::default(), &mut BytesMut::new()) - .is_err() - ); } #[test] @@ -2403,7 +1879,7 @@ mod tests { // Well-formed withdrawn_len=0, pa_len=0, followed by a single byte that // declares a prefix length of 8 but provides no prefix bytes. let buf = &[0x00, 0x00, 0x00, 0x00, 0x08]; - let result = UpdateMessage::from_wire(&ctx, buf); + let result = decode_update(&ctx, buf); assert!( result.is_err(), "Expected a parse error for trailing garbage, got {:?}", @@ -2412,43 +1888,50 @@ mod tests { } #[test] - fn test_notification_roundtrip() -> Result<()> { - use crate::errors::{BgpError, CeaseSubcode}; + fn a_notification_round_trips_with_its_data() -> Result<()> { + use crate::errors::{BgpError, CeaseSubcode, FsmSubcode}; let ctx = &default_v6_context(); - let msg = NotificationMessage { - error: BgpError::Cease(CeaseSubcode::AdminShutdown), - data: vec![0xde, 0xad], - }; - - let mut buf = BytesMut::new(); - msg.to_wire(ctx, &mut buf)?; - assert_eq!(buf.as_ref(), &[6, 2, 0xde, 0xad]); - - let (rem, parsed) = NotificationMessage::from_wire(ctx, &buf).unwrap(); - assert!(rem.is_empty()); - assert_eq!(parsed, msg); + for (error, data, body) in [ + ( + BgpError::Cease(CeaseSubcode::AdminShutdown), + vec![0xde, 0xad], + &[6, 2, 0xde, 0xad][..], + ), + ( + BgpError::FsmError(FsmSubcode::UnexpectedEstablished), + vec![], + &[5, 3][..], + ), + ] { + let msg = Message::Notification(NotificationMessage { error, data }); + let mut buf = BytesMut::new(); + msg.to_wire(ctx, &mut buf)?; + assert_eq!(&buf[19..], body); + assert_eq!(Message::decode(ctx, &buf)?, msg); + } Ok(()) } + /// A peer may give a reason we have no name for: a subcode of zero + /// ("unspecific"), or one assigned since. Such a NOTIFICATION used to + /// be refused as malformed, which lost the reason and answered a + /// message that must not be answered. #[test] - fn test_notification_no_data() -> Result<()> { - use crate::errors::{BgpError, FsmSubcode}; + fn a_notification_with_an_unnamed_reason_is_still_read() { + use crate::errors::BgpError; - let ctx = &default_v6_context(); - let msg = NotificationMessage { - error: BgpError::FsmError(FsmSubcode::UnexpectedEstablished), - data: vec![], - }; - - let mut buf = BytesMut::new(); - msg.to_wire(ctx, &mut buf)?; - assert_eq!(buf.as_ref(), &[5, 3]); - - let (rem, parsed) = NotificationMessage::from_wire(ctx, &buf).unwrap(); - assert!(rem.is_empty()); - assert_eq!(parsed, msg); - Ok(()) + let ctx = default_v6_context(); + for (code, subcode) in [(6, 0), (5, 0), (2, 5), (3, 7), (4, 9), (99, 1)] { + let decoded = Message::decode(&ctx, &framed(21, 3, &[code, subcode])).unwrap(); + let Message::Notification(notification) = &decoded else { + panic!("expected a NOTIFICATION, got {decoded:?}"); + }; + assert_eq!(notification.error, BgpError::Other { code, subcode }); + assert_eq!(notification.error.code_subcode(), (code, subcode)); + } + // Too short to hold a code and subcode. + assert!(Message::decode(&ctx, &framed(20, 3, &[6])).is_err()); } #[test] @@ -2462,30 +1945,7 @@ mod tests { assert!(buf[..16].iter().all(|&b| b == 0xFF)); assert_eq!(&buf[16..19], &[0x00, 0x13, 0x04]); - let (rem, parsed) = Message::from_wire(ctx, &buf).unwrap(); - assert!(rem.is_empty()); - assert!(matches!(parsed, Message::KeepAlive)); - Ok(()) - } - - #[test] - fn test_message_notification_roundtrip() -> Result<()> { - use crate::errors::{BgpError, CeaseSubcode}; - - let ctx = &default_v6_context(); - let inner = NotificationMessage { - error: BgpError::Cease(CeaseSubcode::AdminShutdown), - data: vec![], - }; - let msg = Message::Notification(inner); - - let mut buf = BytesMut::new(); - msg.to_wire(ctx, &mut buf)?; - assert_eq!(buf.len(), msg.wire_len(ctx)? as usize); - - let (rem, parsed) = Message::from_wire(ctx, &buf).unwrap(); - assert!(rem.is_empty()); - assert!(matches!(parsed, Message::Notification(_))); + assert_eq!(Message::decode(ctx, &buf)?, Message::KeepAlive); Ok(()) } @@ -2496,7 +1956,7 @@ mod tests { buf[16] = 0x00; buf[17] = 0x13; buf[18] = 0x04; - assert!(Message::from_wire(&ctx, &buf).is_err()); + assert!(Message::decode(&ctx, &buf).is_err()); } #[test] @@ -2506,7 +1966,41 @@ mod tests { buf[16] = 0x00; buf[17] = 0x12; // length = 18, below minimum 19 buf[18] = 0x04; - assert!(Message::from_wire(&ctx, &buf).is_err()); + assert!(Message::decode(&ctx, &buf).is_err()); + } + + /// A message that cannot be encoded leaves nothing behind in the + /// buffer, so that what is already there can still be sent. + #[test] + fn a_message_that_cannot_be_encoded_leaves_the_buffer_as_it_was() { + let ctx = default_v6_context(); + let mut buf = BytesMut::new(); + Message::KeepAlive.to_wire(&ctx, &mut buf).unwrap(); + + let too_long = Message::Notification(NotificationMessage { + error: crate::errors::BgpError::HoldTimer, + data: vec![0; 5000], + }); + assert!(too_long.to_wire(&ctx, &mut buf).is_err()); + assert_eq!(buf.len(), 19); + } + + /// An attribute's length is one octet unless its value needs two. + #[test] + fn a_long_attribute_gets_the_extended_length() { + let ctx = default_v6_context(); + for (communities, extended) in [(63, false), (64, true)] { + let attribute = + PathAttribute::Communities(CommunitiesPathAttribute(vec![(1, 2); communities])); + let mut out = BytesMut::new(); + attribute.to_wire(&ctx, &mut out).unwrap(); + let header = if extended { 4 } else { 3 }; + assert_eq!(out.len(), header + 4 * communities); + assert_eq!(out[0] & 0x10 != 0, extended); + let (rest, parsed) = PathAttribute::from_wire(&ctx, &out).unwrap(); + assert!(rest.is_empty()); + assert_eq!(parsed, attribute); + } } /// A header, with `length` and `msg_type` as given, followed by `body`. @@ -2585,16 +2079,22 @@ mod tests { } #[test] - fn decode_ignores_bytes_after_the_message_and_from_wire_returns_them() { + fn decode_ignores_bytes_after_the_message() { let ctx = default_v6_context(); let mut buf = framed(19, 4, &[]); buf.extend_from_slice(&[1, 2, 3]); + assert_eq!(Message::decode(&ctx, &buf), Ok(Message::KeepAlive)); + } - assert!(matches!( - Message::decode(&ctx, &buf), - Ok(Message::KeepAlive) - )); - let (rest, _) = Message::from_wire(&ctx, &buf).unwrap(); - assert_eq!(rest, &[1, 2, 3]); + /// The optional parameters are the last thing in an OPEN. + #[test] + fn bytes_after_the_optional_parameters_of_an_open_are_refused() { + use crate::errors::{BgpError, OpenMsgSubcode}; + let ctx = default_v6_context(); + let open = framed(31, 1, &[4, 0xfd, 0xe8, 0, 90, 10, 0, 0, 1, 0, 0xaa, 0xbb]); + assert_eq!( + Message::decode(&ctx, &open).unwrap_err().notification, + BgpError::OpenMsg(OpenMsgSubcode::Unspecific) + ); } } diff --git a/crates/packet/src/open.rs b/crates/packet/src/open.rs index a40be99..55c0973 100644 --- a/crates/packet/src/open.rs +++ b/crates/packet/src/open.rs @@ -7,25 +7,42 @@ use nom::{ IResult, Parser, number::complete::{be_u8, be_u16, be_u24, be_u32}, }; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use crate::errors::InvalidValue; -use strum::EnumDiscriminants; use bgp_primitives::{ constants::{Afi, AfiSafi, Safi}, - parser::{BgpParserError, EncodeError, ParserContext, ProtocolErrorValues}, + parser::{BgpParserError, EncodeError, ProtocolErrorValues}, }; +type Parsed<'a, T> = IResult<&'a [u8], T, BgpParserError<&'a [u8]>>; + macro_rules! fail { ( $($arg:expr),* $(,)? ) => { Failure(BgpParserError::Malformed(format!($($arg),*))) }; } -/// Contains messages related to the BGP Open message, -/// i.e. the Open message itself and options / capabilities. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +/// Parses `buf` to its end as a list of `item`s. Unlike nom's `many0`, +/// which stops quietly at an item it cannot read, an error in an item is an +/// error of the list. +fn list<'a, T>(mut buf: &'a [u8], item: impl Fn(&'a [u8]) -> Parsed<'a, T>) -> Parsed<'a, Vec> { + let mut items = Vec::new(); + while !buf.is_empty() { + let (rest, parsed) = item(buf)?; + items.push(parsed); + buf = rest; + } + Ok((buf, items)) +} + +/// The optional parameter type that marks an OPEN whose optional parameters +/// have two-octet lengths (RFC 9072). +const EXTENDED_LENGTH: u8 = 255; + +/// The OPEN message (RFC 4271 §4.2). +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct OpenMessage { /// Version of the BGP protocol in use. pub version: u8, @@ -40,13 +57,10 @@ pub struct OpenMessage { } impl OpenMessage { - /// The from_wire implementation parses a BGP Open message from a buffer - /// containing the payload of the open message (i.e. without the BGP header / type / length) - /// and the buf passed in must contain only the open message. - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { + /// Parses the body of an OPEN (what follows the 19-octet header). + /// Returns what is left of `buf` after the optional parameters, which + /// in a well-formed OPEN is nothing. + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { let (buf, version) = be_u8(buf)?; if version != 4 { return Err(Failure(BgpParserError::ProtocolError( @@ -58,32 +72,22 @@ impl OpenMessage { let (buf, hold_time) = be_u16(buf)?; let (buf, identifier) = be_u32(buf)?; - // RFC 9072: Extended Open Options length. - // To encode a capabilities length greater than 255 the packet structure - // is prefixed with the extended length option (255), a length of 255, - // and then a u16 containing the payload option. - // An OPEN without optional parameters ends at the length byte, so - // there may be nothing after it to look at. + // RFC 9072: when the first parameter type is 255, the real length + // of the optional parameters follows in two octets, and each + // parameter has a two-octet length as well. An OPEN without + // optional parameters ends at the length octet. let (buf, opt_len) = be_u8(buf)?; - let extended = opt_len == 255 && buf.first() == Some(&255); - + let extended = opt_len != 0 && buf.first() == Some(&EXTENDED_LENGTH); let (buf, opt_len) = if extended { - let (buf, _) = be_u8(buf)?; // Skip past the inner type. - be_u16(buf)? + let (buf, _) = be_u8(buf)?; + let (buf, opt_len) = be_u16(buf)?; + (buf, usize::from(opt_len)) } else { - (buf, opt_len as u16) + (buf, usize::from(opt_len)) }; let (buf, opt_buf) = nom::bytes::take(opt_len).parse(buf)?; - let (opt_buf, options) = - nom::multi::many0(|buf| OpenOption::from_wire(ctx, buf)).parse(opt_buf)?; - - if !opt_buf.is_empty() { - return Err(fail!( - "Leftover bytes after parsing open options: {:x?}", - opt_buf - )); - } + let (_, options) = list(opt_buf, |buf| OpenOption::from_wire(extended, buf))?; Ok(( buf, @@ -105,80 +109,84 @@ impl OpenMessage { }) } - pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + /// Writes the body of the OPEN. + pub fn to_wire(&self, out: &mut BytesMut) -> Result<(), EncodeError> { out.put_u8(self.version); out.put_u16(self.asn); out.put_u16(self.hold_time); out.put_u32(self.identifier.into()); - // RFC 9072: Depending on whether the options fit in < 255 bytes - // we encode the length accordingly. - let opt_len = self + // The parameters are written out first: how long they are decides + // which form of lengths the message uses (RFC 9072 §2). + let parameters = self .options .iter() - .map(|o| o.wire_len(ctx)) - .sum::>()?; - if opt_len > u8::MAX as u16 { - out.put_u8(0xff); // Set length to 255 as per RFC 9072. - out.put_u8(0xff); // Open option type for Ext length. - out.put_u16(opt_len); + .map(|option| Ok((option.type_code(), option.value()?))) + .collect::, EncodeError>>()?; + let total = |header: usize| -> usize { + parameters + .iter() + .map(|(_, value)| header + value.len()) + .sum() + }; + let fits_one_octet = parameters.iter().all(|(_, value)| value.len() <= 255); + + if let Ok(length) = u8::try_from(total(2)) + && fits_one_octet + { + out.put_u8(length); + for (type_code, value) in ¶meters { + out.put_u8(*type_code); + out.put_u8(value.len() as u8); + out.put_slice(value); + } } else { - out.put_u8(opt_len as u8); + let too_long = |len| EncodeError::TooLong { + what: "OPEN optional parameters", + len, + max: u16::MAX.into(), + }; + let length = u16::try_from(total(3)).map_err(|_| too_long(total(3)))?; + out.put_u8(255); + out.put_u8(EXTENDED_LENGTH); + out.put_u16(length); + for (type_code, value) in ¶meters { + out.put_u8(*type_code); + // No longer than the total, which fits. + out.put_u16(value.len() as u16); + out.put_slice(value); + } } - - for option in &self.options { - option.to_wire(ctx, out)?; - } - Ok(()) } - - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - let mut total = 10; - let opt_len = self - .options - .iter() - .map(|o| o.wire_len(ctx)) - .sum::>()?; - if opt_len > u8::MAX as u16 { - total += opt_len + 3; // 1+2 bytes extra for the type + extended length. - } else { - total += opt_len; - } - Ok(total) - } } -#[derive(Debug, Clone, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)] -#[repr(u8)] +/// The optional parameter type for capabilities (RFC 5492). +const CAPABILITIES: u8 = 2; + +/// An optional parameter of an OPEN. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub enum OpenOption { - Capabilities(Vec) = 2, + Capabilities(Vec), } impl OpenOption { - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { + /// Parses one optional parameter. `extended` says whether its length is + /// two octets (RFC 9072) or one. + fn from_wire(extended: bool, buf: &[u8]) -> Parsed<'_, Self> { let (buf, type_code) = be_u8(buf)?; - let (buf, length) = be_u8(buf)?; - - let (buf, payload) = nom::bytes::take(length).parse(buf)?; + let (buf, length) = if extended { + let (buf, length) = be_u16(buf)?; + (buf, usize::from(length)) + } else { + let (buf, length) = be_u8(buf)?; + (buf, usize::from(length)) + }; + let (buf, value) = nom::bytes::take(length).parse(buf)?; match type_code { - v if v == OpenOptionDiscriminants::Capabilities as u8 => { - let (payload_consumed, (capabilities, _eof)) = nom::multi::many_till( - |buf| Capability::from_wire(ctx, buf), - nom::combinator::eof, - ) - .parse(payload)?; - - if !payload_consumed.is_empty() { - return Err(fail!( - "Leftover bytes when parsing BGP capabilities: {:x?}", - payload_consumed - )); - } + CAPABILITIES => { + let (_, capabilities) = list(value, Capability::from_wire)?; Ok((buf, Self::Capabilities(capabilities))) } other => Err(Failure(BgpParserError::ProtocolError( @@ -187,43 +195,28 @@ impl OpenOption { } } - pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { + fn type_code(&self) -> u8 { match self { - OpenOption::Capabilities(items) => { - let cap_len = items - .iter() - .map(|i| i.wire_len(ctx)) - .sum::>()?; - if cap_len > u8::MAX as u16 { - return Err(EncodeError::TooLong { - what: "capabilities", - len: cap_len.into(), - max: u8::MAX.into(), - }); - } - out.put_u8(OpenOptionDiscriminants::Capabilities as u8); - out.put_u8(cap_len as u8); - for item in items { - item.to_wire(ctx, out)?; + Self::Capabilities(_) => CAPABILITIES, + } + } + + /// The parameter's value on the wire. + fn value(&self) -> Result { + let mut value = BytesMut::new(); + match self { + Self::Capabilities(capabilities) => { + for capability in capabilities { + capability.to_wire(&mut value)?; } } } - Ok(()) - } - - pub fn wire_len(&self, ctx: &ParserContext) -> Result { - match self { - OpenOption::Capabilities(items) => items - .iter() - .map(|i| i.wire_len(ctx)) - .sum::>() - .map(|v| v + 2), - } + Ok(value) } } /// One address family in a Multiple Labels capability (RFC 8277 §2.1). -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] pub struct MultiLabelEntry { pub afi: Afi, pub safi: Safi, @@ -231,105 +224,122 @@ pub struct MultiLabelEntry { pub count: u8, } -#[derive(Debug, Clone, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)] -#[repr(u8)] +/// Capability codes (the IANA "Capability Codes" registry). +mod cap_type { + pub const MULTI_PROTOCOL: u8 = 1; + pub const ROUTE_REFRESH: u8 = 2; + pub const OUTBOUND_ROUTE_FILTER: u8 = 3; + pub const EXTENDED_NEXT_HOP: u8 = 5; + pub const EXTENDED_MESSAGE: u8 = 6; + pub const BGP_SEC: u8 = 7; + pub const MULTI_LABEL: u8 = 8; + pub const BGP_ROLE: u8 = 9; + pub const GRACEFUL_RESTART: u8 = 64; + pub const FOUR_BYTE_ASN: u8 = 65; + pub const ADD_PATH: u8 = 69; + pub const ENHANCED_ROUTE_REFRESH: u8 = 70; + pub const LONG_LIVED_GRACEFUL_RESTART: u8 = 71; +} + +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub enum Capability { - /// MultiProtocol Extension (RFC 2858). + /// Multiprotocol Extensions (RFC 4760). MultiProtocol { afi: Afi, safi: Safi, - } = 1, + }, /// Route Refresh capability (RFC 2918). - RouteRefresh = 2, - /// Outbound Route Filtering (RFC 5291). - /// https://datatracker.ietf.org/doc/html/rfc5291 - OutboundRouteFilter { - afi: Afi, - safi: Safi, - orfs: Vec, - } = 3, + RouteRefresh, + /// Outbound Route Filtering (RFC 5291): for each address family, the + /// kinds of filter the speaker can send or receive. + OutboundRouteFilter(Vec), /// Extended Next Hop encoding (RFC 8950). ExtendedNextHop { entries: Vec, - } = 5, + }, /// Extended Message (RFC 8654). - ExtendedMessage = 6, + ExtendedMessage, /// BGPSec (RFC 8205). BgpSec { header: BgpSecHeader, afi: Afi, - } = 7, + }, /// Multiple labels (RFC 8277 §2.1): for each address family, how many /// labels the speaker can handle in one NLRI. MultiLabelCompat { entries: Vec, - } = 8, + }, BgpRole { role: BgpRole, - } = 9, + }, /// Graceful restart capability (RFC 4724). GracefulRestart { - /// When set to true it means that the BGP speaker has just restarted - /// and the peer must not wait for the End of RIB marker from the - /// speaker before advertising routing information to the speaker. + /// The speaker has just restarted, and its peer must not wait for + /// its End-of-RIB marker before advertising routes to it. restart_state: bool, - /// This is the estimated time (in seconds) it will take for the - /// BGP session to be re-established after a restart. This can be - /// used to speed up routing convergence by its peer in case that - /// the BGP speaker does not come back after a restart. + /// The estimated time in seconds for the session to be + /// re-established after a restart. restart_time: u16, - /// The AFI and SAFI, taken in combination, indicate that Graceful - /// Restart is supported for routes that are advertised with the - /// same AFI and SAFI. Routes may be explicitly associated with a - /// particular AFI and SAFI using the encoding of [BGP-MP] or - /// implicitly associated with if using - /// the encoding of [BGP-4]. + /// The address families whose routes are kept over a restart. entries: Vec, - } = 64, - /// Four Byte ASN (RFC 4274). + }, + /// Four-octet AS numbers (RFC 6793). FourByteAsn { asn: u32, - } = 65, + }, /// ADD-PATH (RFC 7911): for each address family listed, whether the /// speaker can send several routes for a prefix, receive them, or both. AddPath { entries: Vec, - } = 69, + }, /// Enhanced Route Refresh (RFC 7313). - EnhancedRouteRefresh = 70, + EnhancedRouteRefresh, /// Long Lived Graceful Restart (RFC 9494). - LongLivedGracefulRestart(Vec) = 71, - /// Unknown Capability encapsulates any capabilities that we don't know about. + LongLivedGracefulRestart(Vec), + /// A capability this implementation has no definition for, kept as it + /// came (RFC 5492 §3: what is not understood is ignored). UnknownCapability { type_code: u8, - length: u8, value: Vec, }, } +/// An AFI, a reserved octet and a SAFI: how several capabilities name an +/// address family. +fn afi_reserved_safi(buf: &[u8]) -> Parsed<'_, (Afi, Safi)> { + let (buf, afi) = be_u16(buf)?; + let (buf, _) = be_u8(buf)?; + let (buf, safi) = be_u8(buf)?; + Ok((buf, (Afi(afi), Safi(safi)))) +} + +fn afi_safi(buf: &[u8]) -> Parsed<'_, (Afi, Safi)> { + let (buf, afi) = be_u16(buf)?; + let (buf, safi) = be_u8(buf)?; + Ok((buf, (Afi(afi), Safi(safi)))) +} + impl Capability { - fn discriminant(&self) -> u8 { + fn type_code(&self) -> u8 { + use cap_type::*; match self { - Capability::MultiProtocol { .. } => CapabilityDiscriminants::MultiProtocol as u8, - Capability::RouteRefresh => CapabilityDiscriminants::RouteRefresh as u8, - Capability::OutboundRouteFilter { .. } => { - CapabilityDiscriminants::OutboundRouteFilter as u8 - } - Capability::ExtendedNextHop { .. } => CapabilityDiscriminants::ExtendedNextHop as u8, - Capability::ExtendedMessage => CapabilityDiscriminants::ExtendedMessage as u8, - Capability::BgpSec { .. } => CapabilityDiscriminants::BgpSec as u8, - Capability::MultiLabelCompat { .. } => CapabilityDiscriminants::MultiLabelCompat as u8, - Capability::BgpRole { .. } => CapabilityDiscriminants::BgpRole as u8, - Capability::GracefulRestart { .. } => CapabilityDiscriminants::GracefulRestart as u8, - Capability::FourByteAsn { .. } => CapabilityDiscriminants::FourByteAsn as u8, - Capability::AddPath { .. } => CapabilityDiscriminants::AddPath as u8, - Capability::EnhancedRouteRefresh => CapabilityDiscriminants::EnhancedRouteRefresh as u8, - Capability::LongLivedGracefulRestart(_) => { - CapabilityDiscriminants::LongLivedGracefulRestart as u8 - } - Capability::UnknownCapability { type_code, .. } => *type_code, + Self::MultiProtocol { .. } => MULTI_PROTOCOL, + Self::RouteRefresh => ROUTE_REFRESH, + Self::OutboundRouteFilter(_) => OUTBOUND_ROUTE_FILTER, + Self::ExtendedNextHop { .. } => EXTENDED_NEXT_HOP, + Self::ExtendedMessage => EXTENDED_MESSAGE, + Self::BgpSec { .. } => BGP_SEC, + Self::MultiLabelCompat { .. } => MULTI_LABEL, + Self::BgpRole { .. } => BGP_ROLE, + Self::GracefulRestart { .. } => GRACEFUL_RESTART, + Self::FourByteAsn { .. } => FOUR_BYTE_ASN, + Self::AddPath { .. } => ADD_PATH, + Self::EnhancedRouteRefresh => ENHANCED_ROUTE_REFRESH, + Self::LongLivedGracefulRestart(_) => LONG_LIVED_GRACEFUL_RESTART, + Self::UnknownCapability { type_code, .. } => *type_code, } } + /// The Multiprotocol capability that offers `family` to a peer. pub fn multiprotocol(family: AfiSafi) -> Self { Self::MultiProtocol { @@ -338,205 +348,123 @@ impl Capability { } } - /// `from_wire` parses a Capability message. + /// Parses one capability from the front of `buf`. /// /// Address families are read as numbers, whatever they are: a peer is /// free to advertise families we have never heard of, and RFC 5492 §3 /// has us ignore what we do not support rather than refuse the OPEN. - pub fn from_wire<'a>( - _ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, typ) = be_u8(buf)?; - let (buf, len) = be_u8(buf)?; - // `cap_buf` holds exactly this capability's value; `buf` is what - // follows it and is what the caller's list parser continues with. - let (buf, cap_buf) = nom::bytes::take(len).parse(buf)?; - let capability = Self::parse_value(typ, cap_buf)?; + pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { + let (buf, type_code) = be_u8(buf)?; + let (buf, length) = be_u8(buf)?; + let (buf, value) = nom::bytes::take(length).parse(buf)?; + let capability = Self::parse_value(type_code, value)?; Ok((buf, capability)) } - /// Parses the value of a capability of type `typ`. `cap_buf` is exactly - /// the value. - fn parse_value<'a>( - typ: u8, - cap_buf: &'a [u8], - ) -> Result>> { - macro_rules! require_empty { - ($buf:expr, $cap_type:expr) => { - if !$buf.is_empty() { - return Err(nom::Err::Failure(BgpParserError::Malformed(format!( - "Unexpected leftover bytes {:x?} while parsing {} capability", - $buf, $cap_type - )))); - } - }; - } - match typ { - v if v == CapabilityDiscriminants::MultiProtocol as u8 => { - // MultiProtocol capability consists of an (AFI, SAFI). - let (cap_buf, afi) = be_u16(cap_buf)?; - let (cap_buf, _) = be_u8(cap_buf)?; - let (cap_buf, safi) = be_u8(cap_buf)?; - require_empty!(cap_buf, "MultiProtocol"); - let afi = Afi(afi); - let safi = Safi(safi); - Ok(Self::MultiProtocol { afi, safi }) + /// Makes the capability of type `type_code` from its value. All of + /// `value` has to be used. + fn parse_value(type_code: u8, value: &[u8]) -> Result>> { + use cap_type::*; + let (rest, capability) = match type_code { + MULTI_PROTOCOL => { + let (rest, (afi, safi)) = afi_reserved_safi(value)?; + (rest, Self::MultiProtocol { afi, safi }) } - v if v == CapabilityDiscriminants::RouteRefresh as u8 => { - require_empty!(cap_buf, "RouteRefresh"); - Ok(Self::RouteRefresh) + ROUTE_REFRESH => (value, Self::RouteRefresh), + OUTBOUND_ROUTE_FILTER => { + let (rest, families) = list(value, OrfFamily::from_wire)?; + (rest, Self::OutboundRouteFilter(families)) } - v if v == CapabilityDiscriminants::OutboundRouteFilter as u8 => { - let (cap_buf, afi) = be_u16(cap_buf)?; - let (cap_buf, _) = be_u8(cap_buf)?; // Move past reverved byte. - let (cap_buf, safi) = be_u8(cap_buf)?; - let afi = Afi(afi); - let safi = Safi(safi); - - let (cap_buf, _num_orfs) = be_u8(cap_buf)?; - - let parse_orf = |buf: &'a [u8]| -> IResult< - &'a [u8], - OutboundRouteFilterEntry, - BgpParserError<&'a [u8]>, - > { - let (buf, orf_type) = be_u8(buf)?; - let (buf, send_recv) = be_u8(buf)?; - let orf_type = OrfType::from(orf_type); - let send_recv = OrfSendRecv::try_from(send_recv) - .map_err(|e| fail!("Failed to parse ORF send/recv: {}", e))?; - - Ok(( - buf, - OutboundRouteFilterEntry { - r#type: orf_type, - send_recv, - }, - )) - }; - - let (cap_buf, orfs) = nom::multi::many0(parse_orf).parse(cap_buf)?; - require_empty!(cap_buf, "OutboundRouteFilter"); - - Ok(Self::OutboundRouteFilter { afi, safi, orfs }) - } - v if v == CapabilityDiscriminants::ExtendedNextHop as u8 => { - // parse_enh parses a single extended nexthop entry. - let parse_enh = |buf: &'a [u8]| -> IResult< - &'a [u8], - ExtendedNextHop, - BgpParserError<&'a [u8]>, - > { + EXTENDED_NEXT_HOP => { + let (rest, entries) = list(value, |buf| { let (buf, afi) = be_u16(buf)?; let (buf, safi) = be_u16(buf)?; let (buf, nh_afi) = be_u16(buf)?; - let afi = Afi(afi); // Two octets on the wire here (RFC 8950), one everywhere // else: no SAFI is larger than 255. let safi = u8::try_from(safi) .map(Safi) .map_err(|_| fail!("SAFI {} does not fit in one octet", safi))?; - let nh_afi = Afi(nh_afi); - Ok((buf, ExtendedNextHop { afi, safi, nh_afi })) - }; - let (cap_buf, entries) = nom::multi::many1(parse_enh).parse(cap_buf)?; - require_empty!(cap_buf, "ExtendedNextHop"); - - Ok(Self::ExtendedNextHop { entries }) - } - v if v == CapabilityDiscriminants::ExtendedMessage as u8 => { - require_empty!(cap_buf, "ExtendedMessage"); - Ok(Self::ExtendedMessage) - } - v if v == CapabilityDiscriminants::BgpSec as u8 => { - let (cap_buf, header) = be_u8(cap_buf)?; - let (cap_buf, afi) = be_u16(cap_buf)?; - let afi = Afi(afi); - require_empty!(cap_buf, "BgpSec"); - Ok(Self::BgpSec { - header: BgpSecHeader(header), - afi, - }) - } - v if v == CapabilityDiscriminants::MultiLabelCompat as u8 => { - let parse_entry = |buf: &'a [u8]| -> IResult< - &'a [u8], - MultiLabelEntry, - BgpParserError<&'a [u8]>, - > { - let (buf, afi) = be_u16(buf)?; - let (buf, safi) = be_u8(buf)?; - let (buf, count) = be_u8(buf)?; Ok(( buf, - MultiLabelEntry { + ExtendedNextHop { afi: Afi(afi), - safi: Safi(safi), - count, + safi, + nh_afi: Afi(nh_afi), }, )) - }; - let (cap_buf, entries) = nom::multi::many1(parse_entry).parse(cap_buf)?; - require_empty!(cap_buf, "MultiLabelCompat"); - Ok(Self::MultiLabelCompat { entries }) + })?; + if entries.is_empty() { + return Err(fail!("Extended Next Hop capability with no entries")); + } + (rest, Self::ExtendedNextHop { entries }) } - v if v == CapabilityDiscriminants::BgpRole as u8 => { - let (cap_buf, role) = be_u8(cap_buf)?; - let role = BgpRole::from(role); - require_empty!(cap_buf, "BgpRole"); - Ok(Self::BgpRole { role }) + EXTENDED_MESSAGE => (value, Self::ExtendedMessage), + BGP_SEC => { + let (rest, header) = be_u8(value)?; + let (rest, afi) = be_u16(rest)?; + ( + rest, + Self::BgpSec { + header: BgpSecHeader(header), + afi: Afi(afi), + }, + ) } - v if v == CapabilityDiscriminants::GracefulRestart as u8 => { - // flags_restart_time is: (4 bits: restart flags, 12 bits: restart time (s)). - let (cap_buf, flags_restart_time) = be_u16(cap_buf)?; - let restart_state = ((1 << 15) & flags_restart_time) != 0; - let restart_time = flags_restart_time & 0xfff; - let parse_entry = |buf: &'a [u8]| -> IResult< - &'a [u8], - GracefulRestartEntry, - BgpParserError<&'a [u8]>, - > { - let (buf, afi) = be_u16(buf)?; - let (buf, safi) = be_u8(buf)?; + MULTI_LABEL => { + let (rest, entries) = list(value, |buf| { + let (buf, (afi, safi)) = afi_safi(buf)?; + let (buf, count) = be_u8(buf)?; + Ok((buf, MultiLabelEntry { afi, safi, count })) + })?; + if entries.is_empty() { + return Err(fail!("Multiple Labels capability with no entries")); + } + (rest, Self::MultiLabelCompat { entries }) + } + BGP_ROLE => { + let (rest, role) = be_u8(value)?; + ( + rest, + Self::BgpRole { + role: BgpRole::from(role), + }, + ) + } + GRACEFUL_RESTART => { + // Four bits of flags and twelve of restart time. + let (rest, flags_restart_time) = be_u16(value)?; + let (rest, entries) = list(rest, |buf| { + let (buf, (afi, safi)) = afi_safi(buf)?; let (buf, flags) = be_u8(buf)?; - let afi = Afi(afi); - let safi = Safi(safi); Ok(( buf, GracefulRestartEntry { afi, safi, - preserve_forwarding: (flags & (1 << 7)) != 0, + preserve_forwarding: flags & (1 << 7) != 0, }, )) - }; - let (cap_buf, entries) = nom::multi::many0(parse_entry).parse(cap_buf)?; - require_empty!(cap_buf, "GracefulRestart"); - Ok(Self::GracefulRestart { - restart_state, - restart_time, - entries, - }) + })?; + ( + rest, + Self::GracefulRestart { + restart_state: flags_restart_time & (1 << 15) != 0, + restart_time: flags_restart_time & 0xfff, + entries, + }, + ) } - v if v == CapabilityDiscriminants::FourByteAsn as u8 => { - let (cap_buf, asn) = be_u32(cap_buf)?; - require_empty!(cap_buf, "FourByteAsn"); - Ok(Self::FourByteAsn { asn }) + FOUR_BYTE_ASN => { + let (rest, asn) = be_u32(value)?; + (rest, Self::FourByteAsn { asn }) } - v if v == CapabilityDiscriminants::AddPath as u8 => { + ADD_PATH => { // A list of families, four octets each. None at all says // nothing and is let through: it is also what is left of a // capability whose families were all ignored below. - if !cap_buf.len().is_multiple_of(4) { - return Err(fail!( - "ADD-PATH capability of {} octets is not a list of families", - cap_buf.len() - )); - } - let entries = cap_buf - .as_chunks::<4>() - .0 + let (entries, rest) = value.as_chunks::<4>(); + let entries = entries .iter() .filter_map(|entry| { // A family with a Send/Receive value that is not @@ -548,24 +476,14 @@ impl Capability { }) }) .collect(); - Ok(Self::AddPath { entries }) + (rest, Self::AddPath { entries }) } - v if v == CapabilityDiscriminants::EnhancedRouteRefresh as u8 => { - require_empty!(cap_buf, "EnhancedRouteRefresh"); - Ok(Self::EnhancedRouteRefresh) - } - v if v == CapabilityDiscriminants::LongLivedGracefulRestart as u8 => { - let parse_llgr = |buf: &'a [u8]| -> IResult< - &'a [u8], - LongLivedGracefulRestart, - BgpParserError<&'a [u8]>, - > { - let (buf, afi) = be_u16(buf)?; - let (buf, safi) = be_u8(buf)?; + ENHANCED_ROUTE_REFRESH => (value, Self::EnhancedRouteRefresh), + LONG_LIVED_GRACEFUL_RESTART => { + let (rest, entries) = list(value, |buf| { + let (buf, (afi, safi)) = afi_safi(buf)?; let (buf, flags) = be_u8(buf)?; let (buf, stale_time) = be_u24(buf)?; - let afi = Afi(afi); - let safi = Safi(safi); Ok(( buf, LongLivedGracefulRestart { @@ -575,42 +493,56 @@ impl Capability { stale_time, }, )) - }; - let (cap_buf, entries) = nom::multi::many0(parse_llgr).parse(cap_buf)?; - require_empty!(cap_buf, "LongLivedGracefulRestart"); - Ok(Self::LongLivedGracefulRestart(entries)) + })?; + (rest, Self::LongLivedGracefulRestart(entries)) } - type_code => { - // Handling an unknown capability, so just parse it out and store the value - // as bytes. - Ok(Self::UnknownCapability { + type_code => ( + &[][..], + Self::UnknownCapability { type_code, - length: cap_buf.len() as u8, - value: cap_buf.to_vec(), - }) - } + value: value.to_vec(), + }, + ), + }; + if !rest.is_empty() { + return Err(fail!( + "{} octets left over in capability {type_code}", + rest.len() + )); } + Ok(capability) } - pub fn to_wire(&self, _ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - out.put_u8(self.discriminant()); - out.put_u8(self.payload_len()?); + /// Writes the capability, with its type and length, to the end of + /// `out`. + pub fn to_wire(&self, out: &mut BytesMut) -> Result<(), EncodeError> { + out.put_u8(self.type_code()); + // The length, filled in once the value is written. + let length_at = out.len(); + out.put_u8(0); + self.value_to_wire(out)?; + let length = out.len() - length_at - 1; + out[length_at] = u8::try_from(length).map_err(|_| EncodeError::TooLong { + what: "capability", + len: length, + max: u8::MAX.into(), + })?; + Ok(()) + } + + fn value_to_wire(&self, out: &mut BytesMut) -> Result<(), EncodeError> { match self { Capability::MultiProtocol { afi, safi } => { out.put_u16(afi.0); - out.put_u8(0); // Reserved byte. + out.put_u8(0); // Reserved. out.put_u8(safi.0); } - Capability::RouteRefresh => {} - Capability::OutboundRouteFilter { afi, safi, orfs } => { - out.put_u16(afi.0); - out.put_u8(0); // Reserved byte. - out.put_u8(safi.0); - // payload_len checked that there are few enough to count. - out.put_u8(orfs.len() as u8); - for orf in orfs { - out.put_u8(orf.r#type.clone().into()); - out.put_u8(orf.send_recv.clone() as u8); + Capability::RouteRefresh + | Capability::ExtendedMessage + | Capability::EnhancedRouteRefresh => {} + Capability::OutboundRouteFilter(families) => { + for family in families { + family.to_wire(out)?; } } Capability::ExtendedNextHop { entries } => { @@ -620,7 +552,6 @@ impl Capability { out.put_u16(entry.nh_afi.0); } } - Capability::ExtendedMessage => {} Capability::BgpSec { header, afi } => { out.put_u8(header.0); out.put_u16(afi.0); @@ -632,18 +563,12 @@ impl Capability { out.put_u8(entry.count); } } - Capability::BgpRole { role } => { - out.put_u8(role.into()); - } + Capability::BgpRole { role } => out.put_u8(role.into()), Capability::GracefulRestart { restart_state, restart_time, entries, } => { - let mut flags_restart_time = 0; - if *restart_state { - flags_restart_time |= 1 << 15; - } if *restart_time > 0xfff { return Err(EncodeError::TooLong { what: "graceful restart time", @@ -651,135 +576,135 @@ impl Capability { max: 0xfff, }); } - flags_restart_time |= restart_time; - out.put_u16(flags_restart_time); + out.put_u16(u16::from(*restart_state) << 15 | restart_time); for entry in entries { out.put_u16(entry.afi.0); out.put_u8(entry.safi.0); - let mut flags = 0; - if entry.preserve_forwarding { - flags |= 1 << 7; - } - out.put_u8(flags); + out.put_u8(u8::from(entry.preserve_forwarding) << 7); } } - Capability::FourByteAsn { asn } => { - out.put_u32(*asn); - } + Capability::FourByteAsn { asn } => out.put_u32(*asn), Capability::AddPath { entries } => { for entry in entries { out.put_u16(entry.afi.0); out.put_u8(entry.safi.0); - out.put_u8((&entry.send_recv).into()); + out.put_u8(entry.send_recv as u8); } } - Capability::EnhancedRouteRefresh => {} - Capability::LongLivedGracefulRestart(llgrs) => { - for llgr in llgrs { - out.put_u16(llgr.afi.0); - out.put_u8(llgr.safi.0); - out.put_u8(llgr.flags); - out.put(&llgr.stale_time.to_be_bytes()[1..]); + Capability::LongLivedGracefulRestart(entries) => { + for entry in entries { + out.put_u16(entry.afi.0); + out.put_u8(entry.safi.0); + out.put_u8(entry.flags); + out.put_slice(&entry.stale_time.to_be_bytes()[1..]); } } - Capability::UnknownCapability { value, .. } => out.put(value.as_slice()), + Capability::UnknownCapability { value, .. } => out.put_slice(value), } Ok(()) } +} - pub fn wire_len(&self, _ctx: &ParserContext) -> Result { - Ok(2 + u16::from(self.payload_len()?)) +/// One address family in an Outbound Route Filtering capability, and the +/// kinds of filter offered for it (RFC 5291 §4). +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +pub struct OrfFamily { + pub afi: Afi, + pub safi: Safi, + pub orfs: Vec, +} + +impl OrfFamily { + fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { + let (buf, (afi, safi)) = afi_reserved_safi(buf)?; + let (mut buf, count) = be_u8(buf)?; + let mut orfs = Vec::with_capacity(count.into()); + for _ in 0..count { + let (rest, orf_type) = be_u8(buf)?; + let (rest, send_recv) = be_u8(rest)?; + let send_recv = OrfSendRecv::try_from(send_recv) + .map_err(|e| fail!("Failed to parse ORF send/recv: {}", e))?; + orfs.push(OutboundRouteFilterEntry { + r#type: OrfType(orf_type), + send_recv, + }); + buf = rest; + } + Ok((buf, Self { afi, safi, orfs })) } - /// The length of the capability's value, which has to fit the one octet - /// that carries it. - fn payload_len(&self) -> Result { - let len = match self { - Capability::MultiProtocol { .. } => 4, - Capability::RouteRefresh => 0, - Capability::OutboundRouteFilter { orfs, .. } => 5 + 2 * orfs.len(), - Capability::ExtendedNextHop { entries } => 6 * entries.len(), - Capability::ExtendedMessage => 0, - Capability::BgpSec { .. } => 3, - Capability::MultiLabelCompat { entries } => 4 * entries.len(), - Capability::BgpRole { .. } => 1, - Capability::GracefulRestart { entries, .. } => 2 + 4 * entries.len(), - Capability::FourByteAsn { .. } => 4, - Capability::AddPath { entries } => 4 * entries.len(), - Capability::EnhancedRouteRefresh => 0, - Capability::LongLivedGracefulRestart(entries) => 7 * entries.len(), - Capability::UnknownCapability { value, .. } => value.len(), - }; - u8::try_from(len).map_err(|_| EncodeError::TooLong { - what: "capability", - len, + fn to_wire(&self, out: &mut BytesMut) -> Result<(), EncodeError> { + out.put_u16(self.afi.0); + out.put_u8(0); // Reserved. + out.put_u8(self.safi.0); + let count = u8::try_from(self.orfs.len()).map_err(|_| EncodeError::TooLong { + what: "ORF types of one address family", + len: self.orfs.len(), max: u8::MAX.into(), - }) + })?; + out.put_u8(count); + for orf in &self.orfs { + out.put_u8(orf.r#type.0); + out.put_u8(orf.send_recv as u8); + } + Ok(()) } } -/// Represents an entry for the OutboundRouteFilter capability. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +/// One kind of filter in an Outbound Route Filtering capability. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct OutboundRouteFilterEntry { pub r#type: OrfType, pub send_recv: OrfSendRecv, } -#[derive(Debug, Clone, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)] +/// An ORF type. Any number is one: most are unassigned or vendor-specific. +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +#[serde(transparent)] +pub struct OrfType(pub u8); + +impl OrfType { + pub const ADDRESS_PREFIX: Self = Self(64); + pub const COVERING_PREFIX: Self = Self(65); + pub const VPN_PREFIX: Self = Self(66); +} + +/// Whether a speaker sends, receives, or does both, in a capability that +/// is offered for each direction apart: ORF (RFC 5291) and ADD-PATH +/// (RFC 7911) use the same three values. +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] #[repr(u8)] -pub enum OrfType { - Reserved = 0, - Unassigned(u8), - AddressPrefix = 64, - CoveringPrefix = 65, - VpnPrefix = 66, - VendorSpecific(u8), -} - -impl From for OrfType { - fn from(value: u8) -> Self { - match value { - v if v == OrfTypeDiscriminants::Reserved as u8 => Self::Reserved, - v if (1..64).contains(&v) => Self::Unassigned(v), - v if v == OrfTypeDiscriminants::AddressPrefix as u8 => Self::AddressPrefix, - v if v == OrfTypeDiscriminants::CoveringPrefix as u8 => Self::CoveringPrefix, - v if v == OrfTypeDiscriminants::VpnPrefix as u8 => Self::VpnPrefix, - other => Self::VendorSpecific(other), - } - } -} - -impl From for u8 { - fn from(value: OrfType) -> Self { - match value { - OrfType::Reserved => OrfTypeDiscriminants::Reserved as u8, - OrfType::Unassigned(inner) => inner, - OrfType::AddressPrefix => OrfTypeDiscriminants::AddressPrefix as u8, - OrfType::CoveringPrefix => OrfTypeDiscriminants::CoveringPrefix as u8, - OrfType::VpnPrefix => OrfTypeDiscriminants::VpnPrefix as u8, - OrfType::VendorSpecific(inner) => inner, - } - } -} - -#[derive(Debug, Clone, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)] -#[repr(u8)] -pub enum OrfSendRecv { +pub enum SendReceive { Receive = 1, Send = 2, Both = 3, } -impl TryFrom for OrfSendRecv { +pub type OrfSendRecv = SendReceive; +pub type AddPathSendRecv = SendReceive; + +impl SendReceive { + /// Whether the speaker that advertised this sends. + pub fn sends(self) -> bool { + matches!(self, Self::Send | Self::Both) + } + + /// Whether the speaker that advertised this receives. + pub fn receives(self) -> bool { + matches!(self, Self::Receive | Self::Both) + } +} + +impl TryFrom for SendReceive { type Error = InvalidValue; fn try_from(value: u8) -> Result { match value { - value if value == OrfSendRecv::Receive as u8 => Ok(OrfSendRecv::Receive), - value if value == OrfSendRecv::Send as u8 => Ok(OrfSendRecv::Send), - value if value == OrfSendRecv::Both as u8 => Ok(OrfSendRecv::Both), + 1 => Ok(Self::Receive), + 2 => Ok(Self::Send), + 3 => Ok(Self::Both), other => Err(InvalidValue { - what: "ORF send/receive", + what: "send/receive", value: other.into(), }), } @@ -787,7 +712,7 @@ impl TryFrom for OrfSendRecv { } /// One address family in an ADD-PATH capability. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct AddPathEntry { pub afi: Afi, pub safi: Safi, @@ -800,57 +725,10 @@ impl AddPathEntry { } } -#[derive(Debug, Clone, Copy, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)] -#[repr(u8)] -pub enum AddPathSendRecv { - Receive = 1, - Send = 2, - Both = 3, -} - -impl AddPathSendRecv { - /// Whether the speaker that advertised this can send several routes - /// for a prefix. - pub fn sends(self) -> bool { - matches!(self, Self::Send | Self::Both) - } - - /// Whether the speaker that advertised this can receive them. - pub fn receives(self) -> bool { - matches!(self, Self::Receive | Self::Both) - } -} - -impl TryFrom for AddPathSendRecv { - type Error = InvalidValue; - - fn try_from(value: u8) -> Result { - match value { - value if value == AddPathSendRecv::Receive as u8 => Ok(AddPathSendRecv::Receive), - value if value == AddPathSendRecv::Send as u8 => Ok(AddPathSendRecv::Send), - value if value == AddPathSendRecv::Both as u8 => Ok(AddPathSendRecv::Both), - other => Err(InvalidValue { - what: "ADD-PATH send/receive", - value: other.into(), - }), - } - } -} - -impl From<&AddPathSendRecv> for u8 { - fn from(value: &AddPathSendRecv) -> Self { - match value { - AddPathSendRecv::Receive => AddPathSendRecv::Receive as u8, - AddPathSendRecv::Send => AddPathSendRecv::Send as u8, - AddPathSendRecv::Both => AddPathSendRecv::Both as u8, - } - } -} - /// `ExtendedNextHop` allows for the nexthop to be for a different address family /// than the one being used to advertise prefixes for. E.g. it allows for sending /// IPv4 prefixes with an IPv6 nexthop. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct ExtendedNextHop { /// The Address Family to update the nexthop format for. pub afi: Afi, @@ -860,25 +738,25 @@ pub struct ExtendedNextHop { pub nh_afi: Afi, } -#[derive(Debug, Clone, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)] -#[repr(u8)] +/// The role a speaker has towards its peer (RFC 9234 §4). +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] pub enum BgpRole { - Provider = 0, - RouteServer = 1, - RouteServerClient = 2, - Customer = 3, - Peer = 4, + Provider, + RouteServer, + RouteServerClient, + Customer, + Peer, Unknown(u8), } impl From for BgpRole { fn from(value: u8) -> Self { match value { - v if v == BgpRoleDiscriminants::Provider as u8 => Self::Provider, - v if v == BgpRoleDiscriminants::RouteServer as u8 => Self::RouteServer, - v if v == BgpRoleDiscriminants::RouteServerClient as u8 => Self::RouteServerClient, - v if v == BgpRoleDiscriminants::Customer as u8 => Self::Customer, - v if v == BgpRoleDiscriminants::Peer as u8 => Self::Peer, + 0 => Self::Provider, + 1 => Self::RouteServer, + 2 => Self::RouteServerClient, + 3 => Self::Customer, + 4 => Self::Peer, other => Self::Unknown(other), } } @@ -887,17 +765,17 @@ impl From for BgpRole { impl From<&BgpRole> for u8 { fn from(value: &BgpRole) -> Self { match value { - BgpRole::Provider => BgpRoleDiscriminants::Provider as u8, - BgpRole::RouteServer => BgpRoleDiscriminants::RouteServer as u8, - BgpRole::RouteServerClient => BgpRoleDiscriminants::RouteServerClient as u8, - BgpRole::Customer => BgpRoleDiscriminants::Customer as u8, - BgpRole::Peer => BgpRoleDiscriminants::Peer as u8, + BgpRole::Provider => 0, + BgpRole::RouteServer => 1, + BgpRole::RouteServerClient => 2, + BgpRole::Customer => 3, + BgpRole::Peer => 4, BgpRole::Unknown(other) => *other, } } } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct LongLivedGracefulRestart { pub afi: Afi, pub safi: Safi, @@ -908,7 +786,7 @@ pub struct LongLivedGracefulRestart { } bitfield! { - #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] + #[derive(Clone, Debug, Serialize, PartialEq, Eq)] pub struct BgpSecHeader(u8); impl new; u8, version, set_version : 3, 0; // 4 bits @@ -917,7 +795,7 @@ bitfield! { } /// Represents an entry in the BGP Graceful Restart capability. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct GracefulRestartEntry { pub afi: Afi, pub safi: Safi, @@ -936,38 +814,26 @@ mod tests { use crate::open::{ AddPathEntry, AddPathSendRecv, Capability, ExtendedNextHop, GracefulRestartEntry, - MultiLabelEntry, OpenMessage, OpenOption, + MultiLabelEntry, OpenMessage, OpenOption, OrfFamily, OrfSendRecv, OrfType, + OutboundRouteFilterEntry, }; - use bgp_primitives::{ - constants::{Afi, Safi}, - parser::ParserContext, - }; + use bgp_primitives::constants::{Afi, Safi}; macro_rules! test_capability_roundtrip { ($name:ident, $input_bytes: expr, $expected: expr) => { #[test] fn $name() -> Result<()> { - let ctx = &default_v6_context(); - let (buf, parsed) = Capability::from_wire(ctx, $input_bytes)?; + let (buf, parsed) = Capability::from_wire($input_bytes)?; assert_eq!(parsed, $expected); assert!(buf.is_empty()); let mut out = BytesMut::with_capacity(u16::MAX as usize); - parsed.to_wire(ctx, &mut out)?; + parsed.to_wire(&mut out)?; assert_eq!(out.to_vec(), $input_bytes); Ok(()) } }; } - /// Creates a default context for evaluating the test cases below. - /// This uses `four_octet_asn` set to true and address_family to IPv6. - fn default_v6_context() -> ParserContext { - ParserContext { - four_octet_asn: Some(true), - ibgp: false, - add_path: Vec::new(), - } - } test_capability_roundtrip!( test_multiptotocol, @@ -985,7 +851,6 @@ mod tests { &[0x80, 0x00], Capability::UnknownCapability { type_code: 0x80, - length: 0x00, value: vec![] } ); @@ -1058,7 +923,6 @@ mod tests { }]), OpenOption::Capabilities(vec![Capability::UnknownCapability { type_code: 0x80, - length: 0x00, value: vec![], }]), OpenOption::Capabilities(vec![Capability::RouteRefresh]), @@ -1094,11 +958,10 @@ mod tests { ], }; - let ctx = default_v6_context(); - let (_, parsed) = OpenMessage::from_wire(&ctx, buf)?; + let (_, parsed) = OpenMessage::from_wire(buf)?; assert_eq!(parsed, expected); let mut out = BytesMut::with_capacity(u16::MAX as usize); - parsed.to_wire(&ctx, &mut out)?; + parsed.to_wire(&mut out)?; assert_eq!(buf.to_vec(), out.to_vec()); Ok(()) } @@ -1110,10 +973,9 @@ mod tests { /// the session task down with it. #[test] fn multiple_labels_capability_round_trips() { - let ctx = ParserContext::default(); // code 8, length 8: (IPv4, labelled unicast, 2) and (IPv6, labelled unicast, 3) let wire = [0x08, 0x08, 0x00, 0x01, 0x04, 0x02, 0x00, 0x02, 0x04, 0x03]; - let (rest, cap) = Capability::from_wire(&ctx, &wire).unwrap(); + let (rest, cap) = Capability::from_wire(&wire).unwrap(); assert!(rest.is_empty()); assert_eq!( cap, @@ -1133,11 +995,11 @@ mod tests { } ); let mut out = BytesMut::new(); - cap.to_wire(&ctx, &mut out).unwrap(); + cap.to_wire(&mut out).unwrap(); assert_eq!(&out[..], &wire[..]); // A truncated entry is a parse error, not a panic. - assert!(Capability::from_wire(&ctx, &[0x08, 0x03, 0x00, 0x01, 0x04]).is_err()); + assert!(Capability::from_wire(&[0x08, 0x03, 0x00, 0x01, 0x04]).is_err()); } /// A peer may offer families this implementation does not support @@ -1145,7 +1007,6 @@ mod tests { /// The family is read as the numbers it is, and what follows it too. #[test] fn a_capability_may_name_any_address_family() { - let ctx = ParserContext::default(); let evpn = (Afi::L2VPN, Safi::EVPN); let samples: &[(&[u8], Capability)] = &[ ( @@ -1181,16 +1042,16 @@ mod tests { for (wire, expected) in samples { let mut input = wire.to_vec(); input.extend_from_slice(&[0x02, 0x00]); - let (rest, cap) = Capability::from_wire(&ctx, &input).unwrap(); + let (rest, cap) = Capability::from_wire(&input).unwrap(); assert_eq!(rest, [0x02, 0x00], "{expected:?}"); assert_eq!(cap, *expected); let mut out = BytesMut::new(); - cap.to_wire(&ctx, &mut out).unwrap(); + cap.to_wire(&mut out).unwrap(); assert_eq!(&out[..], *wire, "{expected:?}"); } // A capability that is simply malformed is still an error. - assert!(Capability::from_wire(&ctx, &[0x01, 0x03, 0x00, 0x01, 0x00]).is_err()); + assert!(Capability::from_wire(&[0x01, 0x03, 0x00, 0x01, 0x00]).is_err()); } /// One ADD-PATH capability lists every family it is for. Reading only @@ -1198,10 +1059,9 @@ mod tests { /// malformed, so the session never came up. #[test] fn add_path_capability_lists_several_families() { - let ctx = ParserContext::default(); // IPv4 unicast: receive. IPv6 unicast: send and receive. let wire = [0x45, 0x08, 0x00, 0x01, 0x01, 0x01, 0x00, 0x02, 0x01, 0x03]; - let (rest, cap) = Capability::from_wire(&ctx, &wire).unwrap(); + let (rest, cap) = Capability::from_wire(&wire).unwrap(); assert!(rest.is_empty()); assert_eq!( cap, @@ -1221,18 +1081,18 @@ mod tests { } ); let mut out = BytesMut::new(); - cap.to_wire(&ctx, &mut out).unwrap(); + cap.to_wire(&mut out).unwrap(); assert_eq!(&out[..], &wire[..]); // A family with a Send/Receive value nobody defined is left out; // the others stand. let odd = [0x45, 0x08, 0x00, 0x01, 0x01, 0x07, 0x00, 0x02, 0x01, 0x02]; - let (_, cap) = Capability::from_wire(&ctx, &odd).unwrap(); + let (_, cap) = Capability::from_wire(&odd).unwrap(); assert!( matches!(&cap, Capability::AddPath { entries } if entries.len() == 1 && entries[0].afi == Afi::IPV6) ); // Not a whole number of families: malformed. - assert!(Capability::from_wire(&ctx, &[0x45, 0x05, 0, 1, 1, 1, 0]).is_err()); + assert!(Capability::from_wire(&[0x45, 0x05, 0, 1, 1, 1, 0]).is_err()); } // Found by the property tests in tests/properties.rs: both were written @@ -1241,34 +1101,115 @@ mod tests { #[test] fn outbound_route_filter_capability_is_written_with_its_count() { - let ctx = ParserContext::default(); // IPv4 unicast, one ORF: address prefix (64), both directions. let wire = [0x03, 0x07, 0x00, 0x01, 0x00, 0x01, 0x01, 0x40, 0x03]; - let (rest, cap) = Capability::from_wire(&ctx, &wire).unwrap(); + let (rest, cap) = Capability::from_wire(&wire).unwrap(); assert!(rest.is_empty()); let mut out = BytesMut::new(); - cap.to_wire(&ctx, &mut out).unwrap(); + cap.to_wire(&mut out).unwrap(); assert_eq!(&out[..], &wire[..]); } + /// One ORF capability may list several address families, each with its + /// own count of ORF types (RFC 5291 §4). The second family used to be + /// read as more ORF types of the first, which made the OPEN of a router + /// offering prefix filters for IPv4 and IPv6 malformed. + #[test] + fn outbound_route_filter_capability_lists_several_families() { + #[rustfmt::skip] + let wire = [ + 0x03, 0x0e, + 0x00, 0x01, 0x00, 0x01, 0x01, 0x40, 0x03, // IPv4 unicast: prefix, both + 0x00, 0x02, 0x00, 0x01, 0x01, 0x40, 0x01, // IPv6 unicast: prefix, receive + ]; + let (rest, cap) = Capability::from_wire(&wire).unwrap(); + assert!(rest.is_empty()); + let family = |afi, send_recv| OrfFamily { + afi, + safi: Safi::UNICAST, + orfs: vec![OutboundRouteFilterEntry { + r#type: OrfType::ADDRESS_PREFIX, + send_recv, + }], + }; + assert_eq!( + cap, + Capability::OutboundRouteFilter(vec![ + family(Afi::IPV4, OrfSendRecv::Both), + family(Afi::IPV6, OrfSendRecv::Receive), + ]) + ); + let mut out = BytesMut::new(); + cap.to_wire(&mut out).unwrap(); + assert_eq!(&out[..], &wire[..]); + + // A count that promises more ORF types than there are. + assert!(Capability::from_wire(&[0x03, 0x05, 0x00, 0x01, 0x00, 0x01, 0x01]).is_err()); + } + + /// Optional parameters longer than 255 octets use the lengths of + /// RFC 9072: two octets for the total and for each parameter. + #[test] + fn long_optional_parameters_use_two_octet_lengths() { + let capabilities: Vec = (0..60) + .map(|i| Capability::UnknownCapability { + type_code: 200, + value: vec![i; 4], + }) + .collect(); + let open = OpenMessage { + version: 4, + asn: 65000, + hold_time: 90, + identifier: Ipv4Addr::new(10, 0, 0, 1), + options: vec![OpenOption::Capabilities(capabilities)], + }; + let mut out = BytesMut::new(); + open.to_wire(&mut out).unwrap(); + // 60 capabilities of 6 octets in one parameter. + #[rustfmt::skip] + assert_eq!( + &out[9..16], + [ + 255, 255, 0x01, 0x6b, // extended: 363 octets of parameters + 2, 0x01, 0x68, // capabilities, 360 octets + ] + ); + assert_eq!(out.len(), 16 + 360); + let (rest, parsed) = OpenMessage::from_wire(&out).unwrap(); + assert!(rest.is_empty()); + assert_eq!(parsed, open); + + // The same form is read when it was not needed. + #[rustfmt::skip] + let short = [ + 4, 0xfd, 0xe8, 0, 90, 10, 0, 0, 1, + 255, 255, 0, 5, + 2, 0, 2, 0x02, 0x00, + ]; + let (rest, parsed) = OpenMessage::from_wire(&short).unwrap(); + assert!(rest.is_empty()); + assert_eq!( + parsed.capabilities().collect::>(), + [&Capability::RouteRefresh] + ); + } + #[test] fn long_lived_graceful_restart_entries_are_seven_octets() { - let ctx = ParserContext::default(); // IPv4 unicast, no flags, stale time 3600 s. let wire = [0x47, 0x07, 0x00, 0x01, 0x01, 0x00, 0x00, 0x0e, 0x10]; - let (rest, cap) = Capability::from_wire(&ctx, &wire).unwrap(); + let (rest, cap) = Capability::from_wire(&wire).unwrap(); assert!(rest.is_empty()); let mut out = BytesMut::new(); - cap.to_wire(&ctx, &mut out).unwrap(); + cap.to_wire(&mut out).unwrap(); assert_eq!(&out[..], &wire[..]); - assert_eq!(cap.wire_len(&ctx).unwrap(), 9); } /// A capability's value has a one-octet length. One that does not fit /// used to be written with the length wrapped around. #[test] fn a_capability_too_long_for_its_length_octet_is_refused() { - let ctx = ParserContext::default(); let entry = ExtendedNextHop { afi: Afi::IPV4, safi: Safi::UNICAST, @@ -1278,19 +1219,17 @@ mod tests { entries: vec![entry.clone(); 42], }; let mut out = BytesMut::new(); - fits.to_wire(&ctx, &mut out).unwrap(); + fits.to_wire(&mut out).unwrap(); assert_eq!(out.len(), 2 + 252); let too_long = Capability::ExtendedNextHop { entries: vec![entry; 43], }; - assert!(too_long.wire_len(&ctx).is_err()); - assert!(too_long.to_wire(&ctx, &mut BytesMut::new()).is_err()); + assert!(too_long.to_wire(&mut BytesMut::new()).is_err()); } #[test] fn capability_parsers_leave_the_following_bytes() { - let ctx = ParserContext::default(); let trailer: &[u8] = &[0x41, 0x04, 0x00, 0x03, 0x34, 0x74]; let samples: &[(&str, &[u8])] = &[ ("multiprotocol", &[0x01, 0x04, 0x00, 0x02, 0x00, 0x01]), @@ -1315,7 +1254,7 @@ mod tests { for (name, bytes) in samples { let mut input = bytes.to_vec(); input.extend_from_slice(trailer); - let (rest, _) = Capability::from_wire(&ctx, &input) + let (rest, _) = Capability::from_wire(&input) .unwrap_or_else(|e| panic!("{name}: failed to parse: {e:?}")); assert_eq!( rest, trailer, @@ -1327,7 +1266,6 @@ mod tests { /// The capability list of a BIRD 2 OPEN, end to end. #[test] fn bird_capability_list_is_parsed_completely() { - let ctx = ParserContext::default(); #[rustfmt::skip] let param: &[u8] = &[ 0x02, 0x18, @@ -1339,7 +1277,7 @@ mod tests { 0x46, 0x00, 0x47, 0x00, ]; - let (rest, OpenOption::Capabilities(caps)) = OpenOption::from_wire(&ctx, param).unwrap(); + let (rest, OpenOption::Capabilities(caps)) = OpenOption::from_wire(false, param).unwrap(); assert!(rest.is_empty()); assert_eq!(caps.len(), 7, "{caps:?}"); assert!(caps.contains(&Capability::FourByteAsn { asn: 210036 })); diff --git a/crates/packet/src/update_decode.rs b/crates/packet/src/update_decode.rs index 34a84f5..259bc2b 100644 --- a/crates/packet/src/update_decode.rs +++ b/crates/packet/src/update_decode.rs @@ -19,16 +19,15 @@ //! be acted on, with treat-as-withdraw and attribute discard already carried //! out, together with a report of what was wrong, for the log. -use std::collections::HashSet; - -use bgp_primitives::constants::{AddressFamilyId, AfiSafi}; +use bgp_primitives::constants::{AfiSafi, IpVersion}; use bgp_primitives::ip_prefix::Nlri; -use bgp_primitives::parser::{BgpParserError, ParserContext}; +use bgp_primitives::parser::ParserContext; use crate::errors::{BgpError, DecodeError, UpdateMsgSubcode}; +use crate::message::attr_type::*; use crate::message::{ - MpUnreachNlriPathAttribute, PathAttribute, PathAttributeDiscriminants, PathAttributeFlags, - UpdateMessage, defined_flags, describe, parse_nlri, + MpUnreachNlriPathAttribute, NotAnAttribute, PathAttribute, RawAttribute, UpdateMessage, + defined_flags, describe, parse_nlri_list, split_attribute, }; /// How an error in an UPDATE was handled, short of resetting the session. @@ -70,25 +69,12 @@ pub struct DecodedUpdate { /// again. MP_REACH_NLRI and MP_UNREACH_NLRI are left out, since they /// name prefixes, and so is every attribute that was discarded. /// - /// `None` when there are no routes for them to belong to, or when the - /// bytes are not in the one form such a table keeps: on a session with - /// two-octet AS numbers. + /// `None` when the UPDATE has no such attributes, when its routes were + /// withdrawn because of an error, or when the bytes are not in the one + /// form such a table keeps: on a session with two-octet AS numbers. pub attributes_wire: Option>, } -const ORIGIN: u8 = 1; -const AS_PATH: u8 = 2; -const NEXT_HOP: u8 = 3; -const LOCAL_PREF: u8 = 5; -const ATOMIC_AGGREGATE: u8 = 6; -const AGGREGATOR: u8 = 7; -const ORIGINATOR_ID: u8 = 9; -const CLUSTER_LIST: u8 = 10; -const MP_REACH_NLRI: u8 = 14; -const MP_UNREACH_NLRI: u8 = 15; -const AS4_PATH: u8 = 17; -const AS4_AGGREGATOR: u8 = 18; - /// How a malformed value of the attribute with this type code is handled /// (RFC 7606 §7, and RFC 6793 §6 for the AS4 attributes). `None` means the /// session is reset: an error inside MP_REACH_NLRI or MP_UNREACH_NLRI leaves @@ -115,22 +101,37 @@ fn reset(subcode: UpdateMsgSubcode, data: Vec, detail: String) -> DecodeErro /// Parses a field of the message body that is nothing but prefixes. An /// error here cannot be recovered from: the prefixes are what /// treat-as-withdraw would act on. -fn prefixes(ctx: &ParserContext, field: &str, mut buf: &[u8]) -> Result, DecodeError> { +fn prefixes(ctx: &ParserContext, field: &str, buf: &[u8]) -> Result, DecodeError> { // The prefixes in the body are IPv4 unicast (RFC 4271 §4.3). let with_path_id = ctx.has_add_path(AfiSafi::IPV4_UNICAST); - let mut prefixes = Vec::new(); - while !buf.is_empty() { - let (rest, prefix) = parse_nlri(AddressFamilyId::Ipv4, with_path_id, buf).map_err(|e| { - reset( - UpdateMsgSubcode::InvalidNetworkField, - vec![], - format!("malformed {field}: {}", describe(&e)), - ) - })?; - prefixes.push(prefix); - buf = rest; + match parse_nlri_list(IpVersion::Ipv4, with_path_id, buf) { + Ok((_, prefixes)) => Ok(prefixes), + Err(e) => Err(reset( + UpdateMsgSubcode::InvalidNetworkField, + vec![], + format!("malformed {field}: {}", describe(&e)), + )), } - Ok(prefixes) +} + +/// Splits off the field at the front of `buf` that begins with its own +/// two-octet length: the field, and what follows it. The two such fields +/// frame an UPDATE; if they do not fit, there is no telling where anything +/// is (RFC 7606 §3.e). +fn length_prefixed<'a>(buf: &'a [u8], what: &str) -> Result<(&'a [u8], &'a [u8]), DecodeError> { + let framing = |detail: String| { + reset( + UpdateMsgSubcode::MalformedAttrs, + vec![], + format!("malformed UPDATE: {detail}"), + ) + }; + let Some((length, rest)) = buf.split_first_chunk::<2>() else { + return Err(framing(format!("no room for the {what} length"))); + }; + let length = usize::from(u16::from_be_bytes(*length)); + rest.split_at_checked(length) + .ok_or_else(|| framing(format!("{what} length {length} exceeds the message"))) } impl UpdateMessage { @@ -140,66 +141,42 @@ impl UpdateMessage { /// `Err` is an error that resets the session, with the NOTIFICATION to /// send. Every other error is handled here and listed in the result. pub fn decode(ctx: &ParserContext, body: &[u8]) -> Result { - // The two length fields frame the message. If they do not fit, there - // is no telling where anything is (RFC 7606 §3.e). - let framing = |detail: &str| { - reset( - UpdateMsgSubcode::MalformedAttrs, - vec![], - format!("malformed UPDATE: {detail}"), - ) - }; - let split_field = |buf: &'_ [u8], what: &str| -> Result<(usize, usize), DecodeError> { - let length = buf - .get(..2) - .map(|l| usize::from(u16::from_be_bytes([l[0], l[1]]))) - .ok_or_else(|| framing(&format!("no room for the {what} length")))?; - if buf.len() - 2 < length { - return Err(framing(&format!( - "{what} length {length} exceeds the message" - ))); - } - Ok((2, 2 + length)) - }; - let (start, end) = split_field(body, "withdrawn routes")?; - let withdrawn_buf = &body[start..end]; - let body = &body[end..]; - let (start, end) = split_field(body, "path attribute")?; - let attributes_buf = &body[start..end]; - let nlri_buf = &body[end..]; + let (withdrawn_buf, body) = length_prefixed(body, "withdrawn routes")?; + let (attributes_buf, nlri_buf) = length_prefixed(body, "path attribute")?; let withdrawn_routes = prefixes(ctx, "withdrawn routes", withdrawn_buf)?; let announced_routes = prefixes(ctx, "NLRI", nlri_buf)?; - let mut errors = Vec::new(); - let mut kept_wire = Vec::with_capacity(attributes_buf.len()); - let path_attributes = attributes(ctx, attributes_buf, &mut errors, &mut kept_wire)?; + let mut reader = AttributeReader::new(ctx, attributes_buf.len()); + reader.read_all(attributes_buf)?; + let AttributeReader { + attributes: path_attributes, + mut errors, + kept_wire, + .. + } = reader; let mut update = UpdateMessage { withdrawn_routes, path_attributes, announced_routes, }; - if !errors + let mut withdrawn = errors .iter() - .any(|e| e.handling == Handling::TreatAsWithdraw) - && let Some(missing) = missing_mandatory(ctx, &update) - { + .any(|e| e.handling == Handling::TreatAsWithdraw); + if !withdrawn && let Some(missing) = missing_mandatory(ctx, &update) { errors.push(UpdateError { handling: Handling::TreatAsWithdraw, attribute: Some(missing), detail: format!("mandatory attribute {missing} is missing"), }); + withdrawn = true; } - let withdrawn = errors - .iter() - .any(|e| e.handling == Handling::TreatAsWithdraw); if withdrawn { update = as_withdrawal(update); } let attributes_wire = - (!withdrawn && ctx.four_octet_asn == Some(true) && !kept_wire.is_empty()) - .then_some(kept_wire); + (!withdrawn && ctx.four_octet_asn && !kept_wire.is_empty()).then_some(kept_wire); Ok(DecodedUpdate { update, errors, @@ -208,86 +185,112 @@ impl UpdateMessage { } } -/// Reads the path attributes, leaving out the ones that are discarded. The -/// bytes of the ones that are kept and are a property of a route (all but -/// MP_REACH_NLRI and MP_UNREACH_NLRI) are added to `kept_wire`. -fn attributes( - ctx: &ParserContext, - mut buf: &[u8], - errors: &mut Vec, - kept_wire: &mut Vec, -) -> Result, DecodeError> { - let mut attributes = Vec::new(); - let mut seen = HashSet::new(); - let mut error = |handling, attribute: Option, detail: String| { - errors.push(UpdateError { +/// What to do with an attribute, once it is clear. +enum Verdict { + /// Use it. `route_property` is false for one that was read only to find + /// the routes it names, and is no part of the routes' attributes. + Keep { + attribute: PathAttribute, + route_property: bool, + }, + /// Leave it out, having noted why. + Skip, +} + +/// Reads the path attributes of one UPDATE, leaving out the ones that are +/// discarded and noting every error that does not reset the session. +struct AttributeReader<'a> { + ctx: &'a ParserContext, + attributes: Vec, + errors: Vec, + /// The bytes of the attributes that are kept and are a property of a + /// route: all but MP_REACH_NLRI and MP_UNREACH_NLRI. + kept_wire: Vec, + /// The type codes met so far, one bit each. + seen: [u64; 4], +} + +impl<'a> AttributeReader<'a> { + fn new(ctx: &'a ParserContext, wire_len: usize) -> Self { + Self { + ctx, + attributes: Vec::new(), + errors: Vec::new(), + kept_wire: Vec::with_capacity(wire_len), + seen: [0; 4], + } + } + + fn error(&mut self, handling: Handling, attribute: Option, detail: String) { + self.errors.push(UpdateError { handling, attribute, detail, - }) - }; + }); + } - while !buf.is_empty() { - // An attribute that does not fit in what is left of the attributes - // (RFC 7606 §4). Where the next one would start is unknown, so the - // rest is not read. Routes found so far are withdrawn. - let header = match buf { - [flags, type_code, rest @ ..] => { - let flags = PathAttributeFlags(*flags); - match (flags.extended_length(), rest) { - (true, [hi, lo, rest @ ..]) => Some(( - flags, - *type_code, - usize::from(u16::from_be_bytes([*hi, *lo])), - rest, - )), - (false, [length, rest @ ..]) => { - Some((flags, *type_code, usize::from(*length), rest)) - } - _ => None, + /// Notes that `type_code` has been met, and returns whether it had + /// been already. + fn seen_before(&mut self, type_code: u8) -> bool { + let (word, bit) = (usize::from(type_code / 64), 1u64 << (type_code % 64)); + let before = self.seen[word] & bit != 0; + self.seen[word] |= bit; + before + } + + fn read_all(&mut self, mut buf: &[u8]) -> Result<(), DecodeError> { + while !buf.is_empty() { + let (raw, rest) = match split_attribute(buf) { + Ok(split) => split, + Err(e) => { + // An attribute that does not fit in what is left of the + // attributes (RFC 7606 §4). Where the next one would + // start is unknown, so the rest is not read. Routes + // found so far are withdrawn. + let attribute = match e { + NotAnAttribute::NoHeader => None, + NotAnAttribute::Truncated { type_code, .. } => Some(type_code), + }; + self.error(Handling::TreatAsWithdraw, attribute, e.to_string()); + return Ok(()); } + }; + buf = rest; + if let Verdict::Keep { + attribute, + route_property, + } = self.judge(&raw)? + { + if route_property { + self.kept_wire.extend_from_slice(raw.wire); + } + self.attributes.push(attribute); } - _ => None, - }; - let Some((flags, type_code, length, rest)) = header else { - error( - Handling::TreatAsWithdraw, - None, - format!("{} octets are too few for an attribute", buf.len()), - ); - break; - }; - if rest.len() < length { - error( - Handling::TreatAsWithdraw, - Some(type_code), - format!( - "attribute {type_code} has length {length} but only {} octets remain", - rest.len() - ), - ); - break; } - let (value, rest) = rest.split_at(length); - let attribute_wire = &buf[..buf.len() - rest.len()]; - buf = rest; + Ok(()) + } + + /// Decides what becomes of one attribute. `Err` resets the session. + fn judge(&mut self, raw: &RawAttribute<'_>) -> Result { + let (flags, type_code, wire) = (&raw.flags, raw.type_code, raw.wire); + let names_routes = matches!(type_code, MP_REACH_NLRI | MP_UNREACH_NLRI); // RFC 7606 §3.g: a second MP_REACH_NLRI or MP_UNREACH_NLRI makes it // unclear which routes are meant; a second anything else is ignored. - if !seen.insert(type_code) { - if matches!(type_code, MP_REACH_NLRI | MP_UNREACH_NLRI) { + if self.seen_before(type_code) { + if names_routes { return Err(reset( UpdateMsgSubcode::MalformedAttrs, vec![], format!("attribute {type_code} appears more than once"), )); } - error( + self.error( Handling::AttributeDiscard, Some(type_code), format!("attribute {type_code} appears more than once; kept the first"), ); - continue; + return Ok(Verdict::Skip); } let Some((optional, transitive)) = defined_flags(type_code) else { @@ -297,23 +300,20 @@ fn attributes( if !flags.optional() { return Err(reset( UpdateMsgSubcode::UnrecognizedWellKnownAttr, - attribute_wire.to_vec(), + wire.to_vec(), format!("unrecognised well-known attribute {type_code}"), )); } - attributes.push(PathAttribute::UnknownPathAttribute { - flags, - type_code, - payload: value.to_vec(), + return Ok(Verdict::Keep { + attribute: opaque(raw), + route_property: true, }); - kept_wire.extend_from_slice(attribute_wire); - continue; }; let flags_are_wrong = flags.optional() != optional || flags.transitive() != transitive; if flags_are_wrong { // RFC 7606 §3.c. The value is still read below if it is one that // names routes, since those have to be withdrawn. - error( + self.error( Handling::TreatAsWithdraw, Some(type_code), format!( @@ -321,99 +321,89 @@ fn attributes( flags.0 ), ); - if !matches!(type_code, MP_REACH_NLRI | MP_UNREACH_NLRI) { - continue; + if !names_routes { + return Ok(Verdict::Skip); } } // Attributes that only mean something between internal peers are // dropped when an external one sends them (RFC 7606 §7.5, §7.9, // §7.10), well formed or not. - if !ctx.ibgp && matches!(type_code, LOCAL_PREF | ORIGINATOR_ID | CLUSTER_LIST) { - error( + if !self.ctx.ibgp && matches!(type_code, LOCAL_PREF | ORIGINATOR_ID | CLUSTER_LIST) { + self.error( Handling::AttributeDiscard, Some(type_code), format!("attribute {type_code} from an external peer"), ); - continue; + return Ok(Verdict::Skip); } // A speaker with four-octet AS numbers has no use for AS4_PATH and // AS4_AGGREGATOR and must not send them to another (RFC 6793 §6). - if ctx.four_octet_asn == Some(true) && matches!(type_code, AS4_PATH | AS4_AGGREGATOR) { - error( + if self.ctx.four_octet_asn && matches!(type_code, AS4_PATH | AS4_AGGREGATOR) { + self.error( Handling::AttributeDiscard, Some(type_code), format!("attribute {type_code} on a session with four-octet AS numbers"), ); - continue; + return Ok(Verdict::Skip); } - let parsed = match type_code { - // Known by their definition but not modelled: kept as bytes, - // once their length has been checked. - ORIGINATOR_ID | CLUSTER_LIST | AS4_AGGREGATOR => { - let well_formed = match type_code { - ORIGINATOR_ID => value.len() == 4, - CLUSTER_LIST => !value.is_empty() && value.len().is_multiple_of(4), - _ => value.len() == 8, - }; - if well_formed { - Ok(PathAttribute::UnknownPathAttribute { - flags, - type_code, - payload: value.to_vec(), - }) - } else { - Err(format!("a length of {} is not valid", value.len())) - } - } - _ => { - PathAttribute::parse_value(ctx, PathAttributeDiscriminants::from(type_code), value) - .map_err(|e: nom::Err>| describe(&e)) - } - }; - match parsed { - Ok(attribute) => { - // With wrong flags it was read only to find its routes. - if !flags_are_wrong && !matches!(type_code, MP_REACH_NLRI | MP_UNREACH_NLRI) { - kept_wire.extend_from_slice(attribute_wire); - } - attributes.push(attribute); - } - Err(detail) => match handling_of_malformed(type_code) { - Some(handling) => error( - handling, - Some(type_code), - format!("malformed attribute {type_code}: {detail}"), - ), - None => { - return Err(reset( + match self.parse(raw) { + Ok(attribute) => Ok(Verdict::Keep { + attribute, + route_property: !flags_are_wrong && !names_routes, + }), + Err(detail) => { + let detail = format!("malformed attribute {type_code}: {detail}"); + match handling_of_malformed(type_code) { + Some(handling) => { + self.error(handling, Some(type_code), detail); + Ok(Verdict::Skip) + } + None => Err(reset( UpdateMsgSubcode::OptionalAttribute, - attribute_wire.to_vec(), - format!("malformed attribute {type_code}: {detail}"), - )); + wire.to_vec(), + detail, + )), } - }, + } } } - Ok(attributes) + + /// Reads the value of an attribute whose definition is known. The error + /// is what was wrong with it, as text. + fn parse(&self, raw: &RawAttribute<'_>) -> Result { + let value = raw.value; + // Known by their definition but not modelled: kept as bytes, once + // their length has been checked. + let length_is_valid = match raw.type_code { + ORIGINATOR_ID => Some(value.len() == 4), + CLUSTER_LIST => Some(!value.is_empty() && value.len().is_multiple_of(4)), + AS4_AGGREGATOR => Some(value.len() == 8), + _ => None, + }; + match length_is_valid { + Some(true) => Ok(opaque(raw)), + Some(false) => Err(format!("a length of {} is not valid", value.len())), + None => PathAttribute::parse_value(self.ctx, raw.flags.clone(), raw.type_code, value) + .map_err(|e| describe(&e)), + } + } +} + +/// The attribute kept as bytes. +fn opaque(raw: &RawAttribute<'_>) -> PathAttribute { + PathAttribute::UnknownPathAttribute { + flags: raw.flags.clone(), + type_code: raw.type_code, + payload: raw.value.to_vec(), + } } /// The type code of a mandatory attribute that an UPDATE announcing routes /// lacks (RFC 4271 §5, RFC 4760 §3, RFC 7606 §3.d). An UPDATE that only /// withdraws needs no attributes. fn missing_mandatory(ctx: &ParserContext, update: &UpdateMessage) -> Option { - let has = |type_code: u8| { - update.path_attributes.iter().any(|a| { - matches!( - (type_code, a), - (ORIGIN, PathAttribute::Origin(_)) - | (AS_PATH, PathAttribute::AsPath(_)) - | (NEXT_HOP, PathAttribute::NextHop(_)) - | (LOCAL_PREF, PathAttribute::LocalPref(_)) - ) - }) - }; let in_body = !update.announced_routes.is_empty(); let in_mp_reach = update .path_attributes @@ -422,15 +412,24 @@ fn missing_mandatory(ctx: &ParserContext, update: &UpdateMessage) -> Option if !in_body && !in_mp_reach { return None; } - let mut required = vec![ORIGIN, AS_PATH]; - if in_body { - // Routes in MP_REACH_NLRI carry their next hop there. - required.push(NEXT_HOP); - } - if ctx.ibgp { - required.push(LOCAL_PREF); - } - required.into_iter().find(|type_code| !has(*type_code)) + // Routes in MP_REACH_NLRI carry their next hop there; LOCAL_PREF is + // for internal peers. + let required = [ + (ORIGIN, true), + (AS_PATH, true), + (NEXT_HOP, in_body), + (LOCAL_PREF, ctx.ibgp), + ]; + required + .into_iter() + .filter(|(_, needed)| *needed) + .map(|(type_code, _)| type_code) + .find(|type_code| { + !update + .path_attributes + .iter() + .any(|attribute| attribute.type_code() == *type_code) + }) } /// `update` with every route it announces withdrawn instead, and no other @@ -440,21 +439,15 @@ fn as_withdrawal(update: UpdateMessage) -> UpdateMessage { withdrawn_routes.extend(update.announced_routes); let mut unreach: Vec = Vec::new(); - let mut withdraw = |afi, safi, prefixes: Vec| match unreach - .iter_mut() - .find(|u| u.afi == afi && u.safi == safi) - { - Some(existing) => existing.prefixes.extend(prefixes), - None => unreach.push(MpUnreachNlriPathAttribute { - afi, - safi, - prefixes, - }), - }; + let mut withdraw = + |family, prefixes: Vec| match unreach.iter_mut().find(|u| u.family == family) { + Some(existing) => existing.prefixes.extend(prefixes), + None => unreach.push(MpUnreachNlriPathAttribute { family, prefixes }), + }; for attribute in update.path_attributes { match attribute { - PathAttribute::MpUnreachNlri(mp) => withdraw(mp.afi, mp.safi, mp.prefixes), - PathAttribute::MpReachNlri(mp) => withdraw(mp.afi, mp.safi, mp.prefixes), + PathAttribute::MpUnreachNlri(mp) => withdraw(mp.family, mp.prefixes), + PathAttribute::MpReachNlri(mp) => withdraw(mp.family, mp.prefixes), _ => {} } } @@ -472,7 +465,7 @@ fn as_withdrawal(update: UpdateMessage) -> UpdateMessage { mod tests { use std::net::Ipv4Addr; - use bgp_primitives::constants::{Afi, Safi}; + use crate::message::PathAttributeFlags; use bgp_primitives::ip_prefix::IpPrefix; use super::*; @@ -492,7 +485,7 @@ mod tests { fn ebgp() -> ParserContext { ParserContext { - four_octet_asn: Some(true), + four_octet_asn: true, ibgp: false, add_path: Vec::new(), } @@ -500,7 +493,7 @@ mod tests { fn ibgp() -> ParserContext { ParserContext { - four_octet_asn: Some(true), + four_octet_asn: true, ibgp: true, add_path: Vec::new(), } @@ -619,7 +612,7 @@ mod tests { // Nor on a session with two-octet AS numbers, where the bytes are // not in the form a table keeps. let two_octet = ParserContext { - four_octet_asn: Some(false), + four_octet_asn: false, ibgp: false, add_path: Vec::new(), }; @@ -851,7 +844,7 @@ mod tests { #[test] fn as4_path_is_read_from_a_two_octet_peer_and_dropped_if_malformed() { let ctx = ParserContext { - four_octet_asn: Some(false), + four_octet_asn: false, ibgp: false, add_path: Vec::new(), }; @@ -911,8 +904,7 @@ mod tests { assert_eq!( decoded.update.path_attributes, [PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, prefixes: vec![v6.into()], })], "{case}" @@ -925,7 +917,7 @@ mod tests { assert!(decoded.errors.is_empty()); assert!(matches!( &decoded.update.path_attributes[0], - PathAttribute::MpReachNlri(mp) if mp.family() == AfiSafi::IPV6_UNICAST + PathAttribute::MpReachNlri(mp) if mp.family == AfiSafi::IPV6_UNICAST )); withdrawn( diff --git a/crates/packet/tests/properties.rs b/crates/packet/tests/properties.rs index be11ea9..3117dd4 100644 --- a/crates/packet/tests/properties.rs +++ b/crates/packet/tests/properties.rs @@ -3,7 +3,7 @@ //! Two things are checked for every kind of message: //! //! - **Round trip.** A message that can be written can be read back, comes -//! back equal, and is exactly as long as `wire_len` said. +//! back equal. //! - **Hostile input.** Whatever bytes arrive, decoding returns a message or //! an error: it never panics. What it does accept can be shown, written //! again, and read back as the same message. @@ -27,9 +27,9 @@ use bgp_packet::message::{ use bgp_packet::open::{ AddPathEntry, AddPathSendRecv, BgpRole, BgpSecHeader, Capability, ExtendedNextHop, GracefulRestartEntry, LongLivedGracefulRestart, MultiLabelEntry, OpenMessage, OpenOption, - OrfSendRecv, OrfType, OutboundRouteFilterEntry, + OrfFamily, OrfSendRecv, OrfType, OutboundRouteFilterEntry, }; -use bgp_primitives::constants::{AddressFamilyId, Afi, AfiSafi, Safi}; +use bgp_primitives::constants::{Afi, AfiSafi, IpVersion, Safi}; use bgp_primitives::ip_prefix::{IpPrefix, Nlri}; use bgp_primitives::parser::{EncodeError, ParserContext}; use bytes::BytesMut; @@ -49,7 +49,7 @@ struct Session { impl Session { fn ctx(self) -> ParserContext { ParserContext { - four_octet_asn: Some(self.four_octet_asn), + four_octet_asn: self.four_octet_asn, ibgp: self.ibgp, add_path: [AfiSafi::IPV4_UNICAST, AfiSafi::IPV6_UNICAST] .into_iter() @@ -59,11 +59,11 @@ impl Session { } } - fn has_add_path(self, version: AddressFamilyId, safi: Safi) -> bool { + fn has_add_path(self, version: IpVersion, safi: Safi) -> bool { safi == Safi::UNICAST && match version { - AddressFamilyId::Ipv4 => self.add_path[0], - AddressFamilyId::Ipv6 => self.add_path[1], + IpVersion::Ipv4 => self.add_path[0], + IpVersion::Ipv6 => self.add_path[1], } } } @@ -80,10 +80,10 @@ fn session() -> impl Strategy { // Generators // --------------------------------------------------------------------------- -fn prefix(version: AddressFamilyId) -> impl Strategy { +fn prefix(version: IpVersion) -> impl Strategy { let max_len = match version { - AddressFamilyId::Ipv4 => 32u8, - AddressFamilyId::Ipv6 => 128, + IpVersion::Ipv4 => 32u8, + IpVersion::Ipv6 => 128, }; (0..=max_len, any::<[u8; 16]>()).prop_map(move |(length, bytes)| { IpPrefix::new(version, bytes, length).expect("16 bytes are enough for any prefix") @@ -92,11 +92,7 @@ fn prefix(version: AddressFamilyId) -> impl Strategy { /// Prefixes as an UPDATE names them. Without ADD-PATH for the family there /// is no identifier on the wire, and it reads back as zero. -fn nlri( - session: Session, - version: AddressFamilyId, - safi: Safi, -) -> impl Strategy> { +fn nlri(session: Session, version: IpVersion, safi: Safi) -> impl Strategy> { let with_path_id = session.has_add_path(version, safi); vec( (prefix(version), any::()).prop_map(move |(prefix, path_id)| Nlri { @@ -135,14 +131,8 @@ fn as_path(asn: BoxedStrategy) -> impl Strategy { }) } -/// Extended communities are classified by their type octets when read, so -/// they are generated from the eight octets they are on the wire. fn extended_community() -> impl Strategy { - any::<[u8; 8]>().prop_map(|bytes| { - ExtendedCommunity::from_wire(&ParserContext::default(), &bytes) - .expect("any eight octets are an extended community") - .1 - }) + any::<[u8; 8]>().prop_map(ExtendedCommunity) } fn mp_reach(session: Session) -> impl Strategy { @@ -152,12 +142,11 @@ fn mp_reach(session: Session) -> impl Strategy ( Just(safi), any::<[u8; 4]>(), - nlri(session, AddressFamilyId::Ipv4, safi), + nlri(session, IpVersion::Ipv4, safi), ) }) .prop_map(|(safi, next_hop, prefixes)| MpReachNlriPathAttribute { - afi: Afi::IPV4, - safi, + family: AfiSafi::new(Afi::IPV4, safi), next_hop: NlriNextHop::Ipv4(Ipv4Addr::from(next_hop)), prefixes, }); @@ -177,12 +166,11 @@ fn mp_reach(session: Session) -> impl Strategy ( Just(safi), v6_next_hop(), - nlri(session, AddressFamilyId::Ipv6, safi), + nlri(session, IpVersion::Ipv6, safi), ) }) .prop_map(|(safi, next_hop, prefixes)| MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi, + family: AfiSafi::new(Afi::IPV6, safi), next_hop, prefixes, }); @@ -190,14 +178,13 @@ fn mp_reach(session: Session) -> impl Strategy } fn mp_unreach(session: Session) -> impl Strategy { - let of = move |version: AddressFamilyId| { + let of = move |version: IpVersion| { nlri(session, version, Safi::UNICAST).prop_map(move |prefixes| MpUnreachNlriPathAttribute { - afi: version.into(), - safi: Safi::UNICAST, + family: AfiSafi::new(version.into(), Safi::UNICAST), prefixes, }) }; - prop_oneof![of(AddressFamilyId::Ipv4), of(AddressFamilyId::Ipv6)] + prop_oneof![of(IpVersion::Ipv4), of(IpVersion::Ipv6)] } /// Type codes whose definition the decoder knows; an unknown attribute has @@ -331,9 +318,9 @@ fn path_attributes(session: Session) -> impl Strategy fn update(session: Session) -> impl Strategy { ( - nlri(session, AddressFamilyId::Ipv4, Safi::UNICAST), + nlri(session, IpVersion::Ipv4, Safi::UNICAST), path_attributes(session), - nlri(session, AddressFamilyId::Ipv4, Safi::UNICAST), + nlri(session, IpVersion::Ipv4, Safi::UNICAST), ) .prop_map( |(withdrawn_routes, path_attributes, announced_routes)| UpdateMessage { @@ -349,7 +336,7 @@ const KNOWN_CAPABILITIES: [u8; 13] = [1, 2, 3, 5, 6, 7, 8, 9, 64, 65, 69, 70, 71 fn capability() -> impl Strategy { let orf = ( - any::().prop_map(OrfType::from), + any::().prop_map(OrfType), prop_oneof![ Just(OrfSendRecv::Receive), Just(OrfSendRecv::Send), @@ -365,8 +352,14 @@ fn capability() -> impl Strategy { prop_oneof![ (afi(), safi()).prop_map(|(afi, safi)| Capability::MultiProtocol { afi, safi }), Just(Capability::RouteRefresh), - (afi(), safi(), vec(orf, 0..4)) - .prop_map(|(afi, safi, orfs)| Capability::OutboundRouteFilter { afi, safi, orfs }), + vec((afi(), safi(), vec(orf, 0..4)), 0..3).prop_map(|families| { + Capability::OutboundRouteFilter( + families + .into_iter() + .map(|(afi, safi, orfs)| OrfFamily { afi, safi, orfs }) + .collect(), + ) + }), vec((afi(), safi(), afi()), 1..4).prop_map(|entries| Capability::ExtendedNextHop { entries: entries .into_iter() @@ -439,11 +432,7 @@ fn capability() -> impl Strategy { }), vec(any::(), 0..20) ) - .prop_map(|(type_code, value)| Capability::UnknownCapability { - type_code, - length: value.len() as u8, - value, - }), + .prop_map(|(type_code, value)| Capability::UnknownCapability { type_code, value }), ] } @@ -466,14 +455,13 @@ fn open() -> impl Strategy { } fn notification() -> impl Strategy { - (1u8..=8, 0u8..12, vec(any::(), 0..40)).prop_filter_map( - "an error code and subcode that are defined", - |(code, subcode, data)| { - BgpError::from_code_subcode(code, subcode) - .ok() - .map(|error| NotificationMessage { error, data }) - }, - ) + // Any code and subcode: a peer may send ones we have no name for. + (any::(), any::(), vec(any::(), 0..40)).prop_map(|(code, subcode, data)| { + NotificationMessage { + error: BgpError::from_code_subcode(code, subcode), + data, + } + }) } fn message(session: Session) -> impl Strategy { @@ -540,7 +528,6 @@ fn check_decoding(ctx: &ParserContext, wire: &[u8]) -> Result<(), TestCaseError> ))); } } - prop_assert_eq!(usize::from(msg.wire_len(ctx).unwrap()), rewritten.len()); if repairs.is_empty() { let again = Message::decode_with_report(ctx, &rewritten) .map_err(|e| TestCaseError::fail(format!("written but cannot be read: {e}")))?; @@ -551,15 +538,13 @@ fn check_decoding(ctx: &ParserContext, wire: &[u8]) -> Result<(), TestCaseError> } /// Writes a generated message. `None` when it is too big for one BGP -/// message, which the generators can produce and the encoder must refuse; -/// any other refusal is a bug, since everything generated is meant to fit -/// the session. -fn written(ctx: &ParserContext, msg: &Message) -> Result, TestCaseError> { +/// message, or some part of it for its length field, which the generators +/// can produce and the encoder must refuse. +fn written(ctx: &ParserContext, msg: &Message) -> Option { let mut wire = BytesMut::new(); match msg.to_wire(ctx, &mut wire) { - Ok(()) => Ok(Some(wire)), - Err(EncodeError::TooLong { .. }) => Ok(None), - Err(e) => Err(TestCaseError::fail(format!("cannot be written: {e}"))), + Ok(()) => Some(wire), + Err(EncodeError::TooLong { .. }) => None, } } @@ -638,18 +623,14 @@ proptest! { #[test] fn a_message_that_can_be_written_reads_back_equal((session, msg) in session_and_message()) { let ctx = session.ctx(); - let Some(wire) = written(&ctx, &msg)? else { + let Some(wire) = written(&ctx, &msg) else { return Ok(()); }; - prop_assert_eq!(usize::from(msg.wire_len(&ctx).unwrap()), wire.len()); - let decoded = Message::decode_with_report(&ctx, &wire) .map_err(|e| TestCaseError::fail(format!("{e}")))?; // What we write is well formed: nothing for the decoder to repair. prop_assert!(decoded.update_errors.is_empty(), "{:?}", decoded.update_errors); prop_assert_eq!(&decoded.message, &msg); - let (rest, _) = Message::from_wire(&ctx, &wire).unwrap(); - prop_assert!(rest.is_empty()); } #[test] @@ -672,7 +653,7 @@ proptest! { damage in vec(damage(), 1..4), ) { let ctx = session.ctx(); - let Some(wire) = written(&ctx, &msg)? else { + let Some(wire) = written(&ctx, &msg) else { return Ok(()); }; check_decoding(&ctx, &damaged(&wire, &damage))?; @@ -685,11 +666,9 @@ proptest! { bytes in vec(any::(), 0..120), ) { let ctx = session.ctx(); - let _ = OpenMessage::from_wire(&ctx, &bytes); - let _ = OpenOption::from_wire(&ctx, &bytes); - let _ = Capability::from_wire(&ctx, &bytes); + let _ = OpenMessage::from_wire(&bytes); + let _ = Capability::from_wire(&bytes); let _ = UpdateMessage::decode(&ctx, &bytes); let _ = PathAttribute::from_wire(&ctx, &bytes); - let _ = NotificationMessage::from_wire(&ctx, &bytes); } } diff --git a/crates/primitives/src/constants.rs b/crates/primitives/src/constants.rs index 7141aaf..337f8d5 100644 --- a/crates/primitives/src/constants.rs +++ b/crates/primitives/src/constants.rs @@ -1,7 +1,7 @@ use std::fmt::Display; -use serde::{Deserialize, Serialize}; -use serde_repr::{Deserialize_repr, Serialize_repr}; +use serde::Serialize; +use serde_repr::Serialize_repr; use thiserror::Error; /// An address family number this implementation has no name for. @@ -11,14 +11,14 @@ pub struct UnknownAfi(pub u16); /// The IP version of a prefix. For the address family field of a capability /// or an MP_REACH_NLRI, which can name families that are not IP, see [`Afi`]. -#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Serialize_repr, Deserialize_repr)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Serialize_repr)] #[repr(u16)] -pub enum AddressFamilyId { +pub enum IpVersion { Ipv4 = 1, Ipv6 = 2, } -impl AddressFamilyId { +impl IpVersion { /// How many bits an address of this version has. pub fn bits(self) -> u8 { match self { @@ -28,7 +28,7 @@ impl AddressFamilyId { } } -impl TryFrom for AddressFamilyId { +impl TryFrom for IpVersion { type Error = UnknownAfi; fn try_from(value: u16) -> Result { @@ -40,11 +40,11 @@ impl TryFrom for AddressFamilyId { } } -impl Display for AddressFamilyId { +impl Display for IpVersion { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - AddressFamilyId::Ipv4 => write!(f, "Ipv4"), - AddressFamilyId::Ipv6 => write!(f, "Ipv6"), + IpVersion::Ipv4 => write!(f, "Ipv4"), + IpVersion::Ipv6 => write!(f, "Ipv6"), } } } @@ -55,7 +55,7 @@ impl Display for AddressFamilyId { /// has never heard of, and that must be readable so it can be ignored. Which /// families are *supported* is a separate question, answered where routes /// are stored. -#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize)] #[serde(transparent)] pub struct Afi(pub u16); @@ -66,13 +66,13 @@ impl Afi { pub const BGP_LS: Self = Self(16388); } -impl From for Afi { - fn from(version: AddressFamilyId) -> Self { +impl From for Afi { + fn from(version: IpVersion) -> Self { Self(version as u16) } } -impl TryFrom for AddressFamilyId { +impl TryFrom for IpVersion { type Error = UnknownAfi; fn try_from(afi: Afi) -> Result { @@ -82,7 +82,7 @@ impl TryFrom for AddressFamilyId { /// A Subsequent Address Family Identifier as it appears on the wire. Like /// [`Afi`], any number is a valid value. -#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize)] #[serde(transparent)] pub struct Safi(pub u8); @@ -99,7 +99,7 @@ impl Safi { /// An address family: the pair that says what kind of routes a capability /// or an MP_REACH_NLRI is about. -#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize)] pub struct AfiSafi { pub afi: Afi, pub safi: Safi, @@ -114,7 +114,7 @@ impl AfiSafi { } /// The unicast family of an IP version. - pub fn unicast(version: AddressFamilyId) -> Self { + pub fn unicast(version: IpVersion) -> Self { Self::new(version.into(), Safi::UNICAST) } @@ -162,19 +162,13 @@ mod tests { AfiSafi::new(Afi(999), Safi(7)).to_string(), "AFI 999 SAFI 7" ); - assert_eq!( - AfiSafi::unicast(AddressFamilyId::Ipv4), - AfiSafi::IPV4_UNICAST - ); + assert_eq!(AfiSafi::unicast(IpVersion::Ipv4), AfiSafi::IPV4_UNICAST); } #[test] fn only_ipv4_and_ipv6_are_ip_versions() { - assert_eq!( - AddressFamilyId::try_from(Afi::IPV6), - Ok(AddressFamilyId::Ipv6) - ); - assert_eq!(AddressFamilyId::try_from(Afi::L2VPN), Err(UnknownAfi(25))); - assert_eq!(AddressFamilyId::try_from(3u16), Err(UnknownAfi(3))); + assert_eq!(IpVersion::try_from(Afi::IPV6), Ok(IpVersion::Ipv6)); + assert_eq!(IpVersion::try_from(Afi::L2VPN), Err(UnknownAfi(25))); + assert_eq!(IpVersion::try_from(3u16), Err(UnknownAfi(3))); } } diff --git a/crates/primitives/src/ip_prefix.rs b/crates/primitives/src/ip_prefix.rs index 620532a..8080a1f 100644 --- a/crates/primitives/src/ip_prefix.rs +++ b/crates/primitives/src/ip_prefix.rs @@ -2,12 +2,10 @@ use std::fmt::Display; use std::net::{Ipv4Addr, Ipv6Addr}; use std::str::FromStr; -use bytes::{Buf, BufMut, BytesMut}; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use thiserror::Error; -use crate::constants::AddressFamilyId; -use crate::parser::{EncodeError, ParserContext}; +use crate::constants::IpVersion; /// Why something is not a valid prefix. #[derive(Debug, Clone, PartialEq, Eq, Error)] @@ -33,9 +31,9 @@ pub enum PrefixError { /// value. A prefix has one representation: the bits past its length are /// always zero, so two prefixes are equal exactly when they are the same /// prefix. -#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Serialize)] pub struct IpPrefix { - address_family: AddressFamilyId, + address_family: IpVersion, length: u8, /// The address, left-aligned, zero past `length` bits. address: [u8; 16], @@ -46,7 +44,7 @@ impl IpPrefix { /// hold at least the bytes that `length` bits cover; anything past /// those bits is ignored (RFC 4271 §4.3: such bits are irrelevant). pub fn new( - address_family: AddressFamilyId, + address_family: IpVersion, prefix: impl AsRef<[u8]>, length: u8, ) -> Result { @@ -75,7 +73,7 @@ impl IpPrefix { }) } - pub fn address_family(&self) -> AddressFamilyId { + pub fn address_family(&self) -> IpVersion { self.address_family } @@ -108,20 +106,6 @@ impl IpPrefix { let mask = u8::MAX << (8 - rem_bits); (self.address[full_bytes] & mask) == (other.address[full_bytes] & mask) } - - pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - let bytes = self.bytes(); - // Verify that there is enough space to write the IpPrefix. - if out.remaining() < (bytes.len() + 1) { - return Err(EncodeError::OutBufferOverflow); - } - - // Write length and prefix. - out.put_u8(self.length); - out.put(bytes); - - Ok(()) - } } /// A prefix as an UPDATE names it: the prefix and, on a session where @@ -130,7 +114,7 @@ impl IpPrefix { /// /// Whether the identifier is on the wire is a matter of what the session /// negotiated, not of its value. Where it is not, it is zero here. -#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Serialize)] pub struct Nlri { pub prefix: IpPrefix, pub path_id: u32, @@ -181,10 +165,10 @@ impl TryFrom<&str> for IpPrefix { let (afi, octets) = if addr.contains(':') { let addr = Ipv6Addr::from_str(addr).map_err(|_| address())?; - (AddressFamilyId::Ipv6, addr.octets().to_vec()) + (IpVersion::Ipv6, addr.octets().to_vec()) } else { let addr = Ipv4Addr::from_str(addr).map_err(|_| address())?; - (AddressFamilyId::Ipv4, addr.octets().to_vec()) + (IpVersion::Ipv4, addr.octets().to_vec()) }; IpPrefix::new(afi, octets, length) } @@ -193,11 +177,11 @@ impl TryFrom<&str> for IpPrefix { impl Display for IpPrefix { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self.address_family { - AddressFamilyId::Ipv4 => { + IpVersion::Ipv4 => { let [a, b, c, d, ..] = self.address; write!(f, "{}/{}", Ipv4Addr::new(a, b, c, d), self.length) } - AddressFamilyId::Ipv6 => { + IpVersion::Ipv6 => { write!(f, "{}/{}", Ipv6Addr::from(self.address), self.length) } } @@ -206,7 +190,7 @@ impl Display for IpPrefix { #[cfg(test)] mod tests { - use crate::constants::AddressFamilyId; + use crate::constants::IpVersion; use proptest::prelude::*; @@ -231,7 +215,7 @@ mod tests { verify_roundtrip!( verify_roundtrip_ipv4_24, "10.1.2.0/24", - AddressFamilyId::Ipv4, + IpVersion::Ipv4, vec![10, 1, 2], 24 ); @@ -240,7 +224,7 @@ mod tests { verify_roundtrip!( verify_roundtrip_ipv4_19, "10.245.123.0/19", - AddressFamilyId::Ipv4, + IpVersion::Ipv4, vec![10, 245, 96], 19 ); @@ -249,7 +233,7 @@ mod tests { verify_roundtrip!( verify_roundtrip_ipv4_3, "192.168.1.0/3", - AddressFamilyId::Ipv4, + IpVersion::Ipv4, vec![192], 3 ); @@ -258,7 +242,7 @@ mod tests { verify_roundtrip!( verify_roundtrip_ipv4_0, "0.0.0.0/0", - AddressFamilyId::Ipv4, + IpVersion::Ipv4, vec![], 0 ); @@ -266,7 +250,7 @@ mod tests { verify_roundtrip!( verify_roundtrip_ipv6_48, "2001:db8:cafe::/48", - AddressFamilyId::Ipv6, + IpVersion::Ipv6, vec![32, 1, 13, 184, 202, 254], 48 ); @@ -275,7 +259,7 @@ mod tests { verify_roundtrip!( verify_roundtrip_ipv6_32, "2001:db8:cafe::/32", - AddressFamilyId::Ipv6, + IpVersion::Ipv6, vec![32, 1, 13, 184], 32 ); @@ -307,7 +291,7 @@ mod tests { assert!(IpPrefix::try_from("0.0.0.0/0").is_ok()); assert!(IpPrefix::try_from("2001:db8::1/128").is_ok()); assert_eq!( - IpPrefix::new(AddressFamilyId::Ipv4, vec![10], 16), + IpPrefix::new(IpVersion::Ipv4, vec![10], 16), Err(PrefixError::TooFewBytes { length: 16, needed: 2, @@ -319,11 +303,11 @@ mod tests { #[test] fn a_prefix_has_one_representation() { // Extra bytes and stray bits past the length are dropped. - let noisy = IpPrefix::new(AddressFamilyId::Ipv4, vec![10, 0xff, 3, 4], 12).unwrap(); + let noisy = IpPrefix::new(IpVersion::Ipv4, vec![10, 0xff, 3, 4], 12).unwrap(); assert_eq!(noisy, IpPrefix::try_from("10.240.0.0/12").unwrap()); assert_eq!(noisy.bytes(), [10, 0xf0]); assert_eq!( - IpPrefix::new(AddressFamilyId::Ipv4, vec![0xff; 13], 99), + IpPrefix::new(IpVersion::Ipv4, vec![0xff; 13], 99), Err(PrefixError::LengthTooLong { length: 99, max: 32 @@ -344,7 +328,7 @@ mod tests { proptest! { #[test] fn text_round_trips(version: bool, bytes: [u8; 16], length in 0u8..=128) { - let version = if version { AddressFamilyId::Ipv6 } else { AddressFamilyId::Ipv4 }; + let version = if version { IpVersion::Ipv6 } else { IpVersion::Ipv4 }; let length = length.min(version.bits()); let prefix = IpPrefix::new(version, bytes, length).unwrap(); let text = prefix.to_string(); @@ -365,8 +349,8 @@ mod tests { fn contains_is_a_comparison_of_leading_bits( a: [u8; 4], a_len in 0u8..=32, b: [u8; 4], b_len in 0u8..=32, ) { - let outer = IpPrefix::new(AddressFamilyId::Ipv4, a, a_len).unwrap(); - let inner = IpPrefix::new(AddressFamilyId::Ipv4, b, b_len).unwrap(); + let outer = IpPrefix::new(IpVersion::Ipv4, a, a_len).unwrap(); + let inner = IpPrefix::new(IpVersion::Ipv4, b, b_len).unwrap(); let mask = if a_len == 0 { 0 } else { u32::MAX << (32 - a_len) }; let expected = b_len >= a_len && (u32::from_be_bytes(a) & mask) == (u32::from_be_bytes(b) & mask); @@ -390,11 +374,5 @@ mod tests { assert!(p("2001:db8::/32").contains(&p("2001:db8:cafe::/48"))); } - verify_roundtrip!( - verify_roundtrip_ipv6_0, - "::/0", - AddressFamilyId::Ipv6, - vec![], - 0 - ); + verify_roundtrip!(verify_roundtrip_ipv6_0, "::/0", IpVersion::Ipv6, vec![], 0); } diff --git a/crates/primitives/src/parser.rs b/crates/primitives/src/parser.rs index 9dc48e0..5c849f5 100644 --- a/crates/primitives/src/parser.rs +++ b/crates/primitives/src/parser.rs @@ -3,12 +3,13 @@ use nom::error::{ErrorKind, ParseError}; use crate::constants::AfiSafi; use thiserror::Error; -/// What a parser has to know about the session a message belongs to. +/// What reading or writing a message has to know about the session it +/// belongs to. The default is a session before its OPENs are exchanged; +/// only an UPDATE depends on anything here, and there are none until then. #[derive(Debug, Clone, Default)] pub struct ParserContext { /// Whether AS numbers are four octets wide on this session (RFC 6793). - /// `None` until the OPENs have been exchanged. - pub four_octet_asn: Option, + pub four_octet_asn: bool, /// Whether the peer is in our own AS. Some attributes mean something /// only between internal peers and are dropped when an external one /// sends them (RFC 7606 §7). @@ -29,9 +30,6 @@ impl ParserContext { /// Why a value could not be written to the wire. #[derive(Debug, Clone, PartialEq, Eq, Error)] pub enum EncodeError { - /// There was not enough space in the output buffer. - #[error("output buffer too small")] - OutBufferOverflow, /// A length does not fit the field that has to carry it. #[error("{what} is too long to encode: {len} exceeds {max}")] TooLong { @@ -39,10 +37,6 @@ pub enum EncodeError { len: usize, max: usize, }, - /// The value has no encoding in this context, e.g. a four-octet AS - /// number on a session that negotiated two-octet ones. - #[error("{0}")] - Unencodable(&'static str), } // Custom error type for the parser. diff --git a/crates/rib/src/local_rib.rs b/crates/rib/src/local_rib.rs index 7979e62..8385352 100644 --- a/crates/rib/src/local_rib.rs +++ b/crates/rib/src/local_rib.rs @@ -10,7 +10,7 @@ use std::time::SystemTime; use bgp_packet::message::{LargeCommunity, OriginPathAttribute}; use bgp_primitives::ip_prefix::IpPrefix; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use tokio::sync::{broadcast, mpsc, oneshot}; use tokio::task::AbortHandle; @@ -20,7 +20,7 @@ const EVENT_CHANNEL_CAPACITY: usize = 1024; const CMD_CHANNEL_CAPACITY: usize = 256; /// Attributes of a locally originated route. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct LocalRouteAttrs { pub origin: OriginPathAttribute, /// Number of *additional* copies of the local ASN to prepend to the @@ -46,7 +46,7 @@ impl Default for LocalRouteAttrs { } /// A route originated by this router. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct LocalRoute { pub prefix: IpPrefix, pub attrs: LocalRouteAttrs, diff --git a/crates/rib/src/table/attrs.rs b/crates/rib/src/table/attrs.rs index 48c5a31..b11f89f 100644 --- a/crates/rib/src/table/attrs.rs +++ b/crates/rib/src/table/attrs.rs @@ -11,16 +11,14 @@ //! once, when the set is stored, and kept beside the bytes. use std::hash::BuildHasher; -use std::net::{Ipv4Addr, Ipv6Addr}; -use std::sync::{Arc, Mutex, PoisonError}; +use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; -use bgp_packet::message::{NlriNextHop, OriginPathAttribute, PathAttribute}; +use bgp_packet::message::{NlriNextHop, PathAttribute, attr_type, split_attribute}; use bgp_primitives::parser::{EncodeError, ParserContext}; use bytes::{BufMut, BytesMut}; use foldhash::fast::RandomState; use hashbrown::HashTable; -const MP_REACH_NLRI: u8 = 14; const OPTIONAL: u8 = 0x80; /// What route selection reads from a set of attributes (RFC 4271 §9.1.2.2). @@ -41,107 +39,62 @@ pub struct SelectionKey { pub origin: u8, } -impl SelectionKey { - fn of(attributes: &[PathAttribute]) -> Self { - let mut key = Self { - local_pref: None, - med: None, - neighbor_as: 0, - as_path_len: 0, - // A route without ORIGIN does not get this far; if one did, it - // would be the least preferred. - origin: 2, - }; - for attribute in attributes { - match attribute { - PathAttribute::Origin(origin) => { - key.origin = match origin { - OriginPathAttribute::IGP => 0, - OriginPathAttribute::EGP => 1, - OriginPathAttribute::INCOMPLETE => 2, - } - } - PathAttribute::AsPath(path) => { - let hops: usize = path - .segments - .iter() - .map(|s| if s.ordered { s.path.len() } else { 1 }) - .sum(); - key.as_path_len = u16::try_from(hops).unwrap_or(u16::MAX); - key.neighbor_as = path - .segments - .first() - .filter(|s| s.ordered) - .and_then(|s| s.path.first().copied()) - .unwrap_or(0); - } - PathAttribute::MultiExitDisc(med) => key.med = Some(med.0), - PathAttribute::LocalPref(local_pref) => key.local_pref = Some(local_pref.0), - _ => {} - } - } - key - } -} - /// Reads the selection key straight off attributes in the form an /// [`AttrSet`] keeps, without building anything. `None` if the bytes are /// not a well-formed list of attributes. fn scan(mut wire: &[u8]) -> Option { - const EXTENDED_LENGTH: u8 = 0x10; let mut key = SelectionKey { local_pref: None, med: None, neighbor_as: 0, as_path_len: 0, + // A route without ORIGIN does not get this far; if one did, it + // would be the least preferred. origin: 2, }; let number = |value: &[u8]| Some(u32::from_be_bytes(<[u8; 4]>::try_from(value).ok()?)); while !wire.is_empty() { - let (&flags, rest) = wire.split_first()?; - let (&type_code, rest) = rest.split_first()?; - let (length, rest) = if flags & EXTENDED_LENGTH != 0 { - let (length, rest) = rest.split_first_chunk::<2>()?; - (usize::from(u16::from_be_bytes(*length)), rest) - } else { - let (&length, rest) = rest.split_first()?; - (usize::from(length), rest) - }; - let (value, rest) = rest.split_at_checked(length)?; + let (attribute, rest) = split_attribute(wire).ok()?; wire = rest; - match type_code { - 1 => key.origin = *value.first()?, - 2 => { - let mut segments = value; - let mut hops = 0usize; - let mut first = true; - while let [kind, count, rest @ ..] = segments { - let (asns, rest) = rest.split_at_checked(usize::from(*count) * 4)?; - let ordered = *kind == 2; - hops += if ordered { usize::from(*count) } else { 1 }; - if first && ordered { - key.neighbor_as = asns - .first_chunk::<4>() - .map_or(0, |a| u32::from_be_bytes(*a)); - } - first = false; - segments = rest; - } - if !segments.is_empty() { - return None; - } - key.as_path_len = u16::try_from(hops).unwrap_or(u16::MAX); - } - 4 => key.med = Some(number(value)?), - 5 => key.local_pref = Some(number(value)?), + let value = attribute.value; + match attribute.type_code { + attr_type::ORIGIN => key.origin = *value.first()?, + attr_type::AS_PATH => (key.as_path_len, key.neighbor_as) = scan_as_path(value)?, + attr_type::MULTI_EXIT_DISC => key.med = Some(number(value)?), + attr_type::LOCAL_PREF => key.local_pref = Some(number(value)?), _ => {} } } Some(key) } +/// The length of an AS_PATH as selection counts it, and the neighbouring +/// AS, from the attribute's value with four-octet AS numbers. +fn scan_as_path(mut segments: &[u8]) -> Option<(u16, u32)> { + const AS_SEQUENCE: u8 = 2; + let mut hops = 0usize; + let mut neighbor_as = 0; + let mut first = true; + while let [kind, count, rest @ ..] = segments { + let (asns, rest) = rest.split_at_checked(usize::from(*count) * 4)?; + let ordered = *kind == AS_SEQUENCE; + hops += if ordered { usize::from(*count) } else { 1 }; + if first && ordered { + neighbor_as = asns + .first_chunk::<4>() + .map_or(0, |asn| u32::from_be_bytes(*asn)); + } + first = false; + segments = rest; + } + if !segments.is_empty() { + return None; + } + Some((u16::try_from(hops).unwrap_or(u16::MAX), neighbor_as)) +} + /// One distinct list of path attributes, with the next hop that goes with -/// it. Made by [`AttrStore::intern`]; two routes have the same attributes +/// it. Made by an [`AttrStore`]; two routes have the same attributes /// exactly when they point to the same `AttrSet`. #[derive(Debug)] pub struct AttrSet { @@ -165,15 +118,20 @@ impl AttrSet { /// The attributes, parsed, and the next hop if it came in /// MP_REACH_NLRI. - pub fn attributes(&self) -> (Vec, Option) { - // The bytes were written by `encode`, so they parse. - parse(&self.wire).unwrap_or_default() + /// + /// `None` if they do not parse. Sets made from parsed attributes or + /// from an UPDATE the decoder accepted always do; one made from bytes + /// of another source ([`AttrStore::intern_wire`]) was checked only as + /// far as selection reads it. + pub fn attributes(&self) -> Option<(Vec, Option)> { + parse(&self.wire) } } +/// How the attributes of a set are written: with four-octet AS numbers. fn four_octet() -> ParserContext { ParserContext { - four_octet_asn: Some(true), + four_octet_asn: true, ibgp: true, add_path: Vec::new(), } @@ -197,49 +155,39 @@ fn encode( attribute.to_wire(&ctx, out)?; } if let Some(next_hop) = next_hop { - out.put_u8(OPTIONAL); - out.put_u8(MP_REACH_NLRI); - out.put_u8(1 + next_hop.wire_len()); - out.put_u8(next_hop.wire_len()); - next_hop.to_wire(&ctx, out)?; + encode_next_hop(next_hop, out); } Ok(()) } +/// Writes a next hop as an MP_REACH_NLRI that holds nothing else. +fn encode_next_hop(next_hop: &NlriNextHop, out: &mut BytesMut) { + out.put_u8(OPTIONAL); + out.put_u8(attr_type::MP_REACH_NLRI); + out.put_u8(1 + next_hop.wire_len()); + out.put_u8(next_hop.wire_len()); + next_hop.to_wire(out); +} + /// Parses path attributes in the form [`AttrSet`] keeps and table dumps /// use (RFC 6396 §4.3.4): four-octet AS numbers, and an MP_REACH_NLRI that /// holds only a next hop. `None` if they are malformed. -pub fn parse(mut wire: &[u8]) -> Option<(Vec, Option)> { +fn parse(mut wire: &[u8]) -> Option<(Vec, Option)> { let ctx = four_octet(); let mut attributes = Vec::new(); let mut next_hop = None; while !wire.is_empty() { - if wire.get(1) == Some(&MP_REACH_NLRI) { - let extended = wire[0] & 0x10 != 0; - let (length, header) = if extended { - ( - usize::from(u16::from_be_bytes([*wire.get(2)?, *wire.get(3)?])), - 4, - ) - } else { - (usize::from(*wire.get(2)?), 3) - }; - let value = wire.get(header..header + length)?; - let address = |bytes: &[u8]| Some(Ipv6Addr::from(<[u8; 16]>::try_from(bytes).ok()?)); - next_hop = match value { - [4, a, b, c, d] => Some(NlriNextHop::Ipv4(Ipv4Addr::new(*a, *b, *c, *d))), - [16, rest @ ..] => Some(NlriNextHop::Ipv6(address(rest)?)), - [32, rest @ ..] if rest.len() == 32 => Some(NlriNextHop::Ipv6WithLl { - global: address(&rest[..16])?, - link_local: address(&rest[16..])?, - }), - _ => return None, - }; - wire = &wire[header + length..]; - continue; + let (raw, rest) = split_attribute(wire).ok()?; + if raw.type_code == attr_type::MP_REACH_NLRI { + let (length, address) = raw.value.split_first()?; + if usize::from(*length) != address.len() { + return None; + } + next_hop = Some(NlriNextHop::from_bytes(address)?); + } else { + let (_, attribute) = PathAttribute::from_wire(&ctx, raw.wire).ok()?; + attributes.push(attribute); } - let (rest, attribute) = PathAttribute::from_wire(&ctx, wire).ok()?; - attributes.push(attribute); wire = rest; } Some((attributes, next_hop)) @@ -282,21 +230,9 @@ impl AttrStore { ) -> Result, EncodeError> { let mut wire = BytesMut::with_capacity(256); encode(attributes, next_hop, &mut wire)?; - let hash = self.hasher.hash_one(&wire[..]); - - let mut stripe = self.stripe(hash); - if let Some(existing) = stripe.find(hash, |set| set.wire[..] == wire[..]) { - return Ok(Arc::clone(existing)); - } - let key = SelectionKey::of(attributes); - debug_assert_eq!(scan(&wire), Some(key), "both ways of reading the key agree"); - let set = Arc::new(AttrSet { - hash, - key, - wire: wire[..].into(), - }); - stripe.insert_unique(hash, Arc::clone(&set), |set| set.hash); - Ok(set) + Ok(self + .intern_wire(&wire) + .expect("attributes written here are well formed")) } /// As [`AttrStore::intern`], for attributes that are already in the @@ -336,7 +272,7 @@ impl AttrStore { }; let mut with_next_hop = BytesMut::with_capacity(wire.len() + 36); with_next_hop.put_slice(wire); - encode(&[], Some(next_hop), &mut with_next_hop).ok()?; + encode_next_hop(next_hop, &mut with_next_hop); self.intern_wire(&with_next_hop) } @@ -366,12 +302,11 @@ impl AttrStore { dropped } - fn stripe(&self, hash: u64) -> std::sync::MutexGuard<'_, HashTable>> { - // The high bits: the table inside uses the low ones. - self.stripe_at((hash >> 56) as usize % STRIPES) + fn stripe(&self, hash: u64) -> MutexGuard<'_, HashTable>> { + self.stripe_at(super::spread(hash, STRIPES)) } - fn stripe_at(&self, index: usize) -> std::sync::MutexGuard<'_, HashTable>> { + fn stripe_at(&self, index: usize) -> MutexGuard<'_, HashTable>> { // A panic while holding the lock cannot leave the table half // changed in a way that matters: see `IgnorePoison` in the server. self.stripes[index] @@ -382,9 +317,11 @@ impl AttrStore { #[cfg(test)] mod tests { + use std::net::Ipv4Addr; + use bgp_packet::message::{ AsPathAttribute, AsPathSegment, CommunitiesPathAttribute, LocalPrefPathAttribute, - MultiExitDiscPathAttribute, NextHopPathAttribute, + MultiExitDiscPathAttribute, NextHopPathAttribute, OriginPathAttribute, }; use super::*; @@ -427,7 +364,7 @@ mod tests { }; let original = attributes(vec![65001, 4_200_000_001]); let set = store.intern(&original, Some(&next_hop)).unwrap(); - assert_eq!(set.attributes(), (original, Some(next_hop))); + assert_eq!(set.attributes(), Some((original, Some(next_hop)))); } #[test] @@ -485,7 +422,7 @@ mod tests { assert_eq!(fresh.key(), parsed.key()); assert_eq!(fresh.key().as_path_len, 3); assert_eq!(fresh.key().neighbor_as, 65001); - assert_eq!(fresh.attributes(), (attrs, Some(next_hop))); + assert_eq!(fresh.attributes(), Some((attrs, Some(next_hop)))); // Bytes that are not attributes are refused. assert!(other.intern_wire(&[0x40, 0x01, 0x05, 0x00]).is_none()); diff --git a/crates/rib/src/table/journal.rs b/crates/rib/src/table/journal.rs index fb4be0a..28dd8e3 100644 --- a/crates/rib/src/table/journal.rs +++ b/crates/rib/src/table/journal.rs @@ -57,7 +57,7 @@ use std::sync::atomic::{AtomicU64, Ordering}; use bgp_primitives::ip_prefix::IpPrefix; use foldhash::fast::RandomState; -use hashbrown::HashMap; +use hashbrown::{HashMap, HashTable}; use super::{AttrSet, PeerIndex, RibTable, RouteRef, Row, Shard, read, write}; @@ -89,10 +89,11 @@ struct Entry { prefix: IpPrefix, } -/// One shard's journal. -pub(super) struct Journal { - /// The sequence number of the latest change. Counts every change of a - /// best route in the shard, whether or not anyone was reading. +/// One journal of one shard. +struct Journal { + kind: Kind, + /// The sequence number of the latest change. Counts every change this + /// journal is for, whether or not anyone was reading. seq: u64, /// Entries up to and including this number were dropped without regard /// to the readers. A reader whose cursor is below it missed something. @@ -113,8 +114,9 @@ pub(super) struct Journal { } impl Journal { - pub(super) fn new(limits: Limits) -> Self { + fn new(kind: Kind, limits: Limits) -> Self { Self { + kind, limits, seq: 0, floor: 0, @@ -125,10 +127,9 @@ impl Journal { } } - /// Notes a change to `prefix`, which has a row: of its best route, or - /// of any of its routes, whichever this journal is for. Returns the - /// sequence number for the row to remember. - pub(super) fn changed(&mut self, prefix: IpPrefix) -> u64 { + /// Notes a change to `prefix`, which has a row. Returns the sequence + /// number for the row to remember. + fn changed(&mut self, prefix: IpPrefix) -> u64 { self.seq += 1; if self.readers > 0 { self.entries.push_back(Entry { @@ -141,7 +142,7 @@ impl Journal { /// As [`Journal::changed`], for a prefix whose row was just made: /// if it had gone and is back, the note that it had gone is void. - pub(super) fn appeared(&mut self, prefix: IpPrefix) -> u64 { + fn appeared(&mut self, prefix: IpPrefix) -> u64 { if !self.gone.is_empty() { self.gone.remove(&prefix); } @@ -149,7 +150,7 @@ impl Journal { } /// Notes that `prefix` lost its last route and has no row any more. - pub(super) fn disappeared(&mut self, prefix: IpPrefix) { + fn disappeared(&mut self, prefix: IpPrefix) { self.seq += 1; if self.readers > 0 { self.entries.push_back(Entry { @@ -160,14 +161,76 @@ impl Journal { } } + /// Forgets that `entry`'s prefix had gone, if `entry` is what said so. fn drop_entry(gone: &mut HashMap, entry: &Entry) { if gone.get(&entry.prefix) == Some(&entry.seq) { gone.remove(&entry.prefix); } } - pub(super) fn len(&self) -> usize { - self.entries.len() + /// What `entry` says, if it still says something: it is the latest + /// change of its prefix. The row, or `None` for a prefix that has gone. + fn live<'a>( + &self, + entry: &Entry, + rows: &'a HashTable, + hasher: &RandomState, + ) -> Option> { + let hash = hasher.hash_one(entry.prefix); + match rows.find(hash, |row| row.prefix == entry.prefix) { + Some(row) => (row.seqs.of(self.kind) == entry.seq).then_some(Some(row)), + None => (self.gone.get(&entry.prefix) == Some(&entry.seq)).then_some(None), + } + } + + /// Keeps the journal from growing. Called after every change to the + /// shard; does nothing until there is something worth doing. + fn tidy(&mut self, rows: &HashTable, hasher: &RandomState) { + if self.entries.len() <= self.limits.tidy_at { + return; + } + // What every reader has passed can go. + let passed = self + .cursors + .iter() + .map(|cursor| cursor.load(Ordering::Acquire)) + .min() + .unwrap_or(NO_CURSOR); + while let Some(front) = self.entries.front() { + if front.seq > passed { + break; + } + Self::drop_entry(&mut self.gone, front); + self.entries.pop_front(); + } + + // Entries for prefixes that have a row are at most as many as the + // rows once the dead ones are out. Entries for prefixes that have + // gone are not counted in: they are what a reader that has stopped + // reading makes pile up. + let limit = 2 * (rows.len() + self.limits.slack); + if self.entries.len() <= limit { + return; + } + // Dead entries: their prefix has changed again since. This looks + // every entry up, but only once the journal has doubled, so the + // cost per change stays small. + let mut entries = std::mem::take(&mut self.entries); + entries.retain(|entry| self.live(entry, rows, hasher).is_some()); + self.entries = entries; + + if self.entries.len() <= limit { + return; + } + // Still too long: a reader is not reading while prefixes come and + // go, or more prefixes went at once than are left. The older half + // goes, and a reader that had not read it loses its place. Half, + // so that readers that are only a little behind keep theirs, and + // so that getting a long journal down to size takes few passes. + for entry in self.entries.drain(..self.entries.len() / 2) { + Self::drop_entry(&mut self.gone, &entry); + self.floor = entry.seq; + } } } @@ -180,92 +243,103 @@ pub(super) enum Kind { All, } -impl Kind { - /// The sequence number `row` remembers for this journal. - fn seq_of(self, row: &Row) -> u64 { - match self { - Self::Best => row.changed, - Self::All => row.touched, +/// The sequence numbers a row remembers, one for each journal: of the +/// latest change of its best route, and of any of its routes. +#[derive(Debug, Clone, Copy)] +pub(super) struct Seqs { + best: u64, + all: u64, +} + +impl Seqs { + fn of(self, kind: Kind) -> u64 { + match kind { + Kind::Best => self.best, + Kind::All => self.all, } } } -impl Shard { - fn journal_of(&self, kind: Kind) -> &Journal { +/// The two journals of a shard, written together: every change to a row is +/// a change of one of its routes, and some are a change of its best route. +pub(super) struct Journals { + best: Journal, + all: Journal, +} + +impl Journals { + pub(super) fn new(limits: Limits) -> Self { + Self { + best: Journal::new(Kind::Best, limits), + all: Journal::new(Kind::All, limits), + } + } + + fn of(&self, kind: Kind) -> &Journal { match kind { - Kind::Best => &self.journal, - Kind::All => &self.all_journal, + Kind::Best => &self.best, + Kind::All => &self.all, } } - fn journal_of_mut(&mut self, kind: Kind) -> &mut Journal { + fn of_mut(&mut self, kind: Kind) -> &mut Journal { match kind { - Kind::Best => &mut self.journal, - Kind::All => &mut self.all_journal, + Kind::Best => &mut self.best, + Kind::All => &mut self.all, } } - /// Whether `entry` still says something: it is the latest change of - /// its prefix. - fn is_live(&self, kind: Kind, entry: &Entry) -> bool { - let hash = self.hasher.hash_one(entry.prefix); - match self.rows.find(hash, |row| row.prefix == entry.prefix) { - Some(row) => kind.seq_of(row) == entry.seq, - None => self.journal_of(kind).gone.get(&entry.prefix) == Some(&entry.seq), + /// Notes that `prefix` has its first route, and so a row. Returns the + /// sequence numbers for the new row. + pub(super) fn appeared(&mut self, prefix: IpPrefix) -> Seqs { + Seqs { + best: self.best.appeared(prefix), + all: self.all.appeared(prefix), } } - /// Keeps the journals from growing. Called after every change to the - /// shard; does nothing until there is something worth doing. - pub(super) fn tidy_journals(&mut self) { - self.tidy_journal(Kind::Best); - self.tidy_journal(Kind::All); + /// Notes that a route of `row` changed, and whether its best route did. + pub(super) fn changed(&mut self, row: &mut Row, best_changed: bool) { + if best_changed { + row.seqs.best = self.best.changed(row.prefix); + } + row.seqs.all = self.all.changed(row.prefix); } - fn tidy_journal(&mut self, kind: Kind) { - if self.journal_of(kind).entries.len() <= self.journal_of(kind).limits.tidy_at { - return; - } - // What every reader has passed can go. - let journal = self.journal_of_mut(kind); - let passed = journal - .cursors - .iter() - .map(|cursor| cursor.load(Ordering::Acquire)) - .min() - .unwrap_or(NO_CURSOR); - while let Some(front) = journal.entries.front() { - if front.seq > passed { - break; - } - Journal::drop_entry(&mut journal.gone, front); - journal.entries.pop_front(); - } + /// Notes that `prefix` lost its last route and has no row any more. + pub(super) fn disappeared(&mut self, prefix: IpPrefix) { + self.best.disappeared(prefix); + self.all.disappeared(prefix); + } - let limit = 2 * (self.rows.len() + self.journal_of(kind).limits.slack); - if self.journal_of(kind).entries.len() <= limit { - return; - } - // Dead entries: their prefix has changed again since. This looks - // every entry up, but only once the journal has doubled, so the - // cost per change stays small. - let mut entries = std::mem::take(&mut self.journal_of_mut(kind).entries); - entries.retain(|entry| self.is_live(kind, entry)); - self.journal_of_mut(kind).entries = entries; + /// Keeps the journals from growing; see [`Journal::tidy`]. + pub(super) fn tidy(&mut self, rows: &HashTable, hasher: &RandomState) { + self.best.tidy(rows, hasher); + self.all.tidy(rows, hasher); + } - if self.journal_of(kind).entries.len() <= limit { - return; - } - // Still too long: some reader is not reading, and prefixes keep - // coming and going. It loses its place. - let journal = self.journal_of_mut(kind); - for entry in journal.entries.drain(..limit / 2) { - Journal::drop_entry(&mut journal.gone, &entry); - journal.floor = entry.seq; - } + /// Where the journals are now, to ask [`Journals::has_news_since`] + /// about later. + pub(super) fn mark(&self) -> Mark { + Mark(self.best.seq, self.all.seq) + } + + /// Whether anything was written since `mark` that some reader is there + /// to read. + pub(super) fn has_news_since(&self, mark: Mark) -> bool { + (self.best.readers > 0 && self.best.seq != mark.0) + || (self.all.readers > 0 && self.all.seq != mark.1) + } + + fn len(&self) -> usize { + self.best.entries.len() + self.all.entries.len() } } +/// A point in a shard's journals; see [`Journals::mark`]. +#[derive(Clone, Copy)] +pub(super) struct Mark(u64, u64); + /// The routes of one prefix, as a follower of all routes is given them. #[derive(Clone, Copy)] pub struct Routes<'a>(&'a Row); @@ -295,7 +369,8 @@ impl std::fmt::Debug for Routes<'_> { #[derive(Debug, Clone, Copy)] pub enum Event<'a> { /// Forget what you know. The whole table follows, as [`Event::Best`] - /// for every prefix, and then [`Event::Synced`]. The first thing a new + /// or [`Event::Routes`] for every prefix, and then [`Event::Synced`]. + /// The first thing a new /// reader is given, and what a reader that fell too far behind is /// given in place of what it missed. Reset, @@ -333,13 +408,23 @@ pub struct Progress { } enum Phase { - /// Giving out the whole table; `next` is the shard to do next. + /// Giving out the whole table; `next` is the shard to do next, and + /// `announced` whether [`Event::Reset`] has been given. Resync { next: usize, announced: bool }, /// Following the journals; `next` is the shard to read next. Follow { next: usize }, } -/// Follows the best routes of a table. Made by [`RibTable::reader`]. +impl Phase { + /// The beginning of a resynchronisation. + const RESYNC: Self = Self::Resync { + next: 0, + announced: false, + }; +} + +/// Follows the routes of a table: the best of each prefix, or all of them. +/// Made by [`RibTable::reader`] and its siblings. /// /// A reader holds its place in every shard's journal. Dropping it gives /// the place up, so that the journals need not keep entries for it. @@ -361,13 +446,7 @@ impl RibTable { /// A new follower of this table's best routes. It is given the whole /// table first and every change after. pub fn reader(self: &Arc) -> Reader { - self.register( - Kind::Best, - Phase::Resync { - next: 0, - announced: false, - }, - ) + self.register(Kind::Best, Phase::RESYNC) } /// A new follower that is given only the changes from now on, for @@ -377,7 +456,7 @@ impl RibTable { let mut reader = self.register(Kind::Best, Phase::Follow { next: 0 }); for (index, shard) in self.shards.iter().enumerate() { let shard = read(shard); - reader.set_cursor(index, &shard, shard.journal_of(Kind::Best).seq); + reader.set_cursor(index, &shard, shard.journals.best.seq); } reader } @@ -387,13 +466,7 @@ impl RibTable { /// prefix's routes ([`Event::Routes`]). The whole table first, like /// [`RibTable::reader`]. pub fn reader_of_all_routes(self: &Arc) -> Reader { - self.register( - Kind::All, - Phase::Resync { - next: 0, - announced: false, - }, - ) + self.register(Kind::All, Phase::RESYNC) } fn register(self: &Arc, kind: Kind, phase: Phase) -> Reader { @@ -411,7 +484,7 @@ impl RibTable { slots[slot] = true; for shard in &self.shards { let mut shard = write(shard); - let journal = shard.journal_of_mut(kind); + let journal = shard.journals.of_mut(kind); while journal.cursors.len() <= slot { journal.cursors.push(AtomicU64::new(NO_CURSOR)); } @@ -435,10 +508,7 @@ impl RibTable { pub fn journal_len(&self) -> usize { self.shards .iter() - .map(|shard| { - let shard = read(shard); - shard.journal.len() + shard.all_journal.len() - }) + .map(|shard| read(shard).journals.len()) .sum() } @@ -455,7 +525,7 @@ impl Drop for Reader { fn drop(&mut self) { for shard in &self.table.shards { let mut shard = write(shard); - let journal = shard.journal_of_mut(self.kind); + let journal = shard.journals.of_mut(self.kind); journal.cursors[self.slot].store(NO_CURSOR, Ordering::Release); journal.readers -= 1; if journal.readers == 0 { @@ -487,8 +557,12 @@ impl Reader { /// short, so it can be more). /// /// `visit` is called with a shard of the table held for reading. It - /// should be quick, and it must not change the table. + /// should be quick, and it must not call into the table, even to read: + /// taking the shard's lock again can wait for ever on a writer that is + /// waiting for this hold to end. pub fn poll(&mut self, budget: usize, mut visit: impl FnMut(Event<'_>)) -> Progress { + // A call that may give nothing out would never get anywhere. + let budget = budget.max(1); let shards = self.table.shards.len(); let mut events = 0; loop { @@ -546,10 +620,7 @@ impl Reader { shard += 1; } if lagged { - self.phase = Phase::Resync { - next: 0, - announced: false, - }; + self.phase = Phase::RESYNC; continue; } self.phase = Phase::Follow { next: 0 }; @@ -590,7 +661,7 @@ impl Reader { fn event_for<'a>(&self, row: &'a Row) -> Event<'a> { match self.kind { Kind::Best => { - let best = row.route(usize::from(row.best)); + let best = row.route(row.best()); Event::Best { prefix: row.prefix, peer: best.peer, @@ -609,7 +680,7 @@ impl Reader { /// resynchronisation takes a new one. fn forget_place(&mut self) { for (index, shard) in self.table.shards.iter().enumerate() { - read(shard).journal_of(self.kind).cursors[self.slot] + read(shard).journals.of(self.kind).cursors[self.slot] .store(NO_CURSOR, Ordering::Release); self.cursors[index] = NO_CURSOR; } @@ -617,19 +688,19 @@ impl Reader { fn set_cursor(&mut self, index: usize, shard: &Shard, cursor: u64) { self.cursors[index] = cursor; - shard.journal_of(self.kind).cursors[self.slot].store(cursor, Ordering::Release); + shard.journals.of(self.kind).cursors[self.slot].store(cursor, Ordering::Release); } - /// Gives out every best route in a shard and takes a place at the end - /// of its journal, in one hold of the shard: what changes afterwards is - /// in the journal after that place. + /// Gives out every prefix in a shard and takes a place at the end of + /// its journal, in one hold of the shard: what changes afterwards is in + /// the journal after that place. fn scan(&mut self, index: usize, visit: &mut impl FnMut(Event<'_>)) -> usize { let table = Arc::clone(&self.table); let shard = read(&table.shards[index]); for row in &shard.rows { visit(self.event_for(row)); } - self.set_cursor(index, &shard, shard.journal_of(self.kind).seq); + self.set_cursor(index, &shard, shard.journals.of(self.kind).seq); shard.rows.len() } @@ -638,7 +709,7 @@ impl Reader { fn drain(&mut self, index: usize, max: usize, visit: &mut impl FnMut(Event<'_>)) -> Drained { let table = Arc::clone(&self.table); let shard = read(&table.shards[index]); - let journal = shard.journal_of(self.kind); + let journal = shard.journals.of(self.kind); let mut cursor = self.cursors[index]; if cursor < journal.floor { return Drained::Lagged; @@ -659,21 +730,16 @@ impl Reader { break; } cursor = entry.seq; - let hash = shard.hasher.hash_one(entry.prefix); - match shard.rows.find(hash, |row| row.prefix == entry.prefix) { - Some(row) if self.kind.seq_of(row) == entry.seq => { - visit(self.event_for(row)); - events += 1; - } - None if journal.gone.get(&entry.prefix) == Some(&entry.seq) => { - visit(Event::Withdrawn { - prefix: entry.prefix, - }); - events += 1; - } + let event = match journal.live(entry, &shard.rows, &shard.hasher) { + Some(Some(row)) => self.event_for(row), + Some(None) => Event::Withdrawn { + prefix: entry.prefix, + }, // The prefix has changed again; a later entry has it. - _ => {} - } + None => continue, + }; + visit(event); + events += 1; } if !more { // Past the last entry there is nothing up to the latest change. diff --git a/crates/rib/src/table/journal_tests.rs b/crates/rib/src/table/journal_tests.rs index b7e2415..9c72bc6 100644 --- a/crates/rib/src/table/journal_tests.rs +++ b/crates/rib/src/table/journal_tests.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -use bgp_primitives::constants::AddressFamilyId; +use bgp_primitives::constants::IpVersion; use bgp_primitives::ip_prefix::IpPrefix; use proptest::prelude::*; @@ -146,7 +146,7 @@ impl Mirror { } fn nth_prefix(n: u32) -> IpPrefix { - IpPrefix::new(AddressFamilyId::Ipv4, (n << 8).to_be_bytes(), 24).unwrap() + IpPrefix::new(IpVersion::Ipv4, (n << 8).to_be_bytes(), 24).unwrap() } #[test] @@ -395,6 +395,90 @@ fn a_reader_that_stops_reading_is_reset_and_holds_nothing_up() { stuck_mirror.assert_matches(&fixture.table); } +/// When a peer that was the only source of most prefixes goes, more +/// prefixes disappear at once than are left. The journal still comes down +/// to its bound, and a reader ends up with what the table has. +#[test] +fn many_prefixes_going_at_once_leaves_the_journal_bounded() { + let fixture = Fixture::on(RibTable::new(1), &[external(1), external(2)]); + for n in 0..5000 { + fixture.announce(0, &nth_prefix(n), &Route::via(&[1])); + } + fixture.announce(1, &nth_prefix(0), &Route::via(&[2, 3])); + let mut reader = fixture.table.reader(); + let mut mirror = Mirror::default(); + mirror.catch_up(&mut reader, 1000); + + fixture.table.remove_peer(fixture.peers[0]); + // Some later change, as there always is, lets the journal be tidied. + for _ in 0..8 { + fixture.announce(1, &nth_prefix(1), &Route::via(&[2])); + fixture.withdraw(1, &nth_prefix(1)); + } + let limits = Limits::default(); + assert!( + fixture.table.journal_len() <= limits.tidy_at.max(2 * (1 + limits.slack)) + 2, + "the journal holds {} entries", + fixture.table.journal_len() + ); + mirror.catch_up(&mut reader, 1000); + mirror.assert_matches(&fixture.table); + assert_eq!(mirror.best.len(), 1); +} + +/// Readers are woken to read. An announcement that changes nothing, or +/// nothing a reader follows, writes nothing for them and must not wake +/// them: each would look through every shard to find that out. +#[test] +fn readers_are_not_woken_when_nothing_was_written_for_them() { + let fixture = Fixture::new(&[external(1), external(2)]); + let p = prefix("10.0.0.0/8"); + let version = || fixture.table.version.load(Ordering::Acquire); + + // Nobody reading: nobody to wake. + fixture.announce(0, &p, &Route::via(&[1])); + assert_eq!(version(), 0); + + let _best = fixture.table.reader(); + let before = version(); + // The same route again. + fixture.announce(0, &p, &Route::via(&[1])); + assert_eq!(version(), before); + // A worse route from another peer: the best route stands. + fixture.announce(1, &p, &Route::via(&[2, 3, 4])); + assert_eq!(version(), before); + // A better one. + fixture.announce(1, &p, &Route::via(&[])); + assert_eq!(version(), before + 1); + + // A follower of all routes is told of any change to a route. + let _all = fixture.table.reader_of_all_routes(); + let before = version(); + fixture.announce(0, &p, &Route::via(&[1, 1, 1])); + assert_eq!(version(), before + 1); + fixture.announce(0, &p, &Route::via(&[1, 1, 1])); + assert_eq!(version(), before + 1); +} + +/// A budget of nothing would let a reader ask for ever without getting +/// anywhere; it is taken as the smallest that does. +#[test] +fn a_budget_of_nothing_still_makes_progress() { + let fixture = Fixture::new(&[external(1)]); + for n in 0..20 { + fixture.announce(0, &nth_prefix(n), &Route::via(&[1])); + } + let mut reader = fixture.table.reader(); + let mut mirror = Mirror::default(); + mirror.catch_up(&mut reader, 0); + for n in 0..20 { + fixture.withdraw(0, &nth_prefix(n)); + } + mirror.catch_up(&mut reader, 0); + mirror.assert_matches(&fixture.table); + assert!(mirror.best.is_empty()); +} + #[tokio::test] async fn a_waiting_reader_is_woken_by_a_change_of_best_route() { let fixture = Fixture::new(&[external(1), external(2)]); diff --git a/crates/rib/src/table/mod.rs b/crates/rib/src/table/mod.rs index 40e7313..dc71ae7 100644 --- a/crates/rib/src/table/mod.rs +++ b/crates/rib/src/table/mod.rs @@ -122,14 +122,15 @@ impl Path { struct Row { prefix: IpPrefix, /// Index of the best route in `paths`. A row always has at least one - /// route, and so always a best one. - best: u16, - /// The journal's sequence number for the latest change of this row's - /// best route; see [`journal`]. - changed: u64, - /// The journal's sequence number for the latest change of any of this - /// row's routes, kept only while someone follows all routes. - touched: u64, + /// route, and so always a best one; and the best one always has the + /// highest [`Path::preference`] in the row, which is what lets most + /// changes be settled without looking at the other routes + /// ([`select`]). + best: u32, + /// The sequence numbers of the latest change of this row's best route + /// and of any of its routes, by which the journals tell their entries + /// for it that still say something from those that do not. + seqs: journal::Seqs, /// The routes, in one run of memory: finding a peer's route and /// choosing the best are scans of it. One from each peer that has /// announced the prefix, or several from a peer that sends more than one @@ -143,12 +144,11 @@ struct Row { } impl Row { - fn new(prefix: IpPrefix, path: Path, path_id: u32, changed: u64, touched: u64) -> Self { + fn new(prefix: IpPrefix, path: Path, path_id: u32, seqs: journal::Seqs) -> Self { Self { prefix, best: 0, - changed, - touched, + seqs, paths: vec![path], path_ids: if path_id == 0 { vec![] } else { vec![path_id] }, } @@ -189,6 +189,16 @@ impl Row { self.paths.swap_remove(at) } + fn best(&self) -> usize { + self.best as usize + } + + fn set_best(&mut self, index: usize) { + // An index into `paths`, which cannot have that many: each is + // sixteen bytes. + self.best = index as u32; + } + fn route(&self, index: usize) -> RouteRef<'_> { let path = &self.paths[index]; RouteRef { @@ -196,7 +206,7 @@ impl Row { peer: path.peer(), path_id: self.path_id(index), attrs: &path.attrs, - best: index == usize::from(self.best), + best: index == self.best(), } } @@ -251,11 +261,8 @@ struct Shard { hasher: RandomState, /// Reused between selections. scratch: select::Scratch, - /// Which prefixes' best routes have changed, for those who follow. - journal: journal::Journal, - /// Which prefixes have had any route change, for those who follow all - /// the routes and not only the best. - all_journal: journal::Journal, + /// Which prefixes have changed, for those who follow. + journals: journal::Journals, } /// The routing table. Shared between threads by reference; every method @@ -271,7 +278,8 @@ pub struct RibTable { reader_slots: Mutex>, /// How many readers there are, to skip waking them when there are none. readers: AtomicUsize, - /// Goes up whenever a best route changes while there are readers. + /// Goes up whenever something is written to a journal that has + /// readers. version: AtomicU64, wake: Notify, } @@ -301,8 +309,7 @@ impl RibTable { peers: Vec::new(), hasher: hasher.clone(), scratch: select::Scratch::default(), - journal: journal::Journal::new(limits), - all_journal: journal::Journal::new(limits), + journals: journal::Journals::new(limits), }) }) .collect(), @@ -323,13 +330,18 @@ impl RibTable { /// Makes room for a peer's routes and returns the number to announce /// them under. + /// + /// The number is the peer's until [`RibTable::remove_peer`] is called + /// with it, and may then be given to another. Whoever was given it must + /// not use it after that: the table cannot tell a late announcement + /// from one of the number's next owner. pub fn add_peer(&self, info: PeerInfo) -> Result { let mut peers = self.peers.lock().unwrap_or_else(PoisonError::into_inner); let index = match peers.iter().position(Option::is_none) { Some(free) => free, None => { // A peer number shares a word with other things; see `Path`. - if peers.len() >= usize::from(u16::MAX) { + if peers.len() > usize::from(u16::MAX) { return Err(TooManyPeers); } peers.push(None); @@ -350,58 +362,47 @@ impl RibTable { /// Takes out every route from `peer` and gives its number back for /// reuse (RFC 4271 §8.2.2: the routes of a session that ended are /// deleted). + /// + /// This looks at every prefix in the table. The list of peers is not + /// held while it does, so other peers can come and go meanwhile. pub fn remove_peer(&self, peer: PeerIndex) -> Outcome { - let mut peers = self.peers.lock().unwrap_or_else(PoisonError::into_inner); let mut outcome = Outcome::default(); + let mut news = false; for shard in &self.shards { let mut shard = write(shard); + let before = shard.journals.mark(); outcome += shard.remove_peer(peer); - shard.tidy_journals(); + // From here on the shard takes no more routes from the peer. if let Some(slot) = shard.peers.get_mut(peer.as_usize()) { *slot = None; } + shard.tidy_journals(); + news |= shard.journals.has_news_since(before); } + // Only now can the number be given out again: no shard knows it. + let mut peers = self.peers.lock().unwrap_or_else(PoisonError::into_inner); if let Some(slot) = peers.get_mut(peer.as_usize()) { *slot = None; } - if outcome != Outcome::default() { + drop(peers); + if news { self.wake_readers(); } outcome } - /// Announces `prefixes` from `peer`, all with the attributes `attrs`: - /// what one UPDATE does. A route the peer had announced before for the - /// same prefix is replaced. - pub fn announce( - &self, - peer: PeerIndex, - attrs: &Arc, - prefixes: &[IpPrefix], - ) -> Outcome { - self.announce_each( - peer, - attrs, - prefixes.iter().map(|prefix| Nlri::from(*prefix)), - ) - } - - /// As [`RibTable::announce`], for a peer that may send several routes - /// for a prefix (ADD-PATH, RFC 7911): each route is known by its prefix - /// and path identifier together, and replaces only the route the peer - /// had announced with both the same. - pub fn announce_paths(&self, peer: PeerIndex, attrs: &Arc, nlri: &[Nlri]) -> Outcome { - self.announce_each(peer, attrs, nlri.iter().copied()) - } - - fn announce_each( - &self, - peer: PeerIndex, - attrs: &Arc, - nlri: impl ExactSizeIterator, - ) -> Outcome { + /// Announces `routes` from `peer`, all with the attributes `attrs`: + /// what one UPDATE does. A route is known by its prefix and path + /// identifier together ([`Nlri`]; a plain [`IpPrefix`] is the route + /// with identifier zero, which is every route of a peer that sends one + /// for each prefix). Announcing a route the peer had announced before + /// replaces it. + pub fn announce(&self, peer: PeerIndex, attrs: &Arc, routes: &[N]) -> Outcome + where + N: Copy + Into, + { let mut outcome = Outcome::default(); - self.by_shard(nlri, |shard, placed| { + self.by_shard(routes, |shard, placed| { let Some(info) = shard.peers.get(peer.as_usize()).copied().flatten() else { // The peer was removed while this was on its way. return; @@ -411,71 +412,67 @@ impl RibTable { outcome += shard.announce(*hash, *nlri, peer, preference, attrs); } }); - if outcome != Outcome::default() { - self.wake_readers(); - } outcome } - /// Withdraws `peer`'s routes for `prefixes`. Withdrawing a route that - /// is not there does nothing. - pub fn withdraw(&self, peer: PeerIndex, prefixes: &[IpPrefix]) -> Outcome { - self.withdraw_each(peer, prefixes.iter().map(|prefix| Nlri::from(*prefix))) - } - - /// As [`RibTable::withdraw`], for routes known by prefix and path - /// identifier (ADD-PATH, RFC 7911). - pub fn withdraw_paths(&self, peer: PeerIndex, nlri: &[Nlri]) -> Outcome { - self.withdraw_each(peer, nlri.iter().copied()) - } - - fn withdraw_each(&self, peer: PeerIndex, nlri: impl ExactSizeIterator) -> Outcome { + /// Withdraws `routes` of `peer`. Withdrawing a route that is not there + /// does nothing. + pub fn withdraw(&self, peer: PeerIndex, routes: &[N]) -> Outcome + where + N: Copy + Into, + { let mut outcome = Outcome::default(); - self.by_shard(nlri, |shard, placed| { + self.by_shard(routes, |shard, placed| { for (hash, nlri) in placed { outcome += shard.withdraw(*hash, nlri, peer); } }); - if outcome != Outcome::default() { - self.wake_readers(); - } outcome } - /// Calls `apply` once for each shard that some of `nlri` fall in, with - /// that shard locked and those prefixes. - fn by_shard( - &self, - nlri: impl ExactSizeIterator, - mut apply: impl FnMut(&mut Shard, &[(u64, Nlri)]), - ) { - let shards = self.shards.len() as u64; - // The high bits pick the shard; the table inside uses the low ones. - let shard_of = |hash: u64| ((hash >> 32) * shards) >> 32; - let hashed = |nlri: Nlri| (self.hasher.hash_one(nlri.prefix), nlri); - // An UPDATE has a handful of prefixes; they are sorted on the stack. - const INLINE: usize = 32; - let count = nlri.len(); - let mut nlri = nlri.peekable(); - let Some(first) = nlri.peek().copied() else { + fn shard_of(&self, hash: u64) -> usize { + spread(hash, self.shards.len()) + } + + /// Calls `apply` once for each shard that some of `routes` fall in, + /// with that shard locked and those routes. Wakes the readers if a + /// journal one of them follows was written to. + fn by_shard(&self, routes: &[N], mut apply: impl FnMut(&mut Shard, &[(u64, Nlri)])) + where + N: Copy + Into, + { + let Some(first) = routes.first() else { return; }; - let mut inline = [(0u64, first); INLINE]; + let hashed = |route: &N| { + let nlri: Nlri = (*route).into(); + (self.hasher.hash_one(nlri.prefix), nlri) + }; + // An UPDATE has a handful of prefixes; they are sorted on the stack. + const INLINE: usize = 32; + let mut inline = [hashed(first); INLINE]; let mut spilled = Vec::new(); - let placed: &mut [(u64, Nlri)] = if count <= INLINE { - for (slot, nlri) in inline.iter_mut().zip(nlri) { - *slot = hashed(nlri); + let placed: &mut [(u64, Nlri)] = if routes.len() <= INLINE { + for (slot, route) in inline.iter_mut().zip(routes) { + *slot = hashed(route); } - &mut inline[..count] + &mut inline[..routes.len()] } else { - spilled.extend(nlri.map(hashed)); + spilled.extend(routes.iter().map(hashed)); &mut spilled }; - placed.sort_unstable_by_key(|(hash, _)| shard_of(*hash)); - for run in placed.chunk_by(|a, b| shard_of(a.0) == shard_of(b.0)) { - let mut shard = write(&self.shards[shard_of(run[0].0) as usize]); + placed.sort_unstable_by_key(|(hash, _)| self.shard_of(*hash)); + + let mut news = false; + for run in placed.chunk_by(|a, b| self.shard_of(a.0) == self.shard_of(b.0)) { + let mut shard = write(&self.shards[self.shard_of(run[0].0)]); + let before = shard.journals.mark(); apply(&mut shard, run); shard.tidy_journals(); + news |= shard.journals.has_news_since(before); + } + if news { + self.wake_readers(); } } @@ -496,7 +493,7 @@ impl RibTable { /// The best route for `prefix`: who it is from, and its attributes. pub fn best(&self, prefix: &IpPrefix) -> Option<(PeerIndex, Arc)> { self.with_row(prefix, |row| { - let best = &row.paths[usize::from(row.best)]; + let best = &row.paths[row.best()]; (best.peer(), Arc::clone(&best.attrs)) }) } @@ -509,8 +506,7 @@ impl RibTable { fn with_row(&self, prefix: &IpPrefix, read_row: impl FnOnce(&Row) -> T) -> Option { let hash = self.hasher.hash_one(prefix); - let shards = self.shards.len() as u64; - let shard = read(&self.shards[(((hash >> 32) * shards) >> 32) as usize]); + let shard = read(&self.shards[self.shard_of(hash)]); shard .rows .find(hash, |row| row.prefix == *prefix) @@ -520,10 +516,14 @@ impl RibTable { /// Calls `visit` with the best route of every prefix, in no particular /// order. The table is not stopped: one shard at a time is held for /// reading, so the routes seen are not all of one moment. + /// + /// `visit` is called with a shard held, and must not call into the + /// table itself: taking the shard's lock again can wait for ever on a + /// writer that is waiting for this hold to end. pub fn for_each_best(&self, mut visit: impl FnMut(&IpPrefix, PeerIndex, &Arc)) { for shard in &self.shards { for row in &read(shard).rows { - let best = &row.paths[usize::from(row.best)]; + let best = &row.paths[row.best()]; visit(&row.prefix, best.peer(), &best.attrs); } } @@ -548,7 +548,8 @@ impl RibTable { } /// Calls `visit` with every route in the table. As - /// [`RibTable::for_each_best`], without stopping the table. + /// [`RibTable::for_each_best`]: the table is not stopped, and `visit` + /// must not call into it. pub fn for_each_route(&self, mut visit: impl FnMut(RouteRef<'_>)) { for shard in &self.shards { for row in &read(shard).rows { @@ -568,6 +569,19 @@ fn write(shard: &RwLock) -> RwLockWriteGuard<'_, Shard> { shard.write().unwrap_or_else(PoisonError::into_inner) } +/// Picks one of `count` buckets by a hash, for spreading keys over shards +/// or lock stripes that each hold a hash table keyed by the same hash. +/// +/// That table takes a key's slot from the low bits of the hash and, from +/// the top seven, the tag it compares before it looks at the key itself +/// (hashbrown). The bucket is taken from bits in between: taken from +/// either end, the keys of one bucket would all share those bits, and the +/// table inside would lose them. +fn spread(hash: u64, count: usize) -> usize { + let middle = (hash << 7) >> 32; + ((middle * count as u64) >> 32) as usize +} + /// Adds to a vector without the doubling that `push` does: a row of a few /// hundred routes should not hold room for as many again. Grows by a /// quarter, which keeps adding cheap on average. @@ -579,6 +593,10 @@ fn push_tight(vec: &mut Vec, value: T) { } impl Shard { + fn tidy_journals(&mut self) { + self.journals.tidy(&self.rows, &self.hasher); + } + fn announce( &mut self, hash: u64, @@ -597,9 +615,8 @@ impl Shard { let path = Path::new(preference, peer, attrs); let row = match entry { Entry::Vacant(vacant) => { - let changed = self.journal.appeared(prefix); - let touched = self.all_journal.appeared(prefix); - vacant.insert(Row::new(prefix, path, path_id, changed, touched)); + let seqs = self.journals.appeared(prefix); + vacant.insert(Row::new(prefix, path, path_id, seqs)); self.routes += 1; return Outcome { added: 1, @@ -610,72 +627,40 @@ impl Shard { Entry::Occupied(occupied) => occupied.into_mut(), }; - let best = usize::from(row.best); - let best_before = row.paths[best].preference(); let (outcome, best_changed) = match row.position(peer, path_id) { None => { row.push(path, path_id); self.routes += 1; - let added = row.paths.len() - 1; - // Better than the best in the first steps of selection is - // better than everything; worse than it changes nothing. A - // tie has to be settled by the later steps. - let best_changed = if preference > best_before { - row.best = added as u16; - true - } else if preference < best_before { - false - } else if let Some(wins) = select::settles_tie(row, added, &self.peers) { - if wins { - row.best = added as u16; - } - wins - } else { - select::choose(row, &self.peers, &mut self.scratch) - }; let outcome = Outcome { added: 1, ..Outcome::default() }; - (outcome, best_changed) + ( + outcome, + select::after_add(row, &self.peers, &mut self.scratch), + ) } Some(at) => { let before = std::mem::replace(&mut row.paths[at], path); - let same = Arc::ptr_eq(&before.attrs, attrs) && before.preference() == preference; - let best_changed = if same { - // Nothing changed at all. - return Outcome { - replaced: 1, - ..Outcome::default() - }; - } else if at == best { - // The best route itself changed. If it got better in - // the first steps it is still the best, but it is a - // different route now. - if preference <= best_before { - select::choose(row, &self.peers, &mut self.scratch); - } - true - } else if preference > best_before { - row.best = at as u16; - true - } else if preference < best_before && before.preference() < best_before { - // It was not in the running and still is not. - false - } else { - select::choose(row, &self.peers, &mut self.scratch) - }; let outcome = Outcome { replaced: 1, ..Outcome::default() }; + if Arc::ptr_eq(&before.attrs, attrs) && before.preference() == preference { + // The same route again: nothing changed at all. + return outcome; + } + let best_changed = select::after_replace( + row, + at, + before.preference(), + &self.peers, + &mut self.scratch, + ); (outcome, best_changed) } }; - if best_changed { - row.changed = self.journal.changed(prefix); - } - row.touched = self.all_journal.changed(prefix); + self.journals.changed(row, best_changed); Outcome { best_changed: best_changed.into(), ..outcome @@ -683,8 +668,8 @@ impl Shard { } fn withdraw(&mut self, hash: u64, nlri: &Nlri, peer: PeerIndex) -> Outcome { - let prefix = &nlri.prefix; - let Ok(mut entry) = self.rows.find_entry(hash, |row| row.prefix == *prefix) else { + let prefix = nlri.prefix; + let Ok(mut entry) = self.rows.find_entry(hash, |row| row.prefix == prefix) else { return Outcome::default(); }; let row = entry.get_mut(); @@ -694,19 +679,15 @@ impl Shard { self.routes -= 1; if row.paths.len() == 1 { entry.remove(); - self.journal.disappeared(*prefix); - self.all_journal.disappeared(*prefix); + self.journals.disappeared(prefix); return Outcome { removed: 1, best_changed: 1, ..Outcome::default() }; } - let best_changed = Self::remove_path(row, at, &self.peers, &mut self.scratch); - if best_changed { - row.changed = self.journal.changed(*prefix); - } - row.touched = self.all_journal.changed(*prefix); + let best_changed = select::after_remove(row, at, &self.peers, &mut self.scratch); + self.journals.changed(row, best_changed); Outcome { removed: 1, best_changed: best_changed.into(), @@ -714,59 +695,30 @@ impl Shard { } } - /// Takes the route at `at` out of a row that has others, and returns - /// whether the best route changed. - fn remove_path( - row: &mut Row, - at: usize, - peers: &[Option], - scratch: &mut select::Scratch, - ) -> bool { - let best = usize::from(row.best); - let best_before = row.paths[best].preference(); - let last = row.paths.len() - 1; - let removed = row.swap_remove(at); - if at == best { - select::choose(row, peers, scratch); - return true; - } - // The last route moved into the gap. - if best == last { - row.best = at as u16; - } - // A route that tied with the best in the first steps may have been - // what kept another from being best, by its MED. - if removed.preference() == best_before { - select::choose(row, peers, scratch) - } else { - false - } - } - fn remove_peer(&mut self, peer: PeerIndex) -> Outcome { let mut outcome = Outcome::default(); let peers = &self.peers; let scratch = &mut self.scratch; - let journal = &mut self.journal; - let all_journal = &mut self.all_journal; + let journals = &mut self.journals; self.rows.retain(|row| { + let mut removed = 0; let mut best_changed = false; // A peer that sends several routes for a prefix has several // here. while let Some(at) = row.position_of_peer(peer) { - outcome.removed += 1; + removed += 1; if row.paths.len() == 1 { + outcome.removed += removed; outcome.best_changed += 1; - journal.disappeared(row.prefix); - all_journal.disappeared(row.prefix); + journals.disappeared(row.prefix); return false; } - best_changed |= Self::remove_path(row, at, peers, scratch); - row.touched = all_journal.changed(row.prefix); + best_changed |= select::after_remove(row, at, peers, scratch); } - if best_changed { - outcome.best_changed += 1; - row.changed = journal.changed(row.prefix); + if removed > 0 { + outcome.removed += removed; + outcome.best_changed += usize::from(best_changed); + journals.changed(row, best_changed); } true }); diff --git a/crates/rib/src/table/select.rs b/crates/rib/src/table/select.rs index f841eb7..1a70b45 100644 --- a/crates/rib/src/table/select.rs +++ b/crates/rib/src/table/select.rs @@ -19,6 +19,12 @@ //! one changes nothing, and neither needs a look at any other route. Only a //! tie goes on to [`choose`], which looks at the whole row, and that begins //! with a pass over the numbers to find the routes still in the running. +//! +//! [`after_add`], [`after_replace`] and [`after_remove`] are what the table +//! calls when a row has changed. Each says whether the best route is now +//! another, and takes the short way where there is one. They rely on, and +//! keep, one fact about a row: its best route has the highest preference +//! of all its routes. use super::attrs::SelectionKey; use super::{DEFAULT_LOCAL_PREF, PeerInfo, Row}; @@ -47,14 +53,93 @@ pub(super) fn preference(key: &SelectionKey, peer: &PeerInfo) -> u64 { #[derive(Default)] pub(super) struct Scratch { /// Routes still in the running: (neighbouring AS, MED, index in row). - candidates: Vec<(u32, u32, u16)>, + candidates: Vec<(u32, u32, u32)>, +} + +/// The best route changed after a route was added at the end of `row`? +pub(super) fn after_add(row: &mut Row, peers: &[Option], scratch: &mut Scratch) -> bool { + let added = row.paths.len() - 1; + let preference = row.paths[added].preference(); + let best = row.paths[row.best()].preference(); + // Better than the best in the first steps of selection is better than + // everything; worse than it changes nothing. A tie has to be settled by + // the later steps. + if preference > best { + row.set_best(added); + true + } else if preference < best { + false + } else if let Some(wins) = settles_tie(row, added, peers) { + if wins { + row.set_best(added); + } + wins + } else { + choose(row, peers, scratch) + } +} + +/// The best route changed after the route at `at` was replaced by another +/// from the same peer? `before` is the preference of the route it replaced. +/// The two are known to differ. +pub(super) fn after_replace( + row: &mut Row, + at: usize, + before: u64, + peers: &[Option], + scratch: &mut Scratch, +) -> bool { + let preference = row.paths[at].preference(); + if at == row.best() { + // The best route itself changed. If it got better in the first + // steps it is still the best, but it is a different route now. + if preference <= before { + choose(row, peers, scratch); + } + return true; + } + let best = row.paths[row.best()].preference(); + if preference > best { + row.set_best(at); + true + } else if preference < best && before < best { + // It was not in the running and still is not. + false + } else { + choose(row, peers, scratch) + } +} + +/// Takes the route at `at` out of `row`, which has others, and returns +/// whether the best route changed. +pub(super) fn after_remove( + row: &mut Row, + at: usize, + peers: &[Option], + scratch: &mut Scratch, +) -> bool { + let best = row.best(); + let best_preference = row.paths[best].preference(); + let last = row.paths.len() - 1; + let removed = row.swap_remove(at); + if at == best { + choose(row, peers, scratch); + return true; + } + // The last route moved into the gap. + if best == last { + row.set_best(at); + } + // A route that tied with the best in the first steps may have been + // what kept another from being best, by its MED. + removed.preference() == best_preference && choose(row, peers, scratch) } /// Chooses the best route of `row` from all its routes, records it in the /// row, and returns whether it is another route than before. /// /// The row must have at least one route. -pub(super) fn choose(row: &mut Row, peers: &[Option], scratch: &mut Scratch) -> bool { +fn choose(row: &mut Row, peers: &[Option], scratch: &mut Scratch) -> bool { let before = row.best; let top = row .paths @@ -70,7 +155,7 @@ pub(super) fn choose(row: &mut Row, peers: &[Option], scratch: &mut Sc let key = path.attrs.key(); // A route without a MED is taken to have the lowest (RFC 4271 // §9.1.2.2 c). - candidates.push((key.neighbor_as, key.med.unwrap_or(0), index as u16)); + candidates.push((key.neighbor_as, key.med.unwrap_or(0), index as u32)); } } @@ -80,7 +165,7 @@ pub(super) fn choose(row: &mut Row, peers: &[Option], scratch: &mut Sc // within each neighbouring AS only the routes with its lowest MED stay // in, and the steps after are an order over what is left. candidates.sort_unstable(); - let mut best: Option<(TieBreak, u16)> = None; + let mut best: Option<(TieBreak, u32)> = None; let mut group = None; for &(neighbor_as, med, index) in candidates.iter() { // Sorted by AS and then MED: the first of each AS has its lowest. @@ -94,7 +179,7 @@ pub(super) fn choose(row: &mut Row, peers: &[Option], scratch: &mut Sc if med != lowest { continue; } - let tie_break = TieBreak::of(row, index, peers); + let tie_break = TieBreak::of(row, index as usize, peers); if best.as_ref().is_none_or(|(least, _)| tie_break < *least) { best = Some((tie_break, index)); } @@ -114,16 +199,16 @@ pub(super) fn choose(row: &mut Row, peers: &[Option], scratch: &mut Sc /// best is not one of those, the best is still the best of all the others. /// So the better of the two is the best of all. This is the usual case: /// most routes have no MED. -pub(super) fn settles_tie(row: &Row, added: usize, peers: &[Option]) -> Option { +fn settles_tie(row: &Row, added: usize, peers: &[Option]) -> Option { let new = row.paths[added].attrs.key(); - let best = row.paths[usize::from(row.best)].attrs.key(); + let best = row.paths[row.best()].attrs.key(); if new.med.unwrap_or(0) != 0 { return None; } if best.neighbor_as == new.neighbor_as && best.med.unwrap_or(0) != 0 { return None; } - Some(TieBreak::of(row, added as u16, peers) < TieBreak::of(row, row.best, peers)) + Some(TieBreak::of(row, added, peers) < TieBreak::of(row, row.best(), peers)) } /// Steps 5 to 7, then the peer's number, and then the path identifier for @@ -139,9 +224,9 @@ struct TieBreak { } impl TieBreak { - fn of(row: &Row, index: u16, peers: &[Option]) -> Self { - let peer = row.paths[usize::from(index)].peer(); - let path_id = row.path_id(usize::from(index)); + fn of(row: &Row, index: usize, peers: &[Option]) -> Self { + let peer = row.paths[index].peer(); + let path_id = row.path_id(index); match peers.get(peer.as_usize()).copied().flatten() { Some(info) => Self { internal: info.ibgp, @@ -150,7 +235,8 @@ impl TieBreak { peer: peer.0, path_id, }, - // A peer on its way out; anything known is preferred to it. + // Not expected: a peer is forgotten only once its routes are + // gone. If it happened, anything known would be preferred. None => Self { internal: true, router_id: u32::MAX, diff --git a/crates/rib/src/table/tests.rs b/crates/rib/src/table/tests.rs index 762c31b..ceef3af 100644 --- a/crates/rib/src/table/tests.rs +++ b/crates/rib/src/table/tests.rs @@ -123,7 +123,7 @@ impl Fixture { prefix: *prefix, path_id, }; - self.table.announce_paths(self.peers[peer], &attrs, &[nlri]) + self.table.announce(self.peers[peer], &attrs, &[nlri]) } pub(super) fn withdraw_path(&self, peer: usize, prefix: &IpPrefix, path_id: u32) -> Outcome { @@ -131,7 +131,7 @@ impl Fixture { prefix: *prefix, path_id, }; - self.table.withdraw_paths(self.peers[peer], &[nlri]) + self.table.withdraw(self.peers[peer], &[nlri]) } /// The best route for `prefix`: which peer (by order added) it is from @@ -391,7 +391,7 @@ fn removing_a_peer_takes_its_routes_and_frees_its_number() { let prefixes: Vec = (0..200u32) .map(|i| { IpPrefix::new( - bgp_primitives::constants::AddressFamilyId::Ipv4, + bgp_primitives::constants::IpVersion::Ipv4, (i << 8).to_be_bytes(), 24, ) @@ -692,3 +692,68 @@ proptest! { } } } + +/// Shards and lock stripes are picked by bits of the hash that the hash +/// table inside does not use, so that keys which share a shard still +/// differ where that table looks. +#[test] +fn keys_of_one_shard_differ_in_the_bits_their_table_uses() { + let hasher = RandomState::default(); + let shards = 64; + let mut counts = vec![0usize; shards]; + let mut tags = vec![std::collections::HashSet::new(); shards]; + for n in 0..64_000u32 { + let hash = hasher.hash_one(n); + let shard = spread(hash, shards); + counts[shard] += 1; + // The seven bits hashbrown keeps as a tag. + tags[shard].insert(hash >> 57); + } + for shard in 0..shards { + assert!( + (700..1300).contains(&counts[shard]), + "shard {shard} has {} of 64000 keys", + counts[shard] + ); + assert_eq!(tags[shard].len(), 128, "shard {shard}"); + } + assert_eq!(spread(u64::MAX, 3), 2); + assert_eq!(spread(0, 3), 0); +} + +/// A peer that sends several routes for a prefix can send more than fit a +/// sixteen-bit index. The best route was kept as one, and wrapped. +#[test] +fn a_row_can_hold_more_routes_than_a_sixteen_bit_index_counts() { + let store = AttrStore::new(); + let worse = store + .intern(&Route::via(&[1, 2]).attributes(), None) + .unwrap(); + let better = store.intern(&Route::via(&[1]).attributes(), None).unwrap(); + let info = external(1); + let peers = [Some(info)]; + let peer = PeerIndex(0); + let path = + |attrs: &Arc| Path::new(select::preference(attrs.key(), &info), peer, attrs); + let seqs = journal::Journals::new(journal::Limits::default()).appeared(prefix("10.0.0.0/8")); + + let mut row = Row::new(prefix("10.0.0.0/8"), path(&worse), 1, seqs); + let mut scratch = select::Scratch::default(); + let count = usize::from(u16::MAX) + 10; + for path_id in 2..=count as u32 { + row.push(path(&worse), path_id); + } + // Equal routes from one peer: the lowest identifier is the best. + assert!(!select::after_add(&mut row, &peers, &mut scratch)); + assert_eq!(row.best(), 0); + + row.push(path(&better), count as u32 + 1); + assert!(select::after_add(&mut row, &peers, &mut scratch)); + assert_eq!(row.best(), count); + assert_eq!(row.route(row.best()).path_id, count as u32 + 1); + + // With the better one gone, the choice among all the rest is the + // lowest identifier again. + assert!(select::after_remove(&mut row, count, &peers, &mut scratch)); + assert_eq!(row.route(row.best()).path_id, 1); +} diff --git a/crates/server/src/advertise.rs b/crates/server/src/advertise.rs index 61def4d..4757086 100644 --- a/crates/server/src/advertise.rs +++ b/crates/server/src/advertise.rs @@ -14,7 +14,7 @@ use bgp_packet::message::{ Message, MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, MultiExitDiscPathAttribute, NextHopPathAttribute, NlriNextHop, PathAttribute, UpdateMessage, }; -use bgp_primitives::constants::{AddressFamilyId, Afi, Safi}; +use bgp_primitives::constants::{AfiSafi, IpVersion}; use bgp_primitives::ip_prefix::{IpPrefix, Nlri}; use bgp_rib::{LocalRib, LocalRibEvent, LocalRoute, RibStopped}; use tokio::sync::broadcast::error::RecvError; @@ -179,11 +179,11 @@ pub fn build_announce( let mut announced_routes = vec![]; match (route.prefix.address_family(), next_hop) { - (AddressFamilyId::Ipv4, IpAddr::V4(nh)) => { + (IpVersion::Ipv4, IpAddr::V4(nh)) => { path_attributes.push(PathAttribute::NextHop(NextHopPathAttribute(nh))); announced_routes.push(route.prefix.into()); } - (AddressFamilyId::Ipv6, IpAddr::V6(_)) => {} + (IpVersion::Ipv6, IpAddr::V6(_)) => {} _ => return None, } @@ -197,10 +197,9 @@ pub fn build_announce( attrs.communities.clone(), ))); } - if let (AddressFamilyId::Ipv6, IpAddr::V6(nh)) = (route.prefix.address_family(), next_hop) { + if let (IpVersion::Ipv6, IpAddr::V6(nh)) = (route.prefix.address_family(), next_hop) { path_attributes.push(PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, next_hop: NlriNextHop::Ipv6(nh), prefixes: vec![route.prefix.into()], })); @@ -224,16 +223,15 @@ pub fn build_announce( /// Builds the UPDATE withdrawing `prefix`. pub fn build_withdraw(prefix: &IpPrefix) -> UpdateMessage { match prefix.address_family() { - AddressFamilyId::Ipv4 => UpdateMessage { + IpVersion::Ipv4 => UpdateMessage { withdrawn_routes: vec![Nlri::from(*prefix)], path_attributes: vec![], announced_routes: vec![], }, - AddressFamilyId::Ipv6 => UpdateMessage { + IpVersion::Ipv6 => UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, prefixes: vec![Nlri::from(*prefix)], })], announced_routes: vec![], @@ -300,15 +298,14 @@ mod tests { // Serialise and parse back to make sure the wire encoding is sound. let ctx = ParserContext { - four_octet_asn: Some(true), + four_octet_asn: true, ibgp: false, add_path: Vec::new(), }; let msg = Message::Update(update.clone()); let mut buf = BytesMut::new(); msg.to_wire(&ctx, &mut buf).unwrap(); - let (_, parsed) = Message::from_wire(&ctx, &buf).unwrap(); - let Message::Update(parsed) = parsed else { + let Message::Update(parsed) = Message::decode(&ctx, &buf).unwrap() else { panic!("expected UPDATE") }; assert_eq!(parsed, update); @@ -359,13 +356,13 @@ mod tests { // What the peer reads from the wire. let ctx = ParserContext { - four_octet_asn: Some(false), + four_octet_asn: false, ibgp: false, add_path: Vec::new(), }; let mut buf = BytesMut::new(); Message::Update(update).to_wire(&ctx, &mut buf).unwrap(); - let (_, Message::Update(seen)) = Message::from_wire(&ctx, &buf).unwrap() else { + let Message::Update(seen) = Message::decode(&ctx, &buf).unwrap() else { panic!("expected UPDATE") }; assert_eq!(path(&seen, false), Some(vec![23456; 2]), "AS_TRANS"); diff --git a/crates/server/src/connection.rs b/crates/server/src/connection.rs index bb28153..ff927d5 100644 --- a/crates/server/src/connection.rs +++ b/crates/server/src/connection.rs @@ -355,7 +355,7 @@ impl Session { } Action::Admit(negotiated) => { self.ctx = ParserContext { - four_octet_asn: Some(negotiated.four_octet_asn), + four_octet_asn: negotiated.four_octet_asn, ibgp: negotiated.peer_asn == self.config.local_asn, add_path: negotiated.add_path_receive.iter().collect(), }; diff --git a/crates/server/src/family.rs b/crates/server/src/family.rs index dbc6913..1024417 100644 --- a/crates/server/src/family.rs +++ b/crates/server/src/family.rs @@ -15,7 +15,6 @@ use bgp_packet::message::{PathAttribute, UpdateMessage}; use bgp_packet::open::{Capability, OpenMessage}; use bgp_primitives::constants::AfiSafi; use bgp_primitives::ip_prefix::IpPrefix; -use bgp_primitives::parser::ParserContext; use bytes::BytesMut; /// The address families this speaker can exchange routes for. Supporting a @@ -82,15 +81,11 @@ impl FamilySet { /// These families as Multiprotocol capabilities in wire form: the Data /// of an Unsupported Capability NOTIFICATION (RFC 5492 §5). pub fn capabilities_wire(&self) -> Vec { - let ctx = ParserContext::default(); let mut out = BytesMut::new(); for family in self.iter() { // A Multiprotocol capability always encodes; if that ever // changed, a NOTIFICATION with less Data is still correct. - if Capability::multiprotocol(family) - .to_wire(&ctx, &mut out) - .is_err() - { + if Capability::multiprotocol(family).to_wire(&mut out).is_err() { return vec![]; } } @@ -112,8 +107,8 @@ impl FamilySet { } update.path_attributes.retain(|attribute| { let (family, prefixes) = match attribute { - PathAttribute::MpReachNlri(mp) => (mp.family(), mp.prefixes.len()), - PathAttribute::MpUnreachNlri(mp) => (mp.family(), mp.prefixes.len()), + PathAttribute::MpReachNlri(mp) => (mp.family, mp.prefixes.len()), + PathAttribute::MpUnreachNlri(mp) => (mp.family, mp.prefixes.len()), _ => return true, }; let keep = self.contains(family); @@ -257,8 +252,7 @@ mod tests { path_attributes: vec![ PathAttribute::Origin(OriginPathAttribute::IGP), PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, next_hop: NlriNextHop::Ipv6(Ipv6Addr::LOCALHOST), prefixes: vec![ prefix("2001:db8:1::/48").into(), @@ -266,8 +260,7 @@ mod tests { ], }), PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, prefixes: vec![prefix("2001:db8:3::/48").into()], }), ], diff --git a/crates/server/src/fsm.rs b/crates/server/src/fsm.rs index 20d12e4..2f9a737 100644 --- a/crates/server/src/fsm.rs +++ b/crates/server/src/fsm.rs @@ -425,7 +425,7 @@ mod tests { MpReachNlriPathAttribute, NlriNextHop, NotificationMessage, PathAttribute, RouteRefreshMessage, }; - use bgp_primitives::constants::{Afi, AfiSafi, Safi}; + use bgp_primitives::constants::AfiSafi; use bgp_primitives::ip_prefix::IpPrefix; use super::*; @@ -664,7 +664,7 @@ mod tests { /// offered to send and the other to receive. #[test] fn add_path_is_in_use_where_one_side_sends_and_the_other_receives() { - use AddPathSendRecv::{Both, Receive, Send}; + use bgp_packet::open::SendReceive::{Both, Receive, Send}; let negotiated = |ours: Option, theirs: Vec<(AfiSafi, AddPathSendRecv)>| { let (mut fsm, first) = Fsm::new(FsmConfig { @@ -747,8 +747,7 @@ mod tests { let update = UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: Afi::IPV6, - safi: Safi::UNICAST, + family: AfiSafi::IPV6_UNICAST, next_hop: NlriNextHop::Ipv6(std::net::Ipv6Addr::LOCALHOST), prefixes: vec![IpPrefix::try_from("2001:db8::/32").unwrap().into()], })], diff --git a/crates/server/src/routes.rs b/crates/server/src/routes.rs index 16bac38..606894e 100644 --- a/crates/server/src/routes.rs +++ b/crates/server/src/routes.rs @@ -29,11 +29,11 @@ pub(crate) fn apply( let mut outcome = Outcome::default(); if !update.withdrawn_routes.is_empty() { - outcome += table.withdraw_paths(peer, &update.withdrawn_routes); + outcome += table.withdraw(peer, &update.withdrawn_routes); } for attribute in &update.path_attributes { if let PathAttribute::MpUnreachNlri(mp) = attribute { - outcome += table.withdraw_paths(peer, &mp.prefixes); + outcome += table.withdraw(peer, &mp.prefixes); } } @@ -47,7 +47,7 @@ pub(crate) fn apply( }; let mut announce = |nlri: &[Nlri], next_hop| -> Result<(), EncodeError> { if !nlri.is_empty() { - outcome += table.announce_paths(peer, &attributes(next_hop)?, nlri); + outcome += table.announce(peer, &attributes(next_hop)?, nlri); } Ok(()) }; diff --git a/crates/web/src/api.rs b/crates/web/src/api.rs index 95d4a31..f2dfd29 100644 --- a/crates/web/src/api.rs +++ b/crates/web/src/api.rs @@ -273,7 +273,8 @@ impl PeerNames { /// the route is on its way out of the table. pub(crate) fn view(&self, route: &Route) -> Option { let peer = self.0.get(&route.peer)?; - let (attributes, next_hop) = route.attrs.attributes(); + // What a session stored was accepted by the decoder, and parses. + let (attributes, next_hop) = route.attrs.attributes().unwrap_or_default(); Some(ReceivedRouteView { peer_addr: peer.addr.to_string(), peer_asn: peer.asn, @@ -365,6 +366,10 @@ pub async fn local_routes( /// number. The table can hold hundreds of millions. const DEFAULT_ROUTE_LIMIT: usize = 1000; +/// The most routes [`received_routes`] returns, whatever is asked for: +/// each is copied out of the table and written as JSON. +const MAX_ROUTE_LIMIT: usize = 100_000; + #[derive(Debug, Deserialize)] pub struct RouteQuery { /// At most this many routes. @@ -376,14 +381,26 @@ pub struct RouteQuery { pub async fn received_routes( State(state): State, Query(query): Query, -) -> Json> { +) -> Result>, StatusCode> { + let limit = query + .limit + .unwrap_or(DEFAULT_ROUTE_LIMIT) + .min(MAX_ROUTE_LIMIT); let names = PeerNames::of(&state.registry); - let mut routes: Vec = state - .rib - .some_routes(query.limit.unwrap_or(DEFAULT_ROUTE_LIMIT)) - .iter() - .filter_map(|route| names.view(route)) - .collect(); - routes.sort_by(|a, b| (&a.prefix, &a.peer_addr).cmp(&(&b.prefix, &b.peer_addr))); - Json(routes) + // Copying, parsing and sorting many routes takes long enough to keep + // off the threads that sessions run on. + let routes = tokio::task::spawn_blocking(move || { + let mut routes: Vec = state + .rib + .some_routes(limit) + .iter() + .filter_map(|route| names.view(route)) + .collect(); + routes.sort_by(|a, b| (&a.prefix, &a.peer_addr).cmp(&(&b.prefix, &b.peer_addr))); + routes + }) + .await + // Only if making the views panicked. + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + Ok(Json(routes)) } diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index c6a6b50..2f1825e 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -29,7 +29,7 @@ impl Session { fn context(self) -> ParserContext { ParserContext { - four_octet_asn: Some(self.four_octet_asn), + four_octet_asn: self.four_octet_asn, ibgp: self.ibgp, add_path: [AfiSafi::IPV4_UNICAST, AfiSafi::IPV6_UNICAST] .into_iter() @@ -62,10 +62,6 @@ pub fn decode_and_check(session: Session, wire: &[u8]) { Err(EncodeError::TooLong { .. }) if !repairs.is_empty() => return, Err(e) => panic!("an accepted message cannot be written: {e}"), } - assert_eq!( - usize::from(msg.wire_len(&ctx).expect("and has a length")), - rewritten.len() - ); // An UPDATE the decoder had to repair (RFC 7606) is not what was on the // wire, and need not read back as itself. if repairs.is_empty() {