I have been selected as the General Area Review Team (Gen-ART) reviewer for this draft (for background on Gen-ART, please see http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).
Please resolve these comments along with any other Last Call comments you may receive. Document: draft-ietf-behave-nat-icmp-05.txt Reviewer: Miguel Garcia <[EMAIL PROTECTED]> Review Date: 2007-10-09 IESG LC end date: 2007-10-11 Summary: Comments: - Although the document contains a downward normative reference to RFC 3022, I think it is justified. So, there isn't a proposed action. - Section 4.1 discusses the checksum of the payload for TCP and UDP packets that are embeded in ICMP Error messages. What about UDP-lite (RFC 3828) packets with partial checksum coverage? Applying the same rules as for UDP would drop legitimate packets. I think the draft should say what the NAT should do with respect ICMP Error messages that contain UDP-lite packets. - Section 4.1, I am surprised that the recommendation is for NATs to validate the IP checksum of the payload in ICMP errors. I thought NATs should forward packets fast, not recalculate two checksums (that of the payload, and that of the ICMP packet). Perhaps there is a reason for this behavior. Dropping a packet at the NAT would also make difficult to trace errors in the NATted host. Nits: - The draft should add formal references to RFC 3022 (Section 4.2) and RFC 1822 (several sections) - Abbreviations should be expanded at the first appearance, e.g., NAT and ICMP in the title, ICMP in the Abstract as well, etc. - Capitalization should be consistent. Compare how "Packet too big" and "Error Message" is capitalized in the title of Sections 7.1.1 and 7.1.2. Also, notice the different capitalization styles for ICMP Query Messages and ICMP Error Messages throughout the document. - As a personal taste, I don't like to mention names of working groups in RFCs, because those are temporary names, and they stop existing once the work is done. So, perhaps the authors could do an effort to avoid the usage of the term "BEHAVE" in the document. - Section 3.1, first paragraph, says: "A NAT device MUST permit ICMP query based applications ...." I think the NAT does not know much about _applications_, but knows about _messages_. So I would recommend to replace "ICMP query based applications" with "ICMP Query messages". - Section 5 deals with hairpinning support for ICMP packets. It would be nice to have a single sentence definition of "hairpinning" at the beginning of the section. The reader may or may not be familiar with its meaning. /Miguel -- Miguel A. Garcia tel:+358-50-4804586 Nokia Siemens Networks Espoo, Finland _______________________________________________ Gen-art mailing list [email protected] https://www1.ietf.org/mailman/listinfo/gen-art
