Re: [netmod] [yang-doctors] Yangdoctors last call review of draft-ietf-netmod-nmda-diff-04

Alexander L Clemm <ludwig@clemm.org> Tue, 15 September 2020 21:18 UTC

Return-Path: <ludwig@clemm.org>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 3B9C83A0D5F; Tue, 15 Sep 2020 14:18:58 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.919
X-Spam-Level:
X-Spam-Status: No, score=-1.919 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, NICE_REPLY_A=-0.001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_NONE=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 16MWUlAvW1UY; Tue, 15 Sep 2020 14:18:56 -0700 (PDT)
Received: from mout.perfora.net (mout.perfora.net [74.208.4.197]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id F2FF93A0D5D; Tue, 15 Sep 2020 14:18:52 -0700 (PDT)
Received: from [172.16.0.44] ([73.189.160.186]) by mrelay.perfora.net (mreueus002 [74.208.5.2]) with ESMTPSA (Nemesis) id 0LdHBn-1kiWHT2rFx-00iTqw; Tue, 15 Sep 2020 23:18:50 +0200
To: "Reshad Rahman (rrahman)" <rrahman=40cisco.com@dmarc.ietf.org>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
Cc: "last-call@ietf.org" <last-call@ietf.org>, "draft-ietf-netmod-nmda-diff.all@ietf.org" <draft-ietf-netmod-nmda-diff.all@ietf.org>, "netmod@ietf.org" <netmod@ietf.org>
References: <159942490640.25028.10946254095755778899@ietfa.amsl.com> <EF21460A-8689-491C-AE19-942C6FA84FFC@cisco.com>
From: Alexander L Clemm <ludwig@clemm.org>
Message-ID: <e801c95e-078e-8438-b1a0-18aaf4be3a82@clemm.org>
Date: Tue, 15 Sep 2020 14:18:48 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <EF21460A-8689-491C-AE19-942C6FA84FFC@cisco.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Content-Language: en-US
X-Provags-ID: V03:K1:USRQujB0FS09hruF4EnXvwDOgctIK4th42xaslqS2JbaiwSNjZ3 w12whUtjmFcNeHvQEni9cL1ImJr4UPrHaMMqO2SX3R9vnx5AcI2IlaiRilCN/5T3BHMsl5r nFjK7fZkSfpHGPqp6ALDZ9I6rMTd7TaMCN3ONcvJaq9qrexHW2Vcp/ij/5g5EXiAkKE1X5Z dbXhbWFzJUuEoX5PKgD3g==
X-UI-Out-Filterresults: notjunk:1;V03:K0:YgxtUBjN87A=:Li36wLtRqdY6SP1NB1zEwv Kga5qchxfaF/tDjPlun/zrgSCK6WgwOd1ADGlWZf9iKzI9MexGR4ZtQBhB75hhR4H8PqAsk4T Ew9iUvuV+6ku7aj+//UmPhVJD7ip9UNu4KkQ5XS5iwmEXxYw/pHWDn1OLj5kJaOzFPbLE46nk y9ca8soCBaEh7awbDOH1DmcmzqaEBOhJ2Rim/eWuTmxIE4PMGy9B2CAUNglygzz7QjVVhr2oJ 7Zd/p4wajfG3fkWiHHnjB0GaHs1wBXN8ySBBDabmQdCAoLRaeeCqph+uPzpWzlRzMz1VjpVTi Z33OSkvUW/FvQeaNRB+s80L5fIItR2wJPgtIOGtA/b6svWvcIcWE/oHntVQJw9I0gbRRf1/HG 3WRoVZPnaGMEtogGRrvtlFnIMBnZDnJrmzx4VukgE7LH+crYq+poAaoYzOSE68YoRE95/rCyg VoIKt+SMGxMjRMaRleD3CbKV6miEk1PZQ/ectvd6fge4uvwBSoezjJXFoCZYnyqgUrH9jw+45 //tCF4oX9EbF4H4qOp7nys3bS/IKTyZQOplgyq1EfiYxHNKQY5VFZjr0FEafGV8/HS4PND1v2 1RLQjqDIoR4k/dbEzj5ktI6lXCnwVReJj0hwbQXxwd3BAHdMfS0O2j7a0y22vwd62XAB60Zd2 U/IWzDCZhJcmmkGP5AH3Faq0JXXc0vTNcwXmKI5fA2Q98Xh9TtxYaZ0LqfC/5hDPoG+NGBtX5 M7kP8igiCzPUkv1j421M08tpzZjfShn8GniiLM9fz31jJOr5Q+XbAXuMCU55skNJWZulpHKkh RA71dYkjhKAnBfW9NhNcxgxw/LQzK72yn8nmN+kSWBRHgLMVFswVfBPAROgiJQDpJv+ih5n
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/074bzs2tJzwFL_r0EPjAFUYxJ_k>
Subject: Re: [netmod] [yang-doctors] Yangdoctors last call review of draft-ietf-netmod-nmda-diff-04
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 15 Sep 2020 21:18:58 -0000

Hello Reshad, hello YANG Doctors,

thank you for your review!  Please find my replies inline, <ALEX>.  We
have also just posted -05 (thanks, Yingzhen, for doublechecking my
updates).   

--- Alex on behalf of coauthors

On 9/7/2020 7:06 AM, Reshad Rahman (rrahman) wrote:
> <Here's the same message with hopefully more readable formatting>
>
> Review of rev -04 by Reshad Rahman
>
> The document is clear and well-written. While some issues have been identified, they can be resolved quickly.
>
> Issues
> 	1.	YANG model P8, for “leaf xpath-filter”, add reference to RFC6021. There should also be a normative reference to RFC6021 (as per RFC8407)
<ALEX> Thanks.  Adding reference to 6991 (as 6021 is obsoleted). </ALEX>
> 	2.	Example P10, </interfa should be </interfaces>
<ALEX> Thanks.  Of course! </ALEX>
> 	3.	Example P10, last paragraph talks about preference and explicit-router-id. This seems to be leftover from when the example was based on OSPF model.
<ALEX> Yes, this is a leftover.  Updated the text accordingly.  </ALEX>
> 	4.	Example P12 and P13. The RPC operation has “operational” as source (enabled is true)  and “intended” as target (enabled is false). The differences shown (in RPC output) have “value true” and “source-value false”. But I thought value came from target datastore and source-value from source datastore, so the values are reversed, i.e.. it should be “value false” and “source-value true” instead? Looking at the origin in the output I am wondering if the intent is to have “intended” as source and ”operational” as target. Or am I misunderstanding this?
<ALEX> Yes, the example contains a few mistakes.  The values were
reversed, and in addition what you mentioned I noticed an issue with the
treatment of origin metadata (only part of <operational>.  It is fixed
now.  </ALEX>
> Questions
> 	1.	YANG model: does the operation “delete” make sense for a diff operation? If it is kept, it’d be good to have some text explaining that for a diff operation, “delete” and “replace” are the same? If they’re not the same, please also add some text….

<ALEX> Here we are simply referring to the basic YANG-patch edit
operations per https://tools.ietf.org/html/rfc8072#page-11.  Those are
in turn derived from <edit-config> operations per
https://tools.ietf.org/html/rfc6241#page-37.  I am not sure we need add
to explain those, as we are directly referring to YANG-patch. 

</ALEX>

> 	2.	YANG model: prefix “cp” doesn’t seem to be a great choice since cp is associated with copying. I realize that there is some preference for 2-letter prefixes, but to me “cp” is not a great choice. What about “cmp”? WG/chairs should have a word to say about this.
<ALEX> Changing it to cmp, pending comments by WG chairs </ALEX>
> 	3.	YANG model P9, for the “uses path:yang-patch”, why not have a  reference to RFC8072 (is it because the description above mentions RFC8072)?
<ALEX> We are clearly referencing RFC 8072; are you suggesting to put a
reference substatement below the uses statement?   It looks a little
strange to me but sure, we will add it.   
> 	4.	Section 7 mentions rate limiting requests per client. Should there be a “global” rate-limiting too, i.e not client-specific?

<ALEX> I am not sure this is really needed as I think the number of
management clients will in general be fairly limited to begin with, but
we can certainly add it.  How about the following text:

OLD:

One possibility for an implementation to mitigate against such a
possibility is to limit the number of requests that is served to a
client in any one time interval, rejecting requests made at a higher
frequency than the implementation can reasonably sustain.

NEW:

One possibility for an implementation to mitigate against such a
possibility is to limit the number of requests that is served to a
client, or to any number of clients, in any one time interval, rejecting
requests made at a higher frequency than the implementation can
reasonably sustain.

</ALEX>

> 	5.	Wondering if section 8 should be in an Appendix (or even removed)? Also, the method suggested doesn’t seem to guarantee that the difference persisted for the “dampening” time.

<ALEX> Personally, I do think it makes sense to include a brief
discussion of possible further extensions.  I suggest to keep the
section if it's okay with you, or perhaps leave it to the chair whether
they have a preference to remove it.  

</ALEX>

>
> Nits:
> 	1.	P11 replace <operational<  with <operational>
<ALEX> fixed </ALEX>
>
> On 2020-09-06, 4:42 PM, "yang-doctors on behalf of Reshad Rahman via Datatracker" <yang-doctors-bounces@ietf.org on behalf of noreply@ietf.org> wrote:
>
>     Reviewer: Reshad Rahman
>     Review result: Ready with Issues
>
>     Review of rev -04 by Reshad Rahman
>
>     The document is clear and well-written. While some issues have been identified,
>     they can be resolved quickly.
>
>     Issues
>             1.      YANG model P8, for “leaf xpath-filter”, add reference to
>             RFC6021. There should also be a normative reference to RFC6021 (as per
>             RFC8407) 2.      Example P10, </interfa should be </interfaces> 3.     
>             Example P10, last paragraph talks about preference and
>             explicit-router-id. This seems to be leftover from when the example was
>             based on OSPF model. 4.      Example P12 and P13. The RPC operation has
>             “operational” as source (enabled is true)  and “intended” as target
>             (enabled is false). The differences shown (in RPC output) have “value
>             true” and “source-value false”. But I thought value came from target
>             datastore and source-value from source datastore, so the values are
>             reversed, i.e.. it should be “value false” and “source-value true”
>             instead? Looking at the origin in the output I am wondering if the
>             intent is to have “intended” as source and ”operational” as target. Or
>             am I misunderstanding this?
>
>     Questions
>             1.      YANG model: does the operation “delete” make sense for a diff
>             operation? If it is kept, it’d be good to have some text explaining
>             that for a diff operation, “delete” and “replace” are the same? If
>             they’re not the same, please also add some text…. 2.      YANG model:
>             prefix “cp” doesn’t seem to be a great choice since cp is associated
>             with copying. I realize that there is some preference for 2-letter
>             prefixes, but to me “cp” is not a great choice. What about “cmp”?
>             WG/chairs should have a word to say about this. 3.      YANG model P9,
>             for the “uses path:yang-patch”, why not have a  reference to RFC8072
>             (is it because the description above mentions RFC8072)? 4.      Section
>             7 mentions rate limiting requests per client. Should there be a
>             “global” rate-limiting too, i.e not client-specific? 5.      Wondering
>             if section 8 should be in an Appendix (or even removed)? Also, the
>             method suggested doesn’t seem to guarantee that the difference
>             persisted for the “dampening” time.
>
>     Nits:
>             1.      P11 replace <operational<  with <operational>
>
>
>
>     _______________________________________________
>     yang-doctors mailing list
>     yang-doctors@ietf.org
>     https://www.ietf.org/mailman/listinfo/yang-doctors
>
> _______________________________________________
> netmod mailing list
> netmod@ietf.org
> https://www.ietf.org/mailman/listinfo/netmod