Re: [netconf] Benjamin Kaduk's No Objection on draft-ietf-netconf-notification-capabilities-18: (with COMMENT)

[Benoit Claise <[e-mail]>]

Hi Benjamin,

Thanks for your review.
See inline for addressing your most important comments (in coordination 
with our AD)

On 10/7/2021 8:12 AM, Benjamin Kaduk via Datatracker wrote:
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-netconf-notification-capabilities-18: 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/blog/handling-iesg-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-netconf-notification-capabilities/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Thanks to Barry Leiba for the secdir review.
>
> Section 1
>
>     Implementation-time information is needed by Network Management
>     [...]
>     new network node type can be introduced into the network.
>
>     Implementation-time information is needed by system integrators.
>     [...]
>     node type sometimes depends on these management possibilities.
>
> Commenting here since I read this draft after
> draft-ietf-netmod-yang-instance-file-format, but there seems to be
> pretty strong overlap between the discussion of how it's expensive to
> require NMS implementors to have correclty configured nodes available,
> the need for operators to have information about device capabilities
> for purchasing decisions, etc.  It would be good to check that the
> specific language used to describe these scenarios is in agreement
> across documents, to the extent possible.  (The phrasing in this
> document is probably a better starting point for the convergence, in my
> opinion.)
>
> Section 4.2
>
>          1) search for a datastore-capabilities list entry for
>          the specific datastore. When stating a specific capability, the
>          relative path for any specific capability must be the same
>          under the system-capabilities container and under the
>          per-node-capabilities list: the same grouping for defining
>          the capabilities MUST be used.
>
> It's a little surprising to find the "MUST use same grouping"
> requirement buried in the procedure for finding a capability value for a
> specific data node in a specific datastore.
We propose to  delete “the same grouping for definingthe capabilities 
MUST be used.”, and instead pull this out to a separate paragraph in the 
module description:

    The same grouping MUST be used to define hierarchical capabilities
    supported both at the system level and datastore/data node level.
>
> Section 5.2
>
>             leaf minimum-update-period {
>               [...]
>               description
>                 "Indicates the minimal update period that is
>                  supported for a 'periodic' subscription.
>
>                  A subscription request to the selected data nodes with
>                  a smaller period than what this leaf specifies is
>                  likely to result in a 'period-unsupported' error.";
>               [...]
>             }
>             leaf-list supported-update-period {
>               [...]
>               description
>                 "Supported update period values for a 'periodic'
>                  subscription.
>
>                  A subscription request to the selected data nodes with a
>                  period not included in the leaf-list will result in a
>                  'period-unsupported' error.";
>
> Is it intended that these disagree on whether it is "likely to result"
> or "will result" in a period-unsupported error?
>
>           leaf-list supported-excluded-change-type {
>             if-feature "yp:on-change";
>             type union {
>               type enumeration {
>                 enum none {
>                   value -2;
>                   description
>                     "None of the change types can be excluded.";
>                 }
>                 enum all {
>                   value -1;
>                   description
>                     "Any combination of change types can be excluded.";
>                 }
>               }
>               type yp:change-type;
>
> It seems like this allows for some nonsensical edge cases, like a list
> that includes both "none" and "any", or all the actual values from RFC
> 8641 as well as "none", etc.  Do we need to say anything about how to
> handle such invalid scenarios?
>
> Section 6
>
> I agree with Roman that it would be nice to be able to say something
> about the potential scope of security considerations for future
> augmentations, if nothing else to aid authors of such future
> augmentations in documenting their own security considerations.
We now have in the security considerations section.

       All protocol-accessible data nodes in augmented modules are 
read-only
         and cannot be modified
>
> NITS
>
> Section 4.2, 5.2
>
> I think only one "<CODE ENDS>" tag is needed.
Indeed, already addressed in the latest version.

Thanks and regards, Benoit
>
>
>
> .