From 990deab6996ff35732fd6d9739069f89c828ae72 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Tue, 1 Feb 2022 03:03:45 +0000 Subject: [PATCH] clean up BgpCodec Signed-off-by: FUJITA Tomonori --- daemon/src/event.rs | 21 ++++--- daemon/src/packet/bgp.rs | 117 +++++++++++++++++++++++---------------- daemon/src/packet/bmp.rs | 15 +++-- daemon/src/packet/mrt.rs | 7 ++- daemon/src/table.rs | 7 ++- 5 files changed, 103 insertions(+), 64 deletions(-) diff --git a/daemon/src/event.rs b/daemon/src/event.rs index b45f697..f4acf1f 100644 --- a/daemon/src/event.rs +++ b/daemon/src/event.rs @@ -3172,7 +3172,7 @@ impl Handler { async fn rx_msg( &mut self, - codec: &mut packet::bgp::BgpCodec, + codec: &mut packet::bgp::Codec, local_sockaddr: SocketAddr, remote_sockaddr: SocketAddr, msg: bgp::Message, @@ -3379,12 +3379,19 @@ impl Handler { txbuf_size = std::cmp::min(txbuf_size, r / 2); } - let mut codec = packet::bgp::BgpCodec::new() - .local_as(self.local_asn) - .local_addr(self.local_addr); - if self.rs_client { - codec = codec.keep_aspath(true).keep_nexthop(true); - } + let mut codec = if self.rs_client { + bgp::CodecBuilder::new() + .local_asn(self.local_asn) + .local_addr(self.local_addr) + .keep_aspath(true) + .keep_nexthop(true) + .build() + } else { + bgp::CodecBuilder::new() + .local_asn(self.local_asn) + .local_addr(self.local_addr) + .build() + }; let mut peer_event_rx = Vec::new(); for _ in 0..*NUM_TABLES { diff --git a/daemon/src/packet/bgp.rs b/daemon/src/packet/bgp.rs index a619198..d1e2b2d 100644 --- a/daemon/src/packet/bgp.rs +++ b/daemon/src/packet/bgp.rs @@ -607,7 +607,7 @@ impl Capability { struct CapDesc { code: u8, url: &'static str, - decode: fn(s: &mut BgpCodec, c: &mut dyn io::Read, len: u8) -> Result, + decode: fn(s: &mut Codec, c: &mut dyn io::Read, len: u8) -> Result, } static CAP_DESCS: Lazy> = Lazy::new(|| { @@ -681,7 +681,7 @@ static CAP_DESCS: Lazy> = Lazy::new(|| { return Err(()); } let remote_as = c.read_u32::().unwrap(); - s.remote_as = remote_as; + s.remote_asn = remote_as; Ok(Capability::FourOctetAsNumber(remote_as)) }), }, @@ -994,14 +994,14 @@ impl Attribute { code: u8, dst: Option<&mut BytesMut>, family: Family, - codec: &BgpCodec, + codec: &Codec, ) -> (u16, Option) { match code { Attribute::AS_PATH => { let n = if codec.keep_aspath { self.clone() } else { - self.as_path_prepend(codec.local_as) + self.as_path_prepend(codec.local_asn) }; let l = if let Some(dst) = dst { n.encode(dst).unwrap() @@ -1598,50 +1598,85 @@ pub(crate) fn create_channel( }) } -pub(crate) struct BgpCodec { - extended_length: bool, - local_as: u32, - remote_as: u32, +pub(crate) struct CodecBuilder { + local_asn: u32, + remote_asn: u32, local_addr: IpAddr, + extended_length: bool, keep_aspath: bool, keep_nexthop: bool, - pub(crate) channel: FnvHashMap, + family: Vec, } -impl BgpCodec { +impl CodecBuilder { pub(crate) fn new() -> Self { - BgpCodec { + CodecBuilder { + local_asn: 0, + remote_asn: 0, + local_addr: IpAddr::V4(Ipv4Addr::from(0)), extended_length: false, - local_as: 0, - remote_as: 0, - local_addr: IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), keep_aspath: false, keep_nexthop: false, - channel: FnvHashMap::default(), + family: Vec::new(), } } - pub(crate) fn local_as(mut self, local_as: u32) -> Self { - self.local_as = local_as; + pub(crate) fn build(&mut self) -> Codec { + let channel = self + .family + .iter() + .map(|f| (*f, Channel::new(*f, false, false))) + .collect(); + Codec { + local_asn: self.local_asn, + remote_asn: self.remote_asn, + local_addr: self.local_addr, + extended_length: self.extended_length, + keep_aspath: self.keep_aspath, + keep_nexthop: self.keep_nexthop, + channel, + } + } + + pub(crate) fn local_asn(&mut self, asn: u32) -> &mut Self { + self.local_asn = asn; self } - pub(crate) fn local_addr(mut self, local_addr: IpAddr) -> Self { + pub(crate) fn local_addr(&mut self, local_addr: IpAddr) -> &mut Self { self.local_addr = local_addr; self } - pub(crate) fn keep_aspath(mut self, y: bool) -> Self { + pub(crate) fn keep_aspath(&mut self, y: bool) -> &mut Self { self.keep_aspath = y; self } - pub(crate) fn keep_nexthop(mut self, y: bool) -> Self { + pub(crate) fn keep_nexthop(&mut self, y: bool) -> &mut Self { self.keep_nexthop = y; self } - pub(crate) fn max_message_length(&self) -> usize { + #[cfg(test)] + fn families(&mut self, v: Vec) -> &mut Self { + self.family = v; + self + } +} + +pub(crate) struct Codec { + extended_length: bool, + local_asn: u32, + remote_asn: u32, + local_addr: IpAddr, + keep_aspath: bool, + keep_nexthop: bool, + pub(crate) channel: FnvHashMap, +} + +impl Codec { + fn max_message_length(&self) -> usize { if self.extended_length { Message::MAX_EXTENDED_LENGTH } else { @@ -1650,7 +1685,7 @@ impl BgpCodec { } fn is_ibgp(&self) -> bool { - self.local_as == self.remote_as + self.local_asn == self.remote_asn } fn mp_reach_encode( @@ -1960,7 +1995,7 @@ impl BgpCodec { } } -impl Encoder<&Message> for BgpCodec { +impl Encoder<&Message> for Codec { type Error = Error; fn encode(&mut self, item: &Message, dst: &mut BytesMut) -> Result<(), Error> { @@ -1988,7 +2023,7 @@ impl Encoder<&Message> for BgpCodec { } } -impl Decoder for BgpCodec { +impl Decoder for Codec { type Item = Message; type Error = Error; @@ -2096,9 +2131,9 @@ impl Decoder for BgpCodec { } } if as_number == Capability::TRANS_ASN as u32 { - as_number = self.remote_as; + as_number = self.remote_asn; } else { - self.remote_as = as_number; + self.remote_asn = as_number; } Ok(Some(Message::Open { @@ -2254,7 +2289,7 @@ impl Decoder for BgpCodec { if !error_withdraw { match attr[*seen.get(&Attribute::AS_PATH).unwrap()] - .as_path_count(self.local_as) + .as_path_count(self.local_asn) { Ok(v) => { if v > 0 { @@ -2457,10 +2492,7 @@ fn ipv6_eor() { ]; buf.append(&mut body); let mut b = BytesMut::from(&buf[..]); - let mut codec = BgpCodec::new(); - codec - .channel - .insert(Family::IPV6, Channel::new(Family::IPV6, false, false)); + let mut codec = CodecBuilder::new().families(vec![Family::IPV6]).build(); let ret = codec.decode(&mut b); assert!(ret.is_ok()); } @@ -2495,10 +2527,7 @@ fn parse_ipv6_update() { .map(|n| (n, 0)) .collect(); let mut b = BytesMut::from(&buf[..]); - let mut codec = BgpCodec::new(); - codec - .channel - .insert(Family::IPV6, Channel::new(Family::IPV6, false, false)); + let mut codec = CodecBuilder::new().families(vec![Family::IPV6]).build(); let msg = codec.decode(&mut b).unwrap(); match msg.unwrap() { Message::Update { @@ -2543,10 +2572,10 @@ fn build_many_v4_route() { set.insert((*n, 0)); } - let mut codec = BgpCodec::new().keep_aspath(true); - codec - .channel - .insert(Family::IPV4, Channel::new(Family::IPV4, false, false)); + let mut codec = CodecBuilder::new() + .families(vec![Family::IPV4]) + .keep_aspath(true) + .build(); let mut txbuf = bytes::BytesMut::with_capacity(4096); codec.encode(&msg, &mut txbuf).unwrap(); let mut recv = Vec::new(); @@ -2634,10 +2663,7 @@ fn many_mp_reach() { unreach: None, }; - let mut codec = BgpCodec::new().keep_aspath(true); - codec - .channel - .insert(Family::IPV6, Channel::new(Family::IPV6, false, false)); + let mut codec = CodecBuilder::new().families(vec![Family::IPV6]).build(); let mut txbuf = bytes::BytesMut::with_capacity(4096); codec.encode(&msg, &mut txbuf).unwrap(); let mut recv = Vec::new(); @@ -2687,10 +2713,7 @@ fn many_mp_unreach() { unreach: Some((Family::IPV6, unreach)), }; - let mut codec = BgpCodec::new().keep_aspath(true); - codec - .channel - .insert(Family::IPV6, Channel::new(Family::IPV6, false, false)); + let mut codec = CodecBuilder::new().families(vec![Family::IPV6]).build(); let mut txbuf = bytes::BytesMut::with_capacity(4096); codec.encode(&msg, &mut txbuf).unwrap(); let mut recv = Vec::new(); diff --git a/daemon/src/packet/bmp.rs b/daemon/src/packet/bmp.rs index 22111ee..5907de7 100644 --- a/daemon/src/packet/bmp.rs +++ b/daemon/src/packet/bmp.rs @@ -101,10 +101,11 @@ impl PeerDownReason { fn encode(&self, c: &mut BytesMut) -> Result<(), Error> { c.put_u8(self.code()); c.put_slice(&[0; 3]); + let mut codec = bgp::CodecBuilder::new().build(); match self { Self::LocalNotification(notification) => { let mut buf = bytes::BytesMut::with_capacity(4096); - bgp::BgpCodec::new().encode(notification, &mut buf).unwrap(); + codec.encode(notification, &mut buf).unwrap(); c.put_slice(buf.as_ref()); } Self::LocalFsm(code) => { @@ -112,7 +113,7 @@ impl PeerDownReason { } Self::RemoteNotification(notification) => { let mut buf = bytes::BytesMut::with_capacity(4096); - bgp::BgpCodec::new().encode(notification, &mut buf).unwrap(); + codec.encode(notification, &mut buf).unwrap(); c.put_slice(buf.as_ref()); } _ => {} @@ -176,13 +177,17 @@ impl Message { } pub(crate) struct BmpCodec { - codec: bgp::BgpCodec, + codec: bgp::Codec, } impl BmpCodec { pub(crate) fn new() -> Self { - let codec = bgp::BgpCodec::new().keep_aspath(true).keep_nexthop(true); - BmpCodec { codec } + BmpCodec { + codec: bgp::CodecBuilder::new() + .keep_aspath(true) + .keep_nexthop(true) + .build(), + } } } diff --git a/daemon/src/packet/mrt.rs b/daemon/src/packet/mrt.rs index dc4c79a..b384bba 100644 --- a/daemon/src/packet/mrt.rs +++ b/daemon/src/packet/mrt.rs @@ -109,13 +109,16 @@ pub(crate) enum Message { } pub(crate) struct MrtCodec { - codec: bgp::BgpCodec, + codec: bgp::Codec, } impl MrtCodec { pub(crate) fn new() -> Self { MrtCodec { - codec: bgp::BgpCodec::new().keep_aspath(true).keep_nexthop(true), + codec: bgp::CodecBuilder::new() + .keep_aspath(true) + .keep_nexthop(true) + .build(), } } } diff --git a/daemon/src/table.rs b/daemon/src/table.rs index b82ac95..b502e0c 100644 --- a/daemon/src/table.rs +++ b/daemon/src/table.rs @@ -352,11 +352,12 @@ impl RoutingTable { }) .filter_map(|(_, p)| { if table_type == api::TableType::AdjOut { - let codec = bgp::BgpCodec::new() - .local_as(p.source.local_asn) + let codec = bgp::CodecBuilder::new() + .local_asn(p.source.local_asn) .local_addr(p.source.local_addr) .keep_aspath(p.source.rs_client) - .keep_nexthop(p.source.rs_client); + .keep_nexthop(p.source.rs_client) + .build(); let attr = Arc::new( (*p).pa .attr