Re: [dhcwg] Yangdoctors early review of draft-ietf-dhc-dhcpv6-yang-19

ianfarrer@gmx.com Tue, 01 June 2021 16:48 UTC

Return-Path: <ianfarrer@gmx.com>
X-Original-To: dhcwg@ietfa.amsl.com
Delivered-To: dhcwg@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 42D6D3A1EDC; Tue, 1 Jun 2021 09:48:57 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.896
X-Spam-Level:
X-Spam-Status: No, score=-1.896 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=gmx.net
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 tQojb7Rcu9Vu; Tue, 1 Jun 2021 09:48:52 -0700 (PDT)
Received: from mout.gmx.net (mout.gmx.net [212.227.17.22]) (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 28D7C3A1EFA; Tue, 1 Jun 2021 09:48:51 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1622566124; bh=cqPoAzRFCg1KPdA7nsvq66UjqS42uNFW16c2jH3Rf8k=; h=X-UI-Sender-Class:Subject:From:In-Reply-To:Date:Cc:References:To; b=PCrQPfX4Yrbw0Dgc/zrb60lY7/6RZSKdQ3Fj4qhn0AEb2gXiAKZOKjBxnO9bc/KZN r41nzLUNJwf/061nQlZYvv3Bba4gMgAHeQdt+H8GJkwOSuAbSWzDEUyy918uELiHfr uQYkGnKL5NOb8Rsz1e0yKhGOyGw6n6ohm3pdaW7Y=
X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c
Received: from smtpclient.apple ([78.35.172.230]) by mail.gmx.net (mrgmx105 [212.227.17.174]) with ESMTPSA (Nemesis) id 1M89Gt-1ljT0Z1KBX-005Ey3; Tue, 01 Jun 2021 18:48:44 +0200
Content-Type: text/plain; charset="utf-8"
Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.100.0.2.22\))
From: ianfarrer@gmx.com
In-Reply-To: <162025036687.13890.17585626720117932864@ietfa.amsl.com>
Date: Tue, 01 Jun 2021 18:48:41 +0200
Cc: yang-doctors@ietf.org, dhcwg@ietf.org, draft-ietf-dhc-dhcpv6-yang.all@ietf.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <1773D1AE-F921-46A8-A881-A014596B0397@gmx.com>
References: <162025036687.13890.17585626720117932864@ietfa.amsl.com>
To: Acee Lindem <acee@cisco.com>
X-Mailer: Apple Mail (2.3654.100.0.2.22)
X-Provags-ID: V03:K1:Oz2Z90P/s4outBpd55keCxwg3U7NyB6V1fCVgYAuW3CKIWcJa7Y oa3Mr4+r+qeTsvFGSdWJE6C3DpDoMVZuPh3z2xX7X+Sx2vtbKmiZ2LT1IoNSi6+i7o0m/eu n8d68lSGH0of1fQfT6gQhYcxF8RYidRoOt8ATIM9SjI7Qm7cpqLpq/NvwxS9xUViSAhK8Zw ZdcwKOhY1XdEplRcoLRAg==
X-UI-Out-Filterresults: notjunk:1;V03:K0:IwM+66FrOBw=:aPmSEUb5CI9Tly8UKqoMoD ppyOVyYoMSlmiIpc1256J0ew+cXw3MbLjtBkwJbXWStXuKxdKizY5arcnTig87VANdnWlqn3g lSlkmYsieFPplQAUetzyOF9WboU/AVtFODyU37SeaSQWbIbrfMD1Z34IwMbN7SilXL/gW+z/g ++G2sYA2sornW7yuaJV4PCCqj9ragsufRHtyAAq/5+JfJviLMs5EyeAuF4wwsHMk4DCQuVbUA MbGOfIDb8q+eK6O/NmRFyUrNjDUqb5UYcSac8LPq/A6L8FlOUCRP9sIpswxKRVhxjsgJodhfr 3Ra7S/7Lz+ber1VmqWx2OXVOAJ8lZCDl4xU5QvTVoeHjuxFoZ9FPsko6fzH+33Az1++ti7A6u oUnaa0dqNSl+HMvjsOJYAkC4novTxla1lHOgCTAaf50RvzwabygR5Pzza6ixO2UmV0RC28P0q OpNemdzBmoh4FZ9XjNIY1OLTb4cz3/BvX7v09xpfKR84EHSTZCmlVE1n/3MZYxpCZBTx/28Sm V5gQEa4D7oVHVo26Wj+blBDCHTE5gGrrfCYXFOSakbte0NnO40epwONLmpeySb97W3noCMw0i mNHIu3urvhe2PV48He74ljJ9689BSERAcP3+OvxcrW+9XX46DksQeV38oh939hyphJ6MvCbQM SixwqAU36ul0RZPeka/4DRWo7DFW23+sbeJxs+1PRaicJW0vb3cyMrRY0Uf5gd8i7XZmToslf FXg07XaIMaI9eiaIKkFmfJA0zkC6PncM40V+wkdCxft1wBeeiJJQlK9UQQZr15vGlVCzAjsue Cxo0lfV8SYtKCjy3XZGx/2sJgVOyeMt0vKXSXJcYZHfUhqUqtWXSBZ2QAhOSAPJeH8MeSK1Tv v+gzJg2BjBgGcygCLUMZqBznEVY/nURK5eJSyxUEeJluAppUUCOWO7eOcWlqv03tOSDcHqGE3 6CJPX/ljv5dpm2IkzdFa1TtDSqrt8ir2F+giuTLTYSKyjDb4BY7TeV3faBk2XwmqZgaF9sOsO vcBqxqcwkX3MGV36TBUuENoRerB3Lmd3JJ+M1/aRnoVmuFX6ZJuojLh7cqDI2Npy6vLKs/xUs deUs0mRy5jlN8QBOKs23dX/j+c6VSQ/FxYg
Archived-At: <https://mailarchive.ietf.org/arch/msg/dhcwg/SWOmnijE_r03FNp7dkiRzMQrnJc>
Subject: Re: [dhcwg] Yangdoctors early review of draft-ietf-dhc-dhcpv6-yang-19
X-BeenThere: dhcwg@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: <dhcwg.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/dhcwg>, <mailto:dhcwg-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/dhcwg/>
List-Post: <mailto:dhcwg@ietf.org>
List-Help: <mailto:dhcwg-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/dhcwg>, <mailto:dhcwg-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 01 Jun 2021 16:49:06 -0000

Hi Acee,

Thanks again for your review. My apologies for how long to work through them all and prepare the update. I’ve just posted -20 which aims to address your comments.

Please see inline below.

Additionally in this version, there are some small errors corrected (typos, incorrect regexs) and the example YANG modules have been renamed and have namespaces according to RFC847.

BR,
Ian

> On 5. May 2021, at 23:32, Acee Lindem via Datatracker <noreply@ietf.org> wrote:
> 
> Reviewer: Acee Lindem
> Review result: On the Right Track
> 
> Document: draft-ietf-dhc-dhcpv6-yang-19
> Reviewer: Acee Lindem
> Review Date: May 5, 2021
> Review Type: Early Review
> Intended Status: Standards Track
> Summary: On the right track - Issues and questions need to be resolved.
> 
> Modules: ietf-dhcpv6-server@2021-03-17.yang
>         ietf-dhcpv6-relay@2021-03-17.yang
>         ietf-dhcpv6-client@2021-03-17.yang
>         ietf-dhcpv6-common@2021-03-17.yang
> 
> Tech Summary: The document contains the base configuration and operational
>              YANG model for the DHCPv6 protocol. The basic structure is
>              very good but the major issues need to be addressed prior to WG
>              last call.
> 
> Major Comments:
> 
>    1. Should the DHCP server, relay, and client functions be enabled by
>       default? It seems they are require specific configuration to be
>       viable.


[if - I’ve removed ‘default enabled’ from server and relay and
left it present for client, as discussed]

> 
>    2. The threshold type in the ietf-dhcpv6-common is strange. It is a
>       union with an enumeration to disable the threshold. Normally, if
>       there is no threshold, you would simply not specify it. However,
>       the data nodes of this type are mandatory. I'd make it a simple
>       type and remove the mandatory designations for the data nodes. Also,
>       the range should not start at 0% since this % makes no sense.

[if - Removed enum and changed type to uint8
Removed ‘mandatory true’ from the data nodes in the server module
Changed range to 1..100]

> 
>    3. There are examples of augmenting the ietf-dhcpv6-server module but
>       no "Module Usage Examples" as specified in section 3.12 of RFC 8407.

[if - Added Appendix A with XML examples for all of the element
modules]

> 
> Minor Comments:
> 
>    1. While not required by RFC 8407, many YANG RFCs explcitly call out
>       the interaction with imported YANG modules in a separate section.

[if - I've extended the description in the introduction to describe
interactions]

> 
>    2. No sense in maintaining all the intermediate revisions in the
>       modules. Just update the one that is the initial version and update
>       the date.

[if - Removed]

> 
>    3. The module prefixes are very descriptive but a bit long. Given
>       the examples of augmentations, this will be especially true for
>       DHCPv6 server augmentations. Perhaps, dhc6-serv, dhc6-rly, and
>       dhc6-clnt would be better.

[if - Changed. 'dhcpv6-common’ has also been shortened to ‘dhc6’]

> 
>    4. Can host-reservation prefixes overlap with holes? If so,
>       reserved-prefix may not be unique. If not, no problem.

[if - For the DHCP server implementations that I am familiar with,
the prefixes will be checked when config is applied and any prefix
overlaps will be rejected as invalid.]

> 
>    5. For nodes with patterns, describe what the pattern allows in
>       the description with an example or two. This applies to
>       link-address, duid-base, duid-llt, duid-en, duid-ll,
>       duid-unstructured, and sub-option-data.

[if - added examples in the description fields]
> 
>    6. With respect to link-address, what type of address is this? If it is
>       an IPv6 link-local address, there is an ipv6-adddress type in RFC 6021.

[if - link-address should be a GUA. I’ve changed the type to ipv6-address.]

> 
> Nits:
> 
>    1. IETF documents should use US English - not UK English. I've
>       changed in suggested edits.

[if - Incoporated the proposed changes, see below] 
> 
>    2. Description format - Sometimes starting right have "description"
>       and sometimes starting on the next line.

[if - Moved description text to start on the next line throughout.]

> 
>    3. sol-max-rt-option-group and inf-max-rt-option-group should spell out
>       the words in the description rather than just repeating the short
>       abreviations.

[if - Changed]

> 
>    4. In ietf-dhcpv6-client,  for ia_ta and ia_pd, spell our
>       acronyms in the descriptions rather than just repeating them (which
>       is useless).Is "ia" "interface address"? Don't make the reader
>       go to the DHCPv6 RFC to know what you mean. What is "ia_ta"?

[if - Added expanded version in the description for each IA (e.g. IA_PD (Identity
Association for Prefix Delegation)).]

[if - for the remaining diffs, I’ve incorporated them exactly as suggested, with the
Exception of one comment below:

> ***************
> *** 1216,1222 ****
>             path "/dhcpv6-server/option-sets/option-set/option-set-id";
>           }
>           description "The ID field of relevant set of DHCPv6 options
> !            (option-set) to be provisioned to clients of this
>             network-range.";
>         }
>         leaf valid-lifetime {
> --- 1218,1224 ----
>             path "/dhcpv6-server/option-sets/option-set/option-set-id";
>           }
>           description "The ID field of relevant set of DHCPv6 options
> !            (option-set) to be provisioned to clients of the using
>             network-range.";
>         }
>         leaf valid-lifetime {

[if - I'm not sure if the intended change here is correct. I've
changed the wording to 'clients using the network-range.’]

> 
> 
> 
> 
> _______________________________________________
> dhcwg mailing list
> dhcwg@ietf.org
> https://www.ietf.org/mailman/listinfo/dhcwg