Re: [Lsr] [yang-doctors] Yangdoctors last call review of draft-ietf-isis-yang-is-is-cfg-29

"Acee Lindem (acee)" <acee@cisco.com> Thu, 10 January 2019 12:42 UTC

Return-Path: <acee@cisco.com>
X-Original-To: lsr@ietfa.amsl.com
Delivered-To: lsr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8C85C130E13; Thu, 10 Jan 2019 04:42:14 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.642
X-Spam-Level:
X-Spam-Status: No, score=-14.642 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_MED=-0.142, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, 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 jZ25-NLNck2Y; Thu, 10 Jan 2019 04:42:12 -0800 (PST)
Received: from alln-iport-2.cisco.com (alln-iport-2.cisco.com [173.37.142.89]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A83C3129AB8; Thu, 10 Jan 2019 04:42:11 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=30348; q=dns/txt; s=iport; t=1547124131; x=1548333731; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=KizDDvy7607klZLf8/Jz6ArgqB9L0sJfTcTVCo0WWgQ=; b=LNhEmt8/lZRP4xUeUP4rScTfRrJ4KFMC79+m6g5YSqkjStVsLVkZknLs YLtANoGVZeBvAjTYDVOob9vwSv45Vovgm83QvYCIoX6twfa7Q1I8a52Fn AUZerfLjBxjPNdSBV3IbSLAy8JEcjKWdXX9/TjkjLzBC+eEPgUmZN5Adr 0=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0AFAAC7PDdc/49dJa1gAxkBAQEBAQE?= =?us-ascii?q?BAQEBAQEHAQEBAQEBgVIDAQEBAQELAYIDZoECJwqDdpQIgg2XdxSBZwMIAQE?= =?us-ascii?q?jhEkCF4IMIjUIDQEDAQECAQECbRwMhUoBAQEDASMRRQwEAgEIEAEBAwEBAQI?= =?us-ascii?q?CFA8DAgICMBQBAgYIAgQBDQWDIgGBeQgPqmsFgT2BL4o0BTRXiGyBBoFCF4F?= =?us-ascii?q?/gRABJx+BTlAugx4BAQIBgSMIAQwGAQkCKw8bBgiCOjGCBCICiTBYgUiEJZI?= =?us-ascii?q?ACQKHF4Z1g3EYgWQjhQGKdIluhQ6ILIMSAhEUgSchATVlcXAVOyoBgkGCJgE?= =?us-ascii?q?BFhOITIU/QTEBiAABBggVAoEIgR8BAQ?=
X-IronPort-AV: E=Sophos;i="5.56,461,1539648000"; d="scan'208";a="224045138"
Received: from rcdn-core-7.cisco.com ([173.37.93.143]) by alln-iport-2.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Jan 2019 12:42:09 +0000
Received: from XCH-RTP-013.cisco.com (xch-rtp-013.cisco.com [64.101.220.153]) by rcdn-core-7.cisco.com (8.15.2/8.15.2) with ESMTPS id x0ACg9Hw014275 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=FAIL); Thu, 10 Jan 2019 12:42:09 GMT
Received: from xch-rtp-015.cisco.com (64.101.220.155) by XCH-RTP-013.cisco.com (64.101.220.153) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Thu, 10 Jan 2019 07:42:08 -0500
Received: from xch-rtp-015.cisco.com ([64.101.220.155]) by XCH-RTP-015.cisco.com ([64.101.220.155]) with mapi id 15.00.1395.000; Thu, 10 Jan 2019 07:42:08 -0500
From: "Acee Lindem (acee)" <acee@cisco.com>
To: Andy Bierman <andy@yumaworks.com>, Stephane Litkowski <stephane.litkowski@orange.com>
CC: Juergen Schoenwaelder <j.schoenwaelder@jacobs-university.de>, tom petch <ietfc@btconnect.com>, Ebben Aries <exa@juniper.net>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "draft-ietf-isis-yang-isis-cfg.all@ietf.org" <draft-ietf-isis-yang-isis-cfg.all@ietf.org>, "lsr@ietf.org" <lsr@ietf.org>
Thread-Topic: [yang-doctors] [Lsr] Yangdoctors last call review of draft-ietf-isis-yang-is-is-cfg-29
Thread-Index: AQHUooyeHlP19WSsQ0KaVvaziE5yuqWm7nFg///0qYCAAEJRAIABE5KwgACYfAD//604gA==
Date: Thu, 10 Jan 2019 12:42:08 +0000
Message-ID: <91DE5DF2-1D49-4D5A-80AF-0975A7B4C686@cisco.com>
References: <154025553381.13801.5009678921928527816@ietfa.amsl.com> <03ff01d48641$8f8d8600$4001a8c0@gateway.2wire.net> <19850_1543334259_5BFD6973_19850_302_6_9E32478DFA9976438E7A22F69B08FF924B7768B5@OPEXCLILMA4.corporate.adroot.infra.ftgroup> <009101d4a135$a59c2780$4001a8c0@gateway.2wire.net> <009101d4a28c$905b2300$4001a8c0@gateway.2wire.net> <7264_1546852922_5C331A39_7264_253_1_9E32478DFA9976438E7A22F69B08FF924B78A3A5@OPEXCLILMA4.corporate.adroot.infra.ftgroup> <009901d4a779$cc96bfe0$4001a8c0@gateway.2wire.net> <1676_1547025526_5C35BC76_1676_161_1_9E32478DFA9976438E7A22F69B08FF924B78C4EC@OPEXCLILMA4.corporate.adroot.infra.ftgroup> <01ab01d4a80f$aa07ca00$4001a8c0@gateway.2wire.net> <15144_1547038327_5C35EE77_15144_333_1_9E32478DFA9976438E7A22F69B08FF924B78C5C4@OPEXCLILMA4.corporate.adroot.infra.ftgroup> <20190109130859.mvjj7gkr4vyh6umt@anna.jacobs.jacobs-university.de> <CABCOCHRkpgPV-D5knoZjT+HiJtAb1h_EEqHmW=syFvQLRaYSLw@mail.gmail.com> <18406_1547109255_5C370387_18406_259_1_9E32478DFA9976438E7A22F69B08FF924B78CB16@OPEXCLILMA4.corporate.adroot.infra.ftgroup> <CABCOCHTryWZnQ3_XQquSF-Zg9AaRx-BEUjhZUHQK=b=ZVBa=bQ@mail.gmail.com>
In-Reply-To: <CABCOCHTryWZnQ3_XQquSF-Zg9AaRx-BEUjhZUHQK=b=ZVBa=bQ@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.116.152.196]
Content-Type: text/plain; charset="utf-8"
Content-ID: <4A76B8545C52024397FCD78D7AFCCF89@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-Outbound-SMTP-Client: 64.101.220.153, xch-rtp-013.cisco.com
X-Outbound-Node: rcdn-core-7.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/Ga_vxOrQi_mPSXKL7K-v6QYP6Ss>
Subject: Re: [Lsr] [yang-doctors] Yangdoctors last call review of draft-ietf-isis-yang-is-is-cfg-29
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:lsr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsr/>
List-Post: <mailto:lsr@ietf.org>
List-Help: <mailto:lsr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:lsr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 10 Jan 2019 12:42:15 -0000

Hi Andy, 

On 1/10/19, 7:38 AM, "Andy Bierman" <andy@yumaworks.com>; wrote:

    On Thu, Jan 10, 2019 at 12:34 AM <stephane.litkowski@orange.com>; wrote:
    
    > Hi Andy,
    >
    >
    >
    > What I’m still not catching is the difference you make between having a
    > description statement telling :” A server MUST accept a string up to 64
    > characters in length” and a type string with length “0..64” ?
    >
    > There is probably something that I’m missing here.
    >
    >
    >
    
    A server MAY accept a string longer than 64 characters.
    A range 0..64 means a server MUST NOT accept a string longer than 64
    Characters

I don't think we should set the string range unless the range in specified in the protocol RFCs. In some cases, it may be beneficial to provide guidance in the description. I believe this is similar to Juergen's position. 

Thanks,
Acee
    
    
    
    > Brgds,
    >
    >
    >
    
    
    Andy
    
    
    > *From:* Andy Bierman [mailto:andy@yumaworks.com]
    > *Sent:* Wednesday, January 09, 2019 18:06
    > *To:* Juergen Schoenwaelder; LITKOWSKI Stephane OBS/OINIS; tom petch;
    > Ebben Aries; yang-doctors@ietf.org;
    > draft-ietf-isis-yang-isis-cfg.all@ietf.org; lsr@ietf.org
    > *Subject:* Re: [yang-doctors] [Lsr] Yangdoctors last call review of
    > draft-ietf-isis-yang-is-is-cfg-29
    >
    >
    >
    > Hi,
    >
    >
    >
    > I agree with Juergen.
    >
    > The protocol has a "too-big" error is the server will not accept a big
    > string.
    >
    > There should not be a false choice between an arbitrary maximum and
    > "terabytes".
    >
    >
    >
    > You cannot use a range-stmt to specify the minimum required length that
    > must be supported.
    >
    > length "3..max" does not allow strings of length 0 - 2.
    >
    >
    >
    > The description-stmt can have "A server MUST accept a string up to 64
    > characters in length"
    >
    > which lets the client choose a length from 0 to 64, and this will work on
    > all server implementations.
    >
    >
    >
    > Andy
    >
    >
    >
    >
    >
    > On Wed, Jan 9, 2019 at 5:10 AM Juergen Schoenwaelder <
    > j.schoenwaelder@jacobs-university.de>; wrote:
    >
    > Hi,
    >
    > please see my other email about the distinction I make between a hard
    > length restriction and the minimum length expected to be supported.  I
    > wonder how you can sensibly pick a limit for things like non-best-reason:
    >
    >         leaf non-best-reason {
    >           type string;
    >           description
    >             "Information field to describe why the alternate
    >              is not best.";
    >         }
    >
    > You are simply creating an arbitrary restriction. And humble server is
    > not likely to send you an jpg image (and a bogus server will do so
    > anyway). (There are other similar objects.)
    >
    > Since I am searching for 'type string', I wonder whether these are
    > clear enough definitions.
    >
    >         leaf prefix {
    >           type string;
    >           description
    >             "Protected prefix.";
    >         }
    >         leaf alternate {
    >           type string;
    >           description
    >             "Alternate nexthop for the prefix.";
    >         }
    >
    > What is the (canonical) format of the allowed values? (There are more of
    > these.)
    >
    > /js
    >
    > On Wed, Jan 09, 2019 at 12:52:07PM +0000, stephane.litkowski@orange.com
    > wrote:
    > > Hi Tom,
    > >
    > > If you agree, I will set a length restriction on each string (ops and
    > cfg). It looks clearer for me rather than setting it in the description.
    > >
    > > For the references, I'm working on it.
    > >
    > > Brgds,
    > >
    > >
    > > -----Original Message-----
    > > From: tom petch [mailto:ietfc@btconnect.com]
    > > Sent: Wednesday, January 09, 2019 12:38
    > > To: LITKOWSKI Stephane OBS/OINIS; Ebben Aries; yang-doctors@ietf.org
    > > Cc: draft-ietf-isis-yang-isis-cfg.all@ietf.org; lsr@ietf.org
    > > Subject: Re: [Lsr] Yangdoctors last call review of
    > draft-ietf-isis-yang-is-is-cfg-29
    > >
    > > Stephane
    > >
    > > Thanks for persisting.
    > >
    > > On string lengths, I take your point about no user input to Operational
    > > State; there, my concern is more about giving guidance to implementors
    > > as to what they should cater for - as I said, this has been a topic of
    > > lively discussion in other WG.  Even before that, whenever I see a
    > > string, I think is there an implicit length restriction and if not,
    > > should there be an explicit one which, as Juergen suggested, could be in
    > > the description clause.  My experience is that those working with
    > > networks think about size, about length; those coming from applications
    > > tend to think 'What is a few terabytes between friends?' and are unaware
    > > that sizes that may be commonplace in servers and associated storage can
    > > create difficulties over a network.  Whatever, I leave this one up to
    > > you.
    > >
    > > On references, I would like a change; you say this information is in the
    > > base ISO spec.  Well, yes, to me that means that it should be a
    > > Normative Reference.  I could not understand the description of e.g.
    > > 'i/e' and needed to look it up but seemingly cannot do so with the
    > > listed references of the I-D.  Note that RFC such as RFC5305 and RFC6119
    > > do reference International Standard 10589 and I think that this one
    > > should too, perhaps in s.2.7 and s.5.
    > >
    > > Tom Petch
    > >
    > > ----- Original Message -----
    > > From: <stephane.litkowski@orange.com>;
    > > Sent: Wednesday, January 09, 2019 9:18 AM
    > >
    > > Hi Tom,
    > >
    > > Please find inline answers.
    > >
    > >
    > > -----Original Message-----
    > > From: tom petch [mailto:ietfc@btconnect.com]
    > > Sent: Tuesday, January 08, 2019 18:45
    > >
    > > Ok.  Top-posting the ones that are not 'ok':
    > >
    > > I said that I thought that LSP did not need expanding on first use and
    > > then checked the RFC Editor list to find that it is not one they regard
    > > as well-known and that LSR protocols use it differently to others, so I
    > > suggest expanding LSP on first use.
    > >
    > > [SLI] Already done for the next version.
    > >
    > > On lengths of text messages, perhaps I am too sensitive to buffer
    > > overrun attacks and the like and so want a maximum on many things (and
    > > then people attach a friendly, 20Mbyte photo to their e-mail at
    > > Christmas and
    > > wonder why their ESP rejects the message so I do not congratulate them
    > > on the latest addition to the family:-).  The IDR WG had a lively
    > > discussion about maximum message lengths in 2017 and that has also
    > > stayed in my mind.  I have seen the comments on this by Juergen and
    > > Lada; perhaps as Juergen intimates, something in the Description would
    > > help; and while the server may not be rogue, it may not have a perfect
    > > implementation.
    > >
    > > [SLI] What I need to understand from your comment on string length
    > > enforcement is if it applies to operational state or just config states
    > > ? I do not see any issue of not enforcing the operational state as there
    > > is no input from the user there and so no attack vector, this is purely
    > > internal to the implementation. For config statements, it makes sense as
    > > there is an input from the user that can be anything.
    > >
    > >
    > > On the length of password, I saw a Security AD wanting clarification on
    > > this not too long ago so you may see this comment again from one such .
    > > Likewise, MD5 tends to be a red flag although I see it appears in bgp
    > > yang.
    > >
    > > I like the sort of detail in ippm-twamp-yang, on algorithms, entropy and
    > > such like (although I have not seen a review by Security AD/directorate
    > > of that).
    > >
    > > But I am left confused as to exactly what the cited object is doing.
    > > Yes, TLV10 provides authentication for any PDU, but what are the fields
    > > in the YANG module doing?  Is
    > >        leaf authentication-type {
    > > the first octet of TLV10?  Is
    > >       leaf authentication-key {
    > > the rest of TLV10?  And where is this 'presented' as the YANG module
    > > says?  Are you thinking of a YANG client presenting the field to a user
    > > at a terminal, one router presenting it to another, or what?
    > >
    > > I am using RFC5310 as my source for TLV10 and wondering why that is not
    > > a Normative Reference for this I-D
    > >
    > > [SLI] TLV 10 is defined in the base ISO spec of IS-IS. RFC5310 just adds
    > > the crypto auth as new types.
    > > The authentication-type is the first byte of the TLV (called
    > > Authentication Type as well).
    > > The authentication-key cannot be mapped directly to the
    > > authentication-value field. This is the case for ClearText password
    > > authtype but not crypto which adds a keyID in front of the
    > > authentication data.
    > >
    > >
    > > On the I/E bit, the question is, which standard?  I have 30 Normative
    > > references to choose from.   I found up/down in RFC5305, but only by
    > > accident, and I have not found i/e yet so a reference would be good.
    > >
    > > [SLI] I/E bit is part of the base ISO spec of IS-IS and relevant for
    > > "legacy advertisements" of prefix and links. By the way, after checking,
    > > the position of the i-e leaf is currently wrong and needs to be within
    > > the metric (default, delay...). I will fix this.
    > >
    > >
    > > Tom Petch
    > >
    > > ----- Original Message -----
    > > From: stephane.litkowski@orange.com
    > > Sent: Monday, January 07, 2019 9:22 AM
    > >
    > > Hi Tom,
    > >
    > > Thanks for your comments.
    > > I wish you an happy new year !
    > >
    > > Please find inline comments.
    > >
    > > Brgds,
    > >
    > > -----Original Message-----
    > > From: tom petch [mailto:ietfc@btconnect.com]
    > > Sent: Wednesday, January 02, 2019 12:17
    > >
    > > Here are the rest of my comments on -29 with a slight tweak to the
    > > subject line.  I would regard most of these (but not the first two) as
    > > non-discussable, ie I won't complain if you disagree:-)
    > >
    > > RFC1195 is in the YANG module but not the references of the I-D
    > > [SLI] Will fix it
    > >
    > > RFC5029 is in the YANG module but not the references of the I-D
    > > [SLI] Will fix it
    > >
    > > PSNPs, CSNPs
    > > [SLI] Will fix it
    > >
    > > expand on first use - LSP I think ok
    > >
    > >         leaf best {           type boolean;
    > > what is true and what false?  I can guess from the English semantics of
    > > the name but would rather not guess.
    > > [SLI] Will fix it
    > > To replace the current description which is : ""Indicates if the
    > > alternate is the preferred."", do you prefer: "Set to true when the
    > > alternate is preferred, set to false otherwise" ?
    > >
    > >
    > >         leaf non-best-reason {          type string;
    > > suggest a maximum length, perhaps 127 or 255 ( unless you expect
    > > screenshots or packet traces to be attached).  As it stands, you could
    > > validly receive
    > > a length of 18446744073709551615.
    > > [SLI] Agree, will fix it
    > >
    > > You have a mixture of
    > > System-id system-id System id System ID System Id system id system ID
    > > suggest consistency; system-id wfm
    > > [SLI] Will fix it
    > >
    > > You have a mixture of
    > > lsp-id LSPID LSP ID
    > > here, perhaps lsp-id for the names and LSP ID in the text
    > > [SLI] Will fix it
    > >
    > >       case password {        leaf key {           type string;
    > > perhaps better with a minimum length
    > > [SLI] I agree that it could make sense but is it really something that
    > > we should impose ?
    > >
    > >
    > >         leaf i-e {          type boolean;
    > > what is true and what false?  here I am reluctant even to guess
    > > [SLI] This is coming from the standard, is it really worth repeating it
    > > ? Same for up/down bit.
    > >
    > >
    > > /"Authentication keyto/  "Authentication key to/
    > >
    > > "     the authentication key MUST NOT be presented in"
    > > RFC2119 language means that RFC2119 boilerplate should be in the YANG
    > > module (but without the [..] ie the reference must be plain text not an
    > > anchor).
    > >
    > > [SLI] You are right, it is missing.
    > >
    > >
    > >  It is recommended to use an MD5
    > >            hash to present the authentication-key.";
    > > Mmm I think that this may be a red flag to security AD or directorate as
    > > being too vague as well as MD5 too weak; and I think this should be
    > > explicitly called out in Security Considerations.
    > >
    > > [SLI] I agree that there is a point to discuss here. The fact is that we
    > > must not retrieve passwords in clear text. Maybe it is something with a
    > > wider scope than IS-IS. How do the other models deal with passwords
    > > retrieved through "get" or "get-config" ?
    > >
    > >
    > >       list level-db {        key level;        leaf level {
    > > A common convention is for a list of leaf thing to be named things i.e.
    > >       list levels {         key level;        leaf level {
    > >
    > > [SLI] ack
    > >
    > >   rpc clear-adjacency {
    > >           "Name of the IS-IS protocol instance whose IS-IS
    > >            information is being queried.
    > > queried or cleared?
    > > [SLI] "cleared"
    > >
    > > Tom Petch
    > >
    > >
    > > ----- Original Message -----
    > > From: "tom petch" <ietfc@btconnect.com>;
    > > Sent: Monday, December 31, 2018 6:21 PM
    > >
    > >
    > > > Stephane
    > > >
    > > > A new and different comment.
    > > >
    > > >   grouping neighbor-gmpls-extensions {
    > > >
    > > > has a text reference to RFC5307 which does not appear in the
    > > references
    > > > for the I-D.  However, before adding it, I would like to know why it
    > > is
    > > > a good reference for switching capabilities (which is part of this
    > > > grouping).  I think that the reference for switching capabilities
    > > should
    > > > be RFC7074 (which this I-D does not currently reference and should
    > > IMO).
    > > >
    > > > And that begs the  question, why is switching-capability an
    > > unrestricted
    > > > uint8 when only 12 values are valid and three are deprecated?
    > > >
    > > > So why not use
    > > >
    > > > draft-ietf-teas-yang-te-types?
    > > >
    > > > I have a number of additional comments on cfg-29 but this is the one
    > > > that may take some discussion.
    > > >
    > > > Tom Petch
    > > >
    > > >
    > > > ----- Original Message -----
    > > > From: <stephane.litkowski@orange.com>;
    > > >
    > > > Hi Tom,
    > > >
    > > > Thanks for your comments. I will fix them asap.
    > > > Regarding:
    > > > " Line length is within the RFC limit but the effect is to spread many
    > > > of the description clauses over multiple lines with indentation of 56
    > > > characters, not user friendly e.g.
    > > >                                         description
    > > >                                                 "List of max LSP
    > > > bandwidths for different
    > > >                                                  priorities.";
    > > > "
    > > > What's your suggestion on this one ?
    > > >
    > > > Brgds,
    > > >
    > > > -----Original Message-----
    > > > From: tom petch [mailto:ietfc@btconnect.com]
    > > > Sent: Tuesday, November 27, 2018 12:11
    > > > Subject: Re: [Lsr] Yangdoctors last call review of
    > > > draft-ietf-isis-yang-isis-cfg-24
    > > >
    > > > Some quirks in-25
    > > >
    > > > I see lots of YANG reference statements - good - but no mention of
    > > them
    > > > in the I-D references - not so good.  My list is
    > > >
    > > > 5130
    > > > 5305
    > > > 5306
    > > > 5880
    > > > 5881
    > > > 6119
    > > > 6232
    > > > 7794
    > > > 7810
    > > > 7917
    > > > 8405
    > > >
    > > > Also perhaps
    > > > OLD
    > > >     reference "RFC XXXX - YANG Data Model for Bidirectional
    > > >                Forwarding Detection (BFD).Please replace YYYY with
    > > >                                                          published RFC
    > > > number for draft-ietf-bfd-yang.";
    > > >
    > > > NEW
    > > >     reference "RFC YYYY - YANG Data Model for Bidirectional
    > > >                Forwarding Detection (BFD).
    > > >
    > > > -- Note to RFC Editor Please replace YYYY with published RFC
    > > > number for draft-ietf-bfd-yang.";
    > > >
    > > > OLD
    > > >       reference "draft-ietf-bfd-yang-xx.txt:
    > > >                  YANG Data Model for Bidirectional Forwarding
    > > >                  Detection (BFD)";
    > > > NEW
    > > >     reference "RFC YYYY - YANG Data Model for Bidirectional
    > > >                Forwarding Detection (BFD).
    > > >
    > > > -- Note to RFC Editor Please replace YYYY with published RFC
    > > > number for draft-ietf-bfd-yang.";
    > > >
    > > >
    > > > Line length is within the RFC limit but the effect is to spread many
    > > of
    > > > the description clauses over multiple lines with indentation of 56
    > > > characters, not user friendly
    > > > e.g.
    > > >                                         description
    > > >                                                 "List of max LSP
    > > > bandwidths for different
    > > >                                                  priorities.";
    > > >
    > > >
    > > > Acknowledgements is TBD. I note that the editor list of the YANG
    > > module
    > > > is somewhat longer than the editor list of the I-D.
    > > >
    > > > I note that the augmentation of interfaces seems to have no
    > > conditional
    > > > and so will augment all interfaces. I think that this is a generic
    > > issue
    > > > but do not see it being addressed anywhere.
    > > >
    > > > In a similar vein, you are defining MPLS objects and I am unclear
    > > > whether or not those should be conditional, or part of the MPLS YANG
    > > > modules or both (copying Tarek for this)
    > > >
    > > > Tom Petch
    > > >
    > > > ----- Original Message -----
    > > > From: "Ebben Aries" <exa@juniper.net>;
    > > > Sent: Tuesday, October 23, 2018 12:45 AM
    > > >
    > > > > Reviewer: Ebben Aries
    > > > > Review result: On the Right Track
    > > > >
    > > > > 1 module in this draft:
    > > > > - ietf-isis@2018-08-09.yang
    > > > >
    > > > > No YANG compiler errors or warnings (from pyang 1.7.5 and yanglint
    > > > 0.16.54)
    > > > >
    > > > > "ietf-isis@2018-08-09" module is compatible with the NMDA
    > > > architecture.
    > > > >
    > > > > Module ietf-isis@2018-08-09.yang:
    > > > > - Both the description and the draft name reference that this module
    > > > is
    > > > >   specific to configuration but contains operational state nodes in
    > > > addition
    > > > >   to RPCs and notifications.  Any wording suggesting this is only
    > > > >   configuration should be changed
    > > > > - Module description must contain most recent copyright notice per
    > > > >
    > > >
    > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.1
    > > > > - Module description reads "common across all of the vendor
    > > > implementations".
    > > > >   I don't think this needs to be called out as such as that is the
    > > > overall
    > > > >   intention of *all* IETF models
    > > > > - This module contains '22' features (and the respective OSPF module
    > > > currently
    > > > >   contains '26').  While it is understood the purpose of these
    > > > features in the
    > > > >   module, take precaution as to complexity for clients if they need
    > > to
    > > > >   understand >= quantity of features per module in use on a
    > > > >   network-element.  We are going to end up w/ feature explosion to
    > > > convey
    > > > >   *all* possible features of each network-element leading to
    > > > divergence back
    > > > >   towards native models at the end of the day.  A large amount of
    > > > these
    > > > >   feature names could be defined within a more global namespace
    > > (e.g.
    > > > nsr) but
    > > > >   this gives us a granular yet cumbersome approach (e.g. feature
    > > > isis:nsr,
    > > > >   ospf:nsr, etc..)
    > > > > - RPC 'clear-adjacency' does not have any input leaf that covers
    > > > clearing a
    > > > >   specific neighbor/adjacency (See comments below as well regarding
    > > > RPC
    > > > >   alignment w/ the OSPF model)
    > > > > - RPC 'clear-adjacency' has an input node of 'interface' however
    > > this
    > > > is just
    > > > >   a string type.  Is there any reason this is not a
    > > > leafref/if:interface-ref
    > > > >   (much like in the OSPF model)
    > > > > - Child nodes within a container or list SHOULD NOT replicate the
    > > > parent
    > > > >   identifier per
    > > > >
    > > >
    > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-4.3