Draft: draft-ietf-hip-base-06.txt Reviewer: Gray, Eric [Eric.Gray@marconi.com] Review Date: Wednesday 9/13/2006 7:02 PM CST IETF LC Date: 9/14/2006 Summary: I believe this draft is nearly ready for publishing as an Experimental RFC. I have the following minor comments, NITs and requests for clarification... ============================================================= In the Introduction section (1.0), you say: "Other elements of the HIP architecture are specified in other documents, including how HIP can be combined with a variant of the Encapsulating Security Payload (ESP) for integrity protection and optional encryption, mobility and multi-homing extensions to HIP, extensions to the Domain Name System (DNS) for storing Host Identities there, proposals on added HIP-related infrastructure into the networks, and techniques for NAT traversal." Which "other documents"? It might help if this was set up as a bullet list with specific references for each of the bullets. ____________________________________________________________ Under subsection "Parameter Type" (bottom of page 87), you say "... a HIP parameters ..." Should this be either - "... a HIP parameter ..." or "... HIP parameters ..." ? ____________________________________________________________ Under this same section, after listing type codes assigned by this specification (sections 5.2.3 through 5.2.18), you say: "The type codes 0 through 1023 and 61440 through 65535 are reserved for future base protocol extensions, and are assigned through IETF Consensus." In my quick look at the list, it looks as if these values are all taken from these ranges. I suggest changing the wording to reflect this - for example - by starting with "With the exception of the above assigned type codes, ..." ___________________________________________________________ Can you differentiate the double place-holder names (two for each of ECHO_RESPONSE and ECHO_REQUEST)? In the draft, you list each as two separate values (in each case, one for one context and the other for another context), each with a different assigned value. This seems messy/confusing. Perhaps you could (using the ECHO_RESPONSE as an example) make the place holder names along these lines: ECHO_RESPONSE_COV = 961 ECHO_RESPONSE_UNCOV = 63425 ? I think you could then say - "There are 2 ECHO_RESPONSE types: ECHO_RESPONSE_COV and ECHO_RESPONSE_UNCOV" - (in the text in section 5.2.18). If you want to keep the names short, perhaps ECHO_RESP_COV and ECHO_RESP_UCO - or you could use the "_2" convention you use with HMAC and HIP_SIGNATURE? The editing requirements to fix this are non-trivial as there are places where ECHO_REPONSE is correct and others where it should be one or the other specific type values. I believe ECHO_RESPONSE applies where you talk about the object (format, etc.) while one of the other terms should apply if you are talking about the actual type number. The same observations and analogous changes would apply to ECHO_REQUEST. For one thing, the current wording will likely "sticky things up" during IANA review. Short of including all of the approriate explanatory text in the file they use to list assigned numbers, a simple listing of assigned values is going to be ambiguous. __________________________________________________________ A similar condition exists for NOTIFY - which is the name of both a packet and a parameter. This might be somewhat easier to dis-ambiguate - clearly one is a packet type while the other is a parameter type - but may this still may be a point of confusion. A developer looking at the assigned numbers file may confuse one with the other. We could argue that this is a "darwinian influence" in the long-term viability of individual developers, but we're not really in this to make life difficult, are we? :-) I suggest either NOTIFY_PKT and NOTIFY_PAR, or NOTIFY and NOTIFY_2 (you pick which is associated with the packet and which the parameter) - at least where you are clearly talking about the assigned type number. I believe that - unlike the above case - it is always the case that you are either talking about the NOTIFY packet or the NOTIFY parameter, hence the editing to fix this is simpler. _________________________________________________________ At the top of page 59, you say: "An UPDATE that does not contain a SEQ parameter is simply an ACK of a previous UPDATE and itself MUST not be acked." "... MUST not ..." should be "... MUST NOT ..." _______________________________________________