Hi,

I was assigned to be the YANG doctor for this document. Here is my
review:

**** General comments

***** Data model organisation

The "ietf-dhcpv6" module covers schemas for DHCPv6 servers, clients and
relays. All these data are defined in the same main module, not in
"sub-modules" as the I-D says on p. 2 (unless we make a distinction
between "submodule" and "sub-module"). In my view, though, each of these
three parts should in fact be in a separate *main* module. I assume that
in many (most?) cases a particular managed device will play just one of
the three roles, so it should then advertise only the part of the data
model that's corresponds to its role. If multiple functions are
co-located in the same device, it can advertise multiple modules in YANG
library.

***** Descriptions

Many descriptions in the "ietf-dhcpv6" module provide no information
at all. This is quite typical:

    container oro-options {
        description "oro options";
        ...
    }

Such descriptions need to be expanded. In the case above, the
description should at least explain what "oro" means. In many cases,
it would also be useful to provide references to relevant RFCs and
other documents using the "reference" statement.

On the other hand, some descriptions (e.g., of "mo-tab" container) are
longer but difficult to read because of sloppy formatting. Giving these
descriptions more structure (paragraphs, itemised lists) would
certainly help.

Also, some of the descriptions are copied verbatim in Sec. 2. It may
prove difficult to keep this information synchronised in the future
revisions of the I-D. I'd suggest to trim the items in "Introduction
of important nodes" to a single sentence or so. The authoritative
description of each parameter can always be found in the module.

***** Metadata

My suggestion is to use the same or similar template for module
metadata as the existing NETMOD modules (e.g. in RFC 7223). In
particular:

- The I-D is intended for Standards Track, so the "contact" statement
  must also contain WG mailing-list and web information (see sec. 4.7
  of RFC 6087).

- Adding a new "revision" statement for every I-D revision becomes
  unwieldy. RFC 6087 permits to use just one "revision" statement
  throughout the I-D stage, and just bump the revision date in its
  argument each time after the module was changed. To keep track of
  the changes in the I-D stage, it is better to have a change log
  section in the I-D that will get removed just before the document
  becomes an RFC.

**** Specific comments

- Leafs that are list keys are mandatory by definition, the
  "mandatory" statement is not necessary for them.

- The "duidtype" type is an union with overlapping members. For
  example, in the XML serialisation it is impossible to distinguish
  between a decimal and hexadecimal number if the latter contains only
  digits. I suggest to prefix the hexadecimal string with "0x".

- The top-level nodes "server", "relay" and "client" are containers
  with presence, and their "presence" statement says "Enables server
  (relay, client)". But then each of them has the mandatory "enable"
  child. I think it would be better to define "true" as the default
  value for "enable" - then it has to be present with the "false"
  value only if the corresponding role is to be disabled while keeping
  the container in configuration.

- Each server option also has an "enable" leaf with identical
  definition, so this leaf should be defined in a grouping to avoid
  copying and pasting it to every option. Also, could this leaf have
  the default of "false" instead of being mandatory? The options that
  are not present then needn't appear in the configuration.

- Given the large number of options and their possible combination, I
  wonder whether "uint8" is enough as the type for "option-set-id".

- Are all the options required to be supported by every server
  implementation? If not, then defining corresponding features might
  be good.

- I don't understand how the "new-or-standard-option" is supposed to
  work. It seems it is there to allow for new options to be added in
  the future, but then I don't get why it is called
  "new-or-standard". More explanation in the description and an
  example in the text would be helpful.

- What is the purpose of the "ipv6-address" leafs in
  "server-attributes" and "relay-if"? How are they related to an IPv6
  address that can be configured via the "ietf-ip" module [RFC 7277]?

- Could the leaf-list "interfaces-config" under "serv-attributes"
  contain just leafref references to interfaces configured via
  "ietf-ip"?

-- 
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: E74E8C0C