Merge pull request #58 from NLnetLabs/release-0.5.0-rc1

Released 2024-06-10.

This release features a lot of changes, big and small. The list below is not
exhaustive but tries to highlight and describe the bigger (and perhaps, more
important) ones.


Breaking changes

* Several common basic types (e.g. `Asn` and `Prefix`) are moved out of
  _routecore_ to the new _inetnum_ crate.

* Refactor of PathAttributes

  The introduction of `PaMap` and `RouteWorkshop` (see below) required refactoring
  of the `PathAttributes` enum and related types. Most significantly, an entire
  layer in the enum was removed, changing how one matches on variants.
  Furthermore some of the types in those variants slightly changed.

* Overhaul of address family related code

  The types describing address families, and related traits to parse and compose
  NLRI of such families, have severely changed. This eliminates any possible
  ambiguity regarding the address family of announcements/withdrawals in UPDATE
  messages at compile time.

    * Supported address families are now explicitly and exhaustively enumerated
      in the `AfiSafiType` enum.  As a SAFI by itself does not carry much meaning,
      the separate `SAFI` enum is removed. Almost all of the code now works
      based on the more descriptive `AfiSafiType`.
    * ADD-PATH is now supported on all supported address families, i.e. the
      `AfiSafiType` includes `~AddPath` variants for every single AFI+SAFI
      combination.
    * This now allows for e.g. efficient iterators over NLRI that are generic
      over the `AfiSafiType`.
      But, as a possible downside, this moves the task of determining what
      address family a set of NLRI holds to the caller in order to then use the
      correctly typed iterator. The less efficient, 'easier to use' iterators
      returning an enum instead of a distinct NLRI type are therefore still
      available.

New

* RouteWorkshop / PaMap

  This release introduces the `RouteWorkshop` to create UPDATE messages based on
  an NLRI and a set of attributes. For creation, inspection and manipulation of
  those attributes, the `PaMap` is introduced. These new types work in
  conjunction with the existing `UpdateBuilder`.

* BGP FSM (absorbed from _rotonda-fsm_)

  _routecore_ now contains the code to enable for actual BGP sessions, i.e. the
  BGP FSM and related machinery. By pulling this in into _routecore_ allows for
  less dependency juggling, easier development iterations and more sensible code
  in all parts. All of this has some rough edges and is prone to changes on the
  near future.

  The _rotonda-fsm_ crate for now is left as-is.


* Route Selection fundamentals

  This release introduces a first attempt at providing handles to perform the
  BGP Decision Process as described in RFC4271, colloquially known as 'route
  selection' or 'best path selection'.

  Most of the heavy-lifting for this comes from implementing `Ord` on a wrapper
  struct holding a 'route' (i.e., `PaMap`) and additional information to allow
  tie-breaking as described in the RFC.

  As the tie-breaking in RFC4271 is actually broken and not totally ordered, we
  aim to provide a certain degree of flexibility in the tie-breaking process by
  means of different `OrdStrat` ordering strategies.


Other changes

* Feature flags

  After splitting of parts of _routecore_ into the _inetnum_ crate, the default
  features set resulted in an almost empty library. Therefore the `bgp` flag is
  now on by default, and we introduced an `fsm` flag to enable the BGP FSM code
  absorbed from _rotonda-fsm_.


Known limitations

* Constructed UPDATE messages are MultiProtocol-only

  With regards to announcing and withdrawing NLRI, the `UpdateBuilder` is currently
  limited to putting everything in the MultiProtocol path attributes
  (MP_REACH_NLRI, MP_UNREACH_NLRI), so even for IPv4 Unicast.

  Note that this behaviour is considered preferable as it leads to somewhat more
  flexibility/resilience on the protocol level. But in case one of the peers
  does not signal the capability of doing IPv4 Unicast in MultiProtocol
  attributes, we should allow creation of PDUs in the traditional form anyway,
  so we plan to reintroduce this functionality.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "routecore"
-version = "0.5.0-rc1-dev"
+version = "0.5.0-rc1"
 authors = ["NLnet Labs <routing-team@nlnetlabs.nl>"]
 categories = ["network-programming"]
 description = "A Library with Building Blocks for BGP Routing"

Changelog.md

@@ -1,17 +1,104 @@
-## 0.5.0-rc0
+## 0.5.0-rc1
 
-Released 2024-05-29.
+Released 2024-06-10.
+
+This release features a lot of changes, big and small. The list below is not
+exhaustive but tries to highlight and describe the bigger (and perhaps, more
+important) ones.
 
-TODO
 
 Breaking changes
 
+* Several common basic types (e.g. `Asn` and `Prefix`) are moved out of
+  _routecore_ to the new _inetnum_ crate.
+
+* Refactor of PathAttributes 
+
+  The introduction of `PaMap` and `RouteWorkshop` (see below) required refactoring
+  of the `PathAttributes` enum and related types. Most significantly, an entire
+  layer in the enum was removed, changing how one matches on variants.
+  Furthermore some of the types in those variants slightly changed.
+
+* Overhaul of address family related code
+
+  The types describing address families, and related traits to parse and compose
+  NLRI of such families, have severely changed. This eliminates any possible
+  ambiguity regarding the address family of announcements/withdrawals in UPDATE
+  messages at compile time. 
+
+    * Supported address families are now explicitly and exhaustively enumerated
+      in the `AfiSafiType` enum.  As a SAFI by itself does not carry much meaning,
+      the separate `SAFI` enum is removed. Almost all of the code now works
+      based on the more descriptive `AfiSafiType`.
+    * ADD-PATH is now supported on all supported address families, i.e. the
+      `AfiSafiType` includes `~AddPath` variants for every single AFI+SAFI
+      combination. 
+    * This now allows for e.g. efficient iterators over NLRI that are generic
+      over the `AfiSafiType`.
+      But, as a possible downside, this moves the task of determining what
+      address family a set of NLRI holds to the caller in order to then use the
+      correctly typed iterator. The less efficient, 'easier to use' iterators
+      returning an enum instead of a distinct NLRI type are therefore still
+      available.
+
 New
 
-Bug fixes
+* RouteWorkshop / PaMap
+
+  This release introduces the `RouteWorkshop` to create UPDATE messages based on
+  an NLRI and a set of attributes. For creation, inspection and manipulation of
+  those attributes, the `PaMap` is introduced. These new types work in
+  conjunction with the existing `UpdateBuilder`.
+
+* BGP FSM (absorbed from _rotonda-fsm_)
+
+  _routecore_ now contains the code to enable for actual BGP sessions, i.e. the
+  BGP FSM and related machinery. By pulling this in into _routecore_ allows for
+  less dependency juggling, easier development iterations and more sensible code
+  in all parts. All of this has some rough edges and is prone to changes on the
+  near future.
+
+  The _rotonda-fsm_ crate for now is left as-is.
+
+
+* Route Selection fundamentals
+
+  This release introduces a first attempt at providing handles to perform the
+  BGP Decision Process as described in RFC4271, colloquially known as 'route
+  selection' or 'best path selection'.
+
+  Most of the heavy-lifting for this comes from implementing `Ord` on a wrapper
+  struct holding a 'route' (i.e., `PaMap`) and additional information to allow
+  tie-breaking as described in the RFC.
+
+  As the tie-breaking in RFC4271 is actually broken and not totally ordered, we
+  aim to provide a certain degree of flexibility in the tie-breaking process by
+  means of different `OrdStrat` ordering strategies.
+
 
 Other changes
 
+* Feature flags
+
+  After splitting of parts of _routecore_ into the _inetnum_ crate, the default
+  features set resulted in an almost empty library. Therefore the `bgp` flag is
+  now on by default, and we introduced an `fsm` flag to enable the BGP FSM code
+  absorbed from _rotonda-fsm_.
+
+
+Known limitations
+
+* Constructed UPDATE messages are MultiProtocol-only
+
+  With regards to announcing and withdrawing NLRI, the `UpdateBuilder` is currently
+  limited to putting everything in the MultiProtocol path attributes
+  (MP_REACH_NLRI, MP_UNREACH_NLRI), so even for IPv4 Unicast.
+
+  Note that this behaviour is considered preferable as it leads to somewhat more
+  flexibility/resilience on the protocol level. But in case one of the peers
+  does not signal the capability of doing IPv4 Unicast in MultiProtocol
+  attributes, we should allow creation of PDUs in the traditional form anyway,
+  so we plan to reintroduce this functionality.
 
 
 ## 0.4.0

fuzz/fuzz_targets/path_selection.rs

@@ -1,8 +1,14 @@
 #![no_main]
 
 use std::cmp::Ordering::*;
+use std::fmt::Debug;
 use libfuzzer_sys::fuzz_target;
-use routecore::bgp::{path_attributes::PaMap, path_selection::{OrdRoute, Rfc4271, SkipMed, TiebreakerInfo}};
+use routecore::bgp::{
+    path_attributes::PaMap,
+    path_selection::{OrdStrat, OrdRoute, Rfc4271, SkipMed, TiebreakerInfo,
+        best_backup, best_backup_generic, best
+    }
+};
 
 fn verify_ord<T>(a: &T, b: &T, c: &T)
     where T: Ord
@@ -36,6 +42,71 @@ fn verify_ord<T>(a: &T, b: &T, c: &T)
     }
 }
 
+fn verify_path_selection<'a, T, OS, I>(candidates: I)
+    where
+        I: Clone + Iterator<Item = T>,
+        T: Debug + Ord + core::borrow::Borrow<OrdRoute<'a, OS>>,
+        OS: Debug + OrdStrat,
+{
+    let best1 = best(candidates.clone()).unwrap();
+    let (best2, backup2) = best_backup(candidates.clone());
+    let best2 = best2.unwrap();
+
+    assert_eq!(best1.borrow(), best2.borrow());
+    assert_eq!(best1.borrow().pa_map(), best2.borrow().pa_map());
+    assert_eq!(best1.borrow().tiebreakers(), best2.borrow().tiebreakers());
+
+    let backup2 = match backup2 {
+        Some(route) => route,
+        None => {
+            let mut iter = candidates.clone();
+            let mut cur  = iter.next().unwrap();
+            for c in iter {
+                assert_eq!(cur.borrow().inner(), c.borrow().inner());
+                cur = c;
+            }
+            return;
+        }
+    };
+
+    assert_ne!(
+        (best2.borrow().tiebreakers(), best2.borrow().pa_map()),
+        (backup2.borrow().tiebreakers(), backup2.borrow().pa_map()),
+    );
+
+    let (best_gen, backup_gen) = verify_path_selection_generic(candidates);
+    let (best_gen, _backup_gen) = (best_gen.unwrap(), backup_gen.unwrap());
+
+    assert_eq!(best2, best_gen);
+
+    // Because best_backup_generic can't deal with duplicates, this won't
+    // always hold:
+    //assert_eq!(backup2, backup_gen);
+
+}
+
+fn verify_path_selection_generic<I, T>(candidates: I) -> (Option<T>, Option<T>)
+where
+    I: Iterator<Item = T>,
+    T: Debug + Ord
+{
+
+    let (best, backup) = best_backup_generic(
+        // create tuples of the OrdRoute and a unique 'id'
+        candidates.enumerate().map(|(idx, c)| (c, idx))
+    );
+    // because the returned values are tuples as well, we can now compare
+    // those (instead of comparing the OrdRoute). With the unique IDs
+    // attached, we can check whether `best` and `backup` are not equal in
+    // terms of content.
+    assert!(best.is_some());
+    assert!(backup.is_some());
+    assert_ne!(best, backup);
+
+
+    (best.map(|t| t.0), backup.map(|t| t.0))
+}
+
 // XXX while doing fuzz_target!(|data: &[u8]| ... and then creating an
 // Unstructured from `data` can be useful, the Debug output becomes quite
 // useless. It'll be just a bunch of bytes without any structure.
@@ -65,8 +136,9 @@ fuzz_target!(|data: (
         Err(_) => return,
     };
 
-    //dbg!("skipmed");
     verify_ord(&a, &b, &c);
+    verify_path_selection([&a, &b, &c, &b, &c, &a].into_iter());
+    //verify_path_selection_generic([&a, &b, &c].into_iter());
 
     //dbg!("rfc4271");
     /*