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

["Acee Lindem (acee)" <[e-mail]>]

Hi Ian, 

Thanks for addressing my comments. Sorry for the delay in responding - needed to get a block of time to context switch back to these models. I agree with all you responses but have some inline comments. 

On 6/1/21, 12:49 PM, "[e-mail]" <[e-mail]> wrote:

    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 <[e-mail]> 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: [e-mail]
    >         [e-mail]
    >         [e-mail]
    >         [e-mail]
    > 
    > 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]

I agree for DHCPv6 client. I think it is somewhat product/deployment specific which interfaces will be automatically included in the client-if list. For example, if the product has no prior configuration, the product may want to add all of them. Whereas, if there are manually configured IPv6 addresses or other techniques (e.g. SLAAC) on an interface, it may not be automatically added. Anyway, this isn't something that can be reflected in the model. 

    > 
    >    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.’]

I'm not sure what I was thinking - what you have is probably what I meant. 

Thanks,
Acee

    > 
    > 
    > 
    > 
    > _______________________________________________
    > dhcwg mailing list
    > [e-mail]
    > https://www.ietf.org/mailman/listinfo/dhcwg