Early Review of draft-ietf-rtgwg-ni-model-02

Request Review of draft-ietf-rtgwg-ni-model-02
Requested rev. 02 (document currently at 12)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2017-04-28
Requested 2017-03-21
Requested by Jeff Tantsura
Authors Lou Berger, Christian Hopps, Acee Lindem, Dean Bogdanović, Xufeng Liu
Draft last updated 2017-04-26
Completed reviews Yangdoctors Early review of -02 by Martin Björklund (diff)
Rtgdir Early review of -02 by John Scudder (diff)
Secdir Last Call review of -06 by Liang Xia (diff)
I'd appreciate timely reivew, the draft is getting ready for WGLC.

Many thanks,
Assignment Reviewer Martin Björklund
State Completed
Review review-ietf-rtgwg-ni-model-02-yangdoctors-early-bjorklund-2017-04-26
Reviewed rev. 02 (document currently at 12)
Review result Not Ready
Review completed: 2017-04-26



I am the assigned YANG doctor for draft-ietf-rtgwg-ni-model, and
here are my review comments, based on -02:

1) Add reference to import statements.

     import ietf-interfaces {
       prefix if;
         "RFC 7223: A YANG Data Model for Interface Management";

2) IETF boilerplate

  Use IETF boilerplate with contact, description w/ copyright etc.

3) feature bind-network-instance-name

    feature bind-network-instance-name {
        "Network Instance to which an interface instance is bound";

  This description doesn't make much sense.

  Also, this feature is not used in the model.  Should the feature be
  removed or used?

4) unused groupings

  The module defines 3 groupings that are not used:


  Either they should be removed, or the text needs to explain how they
  are supposed to be used by other modules.

5)  network-instances

  The module has this:

    container network-instances {
        description "Network instances each of which have
                     an independent IP/IPv6 addressing space
                     and protocol instantiations. For layer 3,
                     this consistent with the routing-instance
                     definition in ietf-routing";

  There is no "routing-instance" in "ietf-routing".  This description
  needs to be updated.

6)  leaf type

          leaf type {
              type identityref {
                  base network-instance-type;
                  "The network instance type -- details TBD
                   Likely types include core, L3-VRF, VPLS,
                   L2-cross-connect, L2-VSI, etc.";

  "details TBD" - needs to be fixed.

  Should this leaf be mandatory?  If not, it needs to be specified
  what it means if this leaf is not present.

7)  container network-instance-policy

    container network-instance-policy {
          "Network Instance Policy -- details TBD,
          perhaps based on BESS model";

  "details TBD" - needs to be fixed.

8)  augments

    augment "/if:interfaces/if:interface" {
          "Add a node for the identification of the logical network
          instance (which is within the interface's identified logical
          network element) associated with the IP information
          configured on an interface";

  Does this mean that this model cannot be used without the LNE model?

    augment "/if:interfaces/if:interface/ip:ipv4" {
          "Add a node for the identification of the logical
          network instance (which is within the interface's
          identified physical or virtual device) associated with
          the IP information configured on an interface";

  What does "which is within the interface's identified physical or
  virtual device" mean?

9) leaf bind-network-instance-name

  This leaf is a string.  Shouldn't it be a leafref?

    leaf bind-network-instance-name {
      type leafref {
        path "/network-instances/network-instance/name";

10) inconsistent formatting

  I suggest you run pyang -f yang [--keep-comments] and possibly edit
  the result add/remove comments.

  The following comment doesn't add much and should be removed:

   // namespace
   namespace "urn:ietf:params:xml:ns:yang:ietf-network-instance";

  Also remove the comments about rpcs and notifications.

  Also add period ('.') at the end of all sentences in the

11)  section 2

  Align with section 2 in the LNE document.

12)  section 3

  Can the same interface be bound to both an LNE and an NI?  If not,
  this needs to be explained.

13) YANG tree

  Section 3.2 uses a tree diagram to show instance data.  I think this
  is confusing, and you should use XML or JSON instead.

  (see my comments on YANG tree in the LNE review as well)