Last Call Review of draft-ietf-ospf-sr-yang-28
review-ietf-ospf-sr-yang-28-yangdoctors-lc-rahman-2024-01-13-00

Hi all,

This is my YANG Doctor review of -28, I had previously reviewed -20. Thanks to
the authors for addressing my previous comments. There is 1 comment in my
initial review which concerns RFC9020, I am not convinced yet and may send
another email (or errata).

Comments
========

Should the title explicitly call out OSPFv2 and OSPFv3? The reason I’m asking
is because OSPF may imply v2 only, e.g. RFC8665 says “OSPF Extensions for
Segment Routing”  but then the abstract says OSPFv2.

Section 2. “OSPF base model[RFC9129]”, nit: add a space before the reference

In the following, can there be overlaps? If not, this should be documented
(ideally should have been documented in RFC9020)

          +--rw srgb
          |  +--rw srgb* [lower-bound upper-bound]
          |     +--rw lower-bound    uint32
          |     +--rw upper-bound    uint32

Section 2.1 (the YANG module)

- In grouping ospfv2-prefix-sid-sub-tlvs, leaf-list flags should have a
reference? Same for v3.

- The grouping ospfv2-extended-prefix-range-tlvs has an ‘af’ address family
leaf which is a uint8, why not use address-family from RFC8294 with the
appropriate restrictions. But since this is OSPFv2 specific, is address family
still needed? For v3, I believe the af leaf is needed, although I’d rename it
to address-family and would use address-family enum from RFC8294.

- The grouping ospfv2-extended-prefix-range-tlvs: should there be a range for
prefix-length? Same question, but but different range needed, for OSPFv3.

- In list local-block-tlv, description of leaf range-size has “…The return of a
zero value”. Nit: change to “A value of zero…”

- In container srms-preference-tlv, leaf preference. Nit: “with with 255”.

- Should leaf neighbor-id be mandatory? If not, what happens when it’s not set,
does it need a default value? I believe the description needs to indicate what
happens when it is set or not set.

- In ti-lfa container: the enable flag is not mandatory and does not have a
default value, you should add a default value or make it mandatory. Other
choice is a presence container.

- In the selection-tie-breakers container, can both tie-breakers be enabled
simultaneously?

- For leaf use-segment-routing-path, the description has “…is in effect only
when remote-lfa is enabled”. I did not see any remote-lfa leaf node, not sure
if this is referring to a feature. I think the description needs to be modified
and a reference would be very helpful here.

Appendix A. There is only 1 (simple) example and it covers v2 only. Please add
a v3 example also, ideally with more config data than the current example e.g.
with the neighbor-id (since that augment is in this document). Having an
operational state example for OSPFv2 and OSPFv3 would also really be helpful. I
realize examples can be painful...

Regards,

Reshad.