Add basic BIER support #10

nrybowski · 2024-04-05T15:21:29Z

This PR adds a partial implementation of a modified version of draft-ietf-bier-bier-yang-08. The differences with the original are the following:

sub-domain-id updated from u16 to u8 per RFC8279.
mt_id updated from u16 to u8 per draft-ietf-ospf-mt-ospfv3-03.
address_family updated from custom format to iana-routing-types:address_family.
bsl enum in draft-08 incorrectly copies the underlay-protocol-type enum, bsl enum from draft-07 is reused.
fixed some typos in the YANG model.
max-si updated from uint16 to uint8 per draft-ietf-bier-ospfv3-extensions.

TODO

Add iBus messages for BierCfgEncapUpdate event
Move draft-ietf-bier-bier-yang-08 modifications in YANG augmentations
Enforce BIER sub-domain configuration constraints in validation callback
Enforce BIER sub-domain encapsulation configuration constraints in validation callback

rwestphal · 2024-04-05T20:02:30Z

@nrybowski Really interesting stuff!

I must say I'm not familiar with BIER and multicasting in general, but I'll follow this closely and learn more when I get the chance.

Could you provide a brief summary of the scope of this work? It seems like you're laying the groundwork containing BIER data structures and configuration options, which could eventually lead to implementing BIER IGP extensions like RFC 8444, right?

Regarding your changes done in the BIER YANG module, I'd suggest keeping the draft module intact, and do all changes in the form of deviations. Reporting the problems you found to the module authors would be nice as well, so that they can be fixed in the next draft version.

Regarding the code changes, it looks like you had some fun adding and implementing the BIER northbound callbacks. Yeah, it's not the most exciting work, but it's necessary. I'm hoping to automate the processing of configuration changes with code generation down the line, but we're not quite there yet.

Feel free to reach out to me on our Discord server if you need help with your implementation. I also plan to write more developer documentation in the coming days, which hopefully will help in your development efforts.

Looking forward to seeing more progress on this front!

nrybowski · 2024-04-08T10:12:09Z

Could you provide a brief summary of the scope of this work? It seems like you're laying the groundwork containing BIER data structures and configuration options, which could eventually lead to implementing BIER IGP extensions like RFC 8444, right?

Sure! To be honest, I worked backward. I first started by implementing BIER TLVs for OSPFv3 (draft-ietf-bier-ospfv3-extensions-07) (#16) which is like RFC8444 but for OSPFv3. I tested that with hardcoded values and then worked on this PR. My goal is to implement a functional BIER control plane in Holo and use a separate BIER data plane (e.g. https://github.com/louisna/bier-rust). I'm also thinking about implementing a BIER data plane in Click / FastClick to leverage upcoming support for DPDK in Holo (#14).

Regarding your changes done in the BIER YANG module, I'd suggest keeping the draft module intact, and do all changes in the form of deviations. Reporting the problems you found to the module authors would be nice as well, so that they can be fixed in the next draft version.

You are right, I also thought about using deviations but I was unsure how to indicate other variable types, I will look at that.

Regarding the code changes, it looks like you had some fun adding and implementing the BIER northbound callbacks. Yeah, it's not the most exciting work, but it's necessary. I'm hoping to automate the processing of configuration changes with code generation down the line, but we're not quite there yet.

Feel free to reach out to me on our Discord server if you need help with your implementation. I also plan to write more developer documentation in the coming days, which hopefully will help in your development efforts.

Sounds great!

nrybowski · 2024-04-09T14:35:02Z

The BIER Forwarding Table (BIFT) configuration defined in draft-ietf-bier-bier-yang-08 is left unimplemented since the way to integrate a BIER data-plane with Holo is not defined yet.

rwestphal · 2024-04-09T22:20:09Z

That's pretty awesome. I'm impressed with the progress you've made so far when there's so little developer documentation available (I'm working on that!).

I assume the Linux kernel doesn't support BIER yet, so yeah, we'll need to figure out a way to integrate with custom BIER dataplanes.

Let me know when the code is ready for review! @frederic-loui has some experience with BIER and can help with testing.

Partial implementation of updated draft-ietf-bier-bier-yang-08 - sub-domain-id updated from u16 to u8 per RFC8279 - mt_id updated from u16 to u8 per draft-ietf-ospf-mt-ospfv3-03 - address_family updated from custom format to iana-routing-types:address_family - fixed some typos in the YANG model Signed-off-by: Nicolas Rybowski <nicolas.rybowski@uclouvain.be>

Divergences with draft-ietf-bier-bier-yang-08: - max-si type updated from uint16 to uint8 per draft-ietf-bier-ospfv3-extensions Full BIER sub-domain configuration via YANG callbacks. Signed-off-by: Nicolas Rybowski <nicolas.rybowski@uclouvain.be>

Signed-off-by: Nicolas Rybowski <nicolas.rybowski@uclouvain.be>

nrybowski · 2024-04-10T23:58:38Z

The force push is a rebase on 8ad0dd1 and the cleanup of some commit messages. I still have to enforce the constraints on the acceptable values for the BIER configuration variables in validation callbacks. That being said, IMO the PR is ready for a first review.

rwestphal

@nrybowski This is looking great!

I've left a few review comments below.

Also, please ensure the ietf-bier@2023-09-16.yang YANG module is kept intact and do all your changes in the deviations module. When people see ietf-bier@2023-09-16.yang, they expect the original standard module, so having a modified version with the same name could lead to confusion.

rwestphal · 2024-04-11T14:17:04Z

holo-utils/src/bier.rs

+pub struct BierSubDomainCfg {
+    pub sd_id: SubDomainId,
+    pub addr_family: AddressFamily,
+    pub bfr_prefix: IpNetwork,


In the YANG module, this and all other fields below this one are optional, so you need wrap the types inside an Option (e.g. Option<IpNetwork>) and adjust the northbound callbacks accordingly.

Referencing the YANG tree output below, note that all leafs ending with ? indicate optional nodes:

| +--rw bier:sub-domain* [sub-domain-id address-family] | | +--rw bier:sub-domain-id uint16 | | +--rw bier:address-family identityref | | +--rw bier:bfr-prefix? inet:ip-prefix | | +--rw bier:underlay-protocol-type? underlay-protocol-type | | +--rw bier:mt-id? uint16 | | +--rw bier:bfr-id? uint16 | | +--rw bier:bsl? bsl | | +--rw bier:igp-algorithm? uint8 | | +--rw bier:bier-algorithm? uint8 | | +--rw bier:load-balance-num? uint8 | | +--rw bier:encapsulation* [bsl encapsulation-type]

rwestphal · 2024-04-11T14:21:13Z

holo-routing/src/northbound/configuration.rs

+        .path(bier::sub_domain::PATH)
+        .validate(|args| {
+            let addr_family = args.dnode.get_af_relative("./address-family").unwrap();
+            let mt_id = args.dnode.get_u8_relative("./mt-id").unwrap();


The "mt-id" leaf is optional, so you need to check if it's present (if let Some(...) ...).

rwestphal · 2024-04-11T14:21:43Z

holo-routing/src/northbound/configuration.rs

+                // Update the shared BIER configuration by creating a new reference-counted copy.
+                self.shared.bier_config = Arc::new(self.bier_config.clone());
+
+                // Notify protocol instances about the updated SR configuration.


s/SR/BIER/ :)

rwestphal · 2024-04-11T14:26:01Z

holo-routing/src/northbound/configuration.rs

@@ -61,6 +68,8 @@ pub enum Event {
    SrCfgUpdate,
    SrCfgLabelRangeUpdate,
    SrCfgPrefixSidUpdate(AddressFamily),
+    BierCfgUpdate,
+    BierCfgEncapUpdate(SubDomainId, AddressFamily, Bsl, BierEncapsulationType),


For enum variants with multiple values, I recommend using the following alternative syntax, where you can name each value:

BierCfgEncapUpdate { sd_id :SubDomainId, af: AddressFamily, bsl: Bsl, encap_type: BierEncapsulationType, }

rwestphal · 2024-04-11T14:27:43Z

holo-routing/src/northbound/state.rs

@@ -408,6 +408,16 @@ fn load_callbacks() -> Callbacks<Master> {
                None
            }
        })
+        .path(bier::sub_domain::PATH)
+        .get_iterate(|_context, _args| {
+            // TODO: implement me!


nit: you can replace this comment by // No operational data under this list.

rwestphal · 2024-04-11T14:32:21Z

holo-utils/src/bier.rs

+pub struct BierEncapsulation {
+    pub bsl: Bsl,
+    pub encap_type: BierEncapsulationType,
+    pub max_si: u8,


This field should be optional as well.

rwestphal · 2024-04-11T14:57:50Z

holo-routing/src/northbound/configuration.rs

+            Event::BierCfgEncapUpdate(sd_id, addr_family, bsl, encap_type) => {
+                let _ = self
+                    .ibus_tx
+                    .send(IbusMsg::BierCfgEvent(BierCfgEvent::EncapUpdate(sd_id, addr_family, bsl, encap_type)));


This line is too long. Please run cargo fmt to address this and other style issues.

codecov · 2024-04-11T15:13:29Z

Codecov Report

Attention: Patch coverage is 5.33333% with 71 lines in your changes are missing coverage. Please review.

Project coverage is 60.29%. Comparing base (0f211fc) to head (b0ad353).
Report is 2 commits behind head on master.

Files	Patch %	Lines
holo-utils/src/bier.rs	4.34%	66 Missing ⚠️
holo-utils/src/ibus.rs	0.00%	4 Missing ⚠️
holo-protocol/src/lib.rs	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
- Coverage   60.42%   60.29%   -0.13%     
==========================================
  Files         179      180       +1     
  Lines       31812    31887      +75     
==========================================
+ Hits        19222    19226       +4     
- Misses      12590    12661      +71

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nrybowski mentioned this pull request Apr 8, 2024

WIP: BIER extension for OSPFv3 #16

Draft

6 tasks

rwestphal force-pushed the master branch from b717104 to 3785932 Compare April 8, 2024 20:44

nrybowski marked this pull request as ready for review April 10, 2024 23:41

nrybowski added 5 commits April 11, 2024 01:45

routing, utils: Add iBus messages for BierCfgEncapUpdate event

09ec920

Signed-off-by: Nicolas Rybowski <nicolas.rybowski@uclouvain.be>

routing, yang: Move modifications to YANG model of BIER in deviations

d8b13ba

Signed-off-by: Nicolas Rybowski <nicolas.rybowski@uclouvain.be>

routing: Remove BIER BIFT-related dead code

b0ad353

Signed-off-by: Nicolas Rybowski <nicolas.rybowski@uclouvain.be>

nrybowski force-pushed the bier branch from 59fec09 to b0ad353 Compare April 10, 2024 23:57

nrybowski changed the title ~~WIP: Add basic BIER support~~ Add basic BIER support Apr 11, 2024

rwestphal reviewed Apr 11, 2024

View reviewed changes

rwestphal force-pushed the master branch from 6ddbbc4 to 591de40 Compare May 1, 2024 02:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic BIER support #10

Add basic BIER support #10

nrybowski commented Apr 5, 2024 •

edited

rwestphal commented Apr 5, 2024

nrybowski commented Apr 8, 2024

nrybowski commented Apr 9, 2024

rwestphal commented Apr 9, 2024

nrybowski commented Apr 10, 2024

rwestphal left a comment

rwestphal Apr 11, 2024

rwestphal Apr 11, 2024

rwestphal Apr 11, 2024

rwestphal Apr 11, 2024

rwestphal Apr 11, 2024

rwestphal Apr 11, 2024

rwestphal Apr 11, 2024

codecov bot commented Apr 11, 2024

Add basic BIER support #10

Are you sure you want to change the base?

Add basic BIER support #10

Conversation

nrybowski commented Apr 5, 2024 • edited

TODO

rwestphal commented Apr 5, 2024

nrybowski commented Apr 8, 2024

nrybowski commented Apr 9, 2024

rwestphal commented Apr 9, 2024

nrybowski commented Apr 10, 2024

rwestphal left a comment

Choose a reason for hiding this comment

rwestphal Apr 11, 2024

Choose a reason for hiding this comment

rwestphal Apr 11, 2024

Choose a reason for hiding this comment

rwestphal Apr 11, 2024

Choose a reason for hiding this comment

rwestphal Apr 11, 2024

Choose a reason for hiding this comment

rwestphal Apr 11, 2024

Choose a reason for hiding this comment

rwestphal Apr 11, 2024

Choose a reason for hiding this comment

rwestphal Apr 11, 2024

Choose a reason for hiding this comment

codecov bot commented Apr 11, 2024

Codecov Report

nrybowski commented Apr 5, 2024 •

edited