From ecb014e5273981ce92913748977bdd2375dc081e Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Tue, 15 Jul 2025 14:37:43 -0700 Subject: [PATCH 1/4] refactor: change path_id to Option --- src/encoder/rib_encoder.rs | 10 ++-- src/models/mrt/table_dump_v2.rs | 4 ++ src/models/network/prefix.rs | 59 +++++++++++-------- src/parser/bgp/attributes/attr_14_15_nlri.rs | 22 +++---- src/parser/bgp/attributes/mod.rs | 10 ++-- src/parser/bgp/messages.rs | 14 ++--- .../bmp/messages/peer_up_notification.rs | 2 +- src/parser/bmp/messages/route_mirroring.rs | 2 +- src/parser/filter.rs | 2 +- src/parser/mrt/messages/bgp4mp.rs | 4 +- src/parser/mrt/messages/mod.rs | 9 +-- src/parser/mrt/messages/table_dump.rs | 5 +- .../messages/table_dump_v2/rib_afi_entries.rs | 22 ++++--- src/parser/rislive/mod.rs | 4 +- src/parser/utils.rs | 36 ++++++----- 15 files changed, 109 insertions(+), 96 deletions(-) diff --git a/src/encoder/rib_encoder.rs b/src/encoder/rib_encoder.rs index 1a284f3f..2b330520 100644 --- a/src/encoder/rib_encoder.rs +++ b/src/encoder/rib_encoder.rs @@ -48,16 +48,18 @@ impl MrtRibEncoder { IpAddr::V6(_ip) => Ipv4Addr::from(0), }; let peer = Peer::new(bgp_identifier, elem.peer_ip, elem.peer_asn); - let peer_id = self.index_table.add_peer(peer); + let peer_index = self.index_table.add_peer(peer); + let path_id = elem.prefix.path_id; let prefix = elem.prefix.prefix; let entries_map = self.per_prefix_entries_map.entry(prefix).or_default(); let entry = RibEntry { - peer_index: peer_id, + peer_index, + path_id, originated_time: elem.timestamp as u32, attributes: Attributes::from(elem), }; - entries_map.insert(peer_id, entry); + entries_map.insert(peer_index, entry); } /// Export the data stored in the struct to a byte array. @@ -99,7 +101,7 @@ impl MrtRibEncoder { let mut prefix_rib_entry = RibAfiEntries { rib_type, sequence_number: entry_count as u32, - prefix: NetworkPrefix::new(*prefix, 0), + prefix: NetworkPrefix::new(*prefix, None), rib_entries: vec![], }; for entry in entries_map.values() { diff --git a/src/models/mrt/table_dump_v2.rs b/src/models/mrt/table_dump_v2.rs index 902d9413..570a4535 100644 --- a/src/models/mrt/table_dump_v2.rs +++ b/src/models/mrt/table_dump_v2.rs @@ -131,6 +131,8 @@ pub struct RibGenericEntries { /// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ /// | Originated Time | /// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +/// | Optional Path ID | +/// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ /// | Attribute Length | /// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ /// | BGP Attributes... (variable) @@ -141,6 +143,7 @@ pub struct RibGenericEntries { pub struct RibEntry { pub peer_index: u16, pub originated_time: u32, + pub path_id: Option, pub attributes: Attributes, } @@ -310,6 +313,7 @@ mod tests { let rib_entry = RibEntry { peer_index: 1, originated_time: 1, + path_id: None, attributes: Attributes::default(), }; let rib_afi = TableDumpV2Message::RibAfi(RibAfiEntries { diff --git a/src/models/network/prefix.rs b/src/models/network/prefix.rs index c58761f2..44f5f09c 100644 --- a/src/models/network/prefix.rs +++ b/src/models/network/prefix.rs @@ -9,16 +9,15 @@ use std::str::FromStr; #[derive(PartialEq, Eq, Clone, Copy, Hash)] pub struct NetworkPrefix { pub prefix: IpNet, - pub path_id: u32, + pub path_id: Option, } // Attempt to reduce the size of the debug output impl Debug for NetworkPrefix { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - if self.path_id == 0 { - write!(f, "{}", self.prefix) - } else { - write!(f, "{}#{}", self.prefix, self.path_id) + match self.path_id { + Some(path_id) => write!(f, "{}#{}", self.prefix, path_id), + None => write!(f, "{}", self.prefix), } } } @@ -28,12 +27,15 @@ impl FromStr for NetworkPrefix { fn from_str(s: &str) -> Result { let prefix = IpNet::from_str(s)?; - Ok(NetworkPrefix { prefix, path_id: 0 }) + Ok(NetworkPrefix { + prefix, + path_id: None, + }) } } impl NetworkPrefix { - pub fn new(prefix: IpNet, path_id: u32) -> NetworkPrefix { + pub fn new(prefix: IpNet, path_id: Option) -> NetworkPrefix { NetworkPrefix { prefix, path_id } } @@ -57,16 +59,19 @@ impl NetworkPrefix { /// use bgpkit_parser::models::NetworkPrefix; /// /// let prefix = NetworkPrefix::from_str("192.168.0.0/24").unwrap(); - /// let encoded_bytes = prefix.encode(false); + /// let encoded_bytes = prefix.encode(); /// /// assert_eq!(encoded_bytes.iter().as_slice(), &[24, 192, 168, 0]); /// ``` - pub fn encode(&self, add_path: bool) -> Bytes { + pub fn encode(&self) -> Bytes { let mut bytes = BytesMut::new(); - if add_path { + + // encode path identifier if it exists + if let Some(path_id) = self.path_id { // encode path identifier - bytes.put_u32(self.path_id); + bytes.put_u32(path_id); } + // encode prefix let bit_len = self.prefix.prefix_len(); @@ -108,12 +113,12 @@ mod serde_impl { where S: Serializer, { - if serializer.is_human_readable() && self.path_id == 0 { + if serializer.is_human_readable() && self.path_id.is_none() { self.prefix.serialize(serializer) } else { SerdeNetworkPrefixRepr::WithPathId { prefix: self.prefix, - path_id: self.path_id, + path_id: self.path_id.unwrap_or_default(), } .serialize(serializer) } @@ -126,12 +131,14 @@ mod serde_impl { D: Deserializer<'de>, { match SerdeNetworkPrefixRepr::deserialize(deserializer)? { - SerdeNetworkPrefixRepr::PlainPrefix(prefix) => { - Ok(NetworkPrefix { prefix, path_id: 0 }) - } - SerdeNetworkPrefixRepr::WithPathId { prefix, path_id } => { - Ok(NetworkPrefix { prefix, path_id }) - } + SerdeNetworkPrefixRepr::PlainPrefix(prefix) => Ok(NetworkPrefix { + prefix, + path_id: None, + }), + SerdeNetworkPrefixRepr::WithPathId { prefix, path_id } => Ok(NetworkPrefix { + prefix, + path_id: Some(path_id), + }), } } } @@ -150,20 +157,20 @@ mod tests { network_prefix.prefix, IpNet::from_str("192.168.0.0/24").unwrap() ); - assert_eq!(network_prefix.path_id, 0); + assert_eq!(network_prefix.path_id, None); } #[test] fn test_encode() { let prefix = IpNet::from_str("192.168.0.0/24").unwrap(); - let network_prefix = NetworkPrefix::new(prefix, 1); - let _encoded = network_prefix.encode(true); + let network_prefix = NetworkPrefix::new(prefix, Some(1)); + let _encoded = network_prefix.encode(); } #[test] fn test_display() { let prefix = IpNet::from_str("192.168.0.0/24").unwrap(); - let network_prefix = NetworkPrefix::new(prefix, 1); + let network_prefix = NetworkPrefix::new(prefix, Some(1)); assert_eq!(network_prefix.to_string(), "192.168.0.0/24"); } @@ -171,7 +178,7 @@ mod tests { #[cfg(feature = "serde")] fn test_serialization() { let prefix = IpNet::from_str("192.168.0.0/24").unwrap(); - let network_prefix = NetworkPrefix::new(prefix, 1); + let network_prefix = NetworkPrefix::new(prefix, Some(1)); let serialized = serde_json::to_string(&network_prefix).unwrap(); assert_eq!(serialized, "{\"prefix\":\"192.168.0.0/24\",\"path_id\":1}"); } @@ -185,13 +192,13 @@ mod tests { deserialized.prefix, IpNet::from_str("192.168.0.0/24").unwrap() ); - assert_eq!(deserialized.path_id, 1); + assert_eq!(deserialized.path_id, Some(1)); } #[test] fn test_debug() { let prefix = IpNet::from_str("192.168.0.0/24").unwrap(); - let network_prefix = NetworkPrefix::new(prefix, 1); + let network_prefix = NetworkPrefix::new(prefix, Some(1)); assert_eq!(format!("{network_prefix:?}"), "192.168.0.0/24#1"); } } diff --git a/src/parser/bgp/attributes/attr_14_15_nlri.rs b/src/parser/bgp/attributes/attr_14_15_nlri.rs index 647cd20f..85a3e7ad 100644 --- a/src/parser/bgp/attributes/attr_14_15_nlri.rs +++ b/src/parser/bgp/attributes/attr_14_15_nlri.rs @@ -106,7 +106,7 @@ pub fn parse_nlri( } /// Encode a NLRI attribute. -pub fn encode_nlri(nlri: &Nlri, reachable: bool, add_path: bool) -> Bytes { +pub fn encode_nlri(nlri: &Nlri, reachable: bool) -> Bytes { let mut bytes = BytesMut::new(); // encode address family @@ -138,7 +138,7 @@ pub fn encode_nlri(nlri: &Nlri, reachable: bool, add_path: bool) -> Bytes { // NLRI for prefix in &nlri.prefixes { - bytes.extend(prefix.encode(add_path)); + bytes.extend(prefix.encode()); } bytes.freeze() @@ -266,7 +266,7 @@ mod tests { Ipv4Addr::from_str("192.0.2.1").unwrap() )) ); - let prefix = NetworkPrefix::new(IpNet::from_str("192.0.2.0/24").unwrap(), 123); + let prefix = NetworkPrefix::new(IpNet::from_str("192.0.2.0/24").unwrap(), Some(123)); assert_eq!(nlri.prefixes[0], prefix); assert_eq!(nlri.prefixes[0].path_id, prefix.path_id); } else { @@ -284,10 +284,10 @@ mod tests { )), prefixes: vec![NetworkPrefix { prefix: IpNet::from_str("192.0.1.0/24").unwrap(), - path_id: 0, + path_id: None, }], }; - let bytes = encode_nlri(&nlri, true, false); + let bytes = encode_nlri(&nlri, true); assert_eq!( bytes, Bytes::from(vec![ @@ -312,10 +312,10 @@ mod tests { )), prefixes: vec![NetworkPrefix { prefix: IpNet::from_str("192.0.1.0/24").unwrap(), - path_id: 123, + path_id: Some(123), }], }; - let bytes = encode_nlri(&nlri, true, true); + let bytes = encode_nlri(&nlri, true); assert_eq!( bytes, Bytes::from(vec![ @@ -368,10 +368,10 @@ mod tests { next_hop: None, prefixes: vec![NetworkPrefix { prefix: IpNet::from_str("192.0.1.0/24").unwrap(), - path_id: 0, + path_id: None, }], }; - let bytes = encode_nlri(&nlri, false, false); + let bytes = encode_nlri(&nlri, false); assert_eq!( bytes, Bytes::from(vec![ @@ -398,10 +398,10 @@ mod tests { )), prefixes: vec![NetworkPrefix { prefix: IpNet::from_str("192.0.1.0/24").unwrap(), - path_id: 0, + path_id: None, }], }; - let bytes = encode_nlri(&nlri_with_next_hop, false, false); + let bytes = encode_nlri(&nlri_with_next_hop, false); // The encoded bytes should include the next_hop assert_eq!( bytes, diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index a9aff886..f0973718 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -210,7 +210,7 @@ pub fn parse_attributes( } impl Attribute { - pub fn encode(&self, add_path: bool, asn_len: AsnLength) -> Bytes { + pub fn encode(&self, asn_len: AsnLength) -> Bytes { let mut bytes = BytesMut::new(); let flag = self.flag.bits(); @@ -247,8 +247,8 @@ impl Attribute { } AttributeValue::OriginatorId(v) => encode_originator_id(&IpAddr::from(*v)), AttributeValue::Clusters(v) => encode_clusters(v), - AttributeValue::MpReachNlri(v) => encode_nlri(v, true, add_path), - AttributeValue::MpUnreachNlri(v) => encode_nlri(v, false, add_path), + AttributeValue::MpReachNlri(v) => encode_nlri(v, true), + AttributeValue::MpUnreachNlri(v) => encode_nlri(v, false), AttributeValue::Development(v) => Bytes::from(v.to_owned()), AttributeValue::Deprecated(v) => Bytes::from(v.bytes.to_owned()), AttributeValue::Unknown(v) => Bytes::from(v.bytes.to_owned()), @@ -268,10 +268,10 @@ impl Attribute { } impl Attributes { - pub fn encode(&self, add_path: bool, asn_len: AsnLength) -> Bytes { + pub fn encode(&self, asn_len: AsnLength) -> Bytes { let mut bytes = BytesMut::new(); for attr in &self.inner { - bytes.extend(attr.encode(add_path, asn_len)); + bytes.extend(attr.encode(asn_len)); } bytes.freeze() } diff --git a/src/parser/bgp/messages.rs b/src/parser/bgp/messages.rs index 7b040613..b15cb07b 100644 --- a/src/parser/bgp/messages.rs +++ b/src/parser/bgp/messages.rs @@ -307,21 +307,21 @@ pub fn parse_bgp_update_message( } impl BgpUpdateMessage { - pub fn encode(&self, add_path: bool, asn_len: AsnLength) -> Bytes { + pub fn encode(&self, asn_len: AsnLength) -> Bytes { let mut bytes = BytesMut::new(); // withdrawn prefixes - let withdrawn_bytes = encode_nlri_prefixes(&self.withdrawn_prefixes, add_path); + let withdrawn_bytes = encode_nlri_prefixes(&self.withdrawn_prefixes); bytes.put_u16(withdrawn_bytes.len() as u16); bytes.put_slice(&withdrawn_bytes); // attributes - let attr_bytes = self.attributes.encode(add_path, asn_len); + let attr_bytes = self.attributes.encode(asn_len); bytes.put_u16(attr_bytes.len() as u16); bytes.put_slice(&attr_bytes); - bytes.extend(encode_nlri_prefixes(&self.announced_prefixes, add_path)); + bytes.extend(encode_nlri_prefixes(&self.announced_prefixes)); bytes.freeze() } @@ -369,7 +369,7 @@ impl BgpUpdateMessage { } impl BgpMessage { - pub fn encode(&self, add_path: bool, asn_len: AsnLength) -> Bytes { + pub fn encode(&self, asn_len: AsnLength) -> Bytes { let mut bytes = BytesMut::new(); bytes.put_u32(0); // marker bytes.put_u32(0); // marker @@ -378,7 +378,7 @@ impl BgpMessage { let (msg_type, msg_bytes) = match self { BgpMessage::Open(msg) => (BgpMessageType::OPEN, msg.encode()), - BgpMessage::Update(msg) => (BgpMessageType::UPDATE, msg.encode(add_path, asn_len)), + BgpMessage::Update(msg) => (BgpMessageType::UPDATE, msg.encode(asn_len)), BgpMessage::Notification(msg) => (BgpMessageType::NOTIFICATION, msg.encode()), BgpMessage::KeepAlive => (BgpMessageType::KEEPALIVE, Bytes::new()), }; @@ -629,7 +629,7 @@ mod tests { error: BgpError::MessageHeaderError(MessageHeaderError::BAD_MESSAGE_LENGTH), data: vec![0x00, 0x00], }); - let bytes = bgp_message.encode(false, AsnLength::Bits16); + let bytes = bgp_message.encode(AsnLength::Bits16); assert_eq!( bytes, Bytes::from_static(&[ diff --git a/src/parser/bmp/messages/peer_up_notification.rs b/src/parser/bmp/messages/peer_up_notification.rs index 407f00cb..a39f4035 100644 --- a/src/parser/bmp/messages/peer_up_notification.rs +++ b/src/parser/bmp/messages/peer_up_notification.rs @@ -106,7 +106,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp_open_message_bytes = bgp_open_message.encode(false, AsnLength::Bits32); + let bgp_open_message_bytes = bgp_open_message.encode(AsnLength::Bits32); data.extend_from_slice(&bgp_open_message_bytes); data.extend_from_slice(&bgp_open_message_bytes); diff --git a/src/parser/bmp/messages/route_mirroring.rs b/src/parser/bmp/messages/route_mirroring.rs index 03b1e89e..3b958e81 100644 --- a/src/parser/bmp/messages/route_mirroring.rs +++ b/src/parser/bmp/messages/route_mirroring.rs @@ -81,7 +81,7 @@ mod tests { extended_length: false, opt_params: vec![], }); - let bgp_message_bytes = bgp_message.encode(false, AsnLength::Bits32); + let bgp_message_bytes = bgp_message.encode(AsnLength::Bits32); let expected_asn_len = AsnLength::Bits32; let actual_info_len = bgp_message_bytes.len() as u16; diff --git a/src/parser/filter.rs b/src/parser/filter.rs index cd35a6f3..efa5971f 100644 --- a/src/parser/filter.rs +++ b/src/parser/filter.rs @@ -603,7 +603,7 @@ mod tests { timestamp: 1637437798_f64, peer_ip: IpAddr::from_str("192.168.1.1").unwrap(), peer_asn: Asn::new_32bit(12345), - prefix: NetworkPrefix::new(IpNet::from_str("192.168.1.0/24").unwrap(), 0), + prefix: NetworkPrefix::new(IpNet::from_str("192.168.1.0/24").unwrap(), None), next_hop: None, as_path: Some(AsPath::from_sequence(vec![174, 1916, 52888])), origin_asns: Some(vec![Asn::new_16bit(12345)]), diff --git a/src/parser/mrt/messages/bgp4mp.rs b/src/parser/mrt/messages/bgp4mp.rs index be08d7b4..4c14689e 100644 --- a/src/parser/mrt/messages/bgp4mp.rs +++ b/src/parser/mrt/messages/bgp4mp.rs @@ -102,7 +102,7 @@ pub fn parse_bgp4mp_message( } impl Bgp4MpMessage { - pub fn encode(&self, add_path: bool, asn_len: AsnLength) -> Bytes { + pub fn encode(&self, asn_len: AsnLength) -> Bytes { let mut bytes = BytesMut::new(); bytes.extend(self.peer_asn.encode()); bytes.extend(self.local_asn.encode()); @@ -110,7 +110,7 @@ impl Bgp4MpMessage { bytes.put_u16(address_family(&self.peer_ip)); bytes.extend(encode_ipaddr(&self.peer_ip)); bytes.extend(encode_ipaddr(&self.local_ip)); - bytes.extend(&self.bgp_message.encode(add_path, asn_len)); + bytes.extend(&self.bgp_message.encode(asn_len)); bytes.freeze() } } diff --git a/src/parser/mrt/messages/mod.rs b/src/parser/mrt/messages/mod.rs index d8f3b1f1..ea256aac 100644 --- a/src/parser/mrt/messages/mod.rs +++ b/src/parser/mrt/messages/mod.rs @@ -28,13 +28,6 @@ impl MrtMessage { msg.encode(asn_len) } Bgp4MpEnum::Message(msg) => { - let add_path = matches!( - msg_type, - Bgp4MpType::MessageAddpath - | Bgp4MpType::MessageAs4Addpath - | Bgp4MpType::MessageLocalAddpath - | Bgp4MpType::MessageLocalAs4Addpath - ); let asn_len = match matches!( msg_type, Bgp4MpType::MessageAs4 @@ -45,7 +38,7 @@ impl MrtMessage { true => AsnLength::Bits32, false => AsnLength::Bits16, }; - msg.encode(add_path, asn_len) + msg.encode(asn_len) } } } diff --git a/src/parser/mrt/messages/table_dump.rs b/src/parser/mrt/messages/table_dump.rs index ce422dd1..24c6900c 100644 --- a/src/parser/mrt/messages/table_dump.rs +++ b/src/parser/mrt/messages/table_dump.rs @@ -98,7 +98,7 @@ pub fn parse_table_dump_message( Ok(TableDumpMessage { view_number, sequence_number, - prefix: NetworkPrefix::new(prefix, 0), + prefix: NetworkPrefix::new(prefix, None), status, originated_time: time, peer_ip, @@ -139,9 +139,8 @@ impl TableDumpMessage { // encode attributes let mut attr_bytes = BytesMut::new(); for attr in &self.attributes.inner { - // add_path always false for v1 table dump // asn_len always 16 bites - attr_bytes.extend(attr.encode(false, AsnLength::Bits16)); + attr_bytes.extend(attr.encode(AsnLength::Bits16)); } bytes.put_u16(attr_bytes.len() as u16); diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index 28152e88..d7f7eec9 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -46,7 +46,7 @@ pub fn parse_rib_afi_entries( ) -> Result { let (afi, safi) = extract_afi_safi_from_rib_type(&rib_type)?; - let add_path = matches!( + let is_add_path = matches!( rib_type, TableDumpV2Type::RibIpv4UnicastAddPath | TableDumpV2Type::RibIpv4MulticastAddPath @@ -67,7 +67,7 @@ pub fn parse_rib_afi_entries( // let attr_data_slice = &input.into_inner()[(input.position() as usize)..]; for _i in 0..entry_count { - let entry = match parse_rib_entry(data, add_path, &afi, &safi, prefix) { + let entry = match parse_rib_entry(data, is_add_path, &afi, &safi, prefix) { Ok(entry) => entry, Err(e) => { warn!("early break due to error {}", e); @@ -106,7 +106,7 @@ pub fn parse_rib_afi_entries( /// ``` pub fn parse_rib_entry( input: &mut Bytes, - add_path: bool, + is_add_path: bool, afi: &Afi, safi: &Safi, prefix: NetworkPrefix, @@ -119,9 +119,12 @@ pub fn parse_rib_entry( let peer_index = input.read_u16()?; let originated_time = input.read_u32()?; - if add_path { - let _path_id = input.read_u32()?; - } + + let path_id = match is_add_path { + true => Some(input.read_u32()?), + false => None, + }; + let attribute_length = input.read_u16()? as usize; input.has_n_remaining(attribute_length)?; @@ -129,7 +132,7 @@ pub fn parse_rib_entry( let attributes = parse_attributes( attr_data_slice, &AsnLength::Bits32, - add_path, + is_add_path, Some(*afi), Some(*safi), Some(&[prefix]), @@ -138,6 +141,7 @@ pub fn parse_rib_entry( Ok(RibEntry { peer_index, originated_time, + path_id, attributes, }) } @@ -147,7 +151,7 @@ impl RibAfiEntries { let mut bytes = BytesMut::new(); bytes.put_u32(self.sequence_number); - bytes.extend(self.prefix.encode(false)); + bytes.extend(self.prefix.encode()); let entry_count = self.rib_entries.len(); bytes.put_u16(entry_count as u16); @@ -165,7 +169,7 @@ impl RibEntry { let mut bytes = BytesMut::new(); bytes.put_u16(self.peer_index); bytes.put_u32(self.originated_time); - let attr_bytes = self.attributes.encode(false, AsnLength::Bits32); + let attr_bytes = self.attributes.encode(AsnLength::Bits32); bytes.put_u16(attr_bytes.len() as u16); bytes.extend(attr_bytes); bytes.freeze() diff --git a/src/parser/rislive/mod.rs b/src/parser/rislive/mod.rs index 06a2f446..cc780148 100644 --- a/src/parser/rislive/mod.rs +++ b/src/parser/rislive/mod.rs @@ -161,7 +161,7 @@ pub fn parse_ris_live_message(msg_str: &str) -> Result, ParserRisli peer_asn: ris_msg.peer_asn, prefix: NetworkPrefix { prefix: p, - path_id: 0, + path_id: None, }, next_hop: Some(announcement.next_hop), as_path: path.clone(), @@ -192,7 +192,7 @@ pub fn parse_ris_live_message(msg_str: &str) -> Result, ParserRisli peer_asn: ris_msg.peer_asn, prefix: NetworkPrefix { prefix: p, - path_id: 0, + path_id: None, }, next_hop: None, as_path: None, diff --git a/src/parser/utils.rs b/src/parser/utils.rs index f1e4df8a..1d4d5f38 100644 --- a/src/parser/utils.rs +++ b/src/parser/utils.rs @@ -151,7 +151,11 @@ pub trait ReadUtils: Buf { afi: &Afi, add_path: bool, ) -> Result { - let path_id = if add_path { self.read_u32()? } else { 0 }; + let path_id = if add_path { + Some(self.read_u32()?) + } else { + None + }; // Length in bits let bit_len = self.read_u8()?; @@ -278,10 +282,10 @@ pub fn encode_ipaddr(addr: &IpAddr) -> Vec { } } -pub fn encode_nlri_prefixes(prefixes: &[NetworkPrefix], add_path: bool) -> Bytes { +pub fn encode_nlri_prefixes(prefixes: &[NetworkPrefix]) -> Bytes { let mut bytes = BytesMut::new(); for prefix in prefixes { - bytes.extend(prefix.encode(add_path)); + bytes.extend(prefix.encode()); } bytes.freeze() } @@ -563,14 +567,14 @@ mod tests { let mut buf = Bytes::from_static(&[0x18, 0xC0, 0xA8, 0x01]); let expected = NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 1, 0), 24).unwrap()), - 0, + None, ); assert_eq!(buf.read_nlri_prefix(&Afi::Ipv4, false).unwrap(), expected); let mut buf = Bytes::from_static(&[0x00, 0x00, 0x00, 0x01, 0x18, 0xC0, 0xA8, 0x01]); let expected = NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 1, 0), 24).unwrap()), - 1, + Some(1), ); assert_eq!(buf.read_nlri_prefix(&Afi::Ipv4, true).unwrap(), expected); } @@ -609,31 +613,31 @@ mod tests { let prefixes = vec![ NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 1, 0), 24).unwrap()), - 0, + None, ), NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 2, 0), 24).unwrap()), - 0, + None, ), ]; let expected = Bytes::from_static(&[0x18, 0xC0, 0xA8, 0x01, 0x18, 0xC0, 0xA8, 0x02]); - assert_eq!(encode_nlri_prefixes(&prefixes, false), expected); + assert_eq!(encode_nlri_prefixes(&prefixes), expected); let prefixes = vec![ NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 1, 0), 24).unwrap()), - 1, + Some(1), ), NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 2, 0), 24).unwrap()), - 1, + Some(1), ), ]; let expected = Bytes::from_static(&[ 0x00, 0x00, 0x00, 0x01, 0x18, 0xC0, 0xA8, 0x01, 0x00, 0x00, 0x00, 0x01, 0x18, 0xC0, 0xA8, 0x02, ]); - assert_eq!(encode_nlri_prefixes(&prefixes, true), expected); + assert_eq!(encode_nlri_prefixes(&prefixes), expected); } #[test] @@ -672,11 +676,11 @@ mod tests { let expected = vec![ NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 1, 0), 24).unwrap()), - 0, + None, ), NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 2, 0), 24).unwrap()), - 0, + None, ), ]; assert_eq!(parse_nlri_list(input, false, &Afi::Ipv4).unwrap(), expected); @@ -689,11 +693,11 @@ mod tests { let expected = vec![ NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 1, 0), 24).unwrap()), - 1, + Some(1), ), NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 2, 0), 24).unwrap()), - 2, + Some(2), ), ]; assert_eq!(parse_nlri_list(input, true, &Afi::Ipv4).unwrap(), expected); @@ -702,7 +706,7 @@ mod tests { let input = Bytes::from_static(&[0x00, 0x00, 0x00, 0x01, 0x18, 0xC0, 0xA8, 0x01]); let expected = vec![NetworkPrefix::new( IpNet::V4(Ipv4Net::new(Ipv4Addr::new(192, 168, 1, 0), 24).unwrap()), - 1, + Some(1), )]; assert_eq!(parse_nlri_list(input, false, &Afi::Ipv4).unwrap(), expected); } From 056c5bdffc8260b407c76392931e32c496087f45 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Sat, 26 Jul 2025 20:35:31 -0700 Subject: [PATCH 2/4] update changelog to describe this change --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae02b8f8..b72aa89b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ All notable changes to this project will be documented in this file. +## Unreleased + +### Breaking changes + +* change `path_id` from `u32` to `Option` for proper null handling + * `NetworkPrefix::path_id` field now properly represents absence with `None` instead of using `0` + * `NetworkPrefix::new()` and `NetworkPrefix::encode()` signatures updated accordingly + * `RibEntry` now stores `path_id` for TABLE_DUMP_V2 messages with AddPath support + * fixes issue [#217](https://github.com/bgpkit/bgpkit-parser/issues/217) + +### Bug fixes + +* fixed TABLE_DUMP_V2 parsing to properly store path_id when AddPath is enabled + * previously path_id was read but discarded, now properly stored in `RibEntry` + ## v0.11.1 - 2025-06-06 ### Bug fixes From ec31c793f505ff3af2b5056001f208fa4ff68a43 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Sat, 26 Jul 2025 20:45:33 -0700 Subject: [PATCH 3/4] fix: remove misleading default in path_id serialization --- src/models/network/prefix.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/models/network/prefix.rs b/src/models/network/prefix.rs index 44f5f09c..c051f3bd 100644 --- a/src/models/network/prefix.rs +++ b/src/models/network/prefix.rs @@ -113,14 +113,13 @@ mod serde_impl { where S: Serializer, { - if serializer.is_human_readable() && self.path_id.is_none() { - self.prefix.serialize(serializer) - } else { - SerdeNetworkPrefixRepr::WithPathId { + match self.path_id { + Some(path_id) => SerdeNetworkPrefixRepr::WithPathId { prefix: self.prefix, - path_id: self.path_id.unwrap_or_default(), + path_id, } - .serialize(serializer) + .serialize(serializer), + None => self.prefix.serialize(serializer), } } } From af06448b430c068f9314cbc19a7747bfabb400c2 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Sat, 26 Jul 2025 20:57:09 -0700 Subject: [PATCH 4/4] test: add coverage for missing lines in path_id changes --- src/models/network/prefix.rs | 12 ++++++++++ src/parser/mrt/messages/table_dump.rs | 24 +++++++++++++++++++ .../messages/table_dump_v2/rib_afi_entries.rs | 18 ++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/src/models/network/prefix.rs b/src/models/network/prefix.rs index c051f3bd..6d4e5818 100644 --- a/src/models/network/prefix.rs +++ b/src/models/network/prefix.rs @@ -194,6 +194,18 @@ mod tests { assert_eq!(deserialized.path_id, Some(1)); } + #[test] + #[cfg(feature = "serde")] + fn test_binary_serialization_with_path_id() { + let prefix = IpNet::from_str("192.168.0.0/24").unwrap(); + let network_prefix = NetworkPrefix::new(prefix, Some(42)); + // Test non-human readable serialization (binary-like) + let serialized = serde_json::to_vec(&network_prefix).unwrap(); + let deserialized: NetworkPrefix = serde_json::from_slice(&serialized).unwrap(); + assert_eq!(deserialized.prefix, prefix); + assert_eq!(deserialized.path_id, Some(42)); + } + #[test] fn test_debug() { let prefix = IpNet::from_str("192.168.0.0/24").unwrap(); diff --git a/src/parser/mrt/messages/table_dump.rs b/src/parser/mrt/messages/table_dump.rs index 24c6900c..4d05b3bd 100644 --- a/src/parser/mrt/messages/table_dump.rs +++ b/src/parser/mrt/messages/table_dump.rs @@ -280,4 +280,28 @@ mod tests { panic!("Expected ParseError for invalid sub_type"); } } + + #[test] + fn test_table_dump_message_encode_with_attributes() { + use crate::models::{Asn, AttributeValue, Attributes, Origin}; + use std::str::FromStr; + + let prefix = IpNet::from_str("192.168.0.0/24").unwrap(); + let mut attributes = Attributes::default(); + attributes.add_attr(AttributeValue::Origin(Origin::IGP).into()); + + let table_dump = TableDumpMessage { + view_number: 1, + sequence_number: 2, + prefix: NetworkPrefix::new(prefix, None), + status: 1, + originated_time: 12345, + peer_ip: IpAddr::V4("10.0.0.1".parse().unwrap()), + peer_asn: Asn::from(65000), + attributes, + }; + + // This should exercise the attr.encode(AsnLength::Bits16) line + let _encoded = table_dump.encode(); + } } diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index d7f7eec9..7e0285f5 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -226,4 +226,22 @@ mod tests { let res = extract_afi_safi_from_rib_type(&rib_type); assert!(res.is_err()); } + + #[test] + fn test_rib_entry_encode() { + use crate::models::{AttributeValue, Attributes, Origin}; + + let mut attributes = Attributes::default(); + attributes.add_attr(AttributeValue::Origin(Origin::IGP).into()); + + let rib_entry = RibEntry { + peer_index: 1, + originated_time: 12345, + path_id: Some(42), + attributes, + }; + + // This should exercise the self.attributes.encode(AsnLength::Bits32) line + let _encoded = rib_entry.encode(); + } }