[yang-doctors] Yangdoctors early review of draft-ietf-idr-bgp-model-07

Andy Bierman via Datatracker <noreply@ietf.org> Mon, 30 December 2019 19:48 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: yang-doctors@ietf.org
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id CA8A8120B55; Mon, 30 Dec 2019 11:48:34 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Andy Bierman via Datatracker <noreply@ietf.org>
To: <yang-doctors@ietf.org>
Cc: idr@ietf.org, draft-ietf-idr-bgp-model.all@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.115.0
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Andy Bierman <andy@yumaworks.com>
Message-ID: <157773531468.4577.5049465733732578910@ietfa.amsl.com>
Date: Mon, 30 Dec 2019 11:48:34 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/jUQiy6MdwaOqalQMWLgbv3PHwL0>
Subject: [yang-doctors] Yangdoctors early review of draft-ietf-idr-bgp-model-07
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 30 Dec 2019 19:48:35 -0000

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.

  - 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.