Miguel, Thank you for the comments. Please see my responses inline.
regards, suresh --- Miguel Garcia <[EMAIL PROTECTED]> wrote: > 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. > [suresh] There are a few things to note here. a) REQ-3d is transport protocol agnostic and is stated as follows. This is applicable to UDP-lite as well, assuming the NAT device supports UDP-lite. If the embedded packet contains the entire transport segment, and the transport protocol of the embedded packet requires the recipient to validate the transport checksum, and the checksum fails to validate, drop the error packet. b) UDP-Lite is an independent transport protocol with a protocol identifier of its own (136). The Behave WG has not dicussed protocols other than TCP and UDP. RFC4787 (UDP BEHAVE) does not discuss UDP-lite either. So, I dont believe, it woudl be necessary to include specific comment about UDP-lite in this document. > - 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. > [suresh] Fast is not as important as doing the right thing. RFC1812 says that a router MUST verify the IP checksum of any packet that is received, and MUST discard messages containing invalid checksums. Besides, newer revision endhosts do not accept ICMP error messages with a mismatched IP or TCP checksum in the embedded packet. Hence the recommendation to validate IP checksum. > Nits: > > - The draft should add formal references to RFC 3022 (Section 4.2) and > RFC 1822 (several sections) > [suresh] Will do. Thanks. > - Abbreviations should be expanded at the first appearance, e.g., NAT > and ICMP in the title, ICMP in the Abstract as well, etc. [suresh] OK, I will expand NAT to be "Network Address translator" in the title. However, I dont believe, it is necessary to expand on well known terms like IP and ICMP. > > - 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. > [suresh] OK. I will fix that. > - 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. > [suresh] I agree with what you say. However, this document needed to refer to protocol specific BEHAVE drafts several times. So, I had no choice. This was a conscious use of the term. > - 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". > [suresh] OK. > - 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. [suresh] The first sentence in section 5 reads - "[BEH-UDP] and [BEH-TCP] mandate support for hairpinning for UDP and TCP sessions respectively on NAT devices."; and the first sentence in the terminology section reads - "Definitions for majority of the NAT terms used throughout the document may be found in [NAT-TERM] and [BEH-UDP]." Given this, it does not seem necessary to repeat the definition again in this document, right. regards, suresh > > /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
