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

tom petch <> Mon, 31 December 2018 18:21 UTC

Return-Path: <>
Received: from localhost (localhost []) by (Postfix) with ESMTP id 0A1A51271FF; Mon, 31 Dec 2018 10:21:55 -0800 (PST)
X-Virus-Scanned: amavisd-new at
X-Spam-Flag: NO
X-Spam-Score: 3.196
X-Spam-Level: ***
X-Spam-Status: No, score=3.196 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_MED=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RATWARE_MS_HASH=2.148, RATWARE_OUTLOOK_NONAME=2.95, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001] autolearn=no autolearn_force=no
Authentication-Results: (amavisd-new); dkim=pass (1024-bit key)
Received: from ([]) by localhost ( []) (amavisd-new, port 10024) with ESMTP id JTCg_g4oIylk; Mon, 31 Dec 2018 10:21:52 -0800 (PST)
Received: from ( []) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by (Postfix) with ESMTPS id 20765126C01; Mon, 31 Dec 2018 10:21:48 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;; s=selector1-btconnect-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=eQ/NowmPJDI0JptPUkSrOOLrcGaCAODBvRQLCSalyCo=; b=XaSiapV13+d2YpSTweJB8TBX4T/+Tz+d7+JkvKggP+nlSJ9Q2Xc3TNMsl+T5uv5L0udyYrGC++wP0F09Rts4AItGTO1mR4COfRSgkMnfvw75zZ443LJvcQlQCoQ99d5OItW28uWHG6o+bVxZQv4INuDAWpHWV+lWgXrL6Ym5+Qs=
Received: from ( by ( with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1495.5; Mon, 31 Dec 2018 18:21:46 +0000
Received: from ([fe80::30d7:2d62:cf50:fb2a]) by ([fe80::30d7:2d62:cf50:fb2a%2]) with mapi id 15.20.1495.005; Mon, 31 Dec 2018 18:21:46 +0000
From: tom petch <>
To: "" <>, Ebben Aries <>, "" <>
CC: "" <>, "" <>, "Tarek Saad (tsaad)" <>
Thread-Topic: [Lsr] Yangdoctors last call review of draft-ietf-isis-yang-isis-cfg-24
Thread-Index: AQHUhkHafKiWVvfkhUKDIB9kEUVysw==
Date: Mon, 31 Dec 2018 18:21:46 +0000
Message-ID: <009101d4a135$a59c2780$>
References: <> <03ff01d48641$8f8d8600$> <19850_1543334259_5BFD6973_19850_302_6_9E32478DFA9976438E7A22F69B08FF924B7768B5@OPEXCLILMA4.corporate.adroot.infra.ftgroup>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-clientproxiedby: LO2P265CA0022.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:62::34) To (2603:10a6:208:103::17)
authentication-results: spf=none (sender IP is );
x-ms-exchange-messagesentrepresentingtype: 1
x-originating-ip: []
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; AM0PR07MB4100; 6:FD5Hl7oXLMpNBfyNZBWOFTLS95QnKcN+NJXOXHgcS04ZHOdmmEluRf2kHbqL2NvSpeZ88nckL5MUh+8dWMOxkZYKe0R/cbzYgI9QTE7EgY+Nag1w+UuwQxa+OknSc7O+5vTiAxV9EZzsAXeUdQ5ppH239wz7LQqZorRYPzpz/zqvzU1o5eoE0TIgkdZjh7QLSL5l6IidKKRPkKfNS1SSv64Dj6CSvQXpbQ3i2DGXUpHEopWOXNywuHw42RHl6w6SghhJHvLmoWb2j0WaGznNJbceRTEqbO7/4IVWvEN0L4+hCjTen4nmdNP9ih12f2gM8yFKw4T7lx02HkfQ7+X29HmOHFPCryykvdVSDJSyRahBX0VhwYJEF7QVz4DbwHFzdK7z0q+WQgR9RYN++AAHmdmTkSYfwQZHLfkOuCwYjUt1MKbM1PDC9x3ESM5HX343RleEvWtABVNMnj1+VXcFLg==; 5:fn74JQ3xj94HUqERu8980A4RAtxTEH0i+055xmaEP68+SyV+g5QjF3rkSc9qHnPMw7w6OpcutnW0uq6hwBGotMh7ZWIcvn2obd/sTs+gRz8zAvprblKnvzCqDMGHEOEEFNpoxg0Eh0ko7FrJZQtXS1hH5ccBgWEm/io93PXraW7Px2zVfrLOP6S9ZopJgOXoWBjya0/qxgiPc+BgZbPbXA==; 7:Xpg5Wyfz1SumPUQS2dMFOJjN9iWx+Ay3/6+UK+ZJy+I4yYwqIzF22H8yFJS9dlrKenUxP2cJ2WH//XtgejZHMam4g0306dYchM3eWTOxtyFIVvkqXddWg54vLa+esfV7OZ+9rZzIPfFU+bPbfKEdvw==
x-ms-office365-filtering-correlation-id: e67e807c-01f9-4eea-378f-08d66f4cd2a0
x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600109)(711020)(2017052603328)(7193020); SRVR:AM0PR07MB4100;
x-ms-traffictypediagnostic: AM0PR07MB4100:
x-microsoft-antispam-prvs: <>
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(3230021)(908002)(999002)(5005026)(6040522)(8220055)(2401047)(8121501046)(3002001)(10201501046)(3231475)(944501520)(52105112)(93006095)(93001095)(6055026)(6041310)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(20161123558120)(20161123562045)(201708071742011)(7699051)(76991095); SRVR:AM0PR07MB4100; BCL:0; PCL:0; RULEID:; SRVR:AM0PR07MB4100;
x-forefront-prvs: 0903DD1D85
x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(376002)(366004)(136003)(346002)(39860400002)(396003)(189003)(199004)(13464003)(51444003)(99286004)(66574012)(4326008)(66066001)(26005)(186003)(476003)(1941001)(84392002)(316002)(446003)(25786009)(2906002)(68736007)(486006)(110136005)(6306002)(86152003)(305945005)(966005)(53936002)(2501003)(6486002)(229853002)(54906003)(6512007)(9686003)(6436002)(7736002)(44736005)(14454004)(6506007)(8936002)(386003)(53546011)(33896004)(6246003)(102836004)(1556002)(81166006)(8676002)(105586002)(106356001)(76176011)(478600001)(14496001)(52116002)(256004)(5024004)(14444005)(86362001)(5660300001)(97736004)(81156014)(6116002)(4744004)(3846002)(71200400001)(71190400001); DIR:OUT; SFP:1102; SCL:1; SRVR:AM0PR07MB4100;; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:0;
received-spf: None ( does not designate permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: f4ki1MuRmHiKfKO8qUT7/OShjvwPCoINb2iw/zQuBsyRkkrv2QfCrIJyhuTmift9hZbxEO1OwNLYNfDp9I0PmTZznqZdvEFk3UAq/1qNRv+yBaBdjh3s0TmNUyeX6ki7lY6PtrUrrXw2d8o2IMRUY114QEEYF2Lb3ZrYAED2DjluCOEpdEK/rSgco66rmtA6QplU4Hq9oLwGuy+cJYX7fC5Yk8Q/EAwuva95IqJQoy7/tD+69oztLnAIJDINwAnObx6yI4aPHyzY93dwnCbA1ml4hoDHARd9lfSxVIs98CwO0vLKuVo28ra7MaafTvcd
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="iso-8859-1"
Content-ID: <>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: e67e807c-01f9-4eea-378f-08d66f4cd2a0
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Dec 2018 18:21:46.5791 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: cf8853ed-96e5-465b-9185-806bfe185e30
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR07MB4100
Archived-At: <>
Subject: Re: [Lsr] Yangdoctors last call review of draft-ietf-isis-yang-isis-cfg-24
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Link State Routing Working Group <>
List-Unsubscribe: <>, <>
List-Archive: <>
List-Post: <>
List-Help: <>
List-Subscribe: <>, <>
X-List-Received-Date: Mon, 31 Dec 2018 18:21:55 -0000


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


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: <>;

Hi Tom,

Thanks for your comments. I will fix them asap.
" 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.
                                                "List of max LSP
bandwidths for different
What's your suggestion on this one ?


-----Original Message-----
From: tom petch []
Sent: Tuesday, November 27, 2018 12:11
To: Ebben Aries;
Cc:;; Tarek Saad
Subject: Re: [Lsr] Yangdoctors last call review of

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


Also perhaps
    reference "RFC XXXX - YANG Data Model for Bidirectional
               Forwarding Detection (BFD).Please replace YYYY with
                                                         published RFC
number for draft-ietf-bfd-yang.";

    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.";

      reference "draft-ietf-bfd-yang-xx.txt:
                 YANG Data Model for Bidirectional Forwarding
                 Detection (BFD)";
    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
                                                "List of max LSP
bandwidths for different

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" <>;
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
> "ietf-isis@2018-08-09" module is compatible with the NMDA
> Module ietf-isis@2018-08-09.yang:
> - Both the description and the draft name reference that this module
>   specific to configuration but contains operational state nodes in
>   to RPCs and notifications.  Any wording suggesting this is only
>   configuration should be changed
> - Module description must contain most recent copyright notice per
> - Module description reads "common across all of the vendor
>   I don't think this needs to be called out as such as that is the
>   intention of *all* IETF models
> - This module contains '22' features (and the respective OSPF module
>   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
>   *all* possible features of each network-element leading to
divergence back
>   towards native models at the end of the day.  A large amount of
>   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
>   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
>   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
>   (much like in the OSPF model)
> - Child nodes within a container or list SHOULD NOT replicate the
>   identifier per
>   A case in point is the list /afs/af that has a leaf of 'af'
>       <afs>
>         <af>
>           <af>ipv4</af>
>           <enable>true</enable>
>         </af>
>       </afs>
>   Not only is this replication, but we should likely not abbreviate
'afs' if
>   we are using the expanded 'address-family' in other IETF models such
>   ietf-i2rs-rib
> General comments on the draft + nits:
> - Since YANG tree diagrams are used, please include an informative
>   per
> - Section 1.1 does not need to exist since this would be covered by
>   reference mentioned above
> - Reference to NMDA compliance should be contained within Section 1
>   Section 2) per
> - Section 2: It seems reference should be given to the location of
where the
>   ietf-routing module is defined (As well as reference to NMDA RFC in
>   above reference)
> - Section 2.1: "Additional modules may be created this to support..."
>   slight rewording adjustment
> - Section 3: The RPC operations are named 'clear-adjacency' and
>   'clear-database' rather w/ reliance off namespacing for uniqueness.
>   section refers to 'clear-isis-database' and 'clear-isis-adjacency'
> - Section 4: Notification name mismatch in this section from actual
>   within the module (e.g. 'adjacency-change' should rather be
>   'adjacency-state-change')
> - Section 7: Security Considerations will need updating to be
patterned after
>   the latest version of the template at
> per
> - Section 12: All modules imported within this module MUST be
>   within this section per
>   There are quite a few missing from this section right now
> - Appendix A: Some of the XML elements are off in alignment
> - Appendix A: Examples must be validated.  The example given has the
>   issues:
>   - /routing[name='SLI'] and /routing/description are invalid data
nodes and
>     do not exist.  I'm not sure why they are in the XML example here
>   - The example is meant to reference configuration however
>     /routing/interfaces is a r/o container
>   - The control-plane-protocol 'type' needs to be qualified - e.g.
>     <type
>   - The area-address does not validate against the pattern regex and
must end
>     with a '.'  e.g.
>     <area-address>49.0001.0000.</area-address>
>   - metric-type/value is set to 'wide' which is invalid.  This should
>     be 'wide-only'
>   - isis/afs/af/af is set to 'ipv4-unicast' which is invalid.  This
>     rather be 'ipv4' per iana-routing-types
>   - /interfaces/interface/type must be populated and is invalid.  This
>     rather be qualified as such:
>     <type
>     <type
>   - /interfaces/interface/link-up-down-trap-enable must have a value
>     associated as such:
>     <link-up-down-trap-enable>enabled</link-up-down-trap-enable>
>   - NP container 'priority' has a must statement checking if an
>     is set to 'broadcast' however if you take the XML example from
>     section, it will fail to validate even if <priority> is not
>     underneath an interface-type of 'point-to-point'.  It seems to me
>     this logic may need to be readjusted or not exist at all (priority
>     still be set on implementations on loopback interfaces - which
>     default to 'broadcast' in the example here).  Could you not solve
>     with use of 'when' vs. 'must' as such:
>           when '../interface-type = "broadcast"' {
>               description "Priority can only be set for broadcast
>           }
>   - /interfaces/interface/ipv4/mtu must contain a valid value (and
likely not
>     need to be defined for Loopback0)
>   - 'isis/mpls-te/ipv4-router-id' is invalid and should rather be
>     'isis/mpls/te-rid/ipv4-router-id'
>   - 'isis/afs/af/enabled' is invalid and should rather be
>   - Examples should use IPv6 addresses where appropriate per


Ce message et ses pieces jointes peuvent contenir des informations
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez
recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme
ou falsifie. Merci.

This message and its attachments may contain confidential or privileged
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and
delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have
been modified, changed or falsified.
Thank you.