Re: draft-ietf-6man-udpchecksums-02.txt

> Dear Gorry;
>
> Thanks for the read.
>
> On Tue, Mar 13, 2012 at 4:47 PM, Gorry Fairhurst <gorry@...> wrote:
>>
>> Marshall &  Philip, I've read the new draft, and have a few new comments.
>> Hope these are helpful - happy to discuss if this seems useful.
>>
>> Best wishes,
>>
>> Gorry
>>
>> ----
>> BOILER PLATE:
>> - Please an Updates line:goprry
>> "Updates RFC2460 (if approved)"
>>
>> ----
>> Section 3:
>>  "there is no additional
>>   benefit and indeed some cost to nodes to both compute and check the
>>   UDP checksum of the outer (encapsulating) header."
>>
>> - I do not think this sentence is really true, and I think it's
>> important that we do get this right. I suggest there ARE benefits in
>> performing a UDP integrity check for IPv6, see Section 3 of
>> I-D.ietf-6man-udpzero. However, I agree that omitting this check can
>> reduce the forwarding cost for systems that perform the check in software.
>
> How about
>
> there is both a benefit and a cost to  compute and check the
> UDP checksum of the outer (encapsulating) header. In certain cases,
> where reducing
> the forwarding cost is important, such as for systems that perform the
> check in software,
> the cost may outweigh the benefit; this document describes a means to
> avoid that cost, in
> the case where there is an inner header with a checksum.
>
>

>
>> ----
>> Section 3:
>>  "to set the checksum field of the
>>   outer header only to 0 and skip both the sender and receiver
>>   computation."
>>
>> - Could we insert a "UDP transport" between "outer" and "header" above,
>> so there can be no confusion with the network layer headers.
>
> OK
>
>>
>> ----
>> Section 4:
>>   "In Section 5.1 of [I-D.ietf-6man-udpzero], the authors propose nine
>>   (9) constraints on the usage of a zero checksum for UDP over IPv6.
>>   We agree with the restrictions proposed, and in fact proposed some of
>>   those restrictions ourselves in the previous version of the current
>>   draft.  These restrictions are incorporated into the proposed changes
>>   below."
>>
>> - This reads rather like a paper. ietf-6man-udpzero is also a working
>> group document, and I would like to think we can find better language.
>> I suggest something like:
>>
>>   "Section 5.1 of [I-D.ietf-6man-udpzero], identifies 9 requirements
>>   that introduce constraints on the usage of a zero checksum for
>>   UDP over IPv6. This document is intended to satisfy these
>>   requirements."
>>
>
> This WFM,
>
>> ----
>> Section 4:
>>  "inspection firewall devices or other middleboxes actually checking"
>> - remove "actually"?
>
> WFM
>
>>
>> ----
>> Section 4:
>>  "This is would be an issue also for legacy systems
>>   which have not implemented the change in the IPv6 specification.  So
>>   in any case, there may be packet loss of lightweight tunneling
>>   packets because of mixed new-rule and old-rule nodes."
>>
>> - Seems like it could be improved. I suggest:
>>  "This would be an issue also for any legacy IPv6 system
>>   that has not implemented the change to the IPv6 specification. In
>>   this case, the system will discard the zero-checksum UDP
>>   packets, and should log this as an error."
>>
>
> This is not a prescription, but an observation (i.e. we can't say
> SHOULD log this as an error here).
>

>
>> ----
>> Section 4:
>> "   As an example, we discuss how can errors be detected and "
>>                                  ^^^
>> - remove "can"
>
> ACK
>
>>
>> ----
>> Section 4:
>>  "Note that other (non-tunneling) protocols may have
>>   different approaches."
>> - suggest replace add:
>>  ", but these are not the topic of this update."
>>
>
> WFM
>
>> ----
>> Section 4:
>>  "The control packets can
>>   also contain any negotiation that is necessary to set up the
>>   endpoint/adapters to accept UDP packets with a zero checksum."
>> - this needs to be enabled for each flow, so please add:
>> "The control packets may
>>      also carry any negotiation that is necessary to set up the
>
> how about
>
> s/that is necessary to set up the/required to enable the/
>
>>      endpoint/adapters to identify the set of ports that
>>      need to enable reception UDP datagrams with a zero
>>       checksum."
>>

>> ----
>> Section 4:
>> "Only UDP packets containing tunneled packets should have a UDP
>>      checksum equal to zero."
>> - Please use keywords. e.g.
>>  "A system MUST NOT set the UDP checksum to zero in packets that do
>> not contain tunneled packets."
>>
>
> OK
>
>> ----
>> Section 4:
>> "We note that
>>      these outcomes not equally likely, as most require multiple bit
>>      errors with errored bits in separate fields. "
>> - It seems wrong to suggest that single bit inversions are a likely
>> cause of residual errors at the transport layer. I do not think this is
>> the case. I suggest direct replacement of a sequence of bytes is a
>> likely form of corruption. Suggest you omit the second part of the sentence?
>
> OK. Note that there is a missing "are" before "not," which I have also fixed.
>

>>
>> ----
>> Section 4:
>> " If it is some
>>         other application, with very high probability, the application
>>         will not recognize the contents of the packet."
>> - I disagree - it all comes down to applications design. The detection
>> is only if the app is defined this way, since the transport does not
>> know, etc. RFC 5405 provides recommended best current practice for
>> applications in general.
>
> Do you have alternate text ? How about s/will not/may not/ ?
>

>>
>> ----
>> Section 4:
>> - I think the wording of this section needs to be tightened.It would be
>> good to see more of a focus on what needs to be implemented, rather than
>> discussing the guidance provided in the other working group document.
>>
>> If the guidance itself needs to be updated, then it would be good to
>> explore this - We'd be happy to update the text and guidance if there is
>> something more that should be said.
>>
>> ----
>> Section 5:
>> "   outer encapsulating packet of a lightweight tunneling protocol."
>> - is it intentional to permit this only for *lightweight* tunneling
>> protocols? - The initial request was for *any* tunneling protocol that
>> meets the requirements.
>
> s/lightweight //
>
> I am not sure why we  say "lightweight tunneling protocols" - I think
> that is a legacy from previous discussions. I don't think that there
> is a definition of what "lightweight" means , and propose changing
> most occurrences of it to simple "tunneling protocols."
>

>>
>> ----
>> Section 5:
>> "UDP
>>   endpoints that implement this solution MUST change their behavior and
>>   not discard UDP packets received with a 0 checksum on the outer
>>   packet of tunneling protocols."
>> - I think we really should add a few words about being enabled only for
>> the set of ports that are supported by system for use by tunneling
>> protocol(s).
>>
>
> OK
>
>> ----
>>
>> Section 5:
>> "its behavior should be as specified originally."
>> - replace "should be as specified originally", by "is not updated and
>> uses the method specified in RFC2460".
>>
>
> OK
>
>> ----
>> Section 6:
>> "   The persistence of this issue among a significant number of
>> protocols being developed in the IETF requires a definitive policy."
>> - Is this a recommendation for more work, or a statement of the
>> motivation for this document. I was not sure.
>
> The latter. I will adjust the text to  make that clear.
>
>>
>> ----
>> Section 6:
>> - I think it could be useful to include the points in section 6 possibly
>> also as a part of ietf-6man-udpzero. What do you think?
>>
>
> OK by me.
>
>>
>> ----
>> I'd suggest we add a normative citation to RFC 5405, stating that this
>> provides the IETF guidance on the design of applications using UDP and
>> includes guidelines on the usage of checksums by application designers.
>>
>> - Possibly best placed in the introduction, indicating that this
>> document adds to this for the case of tunneling applications.
>
> OK. I will reference section 3.4
>
> Thanks again !
>
> Regards
> Marshall
>
>>
>> ----
>>
>>
>> --
>> Prof Gorry Fairhurst, School of Engineering, University of Aberdeen.
>> The University of Aberdeen is a charity registered in Scotland,
>> No SC013683.
>> --------------------------------------------------------------------
>> IETF IPv6 working group mailing list
>> ipv6@...
>> Administrative Requests: https://www.ietf.org/mailman/listinfo/ipv6
>> --------------------------------------------------------------------
>