ospf: Add RFC 9339 support

[nrybowski]

This PR adds the support for Reverse Metric and Reverse TE Metric advertisement through the LLS data block of Hello packets, as defined by RFC 9339.

It depends on RFC 5613 whose support has been added with https://github.com/holo-routing/holo/pull/77.

TODO

• [x] Add Reverse (TE) Metric LLS TLVs encoding / decoding
  • [x] Add encoding / decoding tests for OSPFv2
  • [x] Add encoding / decoding tests for OSPFv3
• [x] Enable Reverse Metric configuration
  • [x] Add YANG model. (There is no YANG model specific to OSPF for now, I will adapt the one for IS-IS.)
  • [x] Extend northbound interface
    • [x] Directly send Hello packet upon Reverse Metric enabling / disabling
  • [x] Extend OSPF configuration to store Reverse Metric configuration
• [x] Implement Reverse Metric logic
  • [x] Update metric if needed upon Reverse Metric reception
  • [x] Trigger Router LSA origination upon Reverse Metric reception
  • [ ] Fallback to provisioned metric once Reverse Metric is not advertised anymore by the neighbor.
• [ ] Enable Reverse TE Metric configuration
  • [ ] Add YANG model.
  • [ ] Extend northbound interface
    • [ ] Directly send Hello packet upon Reverse TE Metric enabling / disabling
  • [ ] Extend OSPF configuration to store Reverse TE Metric configuration
• [ ] Implement Reverse TE Metric logic
  • [ ] Update metric if needed upon Reverse TE Metric reception
  • [ ] Trigger Router LSA origination upon Reverse TE Metric reception
  • [ ] Fallback to provisioned metric once Reverse TE Metric is not advertised anymore by the neighbor.

[rwestphal]

Add YANG model. (There is no YANG model specific to OSPF for now, I will adapt the one for IS-IS.)

Indeed. I see there's [email protected] but no equivalent module for OSPF.

There's draft-ietf-lsr-ospf-yang-augmentation-v1, which adds several augmenting modules to ietf-ospf. We can add our own holo-ospf-reverse-metric and potentially ask the IETF folks to integrate it into that draft. That would probably be welcome on their part, especially since it would come with a working implementation.

[nrybowski]

Thanks for this first review!

Also, the "enable-advertise" leaf can be removed, as the presence of "metric" already implies we want to advertise the reverse metric.

This was also my initial though but when I implemented the Default trait for ReverseMetricMtCfg, I added a default value for the metric which enables 'by default' the RM. A more clever way to implement that would be storing the metric value in an Option so that the default value is None. Also, RFC9339 states in Section 7 that "An implementation MUST NOT signal RM to neighbors by default and MUST provide a configuration option to enable the signaling of RM on specific links.", and I understood this sentence as "we have to provide an explicit switch to enable the metric". I will remove the "enable-advertise" leaf and use an Option to store the RM.

Later, once support for RFC 4915 is implemented, a separate augmentation could be added for "interface/topologies/topology".

I prefer defining a module that completely specifies RFC 9339 and then add a deviation that indicates unsupported augmentations.

I think that would be cleaner, especially since "mtid" only makes sense for OSPFv2.

That's a good point that I completely overlooked. I will add two augmentations then, one that accepts a single RM on the interface and enforce that mt-id equals 0 and one for OSPFv2 that adds a RM to "interface/topologies/topology" as you suggested.

We will then add an 'unsupported' deviation for the last one in holo-ospf-dev.yang.

Does that sounds reasonable to you?

[rwestphal]

This was also my initial though but when I implemented the Default trait for ReverseMetricMtCfg, I added a default value for the metric which enables 'by default' the RM. A more clever way to implement that would be storing the metric value in an Option so that the default value is None. Also, RFC9339 states in Section 7 that "An implementation MUST NOT signal RM to neighbors by default and MUST provide a configuration option to enable the signaling of RM on specific links.", and I understood this sentence as "we have to provide an explicit switch to enable the metric". I will remove the "enable-advertise" leaf and use an Option to store the RM.

Awesome. Option is a perfect fit for configuration options without default values.

I prefer defining a module that completely specifies RFC 9339 and then add a deviation that indicates unsupported augmentations.

That's indeed better!

That's a good point that I completely overlooked. I will add two augmentations then, one that accepts a single RM on the interface and enforce that mt-id equals 0 and one for OSPFv2 that adds a RM to "interface/topologies/topology" as you suggested.

We will then add an 'unsupported' deviation for the last one in holo-ospf-dev.yang.

Does that sounds reasonable to you?

Sounds good to me.

Just keep in mind that in "interface/topologies/topology" you can configure a topology name, but not an "mtid", which can be a bit confusing. This is probably because the IETF folks couldn't agree on whether the MT-ID should be explicitly configured or inferred from the selected RIB. In any case, this shouldn't affect your augmentation. I just wanted to point it out in case it causes any confusion :)

[nrybowski]

Just rebased on master to clear the conflicts. https://github.com/holo-routing/holo/pull/78/commits/81df85f61627aa0f60524e0323a88ab17984b1ec contains the changes we just discussed.

Just keep in mind that in "interface/topologies/topology" you can configure a topology name, but not an "mtid", which can be a bit confusing. This is probably because the IETF folks couldn't agree on whether the MT-ID should be explicitly configured or inferred from the selected RIB. In any case, this shouldn't affect your augmentation. I just wanted to point it out in case it causes any confusion :)

The good news is that the "mtid" augmentation for OSPFv2 is gated with the feature "multi-topology" so we don't even need a deviation to "unsupport" the augmentation and we can keep the headache of handling that when RFC 4915 is implemented :)

[rwestphal]

The good news is that the "mtid" augmentation for OSPFv2 is gated with the feature "multi-topology" so we don't even need a deviation to "unsupport" the augmentation and we can keep the headache of handling that when RFC 4915 is implemented :)

Yeah that makes things easier for us :)

From a quick look, it seems most of the RFC is already implemented. Tomorrow I'll run some tests and do a full review. Looking forward to it :)