Skip to main content

Early Review of draft-ietf-spring-sr-yang-09
review-ietf-spring-sr-yang-09-yangdoctors-early-lhotka-2018-10-24-00

Request Review of draft-ietf-spring-sr-yang
Requested revision No specific revision (document currently at 30)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2018-11-05
Requested 2018-10-16
Requested by Bruno Decraene
Authors Stephane Litkowski , Yingzhen Qu , Acee Lindem , Pushpasis Sarkar , Jeff Tantsura
I-D last updated 2018-10-24
Completed reviews Yangdoctors Early review of -09 by Ladislav Lhotka (diff)
Yangdoctors Last Call review of -20 by Ladislav Lhotka (diff)
Rtgdir Early review of -28 by Tal Mizrahi (diff)
Assignment Reviewer Ladislav Lhotka
State Completed
Request Early review on draft-ietf-spring-sr-yang by YANG Doctors Assigned
Reviewed revision 09 (document currently at 30)
Result On the Right Track
Completed 2018-10-24
review-ietf-spring-sr-yang-09-yangdoctors-early-lhotka-2018-10-24-00
*** General comments

    This document contains two YANG modules:
    ietf-segment-routing-common and ietf-segment-routing. The latter
    augments ietf-routing with data necessary for configuring and
    operating segment routing, but also provides groupings that define
    data to be used in segment routing extensions of routing protocols
    and interfaces. This design makes it relatively easy to define
    such IGP extensions.

**** Naming 

     - My suggestion is to review the identifiers defined in the
       modules so as to adhere more closely to the identifier naming
       conventions in sec. 4.3.1 of RFC 8407. It is subjective to
       decide which acronyms are well-known but, in my view, "srgb",
       "srlb" and "msd" do not fall into this category. For example,
       "msd" can be changed to to "max-sid-depth" (both as a data node
       and a feature).
     - On the other hand, the "-cfg" suffix in the names of several
       groupings should be removed - according to RFC 8407,
       identifiers should not carry semantics. (And perhaps the
       same groupings can also be used for state data?)

**** Revisions

     The YANG modules contain revision dates of all development
     versions. Although RFC 8407 doesn't require to do so, I think it
     can be useful for the developmnent and testing of the
     modules. However, these development revisions should be removed
     before publishing the RFC (maybe the authors intend to do so).

**** References

     Normative References should include the RFCs defining YANG
     modules that are imported by the segment-routing modules: 6991,
     7223, 8294 and 8349.

*** Specific comments

    - Containers "connected-prefix-sid-map" and "local-prefix-sid"
      have subcontainers "ipv4" and "ipv6" that only differ in the
      type of the "prefix" leaf. An alternative solution can be to
      change the outer containers into lists with "address-family" as
      the key. Did the authors discuss this option?

    - The text uses the terms "states" and "operational states" in
      several places. The plural form looks weird to me.