Skip to main content

Early Review of draft-ietf-idr-bgp-model-07
review-ietf-idr-bgp-model-07-yangdoctors-early-bierman-2019-12-30-00

Request Review of draft-ietf-idr-bgp-model
Requested revision No specific revision (document currently at 17)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2019-12-08
Requested 2019-11-17
Requested by Susan Hares
Authors Mahesh Jethanandani , Keyur Patel , Susan Hares , Jeffrey Haas
I-D last updated 2019-12-30
Completed reviews Rtgdir Early review of -07 by Yingzhen Qu (diff)
Yangdoctors Early review of -07 by Andy Bierman (diff)
Yangdoctors Early review of -09 by Acee Lindem (diff)
Assignment Reviewer Andy Bierman
State Completed
Request Early review on draft-ietf-idr-bgp-model by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/jUQiy6MdwaOqalQMWLgbv3PHwL0
Reviewed revision 07 (document currently at 17)
Result On the Right Track
Completed 2019-12-30
review-ietf-idr-bgp-model-07-yangdoctors-early-bierman-2019-12-30-00
Major issues:

  - The ietf-bgp module uses (legal) YANG 1.1 constructs that are
    not supported by available tools so it cannot be validated.
    The solution options are
    (A) rewrite the module so it does not place any definitions
        in the main module.  Instead place all definitions in
        submodules. Add YANG 1.0 include-stmts as needed so pyang
        can validate the module
    (B) wait until the opensource tools properly support this
        YANG 1.1 usage and resubmit the module at that time

  - The modules import ietf-routing-policy. Version used was
    2019-03-04.  This module has a fatal error caused because
    ietf-interface-common has apparently been replaced with
    ietf-if-extensions@2019-11-04

      leaf subinterface {
        type leafref {
          path "/if:interfaces/if:interface/if-cmn:encapsulation"
             + "/if-l3-vlan:dot1q-vlan"
             + "/if-l3-vlan:outer-tag/if-l3-vlan:vlan-id";
        }

    The path expression is wrong. if-cmn:encapsulation is now
    if-ext:encapsulation.  You need to check the XPath everywhere
    when you refactor YANG modules.

  - Full review of these modules is not possible at this time without
    proper opensource tools. Processing the YANG statements
    by hand is extremely difficult, given the large amount
    of groupings used which are spread across many modules and
    submodules.


Minor Issues

  - The "clear" actions need to be specific about the exact set
    of objects that are affected. The value of each object once it
    is cleared needs to be specified in every object that is affected
    by the action.  Impact on operations should be explained in
    each action-stmt

  - Some TODO items still remain indicating the YANG definitions
    are temporary and will be replaced by more correct statements.

  - Additional reference-stmts would be useful to implementors
    who are familar with the routing RFCs that the YANG objects
    are based on

Editorial Comments

  - The implementation complexity appears to be very high
    for both server and client developers.  The interactions between
    subtrees is non-trivial and probably requires more clarifications.
    This needs to be done by implementors, not document reviewers.