Subject: Re: [Idr] [yang-doctors] Yangdoctors early review of draft-ietf-idr-bgp-model-07

From: "Acee Lindem (acee)" <[email-masked]>
Date: Thu, 2 Jan 2020 21:18:26 +0000

Hi Andy, et al, 

On 12/30/19, 2:49 PM, "yang-doctors on behalf of Andy Bierman via Datatracker" <[email-masked] on behalf of [email-masked]> wrote:

    Reviewer: Andy Bierman
    Review result: On the Right Track

    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.

I have fixed this in the -08 version. 

https://datatracker.ietf.org/doc/draft-ietf-rtgwg-policy-model/

Now, if only we could get the IETF tools to include the ieee802-dot1q-types.yang model so that ietf-if-l3-vlan.yang parses....

Thanks,
Acee 
    
      - 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.

    _______________________________________________
    yang-doctors mailing list
    [email-masked]
    https://www.ietf.org/mailman/listinfo/yang-doctors

---

From: Andy Bierman <[email-masked]>
Date: Thu, 2 Jan 2020 13:28:02 -0800

On Thu, Jan 2, 2020 at 1:18 PM Acee Lindem (acee) <[email-masked]> wrote:

> Hi Andy, et al,
>
> On 12/30/19, 2:49 PM, "yang-doctors on behalf of Andy Bierman via
> Datatracker" <[email-masked] on behalf of [email-masked]>
> wrote:
>
>     Reviewer: Andy Bierman
> Now, if only we could get the IETF tools to include the
> ieee802-dot1q-types.yang model so that ietf-if-l3-vlan.yang parses....
>
>
I noticed that IEEE modules are being imported into some IETF models now.
I did not raise it in the ietf-bgp review but it does raise a process or
tools issue.

Is it possible to standardize tools based on the github YangModels repo,
using standard module search path. e.g

 -p repo/experimental/ietf-extracted-YANG-models
 -p repo/standard

Thanks,
> Acee

Andy