Subject: Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-sr-te-topo-04

From: Xufeng Liu <[email-masked]>
Date: Tue, 9 Jul 2019 11:56:33 -0400

Hi Lada,

Thanks for the review. We are addressing some of the issues by
https://tools.ietf.org/html/draft-ietf-teas-yang-sr-te-topo-05. Some of the
discussions are in-line below.

Best regards,
- Xufeng

On Mon, Jun 10, 2019 at 9:02 AM Ladislav Lhotka via Datatracker <
[email-masked]> wrote:

> Reviewer: Ladislav Lhotka
> Review result: Ready with Issues
>
> This document and YANG module "ietf-sr-topology" contained therein
> contribute two new types to the family of network topology data
> models: segment routing and segment routing traffic engineering
> topologies. The document also provides a companion module
> "ietf-sr-topology-state" intended for non-NMDA implementations.
>
> >From the YANG point of view, both modules are in a relatively good
> shape. Also, as far as I can tell, they satisfy the requirements
> specified in RFC 8345.
>
> The example instance data contained in Appendix B successfully passes
> formal validation against the date model, which is far from
> commonplace for early versions of documents. See however comment #1
> below.
>
> **** Comments
>
> 1. I have argued several times that using URIs as identifiers of all
>    network topology objects in RFC 8345 is an overkill.

Xufeng]: Agree with you on this, but since the type has been published we
need to figure out the way to fit into it. It is clear for what we need to
do here in this case, so it is appreciated if you can provide a suggestion.

> Now, the
>    present draft demonstrates the consequences: id values used in
>    many places of the example data (Appendix B) are not valid URIs!
>    For example, "link-id" leaves have values like
>    "D1,1-2-1,D2,2-1-1", which is not a URI.
>
>    This type violation isn't caught by validating tools because the
>    "ietf-inet-types:uri" type doesn't specify a regular expession
>    pattern. However, its description states clearly that the type
>    represents URI as defined by STD 66.
>
[Xufeng]: We know that this is not a typical URI, but many of the syntax
components are not relevant here. In term of this particular example, how
does it violate STD66? Specifically Sec 4.2 Relative Reference?
Syntactically can this string be a “path segment” of a “relative-path” (or
“path-noscheme”)?

>
> 2. The module uses (indirectly) the grouping "srlr" from the
>    "ietf-segment-routing-common" module. This grouping defines two
>    leaves, "lower-bound" and "upper-bound". I assume it is expected
>    that the former must not be greater than the latter. This is
>    however not enforced by the data model. My suggestion is to add a
>    "must" statement to the "srlr" grouping corresponding to this
>    constraint.
>
[Xufeng]: We have added a few “must” statements to the module
ietf-sr-topology, and we will work with the authors of
ietf-segment-routing-common to see if the constraint can be added to
ietf-segment-routing-common.

>
> 3. Typos in the module text:
>
>    - description of grouping "sr-topology-type":
>      s/toplogies/topologies/
>
[Xufeng]: Fixed.

>
>    - description of container "sr-mpls":
>      s/Indiates/Indicates/
>
[Xufeng]: Fixed.