diff --git a/crates/integration-tests/src/test_peer.rs b/crates/integration-tests/src/test_peer.rs index b7d9cc1..1987ffc 100644 --- a/crates/integration-tests/src/test_peer.rs +++ b/crates/integration-tests/src/test_peer.rs @@ -2,7 +2,7 @@ use std::net::{Ipv4Addr, SocketAddr}; use bgp_packet::message::{BGP_HEADER_LEN, Message, UpdateMessage}; use bgp_packet::open::{Capability, OpenMessage, OpenOption}; -use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; +use bgp_primitives::constants::{Afi, Safi}; use bgp_primitives::parser::ParserContext; use bytes::BytesMut; use eyre::{Result, bail, eyre}; @@ -104,12 +104,12 @@ impl TestPeer { let capabilities = vec![ Capability::FourByteAsn { asn: local_asn }, Capability::MultiProtocol { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV4, + safi: Safi::UNICAST, }, Capability::MultiProtocol { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, }, Capability::RouteRefresh, ]; diff --git a/crates/integration-tests/tests/basic_connection.rs b/crates/integration-tests/tests/basic_connection.rs index a83e976..dbfa965 100644 --- a/crates/integration-tests/tests/basic_connection.rs +++ b/crates/integration-tests/tests/basic_connection.rs @@ -8,7 +8,7 @@ use bgp_packet::message::{ Message, MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, NlriNextHop, NotificationMessage, PathAttribute, UpdateMessage, }; -use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; +use bgp_primitives::constants::{AddressFamilyId, Afi, Safi}; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::RibEvent; use eyre::Result; @@ -346,8 +346,8 @@ async fn test_server_accepts_ipv6_update() -> Result<()> { let update = Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6("2001:db8::1".parse().unwrap()), prefixes: vec![prefix], })], @@ -453,8 +453,8 @@ async fn test_rib_records_ipv6_announce() -> Result<()> { peer.send(&Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6("2001:db8::1".parse().unwrap()), prefixes: vec![prefix.clone()], })], @@ -492,8 +492,8 @@ async fn test_rib_records_ipv6_withdraw() -> Result<()> { peer.send(&Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6("2001:db8::1".parse().unwrap()), prefixes: vec![prefix.clone()], })], @@ -506,8 +506,8 @@ async fn test_rib_records_ipv6_withdraw() -> Result<()> { peer.send(&Message::Update(UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, prefixes: vec![prefix.clone()], })], announced_routes: vec![], diff --git a/crates/integration-tests/tests/collision.rs b/crates/integration-tests/tests/collision.rs index 56edcb9..6022121 100644 --- a/crates/integration-tests/tests/collision.rs +++ b/crates/integration-tests/tests/collision.rs @@ -40,7 +40,7 @@ async fn server_dialing(listener: &TcpListener) -> Result { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: None, - families: FamilySet::ALL, + families: FamilySet::supported(), }]; }) .await diff --git a/crates/integration-tests/tests/dialer.rs b/crates/integration-tests/tests/dialer.rs index 8b76709..ab233b2 100644 --- a/crates/integration-tests/tests/dialer.rs +++ b/crates/integration-tests/tests/dialer.rs @@ -31,7 +31,7 @@ async fn server_dialing(listener: &TcpListener, remote_asn: u32) -> Result Result<()> { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: None, - families: FamilySet::ALL, + families: FamilySet::supported(), }]; }) .await?; @@ -231,7 +231,7 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: Some(Ipv4Addr::LOCALHOST.into()), - families: FamilySet::ALL, + families: FamilySet::supported(), }]; }) .await?; @@ -247,7 +247,7 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: Some(Ipv4Addr::new(192, 0, 2, 1).into()), - families: FamilySet::ALL, + families: FamilySet::supported(), }]; }) .await?; diff --git a/crates/integration-tests/tests/interop.rs b/crates/integration-tests/tests/interop.rs index da41343..6405cec 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::{AddressFamilyId, SubsequentAfi}; +use bgp_primitives::constants::{Afi, Safi}; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use eyre::Result; @@ -61,9 +61,9 @@ async fn test_server_advertises_ipv4_and_ipv6_unicast() -> Result<()> { OpenOption::Capabilities(c) => c.iter(), }) .collect(); - for afi in [AddressFamilyId::Ipv4, AddressFamilyId::Ipv6] { + for afi in [Afi::IPV4, Afi::IPV6] { assert!( - caps.iter().any(|c| matches!(c, Capability::MultiProtocol { afi: a, safi: SubsequentAfi::Unicast } if *a == afi)), + caps.iter().any(|c| matches!(c, Capability::MultiProtocol { afi: a, safi: Safi::UNICAST } if *a == afi)), "missing MP capability for {afi:?}: {caps:?}" ); } @@ -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 == AddressFamilyId::Ipv6 && m.prefixes[0].to_string() == "2001:db8:1::/48")) + if m.afi == Afi::IPV6 && 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 68325be..7752425 100644 --- a/crates/integration-tests/tests/negotiation.rs +++ b/crates/integration-tests/tests/negotiation.rs @@ -12,11 +12,11 @@ use bgp_packet::message::{ OriginPathAttribute, PathAttribute, UpdateMessage, }; use bgp_packet::open::Capability; -use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; +use bgp_primitives::constants::{Afi, AfiSafi, Safi}; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::config::PeerConfig; -use bgp_server::family::{AddressFamily, FamilySet}; +use bgp_server::family::{FamilySet, SUPPORTED}; use eyre::{Result, bail}; const LOCAL_ASN: u32 = 65000; @@ -28,7 +28,7 @@ fn prefix(s: &str) -> IpPrefix { IpPrefix::try_from(s).unwrap() } -fn only(family: AddressFamily) -> FamilySet { +fn only(family: AfiSafi) -> FamilySet { [family].into_iter().collect() } @@ -58,8 +58,8 @@ fn dual_stack_update() -> UpdateMessage { AsPathAttribute::from_asns(vec![PEER_ASN]), PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1)), prefixes: vec![prefix("2001:db8:1::/48")], }), @@ -79,18 +79,22 @@ async fn test_families_are_the_ones_both_sides_offered() -> Result<()> { PEER_ASN, PEER_ID, 90, - vec![four_octet(), AddressFamily::Ipv4Unicast.capability()], + vec![ + four_octet(), + Capability::multiprotocol(AfiSafi::IPV4_UNICAST), + ], ) .await?; - for family in AddressFamily::ALL { + for family in SUPPORTED { assert!( - open.capabilities().any(|c| *c == family.capability()), + open.capabilities() + .any(|c| *c == Capability::multiprotocol(family)), "the server's OPEN offers {family}" ); } let session = wait_for_session(&server).await?; - assert_eq!(session.families, only(AddressFamily::Ipv4Unicast)); + assert_eq!(session.families, only(AfiSafi::IPV4_UNICAST)); Ok(()) } @@ -104,7 +108,10 @@ async fn test_routes_of_a_family_not_negotiated_are_not_accepted() -> Result<()> PEER_ASN, PEER_ID, 90, - vec![four_octet(), AddressFamily::Ipv4Unicast.capability()], + vec![ + four_octet(), + Capability::multiprotocol(AfiSafi::IPV4_UNICAST), + ], ) .await?; wait_for_session(&server).await?; @@ -146,11 +153,14 @@ async fn test_routes_of_a_family_not_negotiated_are_not_sent() -> Result<()> { PEER_ASN, PEER_ID, 90, - vec![four_octet(), AddressFamily::Ipv6Unicast.capability()], + vec![ + four_octet(), + Capability::multiprotocol(AfiSafi::IPV6_UNICAST), + ], ) .await?; let session = wait_for_session(&server).await?; - assert_eq!(session.families, only(AddressFamily::Ipv6Unicast)); + assert_eq!(session.families, only(AfiSafi::IPV6_UNICAST)); // A second peer that takes IPv4 is sent the route. The advertiser // handles sessions in the order they came up, so by the time this one @@ -178,7 +188,7 @@ async fn test_no_family_in_common_refuses_the_session() -> Result<()> { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: None, - families: only(AddressFamily::Ipv6Unicast), + families: only(AfiSafi::IPV6_UNICAST), }]; // Keep the dialer out of the way. c.connect_retry = Duration::from_secs(3600); @@ -190,7 +200,10 @@ async fn test_no_family_in_common_refuses_the_session() -> Result<()> { PEER_ASN, PEER_ID, 90, - vec![four_octet(), AddressFamily::Ipv4Unicast.capability()], + vec![ + four_octet(), + Capability::multiprotocol(AfiSafi::IPV4_UNICAST), + ], ) .await?; let open = peer.recv_open().await?; @@ -198,7 +211,7 @@ async fn test_no_family_in_common_refuses_the_session() -> Result<()> { .capabilities() .filter(|c| matches!(c, Capability::MultiProtocol { .. })) .collect(); - assert_eq!(offered, [&AddressFamily::Ipv6Unicast.capability()]); + assert_eq!(offered, [&Capability::multiprotocol(AfiSafi::IPV6_UNICAST)]); match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { Message::Notification(n) => { @@ -215,10 +228,10 @@ async fn test_no_family_in_common_refuses_the_session() -> Result<()> { Ok(()) } -/// Real routers offer families we have never heard of. That must not stop +/// Real routers offer families we do not support. That must not stop /// the families we do share from working. #[tokio::test] -async fn test_a_peer_offering_an_unknown_family_is_still_accepted() -> Result<()> { +async fn test_a_peer_offering_an_unsupported_family_is_still_accepted() -> Result<()> { let server = TestServer::start(LOCAL_ASN).await?; let mut peer = TestPeer::connect(server.addr).await?; peer.handshake_with( @@ -227,19 +240,14 @@ async fn test_a_peer_offering_an_unknown_family_is_still_accepted() -> Result<() 90, vec![ four_octet(), - // Multiprotocol for L2VPN EVPN (AFI 25, SAFI 70). - Capability::UnknownCapability { - type_code: 1, - length: 4, - value: vec![0, 25, 0, 70], - }, - AddressFamily::Ipv4Unicast.capability(), + Capability::multiprotocol(AfiSafi::new(Afi::L2VPN, Safi::EVPN)), + Capability::multiprotocol(AfiSafi::IPV4_UNICAST), ], ) .await?; let session = wait_for_session(&server).await?; - assert_eq!(session.families, only(AddressFamily::Ipv4Unicast)); + assert_eq!(session.families, only(AfiSafi::IPV4_UNICAST)); assert!( session.four_octet_asn, "capabilities after it are still read" @@ -265,7 +273,7 @@ async fn test_a_peer_without_capabilities_gets_two_octet_as_paths() -> Result<() peer.handshake_with(PEER_ASN, PEER_ID, 90, vec![]).await?; let session = wait_for_session(&server).await?; assert!(!session.four_octet_asn); - assert_eq!(session.families, only(AddressFamily::Ipv4Unicast)); + assert_eq!(session.families, only(AfiSafi::IPV4_UNICAST)); // The test peer now decodes AS_PATH two octets wide, as such a speaker // would; an UPDATE encoded four octets wide would not parse. diff --git a/crates/packet/src/message.rs b/crates/packet/src/message.rs index b2950b5..08eaa51 100644 --- a/crates/packet/src/message.rs +++ b/crates/packet/src/message.rs @@ -14,8 +14,7 @@ use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; use strum::EnumDiscriminants; -use bgp_primitives::constants::AddressFamilyId; -use bgp_primitives::constants::SubsequentAfi; +use bgp_primitives::constants::{AddressFamilyId, Afi, AfiSafi, Safi}; use bgp_primitives::ip_prefix::IpPrefix; use bgp_primitives::parser::BgpParserError; use bgp_primitives::parser::EncodeError; @@ -120,8 +119,6 @@ impl Message { ProtocolErrorValues::UnknownOpenOption(_) => { OpenMsgSubcode::UnsupportedOptionalParam } - // Handled inside the capability parser. - ProtocolErrorValues::UnknownAddressFamily => OpenMsgSubcode::Unspecific, }, _ => OpenMsgSubcode::Unspecific, }; @@ -1415,80 +1412,85 @@ fn parse_prefix( Ok((buf, prefix)) } +/// The IP version of the prefixes that make up the NLRI of `family`. +/// +/// Any family can be named in MP_REACH_NLRI and MP_UNREACH_NLRI, but only +/// unicast and multicast routes of IPv4 and IPv6 are plain prefixes, and +/// those are the only NLRI this parser reads. A peer does not send routes +/// 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" + ))) + }; + if family.safi != Safi::UNICAST && family.safi != Safi::MULTICAST { + return Err(unsupported()); + } + AddressFamilyId::try_from(family.afi).map_err(|_| unsupported()) +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct MpReachNlriPathAttribute { - pub afi: AddressFamilyId, - pub safi: SubsequentAfi, + pub afi: Afi, + pub safi: Safi, /// Next hop address (either IPv4 or IPv6 for now). 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>( _: &ParserContext, buf: &'a [u8], ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, raw_afi) = be_u16(buf)?; - let afi = AddressFamilyId::try_from(raw_afi) - .map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; - - let (buf, raw_safi) = be_u8(buf)?; - let safi = SubsequentAfi::try_from(raw_safi) - .map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; + 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 version = ip_version_of_nlri(AfiSafi::new(afi, safi))?; let (buf, nh_len) = be_u8(buf)?; - - let (buf, next_hop, prefixes) = match afi { - AddressFamilyId::Ipv4 => { - // Read the length of the nexthop which should equal 4. - if nh_len != 4 { - return Err(Failure(BgpParserError::Malformed(format!( - "Got nexthop address length {} when expected 4 for IPv4 AFI", - nh_len - )))); - } - // Read the nexthop address which should now be an IPv4 address. - let (buf, nh_bytes) = be_u32(buf)?; - let next_hop = NlriNextHop::Ipv4(Ipv4Addr::from(nh_bytes)); - - let (buf, prefixes) = - nom::multi::many0(|buf| parse_prefix(AddressFamilyId::Ipv4, buf)).parse(buf)?; - (buf, next_hop, prefixes) + 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" + )))); } - AddressFamilyId::Ipv6 => { - // https://datatracker.ietf.org/doc/html/rfc2545 defines that the nexthop address may be 16 or 32 bytes long. - let (buf, nh_bytes) = nom::bytes::take(nh_len as usize).parse(buf)?; - let nexthop = match nh_bytes.len() { - 16 => { - // unwrap should never fire since we have explicitly checked the length. - let slice: [u8; 16] = nh_bytes.try_into().unwrap(); - NlriNextHop::Ipv6(Ipv6Addr::from(slice)) - } - 32 => { - // unwrap should never fire since we have explicitly checked the length. - let slice: [u8; 32] = nh_bytes.try_into().unwrap(); - let global_bytes: [u8; 16] = slice[0..16].try_into().unwrap(); - let global = Ipv6Addr::from(global_bytes); - let link_local_bytes: [u8; 16] = slice[16..32].try_into().unwrap(); - let link_local = Ipv6Addr::from(link_local_bytes); - NlriNextHop::Ipv6WithLl { global, link_local } - } - _ => { - return Err(Failure(BgpParserError::Malformed(format!( - "Mismatched IPv6 nexthop length, got {}, want 16 or 32", - nh_bytes.len() - )))); - } - }; - // Reserved 0 byte (formerly SNPA). - let (buf, _) = be_u8(buf)?; - let (buf, prefixes) = - nom::multi::many0(|buf| parse_prefix(AddressFamilyId::Ipv6, buf)).parse(buf)?; - (buf, nexthop, prefixes) + // 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" + )))); } }; + // Reserved byte (formerly the SNPA count), for every family. + let (buf, _) = be_u8(buf)?; + let (buf, prefixes) = nom::multi::many0(|buf| parse_prefix(version, buf)).parse(buf)?; + Ok(( buf, Self { @@ -1501,8 +1503,8 @@ impl MpReachNlriPathAttribute { } pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - out.put_u16(self.afi as u16); - out.put_u8(self.safi as u8); + out.put_u16(self.afi.0); + out.put_u8(self.safi.0); out.put_u8(self.next_hop.wire_len()); self.next_hop.to_wire(ctx, out)?; out.put_u8(0); @@ -1527,25 +1529,25 @@ impl MpReachNlriPathAttribute { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct MpUnreachNlriPathAttribute { - pub afi: AddressFamilyId, - pub safi: SubsequentAfi, + pub afi: Afi, + pub safi: Safi, pub prefixes: Vec, } impl MpUnreachNlriPathAttribute { + pub fn family(&self) -> AfiSafi { + AfiSafi::new(self.afi, self.safi) + } + pub fn from_wire<'a>( _: &ParserContext, buf: &'a [u8], ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (buf, raw_afi) = be_u16(buf)?; - let afi = AddressFamilyId::try_from(raw_afi) - .map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; + 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 version = ip_version_of_nlri(AfiSafi::new(afi, safi))?; - let (buf, raw_safi) = be_u8(buf)?; - let safi = SubsequentAfi::try_from(raw_safi) - .map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; - - let (buf, prefixes) = nom::multi::many0(|buf| parse_prefix(afi, buf)).parse(buf)?; + let (buf, prefixes) = nom::multi::many0(|buf| parse_prefix(version, buf)).parse(buf)?; Ok(( buf, @@ -1558,8 +1560,8 @@ impl MpUnreachNlriPathAttribute { } pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - out.put_u16(self.afi as u16); - out.put_u8(self.safi as u8); + out.put_u16(self.afi.0); + out.put_u8(self.safi.0); for prefix in &self.prefixes { out.put_u8(prefix.length); out.put(&prefix.prefix[..]); @@ -1639,7 +1641,7 @@ mod tests { }; use bgp_primitives::{ - constants::{AddressFamilyId, SubsequentAfi}, + constants::{AddressFamilyId, Afi, Safi}, ip_prefix::IpPrefix, parser::ParserContext, }; @@ -1788,8 +1790,8 @@ mod tests { 0x01, 0x0d, 0xb8 ], PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6WithLl { global: ipv6!("2001:db8::1"), link_local: ipv6!("fe80::12"), @@ -1809,8 +1811,8 @@ mod tests { 0x80, 0x0f, 0x08, 0x00, 0x02, 0x01, 0x20, 0x20, 0x01, 0x0d, 0xb8 ], PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, prefixes: vec![IpPrefix { address_family: AddressFamilyId::Ipv6, prefix: vec![0x20, 0x01, 0x0d, 0xb8], @@ -2025,6 +2027,53 @@ mod tests { ); // Rejects trailing garbage after the NLRI section (a partial prefix). + /// The reserved octet after the next hop is there for every family. It + /// used to be skipped only for IPv6, so an IPv4 MP_REACH_NLRI gained a + /// 0.0.0.0/0 route that the peer never sent. + #[test] + fn mp_reach_for_ipv4_has_no_phantom_default_route() { + let ctx = default_v6_context(); + #[rustfmt::skip] + let wire = [ + 0x00, 0x01, 0x01, // IPv4 unicast + 0x04, 192, 0, 2, 1, // next hop + 0x00, // reserved + 0x18, 10, 1, 2, // 10.1.2.0/24 + ]; + let (rest, parsed) = MpReachNlriPathAttribute::from_wire(&ctx, &wire).unwrap(); + assert!(rest.is_empty()); + assert_eq!( + parsed.family(), + bgp_primitives::constants::AfiSafi::IPV4_UNICAST + ); + assert_eq!( + parsed.next_hop, + NlriNextHop::Ipv4(Ipv4Addr::new(192, 0, 2, 1)) + ); + assert_eq!(parsed.prefixes, vec![v4_prefix(24, &[10, 1, 2])]); + + let mut out = BytesMut::new(); + parsed.to_wire(&ctx, &mut out).unwrap(); + assert_eq!(&out[..], &wire[..]); + } + + /// Any family can be named, but only IP prefixes can be read as NLRI. + /// Reading the routes of another family as prefixes would invent routes. + #[test] + fn nlri_of_a_family_that_is_not_ip_prefixes_is_refused() { + let ctx = default_v6_context(); + // L2VPN EVPN, and IPv4 with labels: neither is a list of prefixes. + for family in [[0x00, 0x19, 0x46], [0x00, 0x01, 0x04]] { + let mut reach = family.to_vec(); + reach.extend_from_slice(&[0x04, 192, 0, 2, 1, 0x00, 0x18, 10, 1, 2]); + assert!(MpReachNlriPathAttribute::from_wire(&ctx, &reach).is_err()); + + let mut unreach = family.to_vec(); + unreach.extend_from_slice(&[0x18, 10, 1, 2]); + assert!(MpUnreachNlriPathAttribute::from_wire(&ctx, &unreach).is_err()); + } + } + /// AS_PATH is as wide as the session negotiated; AS4_PATH is always /// four octets wide (RFC 6793). #[test] diff --git a/crates/packet/src/open.rs b/crates/packet/src/open.rs index e9656bf..8443a02 100644 --- a/crates/packet/src/open.rs +++ b/crates/packet/src/open.rs @@ -13,7 +13,7 @@ use crate::errors::InvalidValue; use strum::EnumDiscriminants; use bgp_primitives::{ - constants::{AddressFamilyId, SubsequentAfi}, + constants::{Afi, AfiSafi, Safi}, parser::{BgpParserError, EncodeError, ParserContext, ProtocolErrorValues}, }; @@ -222,30 +222,11 @@ impl OpenOption { } } -/// An AFI in a capability, or the marker that makes the capability unknown. -fn known_afi<'a>(afi: u16) -> Result>> { - AddressFamilyId::try_from(afi).map_err(|_| unknown_family()) -} - -/// A SAFI in a capability, or the marker that makes the capability unknown. -fn known_safi<'a, T>(safi: T) -> Result>> -where - SubsequentAfi: TryFrom, -{ - SubsequentAfi::try_from(safi).map_err(|_| unknown_family()) -} - -fn unknown_family<'a>() -> nom::Err> { - Failure(BgpParserError::ProtocolError( - ProtocolErrorValues::UnknownAddressFamily, - )) -} - /// One address family in a Multiple Labels capability (RFC 8277 §2.1). #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] pub struct MultiLabelEntry { - pub afi: u16, - pub safi: u8, + pub afi: Afi, + pub safi: Safi, /// Maximum number of labels in one NLRI of this family. pub count: u8, } @@ -255,16 +236,16 @@ pub struct MultiLabelEntry { pub enum Capability { /// MultiProtocol Extension (RFC 2858). MultiProtocol { - afi: AddressFamilyId, - safi: SubsequentAfi, + 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: AddressFamilyId, - safi: SubsequentAfi, + afi: Afi, + safi: Safi, orfs: Vec, } = 3, /// Extended Next Hop encoding (RFC 8950). @@ -276,7 +257,7 @@ pub enum Capability { /// BGPSec (RFC 8205). BgpSec { header: BgpSecHeader, - afi: AddressFamilyId, + afi: Afi, } = 7, /// Multiple labels (RFC 8277 §2.1): for each address family, how many /// labels the speaker can handle in one NLRI. @@ -311,8 +292,8 @@ pub enum Capability { } = 65, /// Additional Path (RFC 7911). AddPath { - afi: AddressFamilyId, - safi: SubsequentAfi, + afi: Afi, + safi: Safi, send_recv: AddPathSendRecv, } = 69, /// Enhanced Route Refresh (RFC 7313). @@ -350,12 +331,19 @@ impl Capability { Capability::UnknownCapability { type_code, .. } => *type_code, } } + /// The Multiprotocol capability that offers `family` to a peer. + pub fn multiprotocol(family: AfiSafi) -> Self { + Self::MultiProtocol { + afi: family.afi, + safi: family.safi, + } + } + /// `from_wire` parses a Capability message. /// - /// A capability we know, about an address family we do not, is kept as - /// an [`Capability::UnknownCapability`]: 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. + /// 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], @@ -365,17 +353,7 @@ impl Capability { // `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 = match Self::parse_value(typ, cap_buf) { - Ok(capability) => capability, - Err(Failure(BgpParserError::ProtocolError( - ProtocolErrorValues::UnknownAddressFamily, - ))) => Self::UnknownCapability { - type_code: typ, - length: len, - value: cap_buf.to_vec(), - }, - Err(e) => return Err(e), - }; + let capability = Self::parse_value(typ, cap_buf)?; Ok((buf, capability)) } @@ -402,8 +380,8 @@ impl Capability { let (cap_buf, _) = be_u8(cap_buf)?; let (cap_buf, safi) = be_u8(cap_buf)?; require_empty!(cap_buf, "MultiProtocol"); - let afi = known_afi(afi)?; - let safi = known_safi(safi)?; + let afi = Afi(afi); + let safi = Safi(safi); Ok(Self::MultiProtocol { afi, safi }) } v if v == CapabilityDiscriminants::RouteRefresh as u8 => { @@ -414,8 +392,8 @@ impl Capability { 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 = known_afi(afi)?; - let safi = known_safi(safi)?; + let afi = Afi(afi); + let safi = Safi(safi); let (cap_buf, _num_orfs) = be_u8(cap_buf)?; @@ -454,9 +432,13 @@ impl Capability { let (buf, afi) = be_u16(buf)?; let (buf, safi) = be_u16(buf)?; let (buf, nh_afi) = be_u16(buf)?; - let afi = known_afi(afi)?; - let safi = known_safi(safi)?; - let nh_afi = known_afi(nh_afi)?; + 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)?; @@ -471,7 +453,7 @@ impl Capability { 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 = known_afi(afi)?; + let afi = Afi(afi); require_empty!(cap_buf, "BgpSec"); Ok(Self::BgpSec { header: BgpSecHeader(header), @@ -479,8 +461,6 @@ impl Capability { }) } v if v == CapabilityDiscriminants::MultiLabelCompat as u8 => { - // The AFI and SAFI are kept as numbers: this capability is - // used with labelled families we have no named constant for. let parse_entry = |buf: &'a [u8]| -> IResult< &'a [u8], MultiLabelEntry, @@ -489,7 +469,14 @@ impl Capability { let (buf, afi) = be_u16(buf)?; let (buf, safi) = be_u8(buf)?; let (buf, count) = be_u8(buf)?; - Ok((buf, MultiLabelEntry { afi, safi, count })) + Ok(( + buf, + MultiLabelEntry { + afi: Afi(afi), + safi: Safi(safi), + count, + }, + )) }; let (cap_buf, entries) = nom::multi::many1(parse_entry).parse(cap_buf)?; require_empty!(cap_buf, "MultiLabelCompat"); @@ -514,8 +501,8 @@ impl Capability { let (buf, afi) = be_u16(buf)?; let (buf, safi) = be_u8(buf)?; let (buf, flags) = be_u8(buf)?; - let afi = known_afi(afi)?; - let safi = known_safi(safi)?; + let afi = Afi(afi); + let safi = Safi(safi); Ok(( buf, GracefulRestartEntry { @@ -542,8 +529,8 @@ impl Capability { let (cap_buf, afi) = be_u16(cap_buf)?; let (cap_buf, safi) = be_u8(cap_buf)?; let (cap_buf, send_recv) = be_u8(cap_buf)?; - let afi = known_afi(afi)?; - let safi = known_safi(safi)?; + let afi = Afi(afi); + let safi = Safi(safi); let send_recv = AddPathSendRecv::try_from(send_recv).map_err(|e| fail!("{}", e))?; require_empty!(cap_buf, "AddPath"); Ok(Self::AddPath { @@ -566,8 +553,8 @@ impl Capability { let (buf, safi) = be_u8(buf)?; let (buf, flags) = be_u8(buf)?; let (buf, stale_time) = be_u24(buf)?; - let afi = known_afi(afi)?; - let safi = known_safi(safi)?; + let afi = Afi(afi); + let safi = Safi(safi); Ok(( buf, LongLivedGracefulRestart { @@ -599,15 +586,15 @@ impl Capability { out.put_u8(self.payload_len()); match self { Capability::MultiProtocol { afi, safi } => { - out.put_u16(*afi as u16); + out.put_u16(afi.0); out.put_u8(0); // Reserved byte. - out.put_u8(*safi as u8); + out.put_u8(safi.0); } Capability::RouteRefresh => {} Capability::OutboundRouteFilter { afi, safi, orfs } => { - out.put_u16(*afi as u16); + out.put_u16(afi.0); out.put_u8(0); // Reserved byte. - out.put_u8(*safi as u8); + out.put_u8(safi.0); for orf in orfs { out.put_u8(orf.r#type.clone().into()); out.put_u8(orf.send_recv.clone() as u8); @@ -615,20 +602,20 @@ impl Capability { } Capability::ExtendedNextHop { entries } => { for entry in entries { - out.put_u16(entry.afi as u16); - out.put_u16(entry.safi as u16); - out.put_u16(entry.nh_afi as u16); + out.put_u16(entry.afi.0); + out.put_u16(u16::from(entry.safi.0)); + out.put_u16(entry.nh_afi.0); } } Capability::ExtendedMessage => {} Capability::BgpSec { header, afi } => { out.put_u8(header.0); - out.put_u16(*afi as u16); + out.put_u16(afi.0); } Capability::MultiLabelCompat { entries } => { for entry in entries { - out.put_u16(entry.afi); - out.put_u8(entry.safi); + out.put_u16(entry.afi.0); + out.put_u8(entry.safi.0); out.put_u8(entry.count); } } @@ -654,8 +641,8 @@ impl Capability { flags_restart_time |= restart_time; out.put_u16(flags_restart_time); for entry in entries { - out.put_u16(entry.afi as u16); - out.put_u8(entry.safi as u8); + out.put_u16(entry.afi.0); + out.put_u8(entry.safi.0); let mut flags = 0; if entry.preserve_forwarding { flags |= 1 << 7; @@ -671,15 +658,15 @@ impl Capability { safi, send_recv, } => { - out.put_u16(*afi as u16); - out.put_u8(*safi as u8); + out.put_u16(afi.0); + out.put_u8(safi.0); out.put_u8(send_recv.into()); } Capability::EnhancedRouteRefresh => {} Capability::LongLivedGracefulRestart(llgrs) => { for llgr in llgrs { - out.put_u16(llgr.afi as u16); - out.put_u8(llgr.safi as u8); + 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..]); } @@ -823,11 +810,11 @@ impl From<&AddPathSendRecv> for u8 { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct ExtendedNextHop { /// The Address Family to update the nexthop format for. - pub afi: AddressFamilyId, + pub afi: Afi, /// The Subsequent Address Family to update the nexthop format for. - pub safi: SubsequentAfi, + pub safi: Safi, /// The Address Family that nexthops will be encoded as. - pub nh_afi: AddressFamilyId, + pub nh_afi: Afi, } #[derive(Debug, Clone, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)] @@ -869,8 +856,8 @@ impl From<&BgpRole> for u8 { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct LongLivedGracefulRestart { - pub afi: AddressFamilyId, - pub safi: SubsequentAfi, + pub afi: Afi, + pub safi: Safi, pub flags: u8, /// stale_time is the time in seconds after which routes are considered stale. /// Note that this is represented as a u24 on the wire. @@ -889,8 +876,8 @@ bitfield! { /// Represents an entry in the BGP Graceful Restart capability. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct GracefulRestartEntry { - afi: AddressFamilyId, - safi: SubsequentAfi, + afi: Afi, + safi: Safi, /// Forwarding State (F) bit, which can be used to indicate whether the /// forwarding state for routes that were advertised with the given AFI /// and SAFI has indeed been preserved during the previous BGP restart. @@ -905,11 +892,12 @@ mod tests { use eyre::Result; use crate::open::{ - Capability, ExtendedNextHop, GracefulRestartEntry, MultiLabelEntry, OpenMessage, OpenOption, + AddPathSendRecv, Capability, ExtendedNextHop, GracefulRestartEntry, MultiLabelEntry, + OpenMessage, OpenOption, }; use bgp_primitives::{ - constants::{AddressFamilyId, SubsequentAfi}, + constants::{AddressFamilyId, Afi, Safi}, parser::ParserContext, }; @@ -941,8 +929,8 @@ mod tests { test_multiptotocol, &[0x01, 0x04, 0x00, 0x01, 0x00, 0x01], Capability::MultiProtocol { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast + afi: Afi::IPV4, + safi: Safi::UNICAST } ); @@ -971,8 +959,8 @@ mod tests { restart_state: false, restart_time: 300, entries: vec![GracefulRestartEntry { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV4, + safi: Safi::UNICAST, preserve_forwarding: false, }] } @@ -987,19 +975,19 @@ mod tests { Capability::ExtendedNextHop { entries: vec![ ExtendedNextHop { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast, - nh_afi: AddressFamilyId::Ipv6, + afi: Afi::IPV4, + safi: Safi::UNICAST, + nh_afi: Afi::IPV6, }, ExtendedNextHop { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Multicast, - nh_afi: AddressFamilyId::Ipv6, + afi: Afi::IPV4, + safi: Safi::MULTICAST, + nh_afi: Afi::IPV6, }, ExtendedNextHop { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::MplsLabelledVpn, - nh_afi: AddressFamilyId::Ipv6 + afi: Afi::IPV4, + safi: Safi::MPLS_VPN, + nh_afi: Afi::IPV6 } ] } @@ -1021,8 +1009,8 @@ mod tests { identifier: Ipv4Addr::new(10, 0, 0, 1), options: vec![ OpenOption::Capabilities(vec![Capability::MultiProtocol { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV4, + safi: Safi::UNICAST, }]), OpenOption::Capabilities(vec![Capability::UnknownCapability { type_code: 0x80, @@ -1035,27 +1023,27 @@ mod tests { restart_state: false, restart_time: 300, entries: vec![GracefulRestartEntry { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV4, + safi: Safi::UNICAST, preserve_forwarding: false, }], }]), OpenOption::Capabilities(vec![Capability::ExtendedNextHop { entries: vec![ ExtendedNextHop { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast, - nh_afi: AddressFamilyId::Ipv6, + afi: Afi::IPV4, + safi: Safi::UNICAST, + nh_afi: Afi::IPV6, }, ExtendedNextHop { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Multicast, - nh_afi: AddressFamilyId::Ipv6, + afi: Afi::IPV4, + safi: Safi::MULTICAST, + nh_afi: Afi::IPV6, }, ExtendedNextHop { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::MplsLabelledVpn, - nh_afi: AddressFamilyId::Ipv6, + afi: Afi::IPV4, + safi: Safi::MPLS_VPN, + nh_afi: Afi::IPV6, }, ], }]), @@ -1088,13 +1076,13 @@ mod tests { Capability::MultiLabelCompat { entries: vec![ MultiLabelEntry { - afi: 1, - safi: 4, + afi: Afi::IPV4, + safi: Safi::MPLS_LABEL, count: 2 }, MultiLabelEntry { - afi: 2, - safi: 4, + afi: Afi::IPV6, + safi: Safi::MPLS_LABEL, count: 3 }, ] @@ -1108,37 +1096,51 @@ mod tests { assert!(Capability::from_wire(&ctx, &[0x08, 0x03, 0x00, 0x01, 0x04]).is_err()); } - /// A peer may offer families we have no name for (here L2VPN EVPN, - /// AFI 25 SAFI 70). That used to fail the whole OPEN. It is kept as an - /// unknown capability, byte for byte, and what follows is still read. + /// A peer may offer families this implementation does not support + /// (here L2VPN EVPN, AFI 25 SAFI 70). That used to fail the whole OPEN. + /// The family is read as the numbers it is, and what follows it too. #[test] - fn a_capability_for_an_unknown_address_family_is_kept_as_unknown() { + fn a_capability_may_name_any_address_family() { let ctx = ParserContext::default(); - let samples: &[(&str, &[u8])] = &[ - ("multiprotocol", &[0x01, 0x04, 0x00, 0x19, 0x00, 0x46]), - ("add-path", &[0x45, 0x04, 0x00, 0x19, 0x46, 0x03]), + let evpn = (Afi::L2VPN, Safi::EVPN); + let samples: &[(&[u8], Capability)] = &[ + ( + &[0x01, 0x04, 0x00, 0x19, 0x00, 0x46], + Capability::MultiProtocol { + afi: evpn.0, + safi: evpn.1, + }, + ), + ( + &[0x45, 0x04, 0x00, 0x19, 0x46, 0x03], + Capability::AddPath { + afi: evpn.0, + safi: evpn.1, + send_recv: AddPathSendRecv::Both, + }, + ), ( - "graceful restart", &[0x40, 0x06, 0x00, 0x78, 0x00, 0x19, 0x46, 0x80], + Capability::GracefulRestart { + restart_state: false, + restart_time: 120, + entries: vec![GracefulRestartEntry { + afi: evpn.0, + safi: evpn.1, + preserve_forwarding: true, + }], + }, ), ]; - for (name, wire) in samples { + 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(); - assert_eq!(rest, [0x02, 0x00], "{name}"); - assert_eq!( - cap, - Capability::UnknownCapability { - type_code: wire[0], - length: wire[1], - value: wire[2..].to_vec(), - }, - "{name}" - ); + assert_eq!(rest, [0x02, 0x00], "{expected:?}"); + assert_eq!(cap, *expected); let mut out = BytesMut::new(); cap.to_wire(&ctx, &mut out).unwrap(); - assert_eq!(&out[..], *wire, "{name}"); + assert_eq!(&out[..], *wire, "{expected:?}"); } // A capability that is simply malformed is still an error. diff --git a/crates/primitives/src/constants.rs b/crates/primitives/src/constants.rs index 704250f..e02182a 100644 --- a/crates/primitives/src/constants.rs +++ b/crates/primitives/src/constants.rs @@ -1,5 +1,6 @@ use std::fmt::Display; +use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; use thiserror::Error; @@ -8,11 +9,8 @@ use thiserror::Error; #[error("unknown address family identifier {0}")] pub struct UnknownAfi(pub u16); -/// A subsequent address family number this implementation has no name for. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Error)] -#[error("unknown subsequent address family identifier {0}")] -pub struct UnknownSafi(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)] #[repr(u16)] pub enum AddressFamilyId { @@ -41,51 +39,100 @@ impl Display for AddressFamilyId { } } -/// Represents a Subsequent Address Family Identifier. -#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Serialize_repr, Deserialize_repr)] -#[repr(u8)] -pub enum SubsequentAfi { - Unicast = 1, - Multicast = 2, - NlriWithMpls = 4, - MplsLabelledVpn = 128, - MulticastMplsVpn = 129, +/// An Address Family Identifier as it appears on the wire (RFC 4760). +/// +/// Any number is a valid value: a peer may name a family this implementation +/// 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)] +#[serde(transparent)] +pub struct Afi(pub u16); + +impl Afi { + pub const IPV4: Self = Self(1); + pub const IPV6: Self = Self(2); + pub const L2VPN: Self = Self(25); + pub const BGP_LS: Self = Self(16388); } -impl TryFrom for SubsequentAfi { - type Error = UnknownSafi; - - fn try_from(value: u8) -> Result { - match value { - x if x == Self::Unicast as u8 => Ok(Self::Unicast), - x if x == Self::Multicast as u8 => Ok(Self::Multicast), - x if x == Self::NlriWithMpls as u8 => Ok(Self::NlriWithMpls), - x if x == Self::MplsLabelledVpn as u8 => Ok(Self::MplsLabelledVpn), - x if x == Self::MulticastMplsVpn as u8 => Ok(Self::MulticastMplsVpn), - other => Err(UnknownSafi(other.into())), - } +impl From for Afi { + fn from(version: AddressFamilyId) -> Self { + Self(version as u16) } } -/// Some encodings carry the SAFI in two octets. Every SAFI we know fits in -/// one, so a larger value is unknown, not a smaller one in disguise. -impl TryFrom for SubsequentAfi { - type Error = UnknownSafi; +impl TryFrom for AddressFamilyId { + type Error = UnknownAfi; - fn try_from(value: u16) -> Result { - let narrow = u8::try_from(value).map_err(|_| UnknownSafi(value))?; - Self::try_from(narrow) + fn try_from(afi: Afi) -> Result { + Self::try_from(afi.0) } } -impl Display for SubsequentAfi { +/// 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)] +#[serde(transparent)] +pub struct Safi(pub u8); + +impl Safi { + pub const UNICAST: Self = Self(1); + pub const MULTICAST: Self = Self(2); + pub const MPLS_LABEL: Self = Self(4); + pub const EVPN: Self = Self(70); + pub const BGP_LS: Self = Self(71); + pub const MPLS_VPN: Self = Self(128); + pub const MULTICAST_MPLS_VPN: Self = Self(129); + pub const FLOWSPEC: Self = Self(133); +} + +/// 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)] +pub struct AfiSafi { + pub afi: Afi, + pub safi: Safi, +} + +impl AfiSafi { + pub const IPV4_UNICAST: Self = Self::new(Afi::IPV4, Safi::UNICAST); + pub const IPV6_UNICAST: Self = Self::new(Afi::IPV6, Safi::UNICAST); + + pub const fn new(afi: Afi, safi: Safi) -> Self { + Self { afi, safi } + } + + /// The unicast family of an IP version. + pub fn unicast(version: AddressFamilyId) -> Self { + Self::new(version.into(), Safi::UNICAST) + } + + /// The usual name of the family, if it has one here. + fn name(self) -> Option<&'static str> { + Some(match (self.afi, self.safi) { + (Afi::IPV4, Safi::UNICAST) => "IPv4 unicast", + (Afi::IPV6, Safi::UNICAST) => "IPv6 unicast", + (Afi::IPV4, Safi::MULTICAST) => "IPv4 multicast", + (Afi::IPV6, Safi::MULTICAST) => "IPv6 multicast", + (Afi::IPV4, Safi::MPLS_LABEL) => "IPv4 labeled unicast", + (Afi::IPV6, Safi::MPLS_LABEL) => "IPv6 labeled unicast", + (Afi::IPV4, Safi::MPLS_VPN) => "VPNv4 unicast", + (Afi::IPV6, Safi::MPLS_VPN) => "VPNv6 unicast", + (Afi::IPV4, Safi::FLOWSPEC) => "IPv4 flowspec", + (Afi::IPV6, Safi::FLOWSPEC) => "IPv6 flowspec", + (Afi::L2VPN, Safi::EVPN) => "L2VPN EVPN", + (Afi::BGP_LS, Safi::BGP_LS) => "BGP-LS", + _ => return None, + }) + } +} + +impl Display for AfiSafi { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - SubsequentAfi::Unicast => write!(f, "Unicast"), - SubsequentAfi::Multicast => write!(f, "Multicast"), - SubsequentAfi::NlriWithMpls => write!(f, "NlriWithMpls"), - SubsequentAfi::MplsLabelledVpn => write!(f, "MplsLabelledVpn"), - SubsequentAfi::MulticastMplsVpn => write!(f, "MulticastMplsVpn"), + match self.name() { + Some(name) => f.write_str(name), + None => write!(f, "AFI {} SAFI {}", self.afi.0, self.safi.0), } } } @@ -95,10 +142,29 @@ mod tests { use super::*; #[test] - fn a_wide_safi_is_not_truncated_into_a_known_one() { - assert_eq!(SubsequentAfi::try_from(1u16), Ok(SubsequentAfi::Unicast)); - // 257 has the low byte of Unicast. - assert_eq!(SubsequentAfi::try_from(257u16), Err(UnknownSafi(257))); + fn any_family_is_a_value_and_the_common_ones_have_names() { + assert_eq!(AfiSafi::IPV6_UNICAST.to_string(), "IPv6 unicast"); + assert_eq!( + AfiSafi::new(Afi::L2VPN, Safi::EVPN).to_string(), + "L2VPN EVPN" + ); + assert_eq!( + AfiSafi::new(Afi(999), Safi(7)).to_string(), + "AFI 999 SAFI 7" + ); + assert_eq!( + AfiSafi::unicast(AddressFamilyId::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))); } } diff --git a/crates/primitives/src/parser.rs b/crates/primitives/src/parser.rs index e049030..12d614a 100644 --- a/crates/primitives/src/parser.rs +++ b/crates/primitives/src/parser.rs @@ -57,8 +57,4 @@ pub enum ProtocolErrorValues { UnsupportedVersion(u8), /// `UnknownOpenOption` is returned with the open option type code that is unknown. UnknownOpenOption(u8), - /// A capability names an address family this implementation has no name - /// for. The capability parser turns this into an unknown capability; it - /// does not reach callers. - UnknownAddressFamily, } diff --git a/crates/rib/src/entry.rs b/crates/rib/src/entry.rs index c2b1f7c..616ca2f 100644 --- a/crates/rib/src/entry.rs +++ b/crates/rib/src/entry.rs @@ -110,7 +110,7 @@ mod tests { use bgp_packet::message::{ MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, OriginPathAttribute, }; - use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; + use bgp_primitives::constants::{Afi, Safi}; use super::*; @@ -120,8 +120,8 @@ mod tests { fn mp_reach(prefixes: Vec) -> PathAttribute { PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6(Ipv6Addr::LOCALHOST), prefixes, }) @@ -167,8 +167,8 @@ mod tests { PathAttribute::Origin(OriginPathAttribute::IGP), mp_reach(vec![prefix("2001:db8:1::/48")]), PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, prefixes: vec![prefix("2001:db8:2::/48")], }), ], diff --git a/crates/server/src/advertise.rs b/crates/server/src/advertise.rs index 41926a7..8d38c2c 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, SubsequentAfi}; +use bgp_primitives::constants::{AddressFamilyId, Afi, Safi}; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRib, LocalRibEvent, LocalRoute}; use tokio::sync::broadcast::error::RecvError; @@ -188,8 +188,8 @@ pub fn build_announce( } if let (AddressFamilyId::Ipv6, IpAddr::V6(nh)) = (route.prefix.address_family, next_hop) { path_attributes.push(PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6(nh), prefixes: vec![route.prefix.clone()], })); @@ -221,8 +221,8 @@ pub fn build_withdraw(prefix: &IpPrefix) -> UpdateMessage { AddressFamilyId::Ipv6 => UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, prefixes: vec![prefix.clone()], })], announced_routes: vec![], diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index e25dd09..de1c58d 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -4,11 +4,12 @@ use std::time::Duration; use bgp_packet::community; use bgp_packet::message::OriginPathAttribute; +use bgp_primitives::constants::AfiSafi; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use crate::error::{ConfigError, OriginateProblem, PeerProblem}; -use crate::family::{AddressFamily, FamilySet}; +use crate::family::FamilySet; use crate::proto::config::{self as proto, BgpConfig, OriginatedRoute, Peer}; const DEFAULT_BGP_PORT: u16 = 179; @@ -157,13 +158,13 @@ fn parse_peer(peer: Peer) -> Result { }; let families = if peer.address_families.is_empty() { - FamilySet::ALL + FamilySet::supported() } else { peer.address_families .iter() .map(|family| match family.enum_value() { - Ok(proto::AddressFamily::IPV4_UNICAST) => Ok(AddressFamily::Ipv4Unicast), - Ok(proto::AddressFamily::IPV6_UNICAST) => Ok(AddressFamily::Ipv6Unicast), + Ok(proto::AddressFamily::IPV4_UNICAST) => Ok(AfiSafi::IPV4_UNICAST), + Ok(proto::AddressFamily::IPV6_UNICAST) => Ok(AfiSafi::IPV6_UNICAST), Ok(proto::AddressFamily::ADDRESS_FAMILY_UNSPECIFIED) | Err(_) => { Err(PeerProblem::AddressFamily) } @@ -325,16 +326,16 @@ mod tests { "{BASE}peers {{ name: \"r1\" endpoint {{ address: \"192.0.2.2\" }} remote_asn: 1 {body} }}" )) .unwrap(); - config.peers[0].families + config.peers[0].families.clone() }; - assert_eq!(families(""), FamilySet::ALL); + assert_eq!(families(""), FamilySet::supported()); assert_eq!( families("address_families: IPV6_UNICAST"), - [AddressFamily::Ipv6Unicast].into_iter().collect() + [AfiSafi::IPV6_UNICAST].into_iter().collect() ); assert_eq!( families("address_families: IPV4_UNICAST address_families: IPV6_UNICAST"), - FamilySet::ALL + FamilySet::supported() ); } diff --git a/crates/server/src/connection.rs b/crates/server/src/connection.rs index ec8b8df..55147f9 100644 --- a/crates/server/src/connection.rs +++ b/crates/server/src/connection.rs @@ -82,7 +82,7 @@ pub async fn run_session( local_asn: config.local_asn, hold_time_secs: config.hold_time_secs, expected_asn: config.expected_asn, - families: config.families, + families: config.families.clone(), }); let mut session = Session { fsm, diff --git a/crates/server/src/dialer.rs b/crates/server/src/dialer.rs index ca34f6a..5ae3bee 100644 --- a/crates/server/src/dialer.rs +++ b/crates/server/src/dialer.rs @@ -37,7 +37,7 @@ pub fn spawn_dialers( direction: Direction::Outbound, peer_name: Some(peer.name.clone()), expected_asn: Some(peer.remote_asn), - families: peer.families, + families: peer.families.clone(), ..base.clone() }; tokio::spawn(dial_loop(Arc::new(peer.clone()), config, connect_retry)) diff --git a/crates/server/src/family.rs b/crates/server/src/family.rs index 5fa54f5..ebaaec6 100644 --- a/crates/server/src/family.rs +++ b/crates/server/src/family.rs @@ -3,161 +3,94 @@ //! Each side lists the families it is willing to exchange in its OPEN, as //! Multiprotocol capabilities (RFC 4760 §8). A family is in use on a session //! only if both sides listed it. +//! +//! Any family can be named, read from a peer and held in a [`FamilySet`]. +//! The ones this speaker can actually exchange routes for are [`SUPPORTED`], +//! and only those are ever offered to a peer. +use std::collections::BTreeSet; use std::fmt; use bgp_packet::message::{PathAttribute, UpdateMessage}; use bgp_packet::open::{Capability, OpenMessage}; -use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; +use bgp_primitives::constants::AfiSafi; use bgp_primitives::ip_prefix::IpPrefix; use bgp_primitives::parser::ParserContext; use bytes::BytesMut; -/// An address family this speaker can exchange routes for. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub enum AddressFamily { - Ipv4Unicast, - Ipv6Unicast, -} - -impl AddressFamily { - pub const ALL: [Self; 2] = [Self::Ipv4Unicast, Self::Ipv6Unicast]; - - pub fn afi(self) -> AddressFamilyId { - match self { - Self::Ipv4Unicast => AddressFamilyId::Ipv4, - Self::Ipv6Unicast => AddressFamilyId::Ipv6, - } - } - - pub fn safi(self) -> SubsequentAfi { - SubsequentAfi::Unicast - } - - fn from_afi_safi(afi: AddressFamilyId, safi: SubsequentAfi) -> Option { - Self::ALL - .into_iter() - .find(|family| family.afi() == afi && family.safi() == safi) - } - - /// The capability that offers this family to a peer. - pub fn capability(self) -> Capability { - Capability::MultiProtocol { - afi: self.afi(), - safi: self.safi(), - } - } - - pub fn name(self) -> &'static str { - match self { - Self::Ipv4Unicast => "IPv4 unicast", - Self::Ipv6Unicast => "IPv6 unicast", - } - } -} - -impl fmt::Display for AddressFamily { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.name()) - } -} +/// The address families this speaker can exchange routes for. Supporting a +/// new one means being able to parse, store and advertise its routes; then +/// it is added here. +pub const SUPPORTED: [AfiSafi; 2] = [AfiSafi::IPV4_UNICAST, AfiSafi::IPV6_UNICAST]; /// A set of address families. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -pub struct FamilySet { - ipv4_unicast: bool, - ipv6_unicast: bool, -} +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct FamilySet(BTreeSet); impl FamilySet { - pub const EMPTY: Self = Self { - ipv4_unicast: false, - ipv6_unicast: false, - }; - pub const ALL: Self = Self { - ipv4_unicast: true, - ipv6_unicast: true, - }; - - pub fn insert(&mut self, family: AddressFamily) { - match family { - AddressFamily::Ipv4Unicast => self.ipv4_unicast = true, - AddressFamily::Ipv6Unicast => self.ipv6_unicast = true, - } + /// Every family in [`SUPPORTED`]. + pub fn supported() -> Self { + SUPPORTED.into_iter().collect() } - pub fn contains(self, family: AddressFamily) -> bool { - match family { - AddressFamily::Ipv4Unicast => self.ipv4_unicast, - AddressFamily::Ipv6Unicast => self.ipv6_unicast, - } + pub fn insert(&mut self, family: AfiSafi) { + self.0.insert(family); } - fn contains_afi_safi(self, afi: AddressFamilyId, safi: SubsequentAfi) -> bool { - AddressFamily::from_afi_safi(afi, safi).is_some_and(|family| self.contains(family)) + pub fn contains(&self, family: AfiSafi) -> bool { + self.0.contains(&family) } /// Whether a unicast route for `prefix` may be exchanged. - pub fn carries(self, prefix: &IpPrefix) -> bool { - self.contains_afi_safi(prefix.address_family, SubsequentAfi::Unicast) + pub fn carries(&self, prefix: &IpPrefix) -> bool { + self.contains(AfiSafi::unicast(prefix.address_family)) } - pub fn is_empty(self) -> bool { - self == Self::EMPTY + pub fn is_empty(&self) -> bool { + self.0.is_empty() } - pub fn intersection(self, other: Self) -> Self { - Self { - ipv4_unicast: self.ipv4_unicast && other.ipv4_unicast, - ipv6_unicast: self.ipv6_unicast && other.ipv6_unicast, - } + pub fn intersection(&self, other: &Self) -> Self { + Self(self.0.intersection(&other.0).copied().collect()) } - pub fn iter(self) -> impl Iterator { - AddressFamily::ALL - .into_iter() - .filter(move |family| self.contains(*family)) + pub fn iter(&self) -> impl Iterator + '_ { + self.0.iter().copied() } - /// The families a peer offered in its OPEN. + /// The families a peer offered in its OPEN, whether or not we support + /// them. /// /// A peer that sends no Multiprotocol capability at all is a speaker of /// plain RFC 4271 BGP, which carries IPv4 unicast and nothing else. One /// that sends any has listed everything it wants, IPv4 unicast included. pub fn offered_by(open: &OpenMessage) -> Self { - let mut listed_any = false; - let mut offered = Self::EMPTY; - for capability in open.capabilities() { - match capability { - Capability::MultiProtocol { afi, safi } => { - listed_any = true; - if let Some(family) = AddressFamily::from_afi_safi(*afi, *safi) { - offered.insert(family); - } - } - // A Multiprotocol capability for a family we have no name - // for is kept as an unknown capability. It still shows that - // the peer lists its families. - Capability::UnknownCapability { type_code: 1, .. } => listed_any = true, - _ => {} - } - } - if listed_any { - offered + let listed: Self = open + .capabilities() + .filter_map(|capability| match capability { + Capability::MultiProtocol { afi, safi } => Some(AfiSafi::new(*afi, *safi)), + _ => None, + }) + .collect(); + if listed.is_empty() { + [AfiSafi::IPV4_UNICAST].into_iter().collect() } else { - [AddressFamily::Ipv4Unicast].into_iter().collect() + listed } } /// These families as Multiprotocol capabilities in wire form: the Data /// of an Unsupported Capability NOTIFICATION (RFC 5492 §5). - pub fn capabilities_wire(self) -> Vec { + 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 family.capability().to_wire(&ctx, &mut out).is_err() { + if Capability::multiprotocol(family) + .to_wire(&ctx, &mut out) + .is_err() + { return vec![]; } } @@ -170,20 +103,20 @@ impl FamilySet { /// The withdrawn routes and NLRI in the body of an UPDATE are IPv4 /// unicast; every other family travels in MP_REACH_NLRI and /// MP_UNREACH_NLRI, which name theirs. - pub fn restrict(self, update: &mut UpdateMessage) -> usize { + pub fn restrict(&self, update: &mut UpdateMessage) -> usize { let mut removed = 0; - if !self.contains(AddressFamily::Ipv4Unicast) { + if !self.contains(AfiSafi::IPV4_UNICAST) { removed += update.withdrawn_routes.len() + update.announced_routes.len(); update.withdrawn_routes.clear(); update.announced_routes.clear(); } update.path_attributes.retain(|attribute| { - let (afi, safi, prefixes) = match attribute { - PathAttribute::MpReachNlri(mp) => (mp.afi, mp.safi, mp.prefixes.len()), - PathAttribute::MpUnreachNlri(mp) => (mp.afi, mp.safi, mp.prefixes.len()), + let (family, prefixes) = match attribute { + PathAttribute::MpReachNlri(mp) => (mp.family(), mp.prefixes.len()), + PathAttribute::MpUnreachNlri(mp) => (mp.family(), mp.prefixes.len()), _ => return true, }; - let keep = self.contains_afi_safi(afi, safi); + let keep = self.contains(family); if !keep { removed += prefixes; } @@ -193,13 +126,9 @@ impl FamilySet { } } -impl FromIterator for FamilySet { - fn from_iter>(families: I) -> Self { - let mut set = Self::EMPTY; - for family in families { - set.insert(family); - } - set +impl FromIterator for FamilySet { + fn from_iter>(families: I) -> Self { + Self(families.into_iter().collect()) } } @@ -212,7 +141,7 @@ impl fmt::Display for FamilySet { if i > 0 { f.write_str(", ")?; } - f.write_str(family.name())?; + write!(f, "{family}")?; } Ok(()) } @@ -226,6 +155,7 @@ mod tests { MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, NlriNextHop, OriginPathAttribute, }; use bgp_packet::open::OpenOption; + use bgp_primitives::constants::{Afi, Safi}; use super::*; @@ -247,7 +177,7 @@ mod tests { IpPrefix::try_from(s).unwrap() } - fn only(family: AddressFamily) -> FamilySet { + fn only(family: AfiSafi) -> FamilySet { [family].into_iter().collect() } @@ -255,59 +185,68 @@ mod tests { fn a_peer_without_multiprotocol_capabilities_speaks_ipv4_unicast() { assert_eq!( FamilySet::offered_by(&open(vec![])), - only(AddressFamily::Ipv4Unicast) + only(AfiSafi::IPV4_UNICAST) ); // Other capabilities do not change that. assert_eq!( FamilySet::offered_by(&open(vec![Capability::RouteRefresh])), - only(AddressFamily::Ipv4Unicast) + only(AfiSafi::IPV4_UNICAST) ); } #[test] fn a_peer_that_lists_families_gets_exactly_those() { assert_eq!( - FamilySet::offered_by(&open(vec![AddressFamily::Ipv6Unicast.capability()])), - only(AddressFamily::Ipv6Unicast), + FamilySet::offered_by(&open(vec![Capability::multiprotocol( + AfiSafi::IPV6_UNICAST + )])), + only(AfiSafi::IPV6_UNICAST), "IPv4 unicast is not implied once a peer lists its families" ); // Capabilities may be spread over several optional parameters. - let mut spread = open(vec![AddressFamily::Ipv4Unicast.capability()]); - spread.options.push(OpenOption::Capabilities(vec![ - AddressFamily::Ipv6Unicast.capability(), + let mut spread = open(vec![Capability::multiprotocol(AfiSafi::IPV4_UNICAST)]); + spread + .options + .push(OpenOption::Capabilities(vec![Capability::multiprotocol( + AfiSafi::IPV6_UNICAST, + )])); + assert_eq!(FamilySet::offered_by(&spread), FamilySet::supported()); + // A family we do not support is still what the peer offered. It + // drops out when intersected with what we offer. + let evpn = AfiSafi::new(Afi::L2VPN, Safi::EVPN); + let offered = FamilySet::offered_by(&open(vec![ + Capability::multiprotocol(evpn), + Capability::multiprotocol(AfiSafi::IPV6_UNICAST), ])); - assert_eq!(FamilySet::offered_by(&spread), FamilySet::ALL); - // IPv6 multicast is a family, but not one we carry. + assert_eq!(offered.to_string(), "IPv6 unicast, L2VPN EVPN"); assert_eq!( - FamilySet::offered_by(&open(vec![Capability::MultiProtocol { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Multicast, - }])), - FamilySet::EMPTY + FamilySet::supported().intersection(&offered), + only(AfiSafi::IPV6_UNICAST) ); - // Nor is one we cannot even name (here L2VPN EVPN, 25/70). - assert_eq!( - FamilySet::offered_by(&open(vec![Capability::UnknownCapability { - type_code: 1, - length: 4, - value: vec![0, 25, 0, 70], - }])), - FamilySet::EMPTY + assert!( + FamilySet::supported() + .intersection(&FamilySet::offered_by(&open(vec![ + Capability::multiprotocol(evpn) + ]))) + .is_empty() ); } #[test] fn sets_intersect_and_describe_themselves() { - let v6 = only(AddressFamily::Ipv6Unicast); - assert_eq!(FamilySet::ALL.intersection(v6), v6); - assert!(only(AddressFamily::Ipv4Unicast).intersection(v6).is_empty()); - assert_eq!(FamilySet::ALL.to_string(), "IPv4 unicast, IPv6 unicast"); - assert_eq!(FamilySet::EMPTY.to_string(), "none"); + let v6 = only(AfiSafi::IPV6_UNICAST); + assert_eq!(FamilySet::supported().intersection(&v6), v6); + assert!(only(AfiSafi::IPV4_UNICAST).intersection(&v6).is_empty()); + assert_eq!( + FamilySet::supported().to_string(), + "IPv4 unicast, IPv6 unicast" + ); + assert_eq!(FamilySet::default().to_string(), "none"); assert!(v6.carries(&prefix("2001:db8::/32"))); assert!(!v6.carries(&prefix("192.0.2.0/24"))); // One capability each: code 1, length 4, AFI, reserved, SAFI. assert_eq!( - FamilySet::ALL.capabilities_wire(), + FamilySet::supported().capabilities_wire(), [1, 4, 0, 1, 0, 1, 1, 4, 0, 2, 0, 1] ); } @@ -318,14 +257,14 @@ mod tests { path_attributes: vec![ PathAttribute::Origin(OriginPathAttribute::IGP), PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6(Ipv6Addr::LOCALHOST), prefixes: vec![prefix("2001:db8:1::/48"), prefix("2001:db8:2::/48")], }), PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, prefixes: vec![prefix("2001:db8:3::/48")], }), ], @@ -336,11 +275,11 @@ mod tests { #[test] fn restrict_removes_the_routes_of_families_not_in_the_set() { let mut update = mixed_update(); - assert_eq!(FamilySet::ALL.restrict(&mut update), 0); + assert_eq!(FamilySet::supported().restrict(&mut update), 0); assert_eq!(update, mixed_update()); let mut update = mixed_update(); - assert_eq!(only(AddressFamily::Ipv4Unicast).restrict(&mut update), 3); + assert_eq!(only(AfiSafi::IPV4_UNICAST).restrict(&mut update), 3); assert_eq!(update.announced_routes, vec![prefix("192.0.2.0/24")]); assert_eq!(update.withdrawn_routes, vec![prefix("198.51.100.0/24")]); assert_eq!( @@ -349,7 +288,7 @@ mod tests { ); let mut update = mixed_update(); - assert_eq!(only(AddressFamily::Ipv6Unicast).restrict(&mut update), 2); + assert_eq!(only(AfiSafi::IPV6_UNICAST).restrict(&mut update), 2); assert!(update.announced_routes.is_empty()); assert!(update.withdrawn_routes.is_empty()); assert_eq!(update.path_attributes.len(), 3); diff --git a/crates/server/src/fsm.rs b/crates/server/src/fsm.rs index 8c4d3cc..97deaa4 100644 --- a/crates/server/src/fsm.rs +++ b/crates/server/src/fsm.rs @@ -65,7 +65,7 @@ pub struct FsmConfig { } /// What the two OPENs settled. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Negotiated { pub peer_asn: u32, pub peer_identifier: Ipv4Addr, @@ -124,7 +124,7 @@ pub enum Action { /// [`State`] together with what is known in it. What the OPENs settled /// exists exactly when the handshake has got that far, so it lives in the /// states that have it and nowhere else. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] enum Phase { OpenSent, OpenConfirm(Negotiated), @@ -164,8 +164,8 @@ impl Fsm { } /// What the OPENs settled; `None` until the peer's OPEN is accepted. - pub fn negotiated(&self) -> Option { - match self.phase { + pub fn negotiated(&self) -> Option<&Negotiated> { + match &self.phase { Phase::OpenConfirm(negotiated) | Phase::Established(negotiated) => Some(negotiated), Phase::OpenSent | Phase::Closed => None, } @@ -193,7 +193,7 @@ impl Fsm { } fn on_message(&mut self, msg: Message) -> Vec { - match (self.phase, msg) { + match (&self.phase, msg) { // A NOTIFICATION ends the session in any state, and is not // answered (RFC 4271 §8.2.2). (_, Message::Notification(notification)) => { @@ -201,9 +201,11 @@ impl Fsm { } (Phase::OpenSent, Message::Open(open)) => self.on_open(open), (Phase::OpenConfirm(negotiated), Message::KeepAlive) => { - self.phase = Phase::Established(negotiated); + let negotiated = negotiated.clone(); + let hold = hold_time(&negotiated); + self.phase = Phase::Established(negotiated.clone()); let mut actions = vec![Action::Established(negotiated)]; - match hold_time(negotiated) { + match hold { Some(hold) => { actions.push(Action::ResetHoldTimer(hold)); actions.push(Action::StartKeepaliveTimer(hold / 3)); @@ -257,9 +259,9 @@ impl Fsm { }); } - let ours = self.config.families; + let ours = self.config.families.clone(); let theirs = FamilySet::offered_by(&open); - let families = ours.intersection(theirs); + let families = ours.intersection(&theirs); if families.is_empty() { return self.fail(SessionError::NoCommonAddressFamily { ours, theirs }); } @@ -271,14 +273,14 @@ impl Fsm { four_octet_asn: capability_asn.is_some(), families, }; - self.phase = Phase::OpenConfirm(negotiated); + // With a hold time of zero the timer will not run once Established, + // but a peer that never confirms must still be given up on. + let confirm_within = hold_time(&negotiated).unwrap_or(INITIAL_HOLD_TIME); + self.phase = Phase::OpenConfirm(negotiated.clone()); vec![ Action::Admit(negotiated), Action::Send(Message::KeepAlive), - // With a hold time of zero the timer will not run once - // Established, but a peer that never confirms must still be - // given up on. - Action::ResetHoldTimer(hold_time(negotiated).unwrap_or(INITIAL_HOLD_TIME)), + Action::ResetHoldTimer(confirm_within), ] } @@ -315,7 +317,7 @@ impl Fsm { } /// Any message from the peer shows it is alive (RFC 4271 §4.4). -fn heard_from_peer(negotiated: Negotiated) -> Vec { +fn heard_from_peer(negotiated: &Negotiated) -> Vec { hold_time(negotiated) .map(Action::ResetHoldTimer) .into_iter() @@ -323,7 +325,7 @@ fn heard_from_peer(negotiated: Negotiated) -> Vec { } /// The hold time as a duration; `None` when the timers do not run. -fn hold_time(negotiated: Negotiated) -> Option { +fn hold_time(negotiated: &Negotiated) -> Option { match negotiated.hold_time_secs { 0 => None, secs => Some(Duration::from_secs(secs.into())), @@ -339,7 +341,7 @@ fn build_open(config: &FsmConfig) -> Message { }]; // One Multiprotocol capability per family we will exchange. A peer does // not accept routes of a family that was not offered (RFC 4760 §8). - capabilities.extend(config.families.iter().map(|family| family.capability())); + capabilities.extend(config.families.iter().map(Capability::multiprotocol)); capabilities.push(Capability::RouteRefresh); Message::Open(OpenMessage { version: BGP4_VERSION, @@ -371,11 +373,11 @@ mod tests { use bgp_packet::message::{ MpReachNlriPathAttribute, NlriNextHop, NotificationMessage, PathAttribute, }; - use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; + use bgp_primitives::constants::{Afi, AfiSafi, Safi}; use bgp_primitives::ip_prefix::IpPrefix; use super::*; - use crate::family::AddressFamily; + use crate::family::SUPPORTED; const LOCAL_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 1); const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); @@ -386,11 +388,11 @@ mod tests { local_asn: 65000, hold_time_secs: 90, expected_asn: None, - families: FamilySet::ALL, + families: FamilySet::supported(), } } - fn only(family: AddressFamily) -> FamilySet { + fn only(family: AfiSafi) -> FamilySet { [family].into_iter().collect() } @@ -478,7 +480,7 @@ mod tests { hold_time_secs: 30, four_octet_asn: false, // The peer listed no families, so it speaks IPv4 unicast. - families: only(AddressFamily::Ipv4Unicast), + families: only(AfiSafi::IPV4_UNICAST), }; assert!(matches!( &actions[..], @@ -544,7 +546,7 @@ mod tests { #[test] fn our_open_offers_exactly_the_configured_families() { let (_, actions) = Fsm::new(FsmConfig { - families: only(AddressFamily::Ipv6Unicast), + families: only(AfiSafi::IPV6_UNICAST), ..config() }); let Action::Send(open) = &actions[0] else { @@ -554,7 +556,7 @@ mod tests { capabilities(open), [ &Capability::FourByteAsn { asn: 65000 }, - &AddressFamily::Ipv6Unicast.capability(), + &Capability::multiprotocol(AfiSafi::IPV6_UNICAST), &Capability::RouteRefresh, ] ); @@ -562,14 +564,14 @@ mod tests { #[test] fn the_session_carries_the_families_both_sides_offered() { - let negotiated = |ours, theirs: Vec| { + let negotiated = |ours, theirs: Vec| { let (mut fsm, _) = Fsm::new(FsmConfig { families: ours, ..config() }); let open = OpenMessage { options: vec![OpenOption::Capabilities( - theirs.into_iter().map(AddressFamily::capability).collect(), + theirs.into_iter().map(Capability::multiprotocol).collect(), )], ..peer_open(90) }; @@ -577,26 +579,20 @@ mod tests { (fsm, actions) }; - let (fsm, _) = negotiated(FamilySet::ALL, vec![AddressFamily::Ipv6Unicast]); + let (fsm, _) = negotiated(FamilySet::supported(), vec![AfiSafi::IPV6_UNICAST]); assert_eq!( fsm.negotiated().unwrap().families, - only(AddressFamily::Ipv6Unicast) - ); - let (fsm, _) = negotiated( - only(AddressFamily::Ipv4Unicast), - AddressFamily::ALL.to_vec(), + only(AfiSafi::IPV6_UNICAST) ); + let (fsm, _) = negotiated(only(AfiSafi::IPV4_UNICAST), SUPPORTED.to_vec()); assert_eq!( fsm.negotiated().unwrap().families, - only(AddressFamily::Ipv4Unicast) + only(AfiSafi::IPV4_UNICAST) ); // Nothing in common: the session could carry no routes at all. The // NOTIFICATION names the capability we needed. - let (fsm, actions) = negotiated( - only(AddressFamily::Ipv4Unicast), - vec![AddressFamily::Ipv6Unicast], - ); + let (fsm, actions) = negotiated(only(AfiSafi::IPV4_UNICAST), vec![AfiSafi::IPV6_UNICAST]); assert!(matches!( &actions[0], Action::Send(Message::Notification(n)) @@ -617,8 +613,8 @@ mod tests { let update = UpdateMessage { withdrawn_routes: vec![], path_attributes: vec![PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, + afi: Afi::IPV6, + safi: Safi::UNICAST, next_hop: NlriNextHop::Ipv6(std::net::Ipv6Addr::LOCALHOST), prefixes: vec![IpPrefix::try_from("2001:db8::/32").unwrap()], })], diff --git a/crates/server/src/server.rs b/crates/server/src/server.rs index 8447a04..b00e5c5 100644 --- a/crates/server/src/server.rs +++ b/crates/server/src/server.rs @@ -116,7 +116,7 @@ impl BgpServer { direction: Direction::Inbound, peer_name: None, expected_asn: None, - families: FamilySet::ALL, + families: FamilySet::supported(), }; let _advertiser = spawn_advertiser( @@ -151,7 +151,7 @@ impl BgpServer { config.peer_name = Some(peer.name.clone()); config.expected_asn = Some(peer.remote_asn); config.hold_time_secs = peer.hold_time_secs; - config.families = peer.families; + config.families = peer.families.clone(); } tokio::spawn(handle_connection(stream, peer_addr, Arc::new(config))); }