Re: YANG doctor review of draft-ietf-rtgwg-lne-model-02

Lou Berger <lberger@labn.net> Sat, 01 July 2017 15:07 UTC

Return-Path: <lberger@labn.net>
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 72685120454 for <rtgwg@ietfa.amsl.com>; Sat, 1 Jul 2017 08:07:49 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.2
X-Spam-Level:
X-Spam-Status: No, score=-4.2 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-2.8, RCVD_IN_SORBS_SPAM=0.5, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (768-bit key) header.d=labn.net
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 mK_C4lVgSsnA for <rtgwg@ietfa.amsl.com>; Sat, 1 Jul 2017 08:07:47 -0700 (PDT)
Received: from gproxy9.mail.unifiedlayer.com (gproxy9-pub.mail.unifiedlayer.com [69.89.20.122]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 721881242F5 for <rtgwg@ietf.org>; Sat, 1 Jul 2017 08:07:47 -0700 (PDT)
Received: from CMOut01 (unknown [10.0.90.82]) by gproxy9.mail.unifiedlayer.com (Postfix) with ESMTP id 2E99B1E09A8 for <rtgwg@ietf.org>; Sat, 1 Jul 2017 09:07:47 -0600 (MDT)
Received: from box313.bluehost.com ([69.89.31.113]) by CMOut01 with id fT7k1v0042SSUrH01T7nZF; Sat, 01 Jul 2017 09:07:47 -0600
X-Authority-Analysis: v=2.2 cv=FvB1xyjq c=1 sm=1 tr=0 a=h1BC+oY+fLhyFmnTBx92Jg==:117 a=h1BC+oY+fLhyFmnTBx92Jg==:17 a=IkcTkHD0fZMA:10 a=xqWC_Br6kY4A:10 a=G3gG6ho9WtcA:10 a=NEAV23lmAAAA:8 a=48vgC7mUAAAA:8 a=zUFlJRoVwiak2uiOrTEA:9 a=QEXdDO2ut3YA:10 a=w1C3t2QeGrPiZgrLijVG:22
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=labn.net; s=default; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:MIME-Version :Date:Message-ID:From:References:Cc:To:Subject:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=4YyU1hwbCcPwxaRioVEZ0hnUskuz5W+/Va0nL6qUWog=; b=0bnDW0vXbde2gIQUtRmY1poLCa yrYyh9WRJNi5YLQnBb7/4wl4lfE4Ly2te8as7GBLVpWlJBfaXwcHACC47xOwNAg1AU9/FXbe0oCWE Pj/u72lA1nPuwpjbFFQLHvF4D;
Received: from pool-100-15-84-20.washdc.fios.verizon.net ([100.15.84.20]:42508 helo=[IPv6:::1]) by box313.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.87) (envelope-from <lberger@labn.net>) id 1dRJzr-000K9h-Un; Sat, 01 Jul 2017 09:07:44 -0600
Subject: Re: YANG doctor review of draft-ietf-rtgwg-lne-model-02
To: Martin Bjorklund <mbj@tail-f.com>, rtgwg@ietf.org
Cc: yang-doctors@ietf.org
References: <20170423.114706.2035453647296311737.mbj@tail-f.com>
From: Lou Berger <lberger@labn.net>
Message-ID: <284387ce-6e5b-557f-8a39-12b05d9004cb@labn.net>
Date: Sat, 1 Jul 2017 11:07:39 -0400
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20170423.114706.2035453647296311737.mbj@tail-f.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
X-AntiAbuse: This header was added to track abuse, please include it with any abuse report
X-AntiAbuse: Primary Hostname - box313.bluehost.com
X-AntiAbuse: Original Domain - ietf.org
X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12]
X-AntiAbuse: Sender Address Domain - labn.net
X-BWhitelist: no
X-Source-IP: 100.15.84.20
X-Exim-ID: 1dRJzr-000K9h-Un
X-Source:
X-Source-Args:
X-Source-Dir:
X-Source-Sender: pool-100-15-84-20.washdc.fios.verizon.net ([IPv6:::1]) [100.15.84.20]:42508
X-Source-Auth: lberger@labn.net
X-Email-Count: 2
X-Source-Cap: bGFibm1vYmk7bGFibm1vYmk7Ym94MzEzLmJsdWVob3N0LmNvbQ==
Archived-At: <https://mailarchive.ietf.org/arch/msg/rtgwg/IIbwxz9P3cp-cffj5cjRGYmOzi8>
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: Sat, 01 Jul 2017 15:07:49 -0000

Martin,

    Sorry about the slow response, but as you know we've been waiting to
update this document until schema-mount was updated/finalized. I think
it's close enough now and certainly we have  an update for this upcoming
meeting, links to formatted text can be found in the repo
https://github.com/ietf-rtg-area-yang-arch-dt/meta-model . See below for
specific responses:


On 4/23/2017 5:47 AM, Martin Bjorklund wrote:
> Hi,
>
> I am the assigned YANG doctor for draft-ietf-rtgwg-lne-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";
>      }
>
okay

> 2) IETF boilerplate
>
>   Use IETF boilerplate with contact, description w/ copyright etc.
Done
>
> 3) feature bind-lne-name
>
>      feature bind-lne-name {
>        description
>          "Logical network element to which an interface 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?
>
 this is vestigial and can be deleted

> 4) leaf managed
>
>       leaf managed {
>         type boolean;
>         description
>           "True if the host can manage the LNE using the root mount
>            point";
>       }
>
>   This description needs to be expanded.  This is a config leaf, so
>   what happens if a client sets this to 'true' for a certain LNE?
>
>   The text in the document says:
>
>     In addition to standard management interfaces, a host device
>     implementation may support accessing LNE configuration and
>     operational YANG models directly from the host system.  When
>     supported, such access is accomplished through a yang-schema-mount
>     mount point
>
>   So are there three cases involved?
>
>      1.  The host device does NOT support accessing LNE data from the
>          host system => in this case no modules will be mounted.
>
>      2.  The host device supports reading LNE data from the host
>          system => in this case the modules are mounted read-only.
>
>      3.  The host device supports reading/writing LNE data from
>          the host system => in this case the modules are mounted
>          read-write.
>
>   I _think_ the idea is that if the server supports 2 or 3, a client
>   can choose to set 'managed' to "false", in which case it turns into
>   case 1.  Is this correct?  If so, would it be useful to allow a
>   client to turn case 3 into 2?
>
>   The text in the document in section 3 has more details on the
>   "managed" leaf.  I suggest you add text from the document to the
>   YANG module.
>
>
> 5) leaf bind-lne-name
>
>   This leaf is a string.  Shouldn't it be a leafref?
>
>     leaf bind-lne-name {
>       type leafref {
>         path "/logical-network-elements/logical-network-element/name";
>       }
>       ...
>     }

agreed. Thanks!

>
> 6) 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-logical-network-element";
>
>   Also remove the comments about rpcs and notifications.
>
>   Also add period ('.') at the end of all sentences in the
>   descriptions.
done.

>
> 7) YANG tree
>
>   The document should explain the tree diagram syntax.
>
>
>   Section 2 has this tree diagram:
>
>              +--rw interfaces
>              |  +--rw interface* [name]
>              |     +--rw name                       string
>              |     +--rw lne:bind-lne-name?         string
>              |     +--rw ethernet
>              |     |  +--rw ni:bind-network-instance-name? string
>              |     |  +--rw aggregates
>              |     |  +--rw rstp
>              |     |  +--rw lldp
>              |     |  +--rw ptp
>              |     +--rw vlans
>              |     +--rw tunnels
>              |     +--rw ipv4
>              |     |  +--rw ni:bind-network-instance-name? string
>              |     |  +--rw arp
>              |     |  +--rw icmp
>              |     |  +--rw vrrp
>              |     |  +--rw dhcp-client
>              |     +--rw ipv6
>              |        +--rw ni:bind-network-instance-name? string
>              |        +--rw vrrp
>              |        +--rw icmpv6
>              |        +--rw nd
>              |        +--rw dhcpv6-client
>
>
>   This diagram is not correct; it seems to indicate that there are
>   nodes 'ethernet', 'vlans', 'ipv4', etc in the ietf-interfaces
>   module.  I suggest you remove the nodes that do not exist, and
>   change 'ipv4' to 'ip:ipv4' (and add a reference to RFC 7277).

this is fully replaced by an examples appendix in the forthcoming version.

>
> 8)  YANG tree (2)
>
>   Section 3 has this diagram:
>
>        module: ietf-logical-network-element
>           +--rw logical-network-inventory
>              +--rw logical-network-element* [name]
>                 +--rw name?   string
>                 +--rw description? string
>                 +--rw managed?     boolean
>                 +--rw root?        yang-schema-mount
>        augment /if:interfaces/if:interface:
>           +--rw bind-lne-name?     string
>
>
>   It needs to be updated to match the YANG model (rename
>   logical-network-inventory), and the 'root' should not be shown as a
>   leaf.  (Yes I know that there currently is no syntax defined for a
>   mount point in a tree)

The document is now aligned with draft-ietf-netmod-yang-tree-diagrams-01

>
> 9)  YANG tree (3)
>
>   Section 3.1 uses a tree diagram to show instance data.  I think this
>   is confusing, and you should use XML or JSON instead.
>

Same comment as NI:The next rev will use JSON representation for example
instance data.


> 10)  typo
>
>    s/The interface management model [RFC7223] is and existing model/
>      The interface management model [RFC7223] is an existing model/
>

Thanks.

Lou

PS I failed to hit send at the same time as the NI response, so this sat
around in my drafts folder for a while.  But this allowed me to update
the above based on the forthcoming rev, so all the better.

>
> /martin
>
> _______________________________________________
> rtgwg mailing list
> rtgwg@ietf.org
> https://www.ietf.org/mailman/listinfo/rtgwg
>