Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-path-computation-11

Italo Busi <Italo.Busi@huawei.com> Wed, 24 February 2021 11:52 UTC

Return-Path: <Italo.Busi@huawei.com>
X-Original-To: teas@ietfa.amsl.com
Delivered-To: teas@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5E6CA3A1479; Wed, 24 Feb 2021 03:52:55 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=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 hJC_Y6ilBdFB; Wed, 24 Feb 2021 03:52:53 -0800 (PST)
Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 6FC423A1490; Wed, 24 Feb 2021 03:52:52 -0800 (PST)
Received: from fraeml713-chm.china.huawei.com (unknown [172.18.147.207]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4DlvHb4m99z67qZn; Wed, 24 Feb 2021 19:45:31 +0800 (CST)
Received: from fraeml715-chm.china.huawei.com (10.206.15.34) by fraeml713-chm.china.huawei.com (10.206.15.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Wed, 24 Feb 2021 12:52:50 +0100
Received: from fraeml715-chm.china.huawei.com ([10.206.15.34]) by fraeml715-chm.china.huawei.com ([10.206.15.34]) with mapi id 15.01.2106.006; Wed, 24 Feb 2021 12:52:50 +0100
From: Italo Busi <Italo.Busi@huawei.com>
To: 'tom petch' <ietfa@btconnect.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>, =?iso-8859-1?Q?Martin_Bj=F6rklund?= <mbj+ietf@4668.se>
CC: "draft-ietf-teas-yang-path-computation.all@ietf.org" <draft-ietf-teas-yang-path-computation.all@ietf.org>, "teas@ietf.org" <teas@ietf.org>
Thread-Topic: [Teas] Yangdoctors early review of draft-ietf-teas-yang-path-computation-11
Thread-Index: AQHW1KapF2DOK+K1Zk2HJGFUSzAY5KpndC2AgAApVwA=
Date: Wed, 24 Feb 2021 11:52:50 +0000
Message-ID: <90431360dc9641f494264cc77d2969b0@huawei.com>
References: <160823152245.15025.13731491639863650578@ietfa.amsl.com> <DB7PR07MB5546474ADCC583030EFDF7BEA29F9@DB7PR07MB5546.eurprd07.prod.outlook.com>
In-Reply-To: <DB7PR07MB5546474ADCC583030EFDF7BEA29F9@DB7PR07MB5546.eurprd07.prod.outlook.com>
Accept-Language: it-IT, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.47.85.8]
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-CFilter-Loop: Reflected
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/6DNdjKPwYFNjYlTN3MkktPg7bM4>
Subject: Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-path-computation-11
X-BeenThere: teas@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Traffic Engineering Architecture and Signaling working group discussion list <teas.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/teas>, <mailto:teas-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/teas/>
List-Post: <mailto:teas@ietf.org>
List-Help: <mailto:teas-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/teas>, <mailto:teas-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 24 Feb 2021 11:52:55 -0000

Hi Tom,

Yes, we have discussed this issue with the authors of ietf-te and the fix has been published in the -26 draft revision

Now the IETF datatracker reports no errors/warnings for the path-computation draft

Thanks to you and Martin for spotting it

Italo

> -----Original Message-----
> From: tom petch [mailto:ietfa@btconnect.com]
> Sent: mercoledì 24 febbraio 2021 11:24
> To: yang-doctors@ietf.org; Martin Björklund <mbj+ietf@4668.se>
> Cc: draft-ietf-teas-yang-path-computation.all@ietf.org; teas@ietf.org
> Subject: Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-path-
> computation-11
> 
> From: Teas <teas-bounces@ietf.org> on behalf of Martin Björklund via
> Datatracker <noreply@ietf.org>
> Sent: 17 December 2020 18:58
> 
> Reviewer: Martin Björklund
> Review result: Ready with Nits
> 
> <tp>
> 
> Martin's review flagged an error in the ietf-te module whereby a path
> statement lacked prefixes causing validation to fail  That module is updated in
> teas-yang-te-26 to add those missing prefix which may overcome  that issue.
> 
> Tom Petch
> 
> o  General
> 
>   The language is called "YANG", not "Yang".
> 
> 
> o  1.2. Tree Diagram
> 
>   The text says:
> 
>     A simplified graphical representation of the data model is used in
>     section 6.1 of this this document.  The meaning of the symbols in
>     these diagrams is defined in [RFC8340].
> 
>   Tree diagrams are used also in chapter 5.  I suggest:
> 
>     Tree diagrams used in this document follow the notation defined in
>     [RFC8340].
> 
> 
> o  Tree diagrams in general
> 
>    You can use pyang -f tree --tree-line-length 68 ... in order to
>    avoid long lines in the RFC.
> 
> 
> o  6.1
> 
>   This section presents a fully expanded tree diagram of the module.
>   Tree diagrams are mainly used to give an overview of a module's
>   structure.  The tree diagram in this section spans 13 pages and is
>   quite hard to read.
> 
>   I also note that a majority of the nodes in this tree diagram come
>   from the expansion of groupings that aren't defined in this
>   document.  Hence, I suggest that you might want to run:
> 
>     pyang -f tree --tree-line-length 68 \
>       --tree-print-groupings --tree-no-expand-uses
> 
> 
> o  There are a number of groupings that are used only once, and do not
>    seem to be defined to be reused by other modules, e.g.,
>    "requested-info", "requested-state", "svec-metrics-bounds" and more.
> 
>    If they are intended to be reused, it should be made clear in their
>    description statements.  If not, I think they should be inlined and
>    removed.
> 
> 
> o  grouping svec-exclude
> 
>    This grouping has an ordered-by user list.  Why is this list user
>    ordered?  If the order matters, it should be explained how it matters.
> 
>    Also, the index leaf has this description:
> 
>      "XRO subobject index"
> 
>    What is "XRO"?  Is this description sufficiently clear?
> 
> 
> o  path-request
> 
>    In the path-request, there is construct for path-refs:
> 
>                    list primary-reverse-path-ref {
>                      key index;
>                      min-elements 1;
>                      description
>                        "The list of primary reverse paths that
>                         reference this path as a candidate
>                         secondary reverse path";
>                      leaf index {
>                        type uint32;
>                        description
>                          "The index used by the
>                           primary-reverse-path-ref list";
>                      }
> 
> 
>   What is this index?  Is it only used as an arbitrary index, or
>   something else?  If it is an arbitraty index, it should be explained
>   in the descriptions.
> 
>   Also note that lists in rpc input don't need an index.
> 
> 
> o  Validation
> 
>    The module fails YANG validation, but that is really due to errors
>    in ietf-te@2020-07-12.yang.  Specifially, the leafref in the
>    grouping "path-compute-info" must have prefixes in its path.
>    Without prefixes, the path refers to nodes in the module that uses
>    the grouping.  (same for other groupings in that module).
> 
> 
> o  Layout
> 
>   I suggest you run the module through
> 
>     pyang -f yang --yang-canonical --yang-line-length 68
> 
>   in order to have the module indented and formatted consistently.
> 
>   This will make the RFC editor's job easier.
> 
> 
> 
> /martin
> 
> 
> 
> _______________________________________________
> Teas mailing list
> Teas@ietf.org
> https://www.ietf.org/mailman/listinfo/teas