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.