Elwyn, thanks for your thorough review. I referenced it in my No Objection ballot.
Alissa > On Jan 18, 2018, at 1:00 PM, Elwyn Davies <elw...@dial.pipex.com> wrote: > > Reviewer: Elwyn Davies > Review result: Almost Ready > > [This is a Last Call Review, not a telechat review]. > > I am the assigned Gen-ART reviewer for this draft. The General Area Review > Team > (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF > Chair. Please treat these comments just like any other last call comments. > > For more information, please see the FAQ at > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-dhc-rfc3315bis-10 > Reviewer: Elwyn Davies > Review Date: 2018-01-18 > IETF LC End Date: 2018-01-24 > IESG Telechat date: 2018-01-25 > > Summary: Amost ready. To the best of my abiity I have done a thorough check > of the merging of the various earlier documents into the new -bis and the > incorporation of the various Errata. All seems to be good from this point of > view. I am slightly surprised that two other RFCs (7283 and 8213) were not > subsumed into this document. Both are pure updates of 3315 with some > important > improvements and are pretty short in themselves. I also found that the new > document did not do a good job on the interactions between relay-agents and > servers. Particularly on the processing of options between relays and servers > and details of reception of Relay-forward messages a little more detail would > halp naive readers. Finally the draft has not made much of an effort to > support possible alternative security algorithms - IETF policy now encourages > protocols to deal with algorithm flexibility - DHCPv6 as it stands is pretty > thoroughly wedded to HMAC-MD5 and would need some (relatively small) IANA > improvements plus some thought to cope with different algorithms. There are > also a fair number of nits, partly due to inconsistent cross referencing in > s18. > > Major Issues: > None > > Minor Issues: > Non-incorporation of RFC 7283: Is there a good reason why the update of relay > agent behaviour (which is apparently of general application across DHCPv6) is > not incorporated into this document as with several other items? The update > is > short and would be readily addressed in s19 here. As it is one is left with > another document with some key modifications left around rather than achieving > the unification aim of this -bis. > > Non-incoporation of RFC 8213: Similarly regarding use of IPsec for > server-relay agent comms. > > Definitions of T1 andT2: I find the definitions of T1 and T2 as replicated in > many places to be a little confusing. T1 and T2 are actually time intervals > rather than absolute times. I suggest a global substitution as follows: OLD: > The time at which the client contacts the server NEW: The time interval after > which the client would be expected to contact the server END > > s6: Might be worth pointing out the pitfalls of defining additional stateful > services as had to be sorted out with RFC 7550. > > s11.2: [This is not explicitly an issue with this document, but affects its > usage] The list of hardware types originally listed for ARP, is now well out > of date - in particular it doesn't cover Ethernet interfaces faster than > 10Mb/s. Perhas somebody could suggest some updates to Carlos Pignatoro (the > designated expert). > > s12.2: >> A client must create at least one distinct >> IA_PD. > Is it not the case that the majority of simple hosts are not concerned with > IA_PD? I think this needs qualifying to say that if a client is interested in > obtaining a prefix, then it has to create an IA_PD. > > s18.3, s18.3.11 and s19, Recording and/or reproduction of the relay address > chain in servers: I found the lack of description of server handling of > received messages embedded in relay-forward messages in s18.3 required > considerable searching to understand what needed to be done, and there is no > mention of the expectation that the relay address chain would (or at least > might) need to be recorded to facilitate later generation of Reconfigure > messages that are originated by the server. I think it would be helpful to > add > some words about this is in s18.3 as follows: ADD (probably as new para 6, et > seq): > All messages sent from clients may arrive encapsulated in Relay-forward > messages. For example, if client C sent a message that was relayed by relay > agent A to relay agent B and then to the server, the server would receive > the following Relay-forward message to process and, in general, generate an > appropriate Relay-reply to be returned to the client: > > msg-type: RELAY-FORWARD > hop-count: 1 > link-address: 0 > peer-address: A > Relay Message option, containing: > msg-type: RELAY-FORWARD > hop-count: 0 > link-address: address from link to which C is attached > peer-address: C > Relay Message option: <client message> > > Figure xx: Relay-forward Example > > Thus the server MUST record the sequence of link addresses and peer > addresses together with their associated hop count values to allow the > response to the received message to be relayed through the same relay agents > as the original client message (see Section 19.3). The server may also need > to record this information in association with the client's DUID [I think?] > in case it is needed to send a Reconfigure message at a later time unless > the server has been configured with addresses that can be used to send > Reconfigure messages (see Section 18.3.11). For messages that are not > encapsulated in a Relay-forward message responses will be sent to the source > address of the received message; this source address becomes the destination > of the Relay-reply message in the case of Relay-forward messages (see > Sections 18.3.10 and 18.3.11). > END > > s19: The treatment of additonal options in communications between > relay-agents > and servers is extremely sketchy. It appears that extensive use is now made > of > this capability which was hardly envisaged when RFC 3315 was written. I think > it would be helpful to include a little more information about how this > capability is used and maybe pointers to a couple of examples in other RFCs > that represent common usages. > > s20.4: Possibility of using alternative HMAC algorithms? Whilst it appears > that > HMAC-MD5 is still adequate and not significantly compromised, the draft does > not make much provision for possible alternatives (there are no registries > for alternative parameters in the authentication option (Section 21.11) and > s20 > is restricted to HMAC-MD5. > > Nits/editorial: > General: Conventionally tables are given titles and numbers. > > s1, para 1: Might be worth adding in a second sentence to clarify the reason > for the relay agents: "The basic operation of DHCP provides configuration for > clients connected to the same link as the server." > > s1, para 4: s/provide only/deliver nothing but/ > > s1, para 4: Mention that the stateless mode was defined in RFC 3736. > > s1.1: It would be helpful to indicate that all the various Errata have been > processed and point at > Appendix A for complete listing of changes. > > s1.1, next to last sentence: s/errate filled/errata filed/ > s1.1, last sentence: s/aforementioned/the aforementioned/ > > s3, para 1: The main IPv6 specification is now RFC 8200 rather than RFC 2460 > [idnits gets very upset!] > > s3, para 2: s/address scope/address scopes/ > > s4.2: The term "encapsulated option" is used in several places in this draft > and defined in [RFC7227]. It would be good to have a brief introduction here > as > well as a pointer to Section 9 of RFC 7227. > > s4.2, "binding": Probably worth notng that IA* and DUID are defined later in > s4.2. > > s4.2, "relaying": Add "typically conected to different links". > > s4.2, "Singleton option": s/a the/a/ > > s5.3: > OLD: > The client then performs the earlier described two message exchange. > NEW: > The client then performs the two message exchange as described earlier. > END > > s6.1, para 1, 2nd sentence: s/Stateless may be used/Stateless DHCP may be > used/ > > s6.3, para 3: s/hence use/hence the use/ (2 places) > > s6.3, para 4: s/provided prefixes/provisioned with prefixes/ > > s6.5, para 1: Is the reference to RFC 3041 essential? RFC 4941 obsoletes it. > [and idnits enquires] > > s6.6, para 3: s/to request larger prefix/to request a larger prefix/ > > s7.2: The references for the DHCP ports at > https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?&page=10 > are currently associated with Jim Bound. Unfortunately Jim has passed on many > years ago. I suggest asking IANA to update the references to this new RFC. > Also the TCP port numbers are reserved for DHCP and are used in other DHCPv6 > functions (e.g., RFC5460 but there is no allocation reference. ) > > ss9.1/9.2: What options might one get along with the encapsulated message? If > so where are they processed? See the Minor Issue raised against s19 above. > > s11.2, para 1: The reference for hardware types probably ought to be RFC 5494 > and it would handy to add in the IANA page address > (https://www.iana.org/assignments/arp-parameters/arp-parameters.xhtml#arp-parameters-2). > As mentioned in Minor Issues, relevant hardware types for higher speed > Ethernets (and potentially other types) are missing from this list. > > s14.2: T1 and T2 are time intervals: s/time(s)/time interval(s)/g > > s15: It would help to indicate that specific values for the various > retransmission parameters for each message type are given in the sections > 18.2.x and use default values taken from the table in s7.6. Suggest adding > after list of variables: ADD: > Specfic values for each of these parameters relevant to the usages of the > various messages are given in the sub-sections of Section 18.2 using values > defined in the table in Section 7.6. The selection algorithm for RAND is > common across all message transmissions. > END > > s17.2, para 2: s/(ISP network)/(typically, connected to an ISP network)/ > > s18, Fig9: Would client always send request to non-selected server? And > would > it renew/release on non-selected server? > > s18 (multiple places): In many places the first reference to an option in a > subsection is accompanied by a pointer to the part of s21 that defines it > (Good!). However this is not consistent. I have picked up several cases > where > the reference is missing but there may be others. Please check and make it > consistent. However, in the case of Client Identifier and Server Identifier > options and the IA options, they appear so many times that it might be worth > just adding a paragraph about the identifiers with the reference pointers in > Section 18. > > s18.1: >> For a rationale of this >> approach, see Section 4.2 of [RFC7550]. > I think the rationale is in s4.3 of RFC 7550. IMO I think I would have > copied > the rationale across - it is short and 7550 is being obsoleted. > > s18.2, para 3: The reference to RFC 7550 does not appear to add any value and > since this doc is obsoleting RFC 7550 I think the ref should be removed. > > s18.2, para 5: s/Server Unicast option Section 21.12/Server Unicast option > (see > Section 21.12)/ > > s18.2.1, para 6: s/IA Prefix options/IA Prefix options (see Section 21.22)/ > > s18.2.1, para 10: s/descynronize/desynchronize/ > > s18.2.1, para 12 (after retransmit parameters): s/Rapid Commit option/Rapid > Commit option (see Section 21.14)/ > > s18.2.3, para 5: s/IA Address options/IA Address options (see Section 21.6)/ > > s18.2.4, para 1: s/IA Address options/IA Address options (see Section 21.6)/, > s/IA Prefix options/IA Prefix options (see Section 21.22)/ > > s18.2.9, para 8: s/The client MUST process SOL_MAX_RT and INF_MAX_RT > options/The client MUST process SOL_MAX_RT and INF_MAX_RT options (see > Sections > 21.24 and 21.25)/ > > s18.2.10, para 2: (same as last comment) > > s18.2.10, para 6: s/Client FQDN option/Client FQDN option [RFC4704]/ > > s18.2.10.1: >> - Record T1 and T2 times, if appropriate for the IA type. > The T1 and T2 'times' received in the message are (as mentioned above) are > actually intervals: The client needs to convert these to absolute times in > some way (may of course be by setting an interval timer running!) but should > the interval just run from the time of receipt of the message - or something > more complicated? This should be noted here. > > s18.2.10.1, para 19 (I think): >> - Sends a Renew/Rebind if any of the IAs are not in the Reply >> message, but in this case the client MUST limit the rate at which >> it sends these messages, to avoid the Renew/Rebind storm. >> > > The term Renew/Rebind storm is used here just once and without definition. I > am sure that I can see that one might happen but I think the circumstances > envisioned need to be spelled out. Maybe already are at the end of s18.2.12? > > s18.3, para 5: s/Option Request option/Option Request option (see Section > 21.7)/ > > ss18.3.1, 18.3.3, 18.3.5, para 1 in each case: s/regardless if/regardless of > whether/ > > s18.3.2, para 6: s/Status Code option/Status Code option (see Section 21.13)/ > > s18.3.2, para 11: s/Reconfigure Accept option/Reconfigure Accept option (see > Section21.20)/ > > s18.3.3, last para: s/Status Code option/Status Code option (see Section > 21.13)/ > > s18.3.4, para 9: s/Status Code option/Status Code option (see Section 21.13)/ > > s18.3.5, para 8: s/Status Code option/Status Code option (see Section 21.13)/ > > s18.3.7, para 4: s/Status Code option/Status Code option (see Section 21.13)/ > > s18.3.8, para 4: s/Status Code option/Status Code option (see Section 21.13)/ > > s18.3.9, para 3: s/Reconfigure Accept option/Reconfigure Accept option (see > Section 21.20)/ > > s18.3.9, para 4: s/Option Request option/Option Request option (see Section > 21.7)/ > > s18.3.10, para 2: s/Interface-id option/Interface-id option (see Section > 21.18)/ > > s18.3.11, para 2: Provide a pointer to Section 20 for DHCP Authentication. > > s18.2.11, para 5: I suggested in the Minor Issues that some text be added to > s18 regarding collection of addressing information for use with Reconfigure > messages. If this is done, a pointer back to this text would be useful here. > > s18.4, para 1: s/Server Unicast option/Server Unicast option (see Section > 21.12)/ > > s18.4, para 2: s/Status Code option/Status Code option (see Section 21.13)/ > > s19.1, para 1: As mentioned above (in Minor Issues), I don't see why the > essence of RFC 7283 is not incorporated into this document. > > s20.3: Does there need to be a registry for RDM values? > > s21: Including a general statement that general numeric values are encoded as > unsigned integers would be helpful - this is not done consistently through the > options at present. > > s21.4, last para: s/is taken to ("infinity")/is taken to mean "infinity"/ I > think this para would be more useful as the third last para, i.e., moved up > two > paras to just after > The values in the T1 and T2 > fields are the number > of > seconds until T1 and T2. > > s21.8: The pref_value should be specified as an unsigned integer. > > s24: It would probably good to take on the allocations of well-known ports as > owned by this document (see comment of s7.2) For future proofing of > authentication, it might be desirable to start registries for some of the > DHCP > Authentication parameters. > > s24, Note (4) on Table 1: Need to make RFC8156 an informative ref. > > s27.1: RFC 7083 is being obsoleted... making it a normative reference is not > allowed. > > s27.2: I would say that RFC 8213 needs to be normative unless (as I would > recommend) the text is brought into this document, in which case it is > unnnecessary. > > > _______________________________________________ > dhcwg mailing list > dh...@ietf.org > https://www.ietf.org/mailman/listinfo/dhcwg _______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
