Adopt new AfiSafi / Nlri types throughout codebase

This commit adapts the codebase to use the types and traits introduced
in this branch. With that, several other concepts required changes, or
became deprecated and were removed.

The new types/traits include:
- trait AfiSafi, describing something that is characterized by a certain
  AFI+SAFI combination;
- trait AfiSafiNlri, an NLRI characterized by a certain AFI+SAFI
  combination;
- enum AfiSafiType, describing all AFI+SAFI combinations we support;
- enum Nlri<Octs>, now holding more explicit variants of dedicated
  types, a la Ipv4UnicastNlri, Ipv6UnicastNlri, etc.  For ADD-PATH,
  dedicated types are generated: Ipv4UnicastAddpathNlri, etc;
- On UpdateMessage, `fn typed_announcements()` provides an iterator
  generic over tpyes implementing AfiSafiParse, thus yielding NLRI of
  these dedicated types instead of enum variants (which is still
  available via the `fn announcements());
- UpdateBuilder is now generic over types that implement NlriCompose,
  making the adding/removing of announcements and withdrawals more
  straightforward. New generic helper methods include `fn
  add_announcements_from_pdu`.

Other changes:
- enum Safi is removed, as an SAFI by itself does not mean anything
  useful. We keep enum Afi and the new enum AfiSafiType. (The name
  `AfiSafi` is now used for the new trait);
- the UpdateMessage does not hold an entire SessionConfig anymore, but a
  trimmed down version of the new type PduParseInfo. The SessionConfig
  is only used in the very first call to `from_octets()` or `parse()`;
- MpReachNlri and MpUnreachNlri are not part of the PathAttribute enum
  anymore, we start treating them differently. Partly because with the
  new builders being generic, it would mean the enum PathAttribute must
  carry generic type info, and PaMap as well. All while we most often do
  not include the MP* attributes in those maps anyway;
- the new bgp::nlri modules replaces everyhing that was in
  bgp::message::nlri

Open issues / questions and other remarks:
- on UpdateMessage, we had several methods to specifically get unicast
  announcements. We need to figure out if those are still wanted;
- we need to figure out what the Workshop should look like. We can
  probably get rid of the afisafi_nlri module to a large extent;
- the UpdateBuilder now puts all withdrawals and announcements in MP
  attributes, so nothing goes into the conventional sections in the PDU.
  Related tests are ignored for now. We need to figure out to what
  extent we like to support those conventional sections in the builder.

fuzz/fuzz_targets/parse_update_message.rs

@@ -5,23 +5,18 @@ use routecore::bgp::message::{UpdateMessage, SessionConfig};
 use routecore::bgp::message::update_builder::UpdateBuilder;
 
 fuzz_target!(|data: (&[u8], SessionConfig)| {
-    if let Ok(upd) =  UpdateMessage::from_octets(data.0, data.1) {
+    if let Ok(upd) =  UpdateMessage::from_octets(data.0, &data.1) {
         if let Ok(pas) = upd.path_attributes() {
             for pa in pas.into_iter() {
-                if let Ok(pa) = pa {
-                    let _ = pa.to_owned();
-                }
+                let _ = pa.unwrap().to_owned();
             }
         }
-        /*
-        if let Ok(builder) = UpdateBuilder::from_update_message(
-            &upd,
-            data.1,
-            target,
-        ) {
-            let _ = builder.into_message(); //.unwrap();
+        if let Ok(iter) =  upd.announcements() {
+            iter.count();
+        }
+        if let Ok(iter) =  upd.withdrawals() {
+            iter.count();
         }
-        */
     }
 });
 

src/bgp/message/mod.rs

@@ -1,11 +1,9 @@
 pub mod open;
 pub mod update;
 pub mod update_builder;
-pub mod nlri;
 pub mod notification;
 pub mod keepalive;
 pub mod routerefresh;
-//pub mod attr_change_set;
 
 use octseq::{Octets, Parser};
 use crate::util::parser::ParseError;
@@ -21,7 +19,7 @@ use log::debug;
 use serde::{Serialize, Deserialize};
 
 pub use open::OpenMessage;
-pub use update::{UpdateMessage, SessionConfig};
+pub use update::{PduParseInfo, SessionConfig, UpdateMessage};
 pub use notification::NotificationMessage;
 pub use keepalive::KeepaliveMessage;
 pub use routerefresh::RouteRefreshMessage;
@@ -130,7 +128,7 @@ impl<Octs: Octets> Message<Octs> {
                     return Err(ParseError::StateRequired)
                 };
                 Ok(Message::Update(
-                        UpdateMessage::from_octets(octets, config)?
+                    UpdateMessage::from_octets(octets, &config)?
                 ))
             },
             MsgType::Notification =>

src/bgp/message/nlri.rs

@@ -3,7 +3,7 @@ use crate::bgp::types::{Afi, AfiSafi, NextHop};
 use crate::addr::Prefix;
 
 use crate::util::parser::{parse_ipv4addr, parse_ipv6addr, ParseError};
-use crate::bgp::message::update::SessionConfig;
+use crate::bgp::message::update::PduParseInfo;
 use crate::flowspec::Component;
 use crate::typeenum;
 use octseq::{Octets, OctetsBuilder, OctetsFrom, Parser};
@@ -1618,6 +1618,7 @@ pub(crate) fn parse_prefix<R: Octets>(parser: &mut Parser<'_, R>, afi: Afi)
     parse_prefix_for_len(parser, prefix_bits, afi)
 }
 
+/*
 impl BasicNlri {
     pub fn check<Octs: Octets>(
         parser: &mut Parser<Octs>,
@@ -1736,12 +1737,13 @@ impl From<(Prefix, Option<PathId>)> for BasicNlri {
         BasicNlri { prefix: tuple.0, path_id: tuple.1 }
     }
 }
+*/
 
 
 impl<Octs: Octets> MplsVpnNlri<Octs> {
     pub fn check(
         parser: &mut Parser<Octs>,
-        config: SessionConfig,
+        config: PduParseInfo,
         afisafi: AfiSafi,
         ) -> Result<(), ParseError>
     {
@@ -1774,7 +1776,7 @@ impl<Octs: Octets> MplsVpnNlri<Octs> {
 impl<Octs: Octets> MplsVpnNlri<Octs> {
     pub fn parse<'a, R>(
         parser: &mut Parser<'a, R>,
-        config: SessionConfig,
+        config: PduParseInfo,
         afisafi: AfiSafi,
     ) -> Result<Self, ParseError>
     where
@@ -1916,7 +1918,7 @@ impl<T: AsRef<[u8]>> MplsVpnNlri<T> {
 impl<Octs: Octets> MplsNlri<Octs> {
     pub fn check(
         parser: &mut Parser<Octs>,
-        config: SessionConfig,
+        config: PduParseInfo,
         afisafi: AfiSafi,
     ) -> Result<(), ParseError> {
         if config.rx_addpath(afisafi) {
@@ -1960,7 +1962,7 @@ impl<T> fmt::Display for MplsNlri<T> {
 impl<Octs: Octets> MplsNlri<Octs> {
     pub fn parse<'a, R>(
         parser: &mut Parser<'a, R>,
-        config: SessionConfig,
+        config: PduParseInfo,
         afisafi: AfiSafi,
     ) -> Result<Self, ParseError>
     where

src/bgp/message/open.rs

@@ -1,6 +1,6 @@
 use crate::bgp::message::{Header, MsgType};
 use crate::asn::Asn;
-use crate::bgp::types::{Afi, Safi, AfiSafi, AddpathFamDir, AddpathDirection};
+use crate::bgp::types::{AfiSafi, AddpathFamDir, AddpathDirection};
 use crate::typeenum; // from util::macros
 use crate::util::parser::ParseError;
 use log::warn;
@@ -173,13 +173,6 @@ impl<Octs: Octets> OpenMessage<Octs> {
         )
     }
 
-    // FIXME this should return a AFI/SAFI combination, not a bool
-    pub fn add_path_capable(&self) -> bool {
-        self.capabilities().any(|c|
-            c.typ() == CapabilityType::AddPath
-        )
-    }
-
     /*
     pub fn addpath_families(&self) -> impl Iterator<Item = (AfiSafi, AddpathDirection)> + '_ {
         self.capabilities().filter(|c|
@@ -208,15 +201,11 @@ impl<Octs: Octets> OpenMessage<Octs> {
             for c in c.value().chunks(4) {
                 let mut parser = Parser::from_ref(c);
 
-                let afi = parser.parse_u16_be()?.into();
-                let safi = parser.parse_u8()?.into();
+                let afi = parser.parse_u16_be()?;
+                let safi = parser.parse_u8()?;
                 let dir = AddpathDirection::try_from(parser.parse_u8()?)
                     .map_err(|_| ParseError::Unsupported)?;
-                res.push((
-                    AfiSafi::try_from((afi, safi))
-                        .map_err(|_| ParseError::Unsupported)?,
-                    dir
-                ));
+                res.push((AfiSafi::from((afi, safi)), dir));
             }
         }
         Ok(res)
@@ -249,9 +238,9 @@ impl<Octs: Octets> OpenMessage<Octs> {
         }).collect::<Vec<_>>()
     }
 
-    /// Returns an iterator over `(Afi, Safi)` tuples listed as
-    /// MultiProtocol Capabilities in the Optional Parameters of this message.
-    pub fn multiprotocol_ids(&self) -> impl Iterator<Item = (Afi,Safi)> + '_ {
+    /// Returns an iterator over `AfiSafi`s listed as MultiProtocol
+    /// Capabilities in the Optional Parameters of this message.
+    pub fn multiprotocol_ids(&self) -> impl Iterator<Item = AfiSafi> + '_ {
         self.capabilities().filter(|c|
             c.typ() == CapabilityType::MultiProtocol
         ).map(|mp_cap| {
@@ -260,7 +249,7 @@ impl<Octs: Octets> OpenMessage<Octs> {
                 mp_cap.value()[1]
             ]);
             let safi = mp_cap.value()[3];
-            (afi.into(), safi.into())
+            (afi, safi).into()
         })
     }
 
@@ -830,15 +819,17 @@ impl<Target: OctetsBuilder + AsMut<[u8]>> OpenBuilder<Target> {
         self.add_capability(Capability::for_slice(s.to_vec()));
     }
 
-    pub fn add_mp(&mut self, afi: Afi, safi: Safi) {
+    pub fn add_mp(&mut self, afisafi: AfiSafi) {
         // code 1
         // length n
         // 2 bytes AFI, rsrvd byte, 1 byte SAFI
-        let mut s = [0x01, 0x04, 0x00, 0x00, 0x00, safi.into()];
-        let a = <Afi as Into<u16>>::into(afi).to_be_bytes();
-        s[2] = a[0];
-        s[3] = a[1];
-        self.add_capability(Capability::<Vec<u8>>::for_slice(s.to_vec()))
+
+        let (afi, safi) = afisafi.into();
+        let mut s = vec![0x01, 0x04];
+        s.extend_from_slice(&afi.to_be_bytes()[..]);
+        s.extend_from_slice(&[0x00, safi]);
+
+        self.add_capability(Capability::<Vec<u8>>::for_slice(s));
     }
 
     pub fn add_addpath(&mut self, afisafi: AfiSafi, dir: AddpathDirection) {
@@ -863,10 +854,9 @@ where Infallible: From<<Target as OctetsBuilder>::AppendError>
                 ]
             );
 
-            for (fam, dir) in self.addpath_families.iter() {
-                let (afi, safi) = fam.split();
-                addpath_cap.extend_from_slice(&u16::from(afi).to_be_bytes());
-                addpath_cap.extend_from_slice(&[safi.into(), *dir as u8]);
+            for (afisafi, dir) in self.addpath_families.iter() {
+                addpath_cap.extend_from_slice(&afisafi.as_bytes());
+                addpath_cap.extend_from_slice(&[u8::from(*dir)]);
             }
             self.add_capability(Capability::new(addpath_cap));
         }
@@ -915,9 +905,11 @@ impl OpenBuilder<Vec<u8>> {
 mod tests {
 
     use super::*;
-    use crate::bgp::message::Message;
+
     use bytes::Bytes;
 
+    use crate::bgp::message::Message;
+
     #[test]
     fn no_optional_parameters() {
         // BGP OPEN message, 2-octet ASN 64496, no opt params
@@ -1053,21 +1045,23 @@ mod tests {
 
         assert_eq!(open.multiprotocol_ids().count(), 15);
         let protocols = [
-            (Afi::Ipv4, Safi::Unicast),
-            (Afi::Ipv4, Safi::Multicast),
-            (Afi::Ipv4, Safi::MplsUnicast),
-            (Afi::Ipv4, Safi::MplsVpnUnicast),
-            (Afi::Ipv4, Safi::RouteTarget),
-            (Afi::Ipv4, Safi::FlowSpec),
-            (Afi::Ipv4, Safi::FlowSpecVpn),
-            (Afi::Ipv6, Safi::Unicast),
-            (Afi::Ipv6, Safi::Multicast),
-            (Afi::Ipv6, Safi::MplsUnicast),
-            (Afi::Ipv6, Safi::MplsVpnUnicast),
-            (Afi::Ipv6, Safi::FlowSpec),
-            (Afi::Ipv6, Safi::FlowSpecVpn),
-            (Afi::L2Vpn, Safi::Vpls),
-            (Afi::L2Vpn, Safi::Evpn),
+            AfiSafi::Ipv4Unicast,
+            AfiSafi::Ipv4Multicast,
+            AfiSafi::Ipv4MplsUnicast,
+            AfiSafi::Ipv4MplsVpnUnicast,
+            AfiSafi::Ipv4RouteTarget,
+            AfiSafi::Ipv4FlowSpec,
+            //AfiSafi::Ipv4FlowSpecVpn,
+            AfiSafi::Unsupported(1, 134),
+            AfiSafi::Ipv6Unicast,
+            AfiSafi::Ipv6Multicast,
+            AfiSafi::Ipv6MplsUnicast,
+            AfiSafi::Ipv6MplsVpnUnicast,
+            AfiSafi::Ipv6FlowSpec,
+            //AfiSafi::Ipv6FlowSpecVpn,
+            AfiSafi::Unsupported(2, 134),
+            AfiSafi::L2VpnVpls,
+            AfiSafi::L2VpnEvpn,
         ];
 
         for (id, protocol) in open.multiprotocol_ids().zip(
@@ -1137,8 +1131,8 @@ mod builder {
         open.set_holdtime(180);
         open.set_bgp_id([1, 2, 3, 4]);
 
-        open.add_mp(Afi::Ipv4, Safi::Unicast);
-        open.add_mp(Afi::Ipv6, Safi::Unicast);
+        open.add_mp(AfiSafi::Ipv4Unicast);
+        open.add_mp(AfiSafi::Ipv6Unicast);
 
         let res = open.into_message();
 
@@ -1152,8 +1146,8 @@ mod builder {
         open.set_holdtime(180);
         open.set_bgp_id([1, 2, 3, 4]);
 
-        open.add_mp(Afi::Ipv4, Safi::Unicast);
-        open.add_mp(Afi::Ipv6, Safi::Unicast);
+        open.add_mp(AfiSafi::Ipv4Unicast);
+        open.add_mp(AfiSafi::Ipv6Unicast);
 
         let res = open.into_message();
 
@@ -1167,8 +1161,8 @@ mod builder {
         open.set_holdtime(180);
         open.set_bgp_id([1, 2, 3, 4]);
 
-        open.add_mp(Afi::Ipv4, Safi::Unicast);
-        open.add_mp(Afi::Ipv6, Safi::Unicast);
+        open.add_mp(AfiSafi::Ipv4Unicast);
+        open.add_mp(AfiSafi::Ipv6Unicast);
         open.add_addpath(AfiSafi::Ipv4Unicast, AddpathDirection::SendReceive);
         open.add_addpath(AfiSafi::Ipv6Unicast, AddpathDirection::SendReceive);
 

src/bgp/message/routerefresh.rs

@@ -1,15 +1,14 @@
 use octseq::{Octets, Parser};
 
 use crate::bgp::message::Header;
-use crate::bgp::types::{Afi, Safi, RouteRefreshSubtype};
+use crate::bgp::types::{Afi, AfiSafi, RouteRefreshSubtype};
 use crate::util::parser::ParseError;
 
 /// BGP RouteRefresh message, variant of the [`Message`] enum.
 #[derive(Clone, Debug)]
 pub struct RouteRefreshMessage<Octets> {
     octets: Octets,
-    afi: Afi,
-    safi: Safi,
+    afisafi: AfiSafi,
     subtype: RouteRefreshSubtype,
 }
 
@@ -24,11 +23,12 @@ impl<Octs: Octets> RouteRefreshMessage<Octs> {
                 ));
             }
         }
-        let afi = parser.parse_u16_be()?.into();
+        let afi = parser.parse_u16_be()?;
         let subtype = parser.parse_u8()?.into();
-        let safi = parser.parse_u8()?.into();
+        let safi = parser.parse_u8()?;
+        let afisafi = (afi, safi).into();
 
-        Ok(RouteRefreshMessage { octets, afi, safi, subtype })
+        Ok(RouteRefreshMessage { octets, afisafi, subtype })
     }
 
     pub fn octets(&self) -> &Octs {
@@ -39,12 +39,12 @@ impl<Octs: Octets> RouteRefreshMessage<Octs> {
 impl<Octs> RouteRefreshMessage<Octs> {
     /// Returns the `Afi` for this Route Refresh message.
     pub fn afi(&self) -> Afi {
-        self.afi
+        self.afisafi.afi()
     }
 
-    /// Returns the `Safi` for this Route Refresh message.
-    pub fn safi(&self) -> Safi {
-        self.safi
+    /// Returns the `AfiSafi` for this Route Refresh message.
+    pub fn afisafi(&self) -> AfiSafi {
+        self.afisafi
     }
 
     /// Returns the `RouteRefreshSubtype` for this Route Refresh message.
@@ -86,8 +86,7 @@ mod tests {
         ];
 
         let rr = RouteRefreshMessage::from_octets(&raw).unwrap();
-        assert_eq!(rr.afi(), Afi::Ipv4);
-        assert_eq!(rr.safi(), Safi::Unicast);
+        assert_eq!(rr.afisafi(), AfiSafi::Ipv4Unicast);
         assert_eq!(rr.subtype(), RouteRefreshSubtype::Begin);
 
     }