Re: [pim] [yang-doctors] Yangdoctors last call review of draft-ietf-pim-yang-12
Benoit Claise <bclaise@cisco.com> Tue, 02 January 2018 18:19 UTC
Return-Path: <bclaise@cisco.com>
X-Original-To: pim@ietfa.amsl.com
Delivered-To: pim@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D28CF12D7EA; Tue, 2 Jan 2018 10:19:14 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.51
X-Spam-Level:
X-Spam-Status: No, score=-14.51 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
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 vRMGUyvEkHkY; Tue, 2 Jan 2018 10:19:12 -0800 (PST)
Received: from aer-iport-1.cisco.com (aer-iport-1.cisco.com [173.38.203.51]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id AF4BF124B17; Tue, 2 Jan 2018 10:19:10 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=22001; q=dns/txt; s=iport; t=1514917151; x=1516126751; h=subject:to:cc:references:from:message-id:date: mime-version:in-reply-to; bh=51nw2j9aDW7HslWkuFzH3xpj3AdAwOtgmvv4jQsudSw=; b=RKRGnGqmF0wwv+Mbb79QSgYv8oCOEvW4wknUWMgZurqFnDQVAO29OnGm L4FsiAz+ZvonmsPAt+kEBWJvt4B4GyoZ2ovsSgONQBp7V3Cbs9v+n45na joGmrogmI6krzcTbP/0hXyS1/ufNZHjlt2fJOIk36/NJEcKM68eZe8JpX s=;
X-IronPort-AV: E=Sophos;i="5.45,498,1508803200"; d="scan'208,217";a="1222475"
Received: from aer-iport-nat.cisco.com (HELO aer-core-2.cisco.com) ([173.38.203.22]) by aer-iport-1.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Jan 2018 18:19:08 +0000
Received: from [10.55.221.36] (ams-bclaise-nitro3.cisco.com [10.55.221.36]) by aer-core-2.cisco.com (8.14.5/8.14.5) with ESMTP id w02IJ848020393; Tue, 2 Jan 2018 18:19:08 GMT
To: Jürgen Schönwälder <j.schoenwaelder@jacobs-university.de>, yang-doctors@ietf.org
Cc: draft-ietf-pim-yang.all@ietf.org, ietf@ietf.org, pim@ietf.org, "bfd-chairs@ietf.org" <netmod-chairs@ietf.org>, Alia Atlas <akatlas@gmail.com>, "Miroslav Kovac -X (mirkovac - PANTHEON TECHNOLOGIES at Cisco)" <mirkovac@cisco.com>
References: <151380493227.2629.15496937140050941194@ietfa.amsl.com>
From: Benoit Claise <bclaise@cisco.com>
Message-ID: <1f41bfe5-65f5-3db4-2283-506e947722f9@cisco.com>
Date: Tue, 02 Jan 2018 19:19:07 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: <151380493227.2629.15496937140050941194@ietfa.amsl.com>
Content-Type: multipart/alternative; boundary="------------F04A5FE119F91450D7C7D0A6"
Content-Language: en-US
Archived-At: <https://mailarchive.ietf.org/arch/msg/pim/usZ_-LdzFEDU2awUElnrcI58e6I>
Subject: Re: [pim] [yang-doctors] Yangdoctors last call review of draft-ietf-pim-yang-12
X-BeenThere: pim@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Protocol Independent Multicast <pim.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/pim>, <mailto:pim-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/pim/>
List-Post: <mailto:pim@ietf.org>
List-Help: <mailto:pim-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/pim>, <mailto:pim-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 02 Jan 2018 18:19:15 -0000
Hi Jürgen, > Reviewer: Jürgen Schönwälder > Review result: Almost Ready > > * General YANG Review Remarks > > This document depends on a number of other I-Ds. Is it safe to > process this document while the other documents this document > depends on are not yet through the process? Who is tracking things > and making sure that any changes in other documents that may impact > the PIM document are detected and handled appropriately before > publication? I do look at the YANG module dependencies, thanks to tooling. Let's look at the draft-ietf-pim-yang-12 dependencies: https://www.yangcatalog.org/yang-search/impact_analysis.php?modules[]=ietf-pim-dm@2017-12-08.yang&modules[]=ietf-pim-sm@2017-12-08.yang&modules[]=ietf-pim-base@2017-12-08.yang&modules[]=ietf-pim-bidir@2017-12-08.yang&modules[]=ietf-pim-rp@2017-12-08.yang&recurse=2&rfcs=1&show_subm=1&show_dir=dependencies [I-D.ietf-rtgwg-routing-types] is RFC 8294. We will update the tool. [I-D.ietf-netmod-rfc7223bis] is on the Jan 11th IESG telechat. [I-D.ietf-netmod-rfc8022bis] is in IETF LC now and on the Jan 25th IESG telechat The only unknown to me is the state of [I-D.ietf-bfd-yang]. Copying Alia and the BFD chairs. In terms of I-D normative references, https://tools.ietf.org/html/draft-ietf-pim-yang-12#section-9.1, there is an extra related one: [I-D.ietf-netmod-revised-datastores] is on the Jan 11th IESG telechat So we're getting there... Regards, Benoit > > It is generally good style to write complete sentences in > description statements. Some of the description statements are just > fractions of a sentence. > > I think we do not recommend anymore to list WG chairs in the contact > statement. > > It is sometimes not clear why you define groupings that are only > used once (and sometimes are likely not reusable since they contain > relative paths in must expressions). > > * Introduction > > - Is there a reason why you refer to RFC 6020 and to RFC 7950 in > sections 1 and 1.2? Why do you need the reference to RFC 6020? > > - What are 'wider' management interfaces? If you mention NETCONF, > why not mention RESTCONF? > > - s/Protocol-Independent Multicast (PIM) devices > /devices supporting Protocol-Independent Multicast (PIM)/ > > - So which YANG terminology applies, RFC 6020 or RFC 7950? I > personally think using YANG 1.1 is pretty safe these days. > > * Design of Data Model > > - I did not really understand Section 2.5. It seems you are > duplicating objects for different address families but on some > systems these duplicate objects must have the same value. If so, > where would the must expression go and how does an implementation > add such a must expression? How many of such must expressions > would there have to be? Did you consider having address family > independent objects and optionally (controlled by a feature) per > address family objects that overwrite the independent settings? > > * Module Structure > > - s/is included/is imported/ > > - The tree diagrams are rather long. It would likely help readers if > the diagram would be split into meaningful units and additional > text added to describe the units. > > Are lists of the form > > | +--rw ipv4-rp* [ipv4-address] > | | +--rw ipv4-address inet:ipv4-address > > designed for exensibility? Otherwise, this may be collapsed into > a simple leaf-list. > > - Since I am not familiar with details of PIM, I can't comment on > the question whether the model makes sense or not. > > * PIM base Module > > - YANG modules compile cleanly according to the tracker page. > > - As said above, consider using YANG 1.1. The ietf-routing module > actually uses YANG 1.1 so you will need a YANG 1.1 compiler > anyway. > > - Consider adding reference statements to the feature definitions > in case a feature is described in a protocol specification. > > - The value of timer-value is not really clear, you could have used > rt-types:timer-value-seconds16 directly. > > - Why is graceful-restart/duration using an ad-hoc type for 16-bit > seconds and not timer-value? Is it because infinity and not-set > does not apply? > > - Does a bfd/hello-interval of 'infinity' or 'not-set' make sense? > > - More explicit description of bfd/hello-holdtime? Is a choice > really appropriate (hello-holdtime-or-multiplier)? Can I not have > both holdtime and multiplier? Perhaps I am just not clear what > holdtime does... > > - Does a bfd/jp-interval of 'infinity' or 'not-set' make sense? > > - More explicit description of bfd/jp-holdtime? Is a choice really > appropriate (jp-holdtime-or-multiplier)? Can I not have both > holdtime and multiplier? Perhaps I am just not clear what holdtime > does... > > - Please provide more meaningful descriptions: > > description "Propagation description"; > description "Override interval"; > > - What is the meaning of the interface augmentation 'oper-status' > relative to 'oper-status' defined by ietf-interfaces? Is this just > a duplicate with fewer states? Or is this somehow more specific to > multicast or PIM packets? In the later case, I think this deserves > to the be explained in the description clause. > > - How do the ip4 and ipv6 addresses relate to ip addresses assigned > to an interface in ietf-ip? > > - What is the meaning of hello-expiration 'not-set'? > > - What is the meaning of expiration 'not-set'? > > - Is it useful to return the uptime in seconds (which is changing on > every get that is not in the same second) or could it be an option > to report the time when something transitioned into the up state > (which is not changing)? Well, it could be that we are simply used > to uptime like objects. Anyway, the description of up-time should > make it clear what exactly is defining the state 'up'. If this > says up for 5 seconds, what exactly transitioned into an 'up' > state 5 seconds ago? > > - Is the any restriction for dr-priority or is it a full 32-bit > unsigned number space? In some vendor documentation I saw 0..65535 > with a default of 1. What do RFCs say? > > - I am not sure what the precise meaning of the error statistic > counters are. What turns an received or sent messages into an > error message? The description of grouping statistics-error does > not explain this. Also, if I receive a hello and I later decide > that this hello must have been an error, is this hello counted > twice? And what about messages that could not be parsed because > they were malformed, where are those counted? > > - Why is 'pim' a presence container? > > - Do comments like 'configuration data nodes' make sense if you > include config false nodes in the same branch of the tree? > > * PIM RP Module > > - Does the feature bsr-election-state depend on the feature bsr? > > - Should there be a default bsr-candidate/priority? > > - Do you need the address/hash-mask-length/priority in > bsr-state-attributes in an NMDA implementation? > > - I _assume_ the bsr-next-bootstrap value has to be interpreted > relative to the time the value was obtained. What about making > this an absolute timestamp instead? Well, actually I am not sure > what the value represents - is it the remaining time interval > until the next bootstrap will be sent? > > - I have no clue what to expect here: > > leaf group-policy { > type string; > description "Group policy."; > } > > What can I expect the string to contain? > > - I am again uncertain how exactly to understand the value of > rp-candidate-next-advertisement, see similar questions above. > > - What are the policy values that can go into this: > > leaf policy-name { > type string; > description > "Static RP policy."; > } > > Is the string just an arbitrary name or does it mean something? > > - How is this supposed to be used? > > leaf policy { > type string; > description > "ACL (Access Control List) policy used to filter group > addresses."; > } > > What is the meaning of <policy>foo</policy>? > > * PIM SM Module > > - What is the meaning of a policy-name value? > > leaf policy-name { > if-feature spt-switch-policy; > type string; > description > "Switch policy."; > } > > - What is the meaning of a range-policy value? > > leaf range-policy { > type string; > description > "Policy used to define SSM address range."; > } > > * PIM DM Module > > - I wonder, would you need an identity for dense mode? > > * PIM BIDIR Module > > - Remove > > /* > * Typedefs > */ > > - What is the meaning of offer-interval or backoff-interval > 'not-set'? > > * Implementation Status > > It seems the trac page pointed to was used to collect information > about what proprietary implementation support, i.e., it does not > document to what extend the models defined in this document have > been implemented. There are some notable differences. For example, > it seems most counters implemented are 32-bit while most counters in > the YANG models are 64-bit. How chatty is PIM, are 64-bit counters > really needed and are implementors willing to move to 64-bit > counters? > > * Security Considerations > > I think this needs to include a bit more details. See > > https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines > > for the latest template that YANG module authors are expected to > use. > > > _______________________________________________ > yang-doctors mailing list > yang-doctors@ietf.org > https://www.ietf.org/mailman/listinfo/yang-doctors
- [pim] Yangdoctors last call review of draft-ietf-… Jürgen Schönwälder
- Re: [pim] [yang-doctors] Yangdoctors last call re… Benoit Claise
- Re: [pim] [yang-doctors] Yangdoctors last call re… Mahesh Jethanandani
- Re: [pim] Yangdoctors last call review of draft-i… Xufeng Liu
- Re: [pim] [yang-doctors] Yangdoctors last call re… Xufeng Liu
- Re: [pim] Yangdoctors last call review of draft-i… Juergen Schoenwaelder
- Re: [pim] Yangdoctors last call review of draft-i… Xufeng Liu
- Re: [pim] Yangdoctors last call review of draft-i… tom p.
- Re: [pim] Yangdoctors last call review of draft-i… Xufeng Liu
- Re: [pim] Yangdoctors last call review of draft-i… tom p.