Re: [Netconf] RtgDir review: draft-ietf-netconf-nmda-netconf-06

[Lou Berger <[e-mail]>]

Hi Kent,

Thanks for the quick response.  I think there's just one open point left 
after your response...


On 7/30/2018 2:44 PM, Kent Watsen wrote:
> Hi Lou,
>
> Thanks for your review!
>
>> Summary:
>>
>> I have some minor concerns about this document that I think should be
>> resolved before publication.
>>
>> Comments:
>>
>> The document is is generally well written and easy to read.  There are
>> several places where I'm sure the authors know exactly what they intend,
>> but the text could be revised to help along those less familiar with the
>> work.  There is also one miss-marked RFC Update reference.
> Just to be sure, all these issues are discussed below, right?
>
Yes, the ones I felt really needed to be addressed are listed.

>> Major Issues:
>>
>> <none>
>>
>> Minor Issues:
>>
>> - Cover/Abstract
>>     Updates: 7950
>>
>>     The update to
>>     RFC 7950 requires the usage of I-D.ietf-netconf-rfc7895bis by NETCONF
>>     servers implementing the Network Management Datastore Architecture.
>>
>> If I read this and the referenced document correctly, this is saying
>> that I-D.ietf-netconf-rfc7895bis updates which version of YANG library
>> is supported by implementations RFC7950 that support NMDA.  If this is
>> the correct reading, this document doesn't update RFC7950, but rather
>> I-D.ietf-netconf-rfc7895bis updates 7950. (this omission was noted in a
>> separate message.)
> The last paragraph in Section 2 says this:
>
>     This document updates [RFC7950], Section 5.6.4, to allow servers to
>     advertise the capability :yang-library:1.1 instead of :yang-
>     library:1.0, and to implement the subtree "/yang-library"
>     [I-D.ietf-netconf-rfc7895bis] instead of "/modules-state".
>
> Note that RFC 7950, Section 5.6.4 says:
>
>     A NETCONF server MUST announce the modules it implements (see
>     Section 5.6.5) by implementing the YANG module "ietf-yang-library"
>     defined in [RFC7895] and listing all implemented modules in the
>     "/modules-state/module" list.
>
> Which is what this document changes.
well doesn't I-D.ietf-netconf-rfc7895bis already say this and as the 
document that has the actual modification, it seems that that is the 
right document to list the 'update'  and this document need only 
reference I-D.ietf-netconf-rfc7895bis without any duplication of 
requirements.
Note that I-D.ietf-netconf-rfc7895bis says:

    All NETCONF servers supporting YANG 1.1 [RFC7950] are required to
    support YANG Library (see Section 5.6.4 of RFC 7950).  NETCONF
    servers implementing the NETCONF extensions to support the NMDA
    [I-D.ietf-netconf-nmda-netconf] MUST implement at least the version
    of the YANG library defined in this document.  Similarly, all
    RESTCONF servers are required to support YANG Library (see Section 10
    of RFC 8040).  RESTCONF servers implementing the RESTCONF extensions
    to support the NMDA [I-D.ietf-netconf-nmda-restconf] MUST implement
    at least the version of the YANG library defined in this document.

isn't that sufficient?

>
>> - section 3.1.1.
>>
>>     The "config-filter" parameter can be used to retrieve only "config
>>     true" or "config false" nodes.
>>
>>     also
>>           leaf config-filter {
>>             type boolean;
>>             description
>>               "Filter for nodes with the given value for their
>>                'config' property.";
>>           }
>>
>> So this means:
>>      absent = provide all
>>      true = provide only true
>>      false = provide only false
>>
>> Right? Either way, I think this could be clarified a bit.  At least say
>> what behavior is expected when the leaf is omitted.
> OLD:
>               "Filter for nodes with the given value for their
>                'config' property.";
> NEW:
>               "Filter for nodes with the given value for their
>                'config' property.  For example, when set to
>                'true', only 'config true' nodes are returned.
>                When unset, all nodes are returned.";
>
> Okay?
Yes. Thanks.

>
>> Nits:
>>
>> - the orders of sections 3.1.1.1. and 3.1.1.2. should be reversed to
>> match the module ordering.
> ...or change the order in the module, which I think might be preferred.
>
WFM - I didn't want to suggest a technical change to address an 
editorial comment.

>> - Section 3.1.2:
>>
>>      The "default-operation" parameter is a copy of the
>>      "default-operation" parameter of the <edit-config> operation.
>>
>>      The "edit-content" choice mirrors the "edit-content" choice of the
>>      <edit-config> operation.  Note, however, that the "config" element in
>>      the "edit-content" choice of <edit-data> uses "anydata" (introduced
>>      in YANG 1.1) while the "config" element in the "edit-content" choice
>>      of <edit-config> used "anyxml".
>>
>> It's fine to say that these nodes mirror <edit-config> nodes, but this
>> document should at least summarize the function of each, e.g.,
>>       The "default-operation" parameter selects the default operation
>>       for this request. It is a copy....
> NEW
>       The "default-operation" parameter selects the default operation to
>       use.  It is a copy of the "default-operation" parameter of the
>       <edit-config> operation.
>
>       The "edit-content" parameter specifies the content for the edit
>       operation.  It mirrors the "edit-content" choice of the
>       <edit-config> operation.  Note, however, that the "config" element in
>       the "edit-content" choice of <edit-data> uses "anydata" (introduced
>       in YANG 1.1) while the "config" element in the "edit-content" choice
>       of <edit-config> used "anyxml".
>
>
> Okay?
Yes.

Thanks!
Lou

>
> Thanks,
> Kent // co-author
>
>
>
>