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

From: Alia Atlas <[email-masked]>
Date: Mon, 30 Oct 2017 07:25:13 -0700

Alia Atlas has entered the following ballot position for
draft-ietf-lime-yang-connectionless-oam-15: 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:
----------------------------------------------------------------------

I took a quick look through version -15 and it looks like it addresses almost
all of my serious Discuss points. The only Discuss-worthy point is (c) below. 
I have a few more points related to the changes that were made; they are just
comments & listed here to be with the original points.

For version 15:

a) In Sec 3.1,  it still says
" o  Router-id to represent the device or node.
      [I-D.ietf-spring-sr-yang]"

but [I-D.ietf.spring-sr-yang] has nothing to do with the router-id

b) In Section 4, thanks for adding
urn:ietf:params:xml:ns:yang:ietf-lime-common-types - but could it be a
meaningful and accurate name like
   ietf-lime-time-types or ietf-time-types  (Benoit would know best structure) 
   that clearly
 shows its intended scope for reuse and please fix the description for it too.

c)  [I-D.ietf-rtgwg-ni-model] is still listed as informative, but the model
defined in there is imported "import ietf-network-instance {
    prefix ni;
  }"   It needs to be normative

d) I-D.ietf-spring-sr-yang is still listed as informative - but not really
correctly used as a reference.

=================

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

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.

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.

5) [I-D.ietf-rtgwg-routing-types] is not an informative reference.  Its module
is imported and used.  It must be normative.

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.

---

From: Qin Wu <[email-masked]>
Date: Mon, 30 Oct 2017 14:51:26 +0000

-----邮件原件-----
发件人: Alia Atlas [mailto:[email-masked]] 
发送时间: 2017年10月30日 22:25
收件人: The IESG
抄送: [email-masked]; Ron Bonica; Carlos Pignataro; [email-masked]; [email-masked]; [email-masked]
主题: Alia Atlas' Discuss on draft-ietf-lime-yang-connectionless-oam-15: (with DISCUSS and COMMENT)

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.)

I took a quick look through version -15 and it looks like it addresses almost all of my serious Discuss points. The only Discuss-worthy point is (c) below. 
I have a few more points related to the changes that were made; they are just comments & listed here to be with the original points.

For version 15:

[Qin]: route-id is defined in ietf-rtgwg-routing-types, we will use ietf-rtgwg-routing-types as reference.

b) In Section 4, thanks for adding
urn:ietf:params:xml:ns:yang:ietf-lime-common-types - but could it be a meaningful and accurate name like
   ietf-lime-time-types or ietf-time-types  (Benoit would know best structure) 
   that clearly
 shows its intended scope for reuse and please fix the description for it too.

[Qin]: I think we could use the former since later on, you may identify more time related type and will fix the description.

c)  [I-D.ietf-rtgwg-ni-model] is still listed as informative, but the model defined in there is imported "import ietf-network-instance {
    prefix ni;
  }"   It needs to be normative

[Qin]: Okay, will fix this.

d) I-D.ietf-spring-sr-yang is still listed as informative - but not really correctly used as a reference.

[Qin]: Agree, I have replace it with ietf-rtgwg-routing-types.
=================

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

          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.

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.

5) [I-D.ietf-rtgwg-routing-types] is not an informative reference.  Its module is imported and used.  It must be normative.

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.

==============================
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.

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.

            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.

---

From: Alia Atlas <[email-masked]>
Date: Mon, 30 Oct 2017 10:56:09 -0400

Qin,

All this sounds good.

Thanks,
Alia

On Mon, Oct 30, 2017 at 10:51 AM, Qin Wu <[email-masked]> wrote:

> 抄送: [email-masked]; Ron Bonica; Carlos
> Pignataro; [email-masked]; [email-masked]; [email-masked]
> I took a quick look through version -15 and it looks like it addresses
> almost all of my serious Discuss points. The only Discuss-worthy point is
> (c) below.
> I have a few more points related to the changes that were made; they are
> just comments & listed here to be with the original points.
>
> For version 15:
>
> [Qin]: I think we could use the former since later on, you may identify
> more time related type and will fix the description.
>
> c)  [I-D.ietf-rtgwg-ni-model] is still listed as informative, but the
> model defined in there is imported "import ietf-network-instance {
>     prefix ni;
> 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;
>   }
> 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.
>
> 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.
>
> 5) [I-D.ietf-rtgwg-routing-types] is not an informative reference.  Its
> module is imported and used.  It must be normative.
>
> 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.
>
>
> 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.
>
> 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.
>

---

From: Qin Wu <[email-masked]>
Date: Mon, 30 Oct 2017 14:58:24 +0000

Thanks Alia. Update is done,
The diff is:
https://www.ietf.org/rfcdiff?url2=draft-ietf-lime-yang-connectionless-oam-16

-Qin
发件人: Alia Atlas [mailto:[email-masked]]
发送时间: 2017年10月30日 22:56
收件人: Qin Wu
抄送: The IESG; Ron Bonica; [email-masked]; [email-masked]; [email-masked]; [email-masked]
主题: Re: Alia Atlas' Discuss on draft-ietf-lime-yang-connectionless-oam-15: (with DISCUSS and COMMENT)

Qin,

All this sounds good.

Thanks,
Alia

On Mon, Oct 30, 2017 at 10:51 AM, Qin Wu <[email-masked]<mailto:[email-masked]>> wrote:
-----邮件原件-----
发件人: Alia Atlas [mailto:[email-masked]<mailto:[email-masked]>]
发送时间: 2017年10月30日 22:25
收件人: The IESG
抄送: [email-masked]<mailto:[email-masked]>; Ron Bonica; Carlos Pignataro; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>
主题: Alia Atlas' Discuss on draft-ietf-lime-yang-connectionless-oam-15: (with DISCUSS and COMMENT)

For version 15:

          description
            "System ID";"

            leaf tunnel-interface {
              type uint32;
              description
                "VPN Prefix ";
            }
          }
          case pw {
            leaf remote-pe-address {
              type inet:ip-address;
              description
                "Remote pe address.";
            }
"

---

From: Alia Atlas <[email-masked]>
Date: Mon, 30 Oct 2017 11:19:26 -0400

Qin,

This looks great.  I'll clear based on this, assuming that you will submit
when able
and leave it in Benoit's capable hands.

Regards,
Alia

On Mon, Oct 30, 2017 at 10:58 AM, Qin Wu <[email-masked]> wrote:

> *发件人:* Alia Atlas [mailto:[email-masked]]
> *发送时间:* 2017年10月30日 22:56
> *收件人:* Qin Wu
> *抄送:* The IESG; Ron Bonica; [email-masked]; [email-masked];
> [email-masked]; [email-masked]
> *主题:* Re: Alia Atlas' Discuss on draft-ietf-lime-yang-connectionless-oam-15:
> (with DISCUSS and COMMENT)
>
>
>
> Qin,
>
>
>
> All this sounds good.
>
>
>
> Thanks,
>
> Alia
>
>
>