Re: [ippm] Robert Wilton's No Objection on draft-ietf-ippm-ioam-yang-12: (with COMMENT)

[Tianran Zhou <[e-mail]>]

Hi Rob,

Thanks very much for your comments.
Please see in line.

Best,
Tianran

-----Original Message-----
From: Robert Wilton via Datatracker [mailto:[e-mail]] 
Sent: Friday, March 1, 2024 12:48 AM
To: The IESG <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
Subject: Robert Wilton's No Objection on draft-ietf-ippm-ioam-yang-12: (with COMMENT)

Robert Wilton has entered the following ballot position for
draft-ietf-ippm-ioam-yang-12: No Objection

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/about/groups/iesg/statements/handling-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ippm-ioam-yang/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Hi,

Thanks for this document,  I noted that this document hadn't had a recent YANG
doctor review.  I have a few suggestions for you to consider that may improve
the YANG.

Minor level comments:

(1) p 3, sec 3.1.  Overview

   The "ioam-info" is a container for all the read-only information that
   assists monitoring systems in the interpretation of the IOAM data.
   module: ietf-ioam
      +--rw ioam
         +--ro ioam-info
         |  +--ro timestamp-type?        identityref
         |  +--ro available-interface* [if-name]
         |     +--ro if-name    if:interface-ref
         +--rw ioam-profiles

Please rename the 'ioam-info' container to 'info' and 'ioam-profiles' to
'profiles', and 'ioam-profile' to 'profile'.  This will make the paths shorter
and reduce duplicate semantic information on the path.

ZTR> Agreed.

(2) p 3, sec 3.1.  Overview

            +--rw admin-config
            |  +--rw enabled?   boolean

Please move 'admin-config' outside of the ioam-profiles container, so that the
container is only wrapping the list entry.

ZTR> Agreed.

(3) p 17, sec 4.  IOAM YANG Module

         description
           "This boolean value indicates whether the sequence number is
            used in the direct export option 32-bit flow identifier. If
            this value is true, the sequence number is used. By default,
            it's turned off.";
       }
     }

Are you allowed to set both enable-sequence-number and a flow-id?  If not, then
it might be worth adding text to this effect.

ZTR> Both sequence-number and flow-id are optional in data plane. The sequence-number is enabled by enable-sequence-number.
If there is configuration on the flow-id, then it will be encapsulated in the data plane. If there is no such configuration, flow-id is not enabled in data plane.

(4) p 20, sec 4.  IOAM YANG Module

             leaf enabled {
               type boolean;
               default false;
               description
                 "When true, apply incremental tracing option to the
                  specified flow identified by the filter.";
             }

You have several similar containers that all an "enabled" leaf, which defaults
to false.  Did you consider setting these containers as presence containers
instead? Which would remove the need for the enable leaf, since the existence
of the container in the config would indicate that it is enabled.

ZTR> Yes. Thanks.

Nit level comments:

(5) p 17, sec 4.  IOAM YANG Module

       leaf node-action {
         type ioam-node-action;
         default action-transit;
         description "This indicates what action the node will take,
         e.g. encapsulation.";

Description isn't properly formatted (although the RFC editor should also fix
this).

ZTR> Yes. Thanks.

(6) p 17, sec 4.  IOAM YANG Module

         description
           "A 32-bit flow identifier. The field is set at the
            encapsulating node. The Flow ID can be uniformly assigned
            by a central controller or algorithmically generated by the
            encapsulating node. The latter approach cannot guarantee
            the uniqueness of Flow ID, yet the conflict probability is
            small due to the large Flow ID space.flow-id is used to
            correlate the exported data of the same flow from multiple
            nodes and from multiple packets.";
       }

".flow-id" => ". flow-id"

ZTR> Yes. Thanks.

(7) p 23, sec 5.  Security Considerations

   *  /ioam/ioam-profiles/ioam-profile: The entries in the list above
      include the whole IOAM profile configurations.  Unexpected changes
      to these entries could lead to the mistake of the IOAM behavior
      for the corresponding flows.  Consequently, it will impact the
      performance monitoring, data analytics, and the assciated reaction
      to network services.

assciated => associated

ZTR> Yes. Thanks.

Regards,
Rob