A YANG Data Model for DHCPv6 Configuration
draft-ietf-dhc-dhcpv6-yang-25

[S1.2; nit]

* s/demonstrate how this is can be/demonstrate how this can be/

[S3.1; nit]

* s/for an failure/for a failure/

[S3.2, S4.3]

* Should RFC 4649 remote-id be included here?  Or, perhaps, did RFC 8415
  S21.18 OPTION_INTERFACE_ID effectively obsolete OPTION_REMOTE_ID use
  cases (or perhaps I'm just confused)?

[S3.3, 4.4; question]

* For the user class and vendor class options, should they be annotated as
  "not anon-profile"?  I'm curious about these elements w.r.t. to
  RFC7844 S4.8.

I suggest breaking Section 6 into two subsections, one for each batch of registry operations.

A possibly ignorant question about YANG modules: Since RFCs 3319 and 8987 are referenced from within the module itself, shouldn't they be normative references?

Thank you to Vincent Roca for the SECDIR review.

** (Editorial) Regex constraining hex strings.  When I first read the pattern constraining typedef duid-base (and the same is true in other places), I didn’t appreciate that this regex was operating on a string containing a hex representation of octets. 

** There are a few places in the three yang modules where it appears that human-readable messages are being returned.  What is the expected approach (if any) for conveying a language tag per the guidance in Section 4.2 of BCP18?  An incomplete list of these places are:

-- Section 4.1. grouping status and status-code-option-group has a leaf message

-- Section 4.2. rpc delete-address-lease and delete-prefix-lease return a leaf return-message

-- Section 4.3. leaf return-message in the rpcs

** Section 4.1. grouping status has a leaf message which is explicitly described as of “type string” and is clarified as being a “UTF-8 encoded string ... that isn’t null-terminated”.  No issues with that guidance.  I’m wonder whether the other strings mentioned in the previous comment should also be described in this way.

** Section 5.  Additionally threats to document would be:

-- Generalize the threat of redirecting clients to services under the attackers’ control (e.g., DNS server or WPAP).  Say:

OLD
* Various attacks based on re-configuring the contents of DHCPv6
      options, leading to several types of security or privacy threats.
      For example, changing the address of a DNS server supplied in a
      DHCP option to point to a rogue server.

NEW
* Various attacks based on re-configuring the contents of DHCPv6 options, leading to several types of security or privacy threats.  These options could redirect clients to services under an attacker’s control. For example, changing the address of a DNS server supplied in a DHCP option to point to a rogue server.

-- Ability to read the leases from the Server or Relay could help the attacker fingerprint device types.

OLD
These subtrees and
   data nodes can be misused to track the activity of a host:

NEW
These subtrees and data nodes can be misused to track the activity or fingerprint the device type of the host:

Thanks to the authors for their diligent efforts on this document. I really get impressed by all the YANG model documents I review. I don't think I have much to comment to improve this specification or something that I am worried about.

I however, think RFC 8987 should be in the normative reference as it is referenced from the "RPCs" section. You can let me know it should not.

Many thanks for the updates in the -25!

I have no further comments other than to apologize for having taken
so long to review the changes and remove my DISCUSS.  Great work!

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 4.1. , paragraph 10, nit:
-        to the license terms contained in, the Simplified BSD License
-                                               ^ ^^^^^^
+        to the license terms contained in, the Revised BSD License
+                                               ^^^ ^

Also elsewhere, please change them all. (The TLP recently changed.)

Section 3.1. , paragraph 3, nit:
>  than 128-octets) content field. Currently there are four defined types of D
>                                  ^^^^^^^^^
A comma may be missing after the conjunctive/linking adverb "Currently".
[SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA]

Section 3.1. , paragraph 5, nit:
> ed when there is a status message for an failure. 3.2. DHCPv6 Relay Tree Dia
>                                       ^^
Use "a" instead of "an" if the following word doesn't start with a vowel sound,
e.g. "a sentence", "a university". [EN_A_VS_AN]

Section 3.3. , paragraph 3, nit:
> th (1-128 octets) content field. Currently there are four defined types of D
>                                  ^^^^^^^^^
A comma may be missing after the conjunctive/linking adverb "Currently".
[SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA]

Section 4.1. , paragraph 20, nit:
>  this option to tell the client whether or not to accept Reconfigure messages
>                                 ^^^^^^^^^^^^^^
Consider shortening this phrase to just "whether". It is correct though if you
mean "regardless of whether". [WHETHER]

Section 4.2. , paragraph 38, nit:
> nitions for DHCPv6 options that can be be sent by the client. Additional opti
>                                     ^^^^^
Possible typo: you repeated a word. [ENGLISH_WORD_REPEAT_RULE]

Section 4.4. , paragraph 29, nit:
> tions The following section provides a example of how the DHCPv6 option defi
>                                      ^
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour". [EN_A_VS_AN]

Section 4.4. , paragraph 29, nit:
> escription "SIP server list identifier identifier."; } leaf sip-serv-domain-n
>                             ^^^^^^^^^^^^^^^^^^^^^
Possible typo: you repeated a word. [ENGLISH_WORD_REPEAT_RULE]

Section 4.4. , paragraph 33, nit:
> on "Configures how the server will stores leases."; choice storage-type { de
>                                    ^^^^^^
The modal verb "will" requires the verb's base form. [MD_BASEFORM]

Section 4.4. , paragraph 35, nit:
> rname { type string; description "User name of the account under which the se
>                                   ^^^^^^^^^
It's more common nowadays to write this noun as one word.
[RECOMMENDED_COMPOUNDS]

Section 5. , paragraph 2, nit:
> rname { type string; description "User name of the account under which the se
>                                   ^^^^^^^^^
It's more common nowadays to write this noun as one word.
[RECOMMENDED_COMPOUNDS]

Thanks for publishing another YANG module.

I found this document well written, and in particular I thought that the section on options extensibility and examples in the appendix to be thoughtful and helpful.  I ​only have a few minor (non blocking) comments on the structure of the YANG module that you may wish to consider:

1. You may want to consider putting the counters under a container to group them together, and perhaps to make it easier for a targeted telemetry subscription.

2. The paths on your RPCs and top level Notifications are using relative paths - it would probably be clearer to use absolute paths here.

3. I would remove "container" from the Option description strings, to improve help string readability.

4. Please make sure that you fix the 3 idnits YANG warnings (e.g., alignment of the revision-date to the extracted file date).

Perhaps in a future draft the IETF will manage to standardize common configuration for the class selection function since that will help interop but I can appreciate that is tricky to get consensus on if the existing models/CLIs are far apart, and you seem to have taken a pragmatic approach here.

Regards,
Rob