Re: [Lime] Alia Atlas' Discuss on draft-ietf-lime-yang-connectionless-oam-14: (with DISCUSS and COMMENT)

[Benoit Claise <[e-mail]>]

Hi Alia,

Some answers below.
I let the authors chime in.
> Alia Atlas has entered the following ballot position for
> draft-ietf-lime-yang-connectionless-oam-14: Discuss
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-lime-yang-connectionless-oam/
>
>
>
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
> Thank you for your work on this document.  I have a number of serious concerns
> - but they all amount to fixing up your references and slight restructuring
> for clarity and reuse.
>
> 1) In Sec 3.1, the reference is system-id to represent the device or
> node.[I-D.ietf-spring-sr-yang] I believe that should be "typedef router-id {
>         type yang:dotted-quad;
>         description
>           "A 32-bit number in the dotted quad format assigned to each
>            router. This number uniquely identifies the router within
>            an Autonomous System.";
>       }"
> from draft-ietf-rtgwg-routing-types.
> Certainly "[I-D.ietf-spring-sr-yang]" is NOT an informative reference with such
> a dependency.
>
> I see that this document actually redefines router-id, instead of using it as
> part of the included import from
>   import ietf-routing-types {
>     prefix rt;
>    }
This was part of my AD review (on the LIME mailing list) in August.

    - Cut and paste the typedefs from
    https://datatracker.ietf.org/doc/draft-ietf-rtgwg-routing-types/

         typedef router-id {
         typedef ipv4-multicast-group-address
         typedef ipv6-multicast-group-address {
    	...

    If published fast enough, you should import the types from
    https://datatracker.ietf.org/doc/draft-ietf-rtgwg-routing-types/

I'm happy to see that draft-ietf-rtgwg-routing-types is finally approved.
So authors, it's time to import the router-id
> On p.27, I see "leaf system-id {
>            type rt:router-id;
>            description
>              "System ID assigned to this node.";
>          }"
> so it is using the routing-yang-types, but renaming it as system-id, there.
> Consistency isn't just the hobgoblin of little minds - it's actually useful.
Make sense to me.
>
> In choice to-location, again "case system-id {
>            leaf system-id-location {
>              type router-id;
>              description
>                "System id location";
>            }
>
>            description
>              "System ID";"
> using the locally defined router-id and renaming it instead of using
> rt:router-id.
>
> 2) On p. 13 & 14, there are many identities associated with time and
> time-stamps.  I cannot believe that the best way to handle these is by having
> them as part of an OAM model!   At a minimum, they should be defined as a
> separate module and then included, even if it is in the same draft.  Then they
> will be available for reuse elsewhere.
>
> 3) This is extending [I-D.ietf-i2rs-yang-network-topo] - I do not believe this
> should be merely an informative reference.

Note sure what you mean:

  augment "/nd:networks/nd:network/nd:node" {
     description
       "Augment test points of connectionless oam.";
         uses test-point-locations;
   }

https://www.yangcatalog.org/yang-search/impact_analysis.php?modules[]=ietf-connectionless-oam&recurse=0&rfcs=1&show_subm=1&show_dir=dependencies
>
> 4) I cannot tell if I-D.ietf-rtgwg-ni-model is informative or normative; it is
> not referenced in the draft - though there are fields that are labeled NI
> without adequate description.
https://www.yangcatalog.org/yang-search/impact_analysis.php?modules[]=ietf-connectionless-oam&recurse=0&rfcs=1&show_subm=1&show_dir=dependencies

The generic question is: if I import a YANG module from draft D, then D 
should be a normative reference.
>
> 5) [I-D.ietf-rtgwg-routing-types] is not an informative reference.  Its module
> is imported and used.  It must be normative.
Yes.

Regards, Benoit
>
> 6) [I-D.ietf-spring-sr-yang] is listed as an informative reference, but if it
> were actually used as described, it would need to be normative. Instead, I
> believe this can be removed as a reference.
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> a) Sec 3.8: It is unfortunate that the cc-session-statistics-data structure is
> not a list of {traffic type, cc-session-statistics} instead of hardcoded
> members for IPv4 and IPv6 traffic only.  While it can still be extended for
> additional traffic types, the naming may be inconsistent and there's no
> requirement that the contents are cc-session-statistics.
>
> b) On p.9: " +--:(system-id)
>        |                 +--rw system-id-location?      router-id"
>
> Why isn't this just named router-id instead of system-id, for consistency?
> This comment applies throughout the draft.
>
> c) The use of "tp" to mean test-point is a bit unfortunate in a model that is
> building off of the network topology one, which uses "tp" for termination-point.
>
> d) On p. 13: "identity address-attribute-types {
>      description
>        "This is base identity of address
>         attribute types which are ip-prefix,
>         bgp, tunnel, pwe3, vpls, etc.";
>    }"
>
> I haven't a clue what is meant by a bgp address attribute type or a tunnel one.
>   Can you please expand the description to be substantially more meaningful?
> How is it used?
>
> On p. 24, I see these defined
> " case bgp {
>              leaf bgp {
>                type inet:ip-prefix;
>                description
>                  "BGP Labeled Prefix ";
>              }
>            }
>            case tunnel {
>
>              leaf tunnel-interface {
>                type uint32;
>                description
>                  "VPN Prefix ";
>              }
>            }
>            case pw {
>              leaf remote-pe-address {
>                type inet:ip-address;
>                description
>                  "Remote pe address.";
>              }
> "
> but unlike the other cases with clear descriptions and references to the
> relevant RFCs, these are NOT clear and do not even fully expand acronyms.
>
> e) "grouping tp-address-ni "  Please expand what NI is the abbreviation for in
> the description.
>
>
> .
>