YANG doctor review of draft-ietf-rtgwg-ni-model-02
Martin Bjorklund <mbj@tail-f.com> Sun, 23 April 2017 09:48 UTC
Return-Path: <mbj@tail-f.com>
X-Original-To: rtgwg@ietfa.amsl.com
Delivered-To: rtgwg@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id AEBE3128D40; Sun, 23 Apr 2017 02:48:40 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.902
X-Spam-Level:
X-Spam-Status: No, score=-1.902 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xeGdg236bClD; Sun, 23 Apr 2017 02:48:39 -0700 (PDT)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id AD4A01200C1; Sun, 23 Apr 2017 02:48:39 -0700 (PDT)
Received: from localhost (h-13-92.a165.priv.bahnhof.se [155.4.13.92]) by mail.tail-f.com (Postfix) with ESMTPSA id 005FB1AE028F; Sun, 23 Apr 2017 11:48:38 +0200 (CEST)
Date: Sun, 23 Apr 2017 11:48:38 +0200
Message-Id: <20170423.114838.190354600276418424.mbj@tail-f.com>
To: rtgwg@ietf.org
Cc: yang-doctors@ietf.org
Subject: YANG doctor review of draft-ietf-rtgwg-ni-model-02
From: Martin Bjorklund <mbj@tail-f.com>
X-Mailer: Mew version 6.7 on Emacs 24.5 / Mule 6.0 (HANACHIRUSATO)
Mime-Version: 1.0
Content-Type: Text/Plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/rtgwg/hGc45m8G-d4fJ6LBTxRatyCDt9Q>
X-BeenThere: rtgwg@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Routing Area Working Group <rtgwg.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/rtgwg>, <mailto:rtgwg-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/rtgwg/>
List-Post: <mailto:rtgwg@ietf.org>
List-Help: <mailto:rtgwg-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rtgwg>, <mailto:rtgwg-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 23 Apr 2017 09:48:40 -0000
Hi, 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; reference "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 { description "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: interface-ip-common ipv4-interface-protocols ipv6-interface-protocols 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; } description "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 { description "Network Instance Policy -- details TBD, perhaps based on BESS model"; } "details TBD" - needs to be fixed. 8) augments augment "/if:interfaces/if:interface" { description "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" { description "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 descriptions. 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) /martin
- YANG doctor review of draft-ietf-rtgwg-ni-model-02 Martin Bjorklund
- Re: YANG doctor review of draft-ietf-rtgwg-ni-mod… Lou Berger