Subject: Re: [dhcwg] I-D Action: draft-ietf-dhc-dhcpv6-yang-17.txt

From: t petch <[email-masked]>
Date: Tue, 2 Feb 2021 11:36:42 +0000

Ian

Looks good, although it will be some time before I digest all the
detail.

Some admin type thoughts to be going on with.

Authors: the I-D has six, which may or may not be ok with the AD, but
the YANG modules have five or six or seven which is not ok! Consistency
please.

Contact in YANG modules must include WG website and e-mail, in the YANG 
modules in Appendices as well.

NMDA or lack thereof needs a mention in Introduction or Abstract

References: the YANG modules  reference
RFC826
RFC2464
RFC4122
<https://www.iana.org/assignments/dhcpv6-parameters>
which need to be in I-D Normative References

References in the YANG modules are patchy. You need them, I think, for
many more leaves, all the timers, all the counts, as RFC and section 
therein, and for IANA Enterprise numbers (the URI)
Without them I do not know where to look to see if the YANG matches the
underlying definitions.

IANA Considerations must register the four namespaces

RFC8513 appears in several places, which I find rather telling

s.1 lacks reference for YANG and Netconf and lacks RESTCONF

Options: I would like a list of all the options supported so I do not
have to reverse engineer the YANG to find them

Abbreviations need expanding on first use

s.2.1
   *  enabled: Enables/disables the function of the //DHCPv6 /server.

s.3.1
        leaf rapid-commit {
          type boolean;
          description "A value of 1 specifies that the pool supports
boolean are true or false

        leaf client-duid {
          type binary;
for me 'duid' cries out for a YANG type definition

s.3.3
          defined in [RFC8415] is unsuccessful.";
looks like markup language which is not allowed in YANG modules

s.3.4
      typedef timer-seconds32 {
        type uint32 {
          range "1..4294967295";
4294967295 looks the maximum value which case you can say 'max'

you exclude zero which used to be a valid value for such as T1 and T2

        leaf type-code {
          type uint16;
           default 65535;
why is the default 65535?

          case duid-unstructured {
          ...
            leaf data {
              type binary;
as above, I think that this should be a type.  Were it binary, I think 
length should be restricted, such as min 3 octet max 128 octet

/description "The replay detection method used/description "The Replay
Detection Method used/

Tom Petch

----- Original Message -----
From: <[email-masked]>
To: <[email-masked]>
Sent: Friday, January 29, 2021 3:25 PM
Subject: Re: [dhcwg] I-D Action: draft-ietf-dhc-dhcpv6-yang-17.txt

> I've just posted -17 of the draft following discussion on the
structure on the netmod mailing list. The discussion is here:
https://mailarchive.ietf.org/arch/msg/netmod/uFNY9XfCcNANQNA34HpLVsF_lU8
/
>
>
> A number of changes were discussed and have been incorporated in this
update. These are:
>
> * The element specific modules previously had a second module which
described options relevant to the node. These options definitions have
been incorporated into the relevant element modules. This means there
are now total 4 modules in the draft  (instead of 7):
>
> Ietf-dhcvp6-common
> Ietf-dhcvp6-client
> Ietf-dhcvp6-relay
> Ietf-dhcvp6-server
>
> * Options which are applicable to more than one node are now defined
in the 'common' module to be imported and used by the relevant elements.
>
> * As a result, the identities for each node type is no longer needed,
so these has been removed.
>
> * Additional option definition modules no long use 'RFCXXXX' in their
naming. Short, descriptive names are used instead.
>
> * The appendix example for defining additional option definitions has
been updated along with the accompanying text.
>
> * 'Enable' nodes have been added to the client, relay and server
modules to enable disable overall function. Client and relay modules
also have enable nodes for each DHCP interface included.
>
>
> In addition, there are a number of small wording cleanups. Also,  in
the security section, a bullet point about reconfiguring the
relay-destination address has been removed. This was a duplicate bullet,
copied in error under the read-only security descriptions.
>
> Thanks,
> Ian
>
> On 29. Jan 2021, at 16:21,
[email-masked]<mailto:[email-masked]> wrote:

---

From: [email-masked]
Date: Mon, 22 Feb 2021 15:09:15 +0100

Hi Tom,

Thanks for your comments. I’ve just posted an update to address these. Also, please see inline below. 

Ian

> On 2. Feb 2021, at 12:36, t petch <[email-masked]> wrote:
> 
> Ian
> 

[if - Fixed]

> 
> Contact in YANG modules must include WG website and e-mail, in the YANG modules in Appendices as well.

[if - done]

> NMDA or lack thereof needs a mention in Introduction or Abstract

[if - done]

> which need to be in I-D Normative References

[if - done]

> many more leaves, all the timers, all the counts, as RFC and section therein, and for IANA Enterprise numbers (the URI)

[if - Done, with the exception of counters and some notifications. These are not formally defined anywhere (RFC, MIB etc.)]

> IANA Considerations must register the four namespaces

[if - Done]

> RFC8513 appears in several places, which I find rather telling

[if - Replaced with RFC XXXX]

> s.1 lacks reference for YANG and Netconf and lacks RESTCONF

[if - Added reference]

> have to reverse engineer the YANG to find them

[if - added table]

> Abbreviations need expanding on first use

[if - done]

>  *  enabled: Enables/disables the function of the //DHCPv6 /server.

[if - done]

> for me 'duid' cries out for a YANG type definition

[if - done]

> looks like markup language which is not allowed in YANG modules

[if - done]

> you exclude zero which used to be a valid value for such as T1 and T2

[if - removed the range statement as it is redundant]

> why is the default 65535?

[if - removed] 

> 
>         case duid-unstructured {
>         ...
>           leaf data {
>             type binary;
> as above, I think that this should be a type.  Were it binary, I think length should be restricted, such as min 3 octet max 128 octet

[if - The DUID type is a string and restricted to a length of 260 (to include the 2 octet type field and max 128 octet DUID)]

> /description "The replay detection method used/description "The Replay

[if - done]