Skip to main content

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

Request Review of draft-ietf-isis-sr-yang
Requested revision No specific revision (document currently at 21)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2023-11-30
Requested 2023-10-31
Requested by Acee Lindem
Authors Stephane Litkowski , Yingzhen Qu , Pushpasis Sarkar , Ing-Wher (Helen) Chen , Jeff Tantsura
I-D last updated 2024-01-13
Completed reviews Yangdoctors Last Call review of -19 by Reshad Rahman (diff)
Rtgdir Last Call review of -17 by Shuping Peng (diff)
Yangdoctors Last Call review of -13 by Jan Lindblad (diff)
Assignment Reviewer Reshad Rahman
State Completed
Request Last Call review on draft-ietf-isis-sr-yang by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/FWbc3qZD4R7aIE-zr3P0_xojr3U
Reviewed revision 19 (document currently at 21)
Result Ready w/issues
Completed 2024-01-13
review-ietf-isis-sr-yang-19-yangdoctors-lc-rahman-2024-01-13-00
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.