Subject: [Lime] AD review: draft-ietf-lime-yang-connectionless-oam-methods

From: Benoit Claise <[email-masked]>
Date: Fri, 11 Aug 2017 17:11:35 +0200

Dear all,

Here is my AD review.

- I see that the draft is NMDA-compliant. Good.
https://yangcatalog.org:8443/search/modules/ietf-connectionless-oam-methods,2017-05-18,ietf

- " This document presents a retrieval method YANG Data model for 
connectionless OAM protocols" is this right?

        rpc path-discovery {
              description
                "Generates path discovery as perRFC7276 <https://tools.ietf.org/html/rfc7276>.";

        rpc continuity-check {
              if-feature coam:continuity-check;
              description
                "Generates continuity-check as perRFC7276 <https://tools.ietf.org/html/rfc7276>.";

AFACT, the RPC triggers an "on-demand" (as opposed to proactive 
draft-ietf-lime-yang-connectionless-oam, to use the 
draft-ietf-lime-yang-connectionless-oam term) OAM mechanism and 
retrieves the results directly.
" This document presents a retrieval method YANG Data model for 
connectionless OAM protocols" makes it sound like "polling" the results, 
which could also be "proactive". You should improve the text

After hours spent on the two LIME drafts ...
If the continuity-check RPC is really "on-demand", why do we have the 
session-type-enum as input?

  rpc continuity-check {
     if-feature "coam:continuity-check";
     description
       "Generates continuity-check as perRFC7276 <https://tools.ietf.org/html/rfc7276>.";
     input {
       container destination-tp {
         uses coam:tp-address;
         description
           "Destination test point.";
       }
       uses coam:session-type;         <==============
       leaf source-interface {
         type if:interface-ref;
         description
           "Source interface.";
       }

 From the other draft (why, btw?)

     grouping session-type {
       description
         "This object indicates the current session
          definition.";
       leaf session-type-enum {
         type enumeration {
           enum "proactive" {
             description
               "The current session is proactive";
           }
           enum "on-demand" {
             description
               "The current session is on-demand.";
           }
         }
         default "on-demand";
         description
           "Session type enum";
       }
     }

This should always be "on-demand", right?

Same remark for the persistent RPCs in the appendix A, which should 
always have a "session-type-enum" value of "proactive".
Trying to understand...

- Abstract:

    The retrieval methods
    model presented here can be extended to include technology specific
    details.

But this is not in line with the appendix A, which is the place that 
speaks about "extensions"
You should have another place (a new appendix) to explain how to augment 
the technology specific details ... in a consistent way, if possible.

-

    RPC - A Remote Procedure Call, as used within the NETCONF protocol

This document is about a YANG data model. So independently of NETCONF or 
RESTCONF or something else.
So referring to NETCONF only is not right. I would remove "as used 
within the NETCONF protocol"
Btw, RFC 7950 makes the distinction between :

    o  RPC: A Remote Procedure Call.

    o  RPC operation: A specific Remote Procedure Call.

You should include RPC and RPC operations in section 2
You want to review all "RPC" instances.
For example, RPC commands should be: RPC operations
Most of the time, RPC should be: RPC operation(s). Ex: 3.1 title.

- Since you import ietf-connectionless-oam, 
draft-ietf-lime-yang-connectionless-oam should probably be a normative 
reference

- The security considerations have been updated: 
https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines

Editorial:
-
OLD:

    It provides a technology-independent
    RPC commands for connectionless OAM protocols.

NEW:

    It provides technology-independent
    RPC commands for connectionless OAM protocols.

-
    It provides a flexible way to retrieve the retrieved-
    data which defined by the "ietf-connectionless-oam.yang"
    [I-D.ietf-lime-yang-connectionless-oam 
<https://tools.ietf.org/html/draft-ietf-lime-yang-connectionless-oam-methods-05#ref-I-D.ietf-lime-yang-connectionless-oam>].

retrieve the data?

-

OLD:
      "This YANG module defines the RPCs for ,
      connectionless OAM to be used within IETF
      in a protocol Independent manner.

NEW:
      "This YANG module defines the RPCs for
      connectionless OAM to be used within IETF
      in a protocol independent manner.

-
Appendix A
OLD:

    The following are some examples of extensions possible to the yang
    model.  The example discusses persistent methods.

NEW:

    The following are some examples of extensions possible to the YANG
    model.  The example discusses persistent methods.

Regards, Benoit

---

From: Qin Wu <[email-masked]>
Date: Fri, 1 Sep 2017 04:52:14 +0000

Thank for valuable comments, your comments have been addressed in v-(06) with other comments on the list:
https://datatracker.ietf.org/doc/draft-ietf-lime-yang-connectionless-oam-methods/

-Qin
发件人: Lime [mailto:[email-masked]] 代表 Benoit Claise
发送时间: 2017年8月11日 23:12
收件人: [email-masked]
抄送: Carl Moberg (camoberg)
主题: [Lime] AD review: draft-ietf-lime-yang-connectionless-oam-methods

Dear all,

Here is my AD review.

- I see that the draft is NMDA-compliant. Good.
https://yangcatalog.org:8443/search/modules/ietf-connectionless-oam-methods,2017-05-18,ietf
- " This document presents a retrieval method YANG Data model for connectionless OAM protocols" is this right?

   rpc path-discovery {

         description

           "Generates path discovery as per RFC7276<https://tools.ietf.org/html/rfc7276>.";

   rpc continuity-check {

         description

           "Generates continuity-check as per RFC7276<https://tools.ietf.org/html/rfc7276>.";
AFACT, the RPC triggers an "on-demand" (as opposed to proactive draft-ietf-lime-yang-connectionless-oam, to use the draft-ietf-lime-yang-connectionless-oam term) OAM mechanism and retrieves the results directly.
" This document presents a retrieval method YANG Data model for connectionless OAM protocols" makes it sound like "polling" the results, which could also be "proactive". You should improve the text

After hours spent on the two LIME drafts ...
If the continuity-check RPC is really "on-demand", why do we have the session-type-enum as input?

 rpc continuity-check {

    description

      "Generates continuity-check as per RFC7276<https://tools.ietf.org/html/rfc7276>.";

    input {

        uses coam:tp-address;

        description

      }

      leaf source-interface {

        type if:interface-ref;

        description

          "Source interface.";

      }

    grouping session-type {

      description

         definition.";

      leaf session-type-enum {

        type enumeration {

          enum "proactive" {

            description

          }

          enum "on-demand" {

            description

          }

        }

        default "on-demand";

        description

          "Session type enum";

      }

    }

Same remark for the persistent RPCs in the appendix A, which should always have a "session-type-enum" value of "proactive".
Trying to understand...

- Abstract:

   The retrieval methods

   details.
But this is not in line with the appendix A, which is the place that speaks about "extensions"
You should have another place (a new appendix) to explain how to augment the technology specific details ... in a consistent way, if possible.

-

   RPC - A Remote Procedure Call, as used within the NETCONF protocol
This document is about a YANG data model. So independently of NETCONF or RESTCONF or something else.
So referring to NETCONF only is not right. I would remove "as used within the NETCONF protocol"
Btw, RFC 7950 makes the distinction between :

- Since you import ietf-connectionless-oam,  draft-ietf-lime-yang-connectionless-oam should probably be a normative reference

- The security considerations have been updated: https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines

Editorial:
-
OLD:

NEW:

-

   [I-D.ietf-lime-yang-connectionless-oam<https://tools.ietf.org/html/draft-ietf-lime-yang-connectionless-oam-methods-05#ref-I-D.ietf-lime-yang-connectionless-oam>].

retrieve the data?
-

OLD:

NEW:

NEW:

Regards, Benoit