rayhaan/ferrix
0
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Names that said the same thing twice, or two things once

Browse Source 
- PathAttribute::UnknownPathAttribute and Capability::UnknownCapability
  are PathAttribute::Unknown and Capability::Unknown: the enum already
  says what kind of thing is unknown.
- The capability entry types are all named XxxEntry now
  (ExtendedNextHopEntry, LongLivedGracefulRestartEntry), rather than two
  of them sharing a name with the variant that holds a list of them; and
  MultiLabelCompat is MultiLabel, since nothing explained the "Compat".
- Two different things were both called Reflection: what a session needs
  to reflect routes (export.rs, still Reflection) and the identifiers a
  reflected route must not carry back to us (policy.rs, now
  ReflectorIds).
- well_known_allow's peer_is_internal is ibgp, as everywhere else.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Rayhaan JaufeerallyandClaude Fable 5.1 committed 2026-10-06 11:35:12 +02:00
1 parent 0829f873f2
commit 673920d48b
11 files changed
+68 -66
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/packet/src/attributes.rs
+5 -5
View File
@@ -86,7 +86,7 @@ pub enum PathAttribute {
ClusterList(Vec<Ipv4Addr>),
/// An attribute this implementation does not interpret, kept as it
/// came so that it can be passed on.
UnknownPathAttribute {
Unknown {
flags: PathAttributeFlags,
type_code: u8,
payload: Vec<u8>,
@@ -198,7 +198,7 @@ impl PathAttribute {
Self::OnlyToCustomer(_) => ONLY_TO_CUSTOMER,
Self::OriginatorId(_) => ORIGINATOR_ID,
Self::ClusterList(_) => CLUSTER_LIST,
Self::UnknownPathAttribute { type_code, .. } => *type_code,
Self::Unknown { type_code, .. } => *type_code,
}
}
@@ -207,7 +207,7 @@ impl PathAttribute {
fn flags(&self) -> PathAttributeFlags {
match self {
// As they came: the Partial bit has to survive (RFC 4271 §5).
Self::UnknownPathAttribute { flags, .. } => flags.clone(),
Self::Unknown { flags, .. } => flags.clone(),
known => {
let (optional, transitive) = defined_flags(known.type_code())
.expect("every attribute with a variant has defined flags");
@@ -304,7 +304,7 @@ impl PathAttribute {
.map(|id| Ipv4Addr::from(*id))
.collect(),
),
_ => Self::UnknownPathAttribute {
_ => Self::Unknown {
flags,
type_code,
payload: value.to_vec(),
@@ -362,7 +362,7 @@ impl PathAttribute {
out.put_slice(&id.octets());
}
}
Self::UnknownPathAttribute { payload, .. } => out.put_slice(payload),
Self::Unknown { payload, .. } => out.put_slice(payload),
}
Ok(())
}
crates/packet/src/message.rs
+2 -2
View File
@@ -639,7 +639,7 @@ mod tests {
test_path_attribute_roundtrip!(
test_unknown_path_attribute,
&[0x80, 0x63, 0x04, 0xde, 0xad, 0xbe, 0xef],
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
flags: PathAttributeFlags::new(
/*optional=*/ true, /*transitive=*/ false, /*partial=*/ false,
/*extended_length=*/ false,
@@ -780,7 +780,7 @@ mod tests {
],
UpdateMessage {
withdrawn_routes: vec![],
path_attributes: vec![PathAttribute::UnknownPathAttribute {
path_attributes: vec![PathAttribute::Unknown {
flags: PathAttributeFlags::new(true, false, false, false),
type_code: 0x63,
payload: vec![0xde, 0xad, 0xbe, 0xef],
crates/packet/src/open.rs
+27 -27
View File
@@ -237,7 +237,7 @@ pub enum Capability {
OutboundRouteFilter(Vec<OrfFamily>),
/// Extended Next Hop encoding (RFC 8950).
ExtendedNextHop {
entries: Vec<ExtendedNextHop>,
entries: Vec<ExtendedNextHopEntry>,
},
/// Extended Message (RFC 8654).
ExtendedMessage,
@@ -248,7 +248,7 @@ pub enum Capability {
},
/// Multiple labels (RFC 8277 §2.1): for each address family, how many
/// labels the speaker can handle in one NLRI.
MultiLabelCompat {
MultiLabel {
entries: Vec<MultiLabelEntry>,
},
BgpRole {
@@ -280,10 +280,10 @@ pub enum Capability {
/// Enhanced Route Refresh (RFC 7313).
EnhancedRouteRefresh,
/// Long Lived Graceful Restart (RFC 9494).
LongLivedGracefulRestart(Vec<LongLivedGracefulRestart>),
LongLivedGracefulRestart(Vec<LongLivedGracefulRestartEntry>),
/// A capability this implementation has no definition for, kept as it
/// came (RFC 5492 §3: what is not understood is ignored).
UnknownCapability {
Unknown {
type_code: u8,
value: Vec<u8>,
},
@@ -308,14 +308,14 @@ impl Capability {
Self::ExtendedNextHop { .. } => EXTENDED_NEXT_HOP,
Self::ExtendedMessage => EXTENDED_MESSAGE,
Self::BgpSec { .. } => BGP_SEC,
Self::MultiLabelCompat { .. } => MULTI_LABEL,
Self::MultiLabel { .. } => 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,
Self::Unknown { type_code, .. } => *type_code,
}
}
@@ -366,7 +366,7 @@ impl Capability {
.map_err(|_| malformed(format!("SAFI {safi} does not fit in one octet")))?;
Ok((
buf,
ExtendedNextHop {
ExtendedNextHopEntry {
afi: Afi(afi),
safi,
nh_afi: Afi(nh_afi),
@@ -399,7 +399,7 @@ impl Capability {
if entries.is_empty() {
return Err(malformed("Multiple Labels capability with no entries"));
}
(rest, Self::MultiLabelCompat { entries })
(rest, Self::MultiLabel { entries })
}
BGP_ROLE => {
let (rest, role) = be_u8(value)?;
@@ -466,7 +466,7 @@ impl Capability {
let (buf, stale_time) = be_u24(buf)?;
Ok((
buf,
LongLivedGracefulRestart {
LongLivedGracefulRestartEntry {
afi,
safi,
flags,
@@ -478,7 +478,7 @@ impl Capability {
}
type_code => (
&[][..],
Self::UnknownCapability {
Self::Unknown {
type_code,
value: value.to_vec(),
},
@@ -524,7 +524,7 @@ impl Capability {
out.put_u8(header.0);
out.put_u16(afi.0);
}
Capability::MultiLabelCompat { entries } => {
Capability::MultiLabel { entries } => {
for entry in entries {
put_afi_safi(AfiSafi::new(entry.afi, entry.safi), out);
out.put_u8(entry.count);
@@ -566,7 +566,7 @@ impl Capability {
out.put_slice(&entry.stale_time.to_be_bytes()[1..]);
}
}
Capability::UnknownCapability { value, .. } => out.put_slice(value),
Capability::Unknown { value, .. } => out.put_slice(value),
}
Ok(())
}
@@ -692,11 +692,11 @@ impl AddPathEntry {
}
}
/// `ExtendedNextHop` allows for the nexthop to be for a different address family
/// `ExtendedNextHopEntry` 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, PartialEq, Eq)]
pub struct ExtendedNextHop {
pub struct ExtendedNextHopEntry {
/// The Address Family to update the nexthop format for.
pub afi: Afi,
/// The Subsequent Address Family to update the nexthop format for.
@@ -743,7 +743,7 @@ impl From<&BgpRole> for u8 {
}
#[derive(Debug, Clone, Serialize, PartialEq, Eq)]
pub struct LongLivedGracefulRestart {
pub struct LongLivedGracefulRestartEntry {
pub afi: Afi,
pub safi: Safi,
pub flags: u8,
@@ -780,7 +780,7 @@ mod tests {
use eyre::Result;
use crate::open::{
AddPathEntry, AddPathSendRecv, Capability, ExtendedNextHop, GracefulRestartEntry,
AddPathEntry, AddPathSendRecv, Capability, ExtendedNextHopEntry, GracefulRestartEntry,
MultiLabelEntry, OpenMessage, OpenOption, OrfFamily, OrfSendRecv, OrfType,
OutboundRouteFilterEntry,
};
@@ -816,7 +816,7 @@ mod tests {
test_capability_roundtrip!(
test_route_refresh_cisco_unsupported,
&[0x80, 0x00],
Capability::UnknownCapability {
Capability::Unknown {
type_code: 0x80,
value: vec![]
}
@@ -868,17 +868,17 @@ mod tests {
],
Capability::ExtendedNextHop {
entries: vec![
ExtendedNextHop {
ExtendedNextHopEntry {
afi: Afi::IPV4,
safi: Safi::UNICAST,
nh_afi: Afi::IPV6,
},
ExtendedNextHop {
ExtendedNextHopEntry {
afi: Afi::IPV4,
safi: Safi::MULTICAST,
nh_afi: Afi::IPV6,
},
ExtendedNextHop {
ExtendedNextHopEntry {
afi: Afi::IPV4,
safi: Safi::MPLS_VPN,
nh_afi: Afi::IPV6
@@ -906,7 +906,7 @@ mod tests {
afi: Afi::IPV4,
safi: Safi::UNICAST,
}]),
OpenOption::Capabilities(vec![Capability::UnknownCapability {
OpenOption::Capabilities(vec![Capability::Unknown {
type_code: 0x80,
value: vec![],
}]),
@@ -924,17 +924,17 @@ mod tests {
}]),
OpenOption::Capabilities(vec![Capability::ExtendedNextHop {
entries: vec![
ExtendedNextHop {
ExtendedNextHopEntry {
afi: Afi::IPV4,
safi: Safi::UNICAST,
nh_afi: Afi::IPV6,
},
ExtendedNextHop {
ExtendedNextHopEntry {
afi: Afi::IPV4,
safi: Safi::MULTICAST,
nh_afi: Afi::IPV6,
},
ExtendedNextHop {
ExtendedNextHopEntry {
afi: Afi::IPV4,
safi: Safi::MPLS_VPN,
nh_afi: Afi::IPV6,
@@ -962,7 +962,7 @@ mod tests {
assert!(rest.is_empty());
assert_eq!(
cap,
Capability::MultiLabelCompat {
Capability::MultiLabel {
entries: vec![
MultiLabelEntry {
afi: Afi::IPV4,
@@ -1136,7 +1136,7 @@ mod tests {
#[test]
fn long_optional_parameters_use_two_octet_lengths() {
let capabilities: Vec<Capability> = (0..60)
.map(|i| Capability::UnknownCapability {
.map(|i| Capability::Unknown {
type_code: 200,
value: vec![i; 4],
})
@@ -1194,7 +1194,7 @@ mod tests {
/// used to be written with the length wrapped around.
#[test]
fn a_capability_too_long_for_its_length_octet_is_refused() {
let entry = ExtendedNextHop {
let entry = ExtendedNextHopEntry {
afi: Afi::IPV4,
safi: Safi::UNICAST,
nh_afi: Afi::IPV6,
crates/packet/src/update_decode.rs
+3 -3
View File
@@ -421,7 +421,7 @@ impl<'a> AttributeReader<'a> {
/// The attribute kept as bytes.
fn opaque(raw: &RawAttribute<'_>) -> PathAttribute {
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
flags: raw.flags.clone(),
type_code: raw.type_code,
payload: raw.value.to_vec(),
@@ -1000,7 +1000,7 @@ mod tests {
assert!(decoded.errors.is_empty());
assert_eq!(
decoded.update.path_attributes.last(),
Some(&PathAttribute::UnknownPathAttribute {
Some(&PathAttribute::Unknown {
flags: PathAttributeFlags(OPTIONAL_TRANSITIVE),
type_code: AS4_AGGREGATOR,
payload: value.to_vec(),
@@ -1204,7 +1204,7 @@ mod tests {
assert!(decoded.errors.is_empty());
assert_eq!(
decoded.update.path_attributes[3],
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
flags: PathAttributeFlags(0xe0),
type_code: 99,
payload: vec![1, 2, 3],
crates/packet/tests/properties.rs
+14 -12
View File
@@ -25,8 +25,8 @@ use bgp_packet::message::{
PathAttributeFlags, RouteRefreshMessage, UpdateMessage,
};
use bgp_packet::open::{
AddPathEntry, AddPathSendRecv, BgpRole, BgpSecHeader, Capability, ExtendedNextHop,
GracefulRestartEntry, LongLivedGracefulRestart, MultiLabelEntry, OpenMessage, OpenOption,
AddPathEntry, AddPathSendRecv, BgpRole, BgpSecHeader, Capability, ExtendedNextHopEntry,
GracefulRestartEntry, LongLivedGracefulRestartEntry, MultiLabelEntry, OpenMessage, OpenOption,
OrfFamily, OrfSendRecv, OrfType, OutboundRouteFilterEntry,
};
use bgp_primitives::constants::{Afi, AfiSafi, IpVersion, Safi};
@@ -220,7 +220,7 @@ fn unknown_attributes() -> impl Strategy<Value = Vec<PathAttribute>> {
if payload.len() > 255 {
flags |= 0x10;
}
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
flags: PathAttributeFlags(flags),
type_code,
payload,
@@ -402,7 +402,7 @@ fn capability() -> impl Strategy<Value = Capability> {
vec((afi(), safi(), afi()), 1..4).prop_map(|entries| Capability::ExtendedNextHop {
entries: entries
.into_iter()
.map(|(afi, safi, nh_afi)| ExtendedNextHop { afi, safi, nh_afi })
.map(|(afi, safi, nh_afi)| ExtendedNextHopEntry { afi, safi, nh_afi })
.collect(),
}),
Just(Capability::ExtendedMessage),
@@ -411,7 +411,7 @@ fn capability() -> impl Strategy<Value = Capability> {
afi,
}),
vec((afi(), safi(), any::<u8>()), 1..4).prop_map(|entries| {
Capability::MultiLabelCompat {
Capability::MultiLabel {
entries: entries
.into_iter()
.map(|(afi, safi, count)| MultiLabelEntry { afi, safi, count })
@@ -458,12 +458,14 @@ fn capability() -> impl Strategy<Value = Capability> {
Capability::LongLivedGracefulRestart(
entries
.into_iter()
.map(|(afi, safi, flags, stale_time)| LongLivedGracefulRestart {
afi,
safi,
flags,
stale_time,
})
.map(
|(afi, safi, flags, stale_time)| LongLivedGracefulRestartEntry {
afi,
safi,
flags,
stale_time,
},
)
.collect(),
)
}),
@@ -473,7 +475,7 @@ fn capability() -> impl Strategy<Value = Capability> {
}),
vec(any::<u8>(), 0..20)
)
.prop_map(|(type_code, value)| Capability::UnknownCapability { type_code, value }),
.prop_map(|(type_code, value)| Capability::Unknown { type_code, value }),
]
}
crates/server/src/config.rs
+3 -3
View File
@@ -17,7 +17,7 @@ use crate::error::{ConfigError, ExportProblem, ImportProblem, OriginateProblem,
use crate::family::FamilySet;
use crate::marks::{Control, Marks, Patterns, Scrub};
use crate::policy::{
ExportLimits, ExportPolicy, ImportPolicy, ImportRule, PeerRole, PrefixSet, Reflection,
ExportLimits, ExportPolicy, ImportPolicy, ImportRule, PeerRole, PrefixSet, ReflectorIds,
};
use crate::proto::config::{self as proto, BgpConfig, OriginatedRoute, Peer};
use crate::restart::GracefulRestartConfig;
@@ -341,7 +341,7 @@ impl Ours {
for peer in peers {
if peer.remote_asn == self.local_asn {
peer.import.originated = Arc::clone(&originated);
peer.import.reflection = Some(Reflection {
peer.import.reflection = Some(ReflectorIds {
router_id: self.router_id,
cluster_id: self.cluster_id,
});
@@ -836,7 +836,7 @@ mod tests {
assert!(config.peers[0].reflector_client);
assert_eq!(
config.peers[0].import.reflection,
Some(Reflection {
Some(ReflectorIds {
router_id: config.router_id,
cluster_id: config.router_id
})
crates/server/src/export.rs
+4 -4
View File
@@ -260,7 +260,7 @@ impl Egress<'_> {
// What we do not understand goes on if it says it may, marked
// as having passed a router that did not look at it; and no
// further if not (RFC 4271 §5).
PathAttribute::UnknownPathAttribute { flags, .. } => {
PathAttribute::Unknown { flags, .. } => {
let passes = flags.optional() && flags.transitive();
if passes {
flags.set_partial(true);
@@ -619,12 +619,12 @@ mod tests {
communities(&[(3356, 3)]),
// Understood by nobody here: one that may go on, one that may
// not.
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
flags: PathAttributeFlags::new(true, true, false, false),
type_code: 200,
payload: vec![1],
},
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
flags: PathAttributeFlags::new(true, false, false, false),
type_code: 201,
payload: vec![2],
@@ -641,7 +641,7 @@ mod tests {
PathAttribute::Origin(OriginPathAttribute::IGP),
AsPathAttribute::from_asns(vec![LOCAL, 3356, 64700]),
communities(&[(3356, 3)]),
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
// Marked as having passed a router that did not know it.
flags: PathAttributeFlags::new(true, true, true, false),
type_code: 200,
crates/server/src/fsm.rs
+3 -3
View File
@@ -17,7 +17,7 @@ use std::time::Duration;
use bgp_packet::errors::{BgpError, CeaseSubcode, DecodeError, OpenMsgSubcode};
use bgp_packet::message::{AS_TRANS, BGP4_VERSION, Message, NotificationMessage, UpdateMessage};
use bgp_packet::open::{
AddPathEntry, AddPathSendRecv, Capability, ExtendedNextHop, OpenMessage, OpenOption,
AddPathEntry, AddPathSendRecv, Capability, ExtendedNextHopEntry, OpenMessage, OpenOption,
};
use bgp_primitives::constants::{Afi, AfiSafi, Safi};
@@ -555,7 +555,7 @@ impl Fsm {
/// IPv4 unicast routes with next hops that are IPv6 addresses, as the
/// Extended Next Hop capability names it (RFC 8950 §3).
const IPV4_WITH_IPV6_NEXT_HOP: ExtendedNextHop = ExtendedNextHop {
const IPV4_WITH_IPV6_NEXT_HOP: ExtendedNextHopEntry = ExtendedNextHopEntry {
afi: Afi::IPV4,
safi: Safi::UNICAST,
nh_afi: Afi::IPV6,
@@ -1066,7 +1066,7 @@ mod tests {
// Offered for something else: IPv6 routes with IPv4 next hops,
// say.
let other = Capability::ExtendedNextHop {
entries: vec![ExtendedNextHop {
entries: vec![ExtendedNextHopEntry {
afi: Afi::IPV6,
safi: Safi::UNICAST,
nh_afi: Afi::IPV4,
crates/server/src/marks.rs
+2 -2
View File
@@ -295,10 +295,10 @@ const NO_EXPORT_SUBCONFED: (u16, u16) = (0xFFFF, 0xFF03);
/// all, as far as the well-known ones say: NO_ADVERTISE keeps it from
/// everyone, NO_EXPORT from other networks. (There are no confederations
/// here, so NO_EXPORT_SUBCONFED is the same as NO_EXPORT.)
pub fn well_known_allow(communities: &[(u16, u16)], peer_is_internal: bool) -> bool {
pub fn well_known_allow(communities: &[(u16, u16)], ibgp: bool) -> bool {
!communities.iter().any(|community| {
*community == NO_ADVERTISE
|| (!peer_is_internal && matches!(*community, NO_EXPORT | NO_EXPORT_SUBCONFED))
|| (!ibgp && matches!(*community, NO_EXPORT | NO_EXPORT_SUBCONFED))
})
}
crates/server/src/policy.rs
+3 -3
View File
@@ -123,7 +123,7 @@ pub struct ImportPolicy {
/// For a peer in our own AS, what a route passed on inside the AS
/// must not say it has already been through (RFC 4456 §8); `None` for
/// a peer in another AS, whose routes carry no such thing.
pub reflection: Option<Reflection>,
pub reflection: Option<ReflectorIds>,
}
/// Who we are to routes passed on from one router of our AS to another:
@@ -131,7 +131,7 @@ pub struct ImportPolicy {
/// whose CLUSTER_LIST holds our cluster ID has been passed on by us
/// already. Either has gone round in a circle, and is not taken.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct Reflection {
pub struct ReflectorIds {
pub router_id: Ipv4Addr,
pub cluster_id: Ipv4Addr,
}
@@ -1068,7 +1068,7 @@ mod tests {
let us = Ipv4Addr::new(10, 0, 0, 1);
let cluster = Ipv4Addr::new(10, 0, 0, 9);
let policy = ImportPolicy {
reflection: Some(Reflection {
reflection: Some(ReflectorIds {
router_id: us,
cluster_id: cluster,
}),
crates/web/src/api.rs
+2 -2
View File
@@ -390,7 +390,7 @@ fn other_attribute(attribute: &PathAttribute) -> Option<OtherAttribute> {
"cluster list",
list(ids.iter().map(Ipv4Addr::to_string).collect(), " "),
),
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
flags,
type_code,
payload,
@@ -1412,7 +1412,7 @@ mod tests {
Ipv4Addr::new(10, 0, 0, 1),
Ipv4Addr::new(10, 0, 0, 2),
]),
PathAttribute::UnknownPathAttribute {
PathAttribute::Unknown {
flags: PathAttributeFlags::new(true, true, false, false),
type_code: 99,
payload: vec![0xab, 0x01],