holo-routing / holo

Holo is a suite of routing protocols designed to support high-scale and automation-driven networks.
MIT License
270 stars 16 forks source link

Add basic BIER support #10

Closed nrybowski closed 3 months ago

nrybowski commented 7 months ago

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:

TODO

rwestphal commented 7 months ago

@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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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.

nrybowski commented 6 months ago

The force push is a rebase on 8ad0dd112a85a9021354deffb70d43c58ec577c7 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.

codecov[bot] commented 6 months ago

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 :warning:
holo-utils/src/ibus.rs 0.00% 4 Missing :warning:
holo-protocol/src/lib.rs 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nrybowski commented 3 months ago

980694896ed393dc66ea4b8b53bd9dcdaa247f1d is the commit on bier-rebased that includes the addition of the YANG model for BIER on top of the latest commit on master (08dcc23). The PR should be ready for merge.