Reviewer: Reshad Rahman
Review result: Ready with Issues
Hi all,
This is my YANG Doctor review of -19. Thanks to the authors for making the
effort to align with draft-ietf-ospf-yang (as previously requested).
Comments
========
Section 3 (YANG Tree)
- There are a few instances of perfix-sid-flags, should bd prefix-sid-flags
- For the list below, can there be overlaps between different entries? If not,
this should be documented (ideally should have been documented in RFC9020)
+--rw protocol-srgb {sr-mpls:protocol-srgb}?
+--rw srgb* [lower-bound upper-bound]
+--rw lower-bound uint32
+--rw upper-bound uint32
YANG Model
- The identities such as r-bit, n-bit etc should all have a reference (not just
the document but also the section)
- All those identities are called x-bit but the description says flag. Which
terminology is typically used in IS-IS?
- Leaf Sid, how do we know whether it’s a 32-bit SID or a 20-bit label (not
sure if we need to know)?
- For global-blocks and local-blocks, a reference would help.
- In grouping sid-sub-tlv, is the container sid-sub-tlv needed?
- In grouping srlb , can there be an overlap of the advertised local blocks?
Need some enhanced description here iMO. Same question for sr-capability and
global blocks.
- In list global-block, why not add a key? I’m assuming the sid would be
unique? Same comment for local-block.
- In grouping srms-preference, leaf preference, no need for the range 0..255
since it is a uint8.
- Nit: a few instances of “This group …”, it should be “This grouping …”
- Leaf ‘af’, nit/suggestion: I would rename to address-family.
- In grouping segment-routing-binding-tlv, leaf range, is that description
accurate?
- Augment with neighbor-system-id, should the leaf node be mandatory?
- Container selection-tie-breakers, can both protection types be enabled
simultaneously?
There should be an appendix with a fairly complete config example and also an
operational state example.
Regards,
Reshad.
_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr
