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

> Dear Gorry;
>
> A few matters in-line.
>
> On Wed, Mar 14, 2012 at 4:31 AM,  <gorry@...> wrote:
>>
>> See in-line:
>>
>>
>> On Tue, 13 Mar 2012, Marshall Eubanks wrote:
>>
>>> 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.
>>>
>>>
>> Looks good.
>>
>>>
>>>> ----
>>>> 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).
>>>
>> The existing spec says systems should log it as an error.
>>
>
> Yes. My point was just that I didn't want to say
>
>>>>   packets, and SHOULD log this as an error."
>
> as we are not prescribing anything for systems that have not been updated.
> If they are going to update, they should update to the new standard.
>
>>
>>> How about
>>>
>>>  "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 should discard the zero-checksum UDP
>
> How about
>
> s/system/system  (according to RFC 2460)/
>
> just to make the point clear.
>
>>>   packets, and log this as an error."
>>>
>> OK
>>
>>>
>>>> ----
>>>> 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."
>>>>
>> Better.
>>
>>
>>>> ----
>>>> 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.
>>>
>> Thanks
>>
>>
>>>>
>>>> ----
>>>> 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/ ?
>>>
>> Applications are recommended to verfiy the context of the packets they
>> receive using UDP, as described in RFC 5405. Applications that verify the
>> context of a datagram are expected to have a high probability of discarding
>> corrupted data. [I-D.ietf-6man-udpzero] presents examples of cases where
>> corruption can inadvertantly impact application state.
>>
>>
>
> WFM
>
>
>>
>>>>
>>>> ----
>>>> 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."
>>>
>> Seems good.
>>
>>>>
>>>> ----
>>>> 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
>>>> --------------------------------------------------------------------
>>>
>>>
>>
>> Looks like we agree on all the important points, which may be a sign that it
>> is worth polishing the text and asking for WG accepatnce?
>>
>
> That was my intention. I can get a revision out early in IETF week.
>
> Thanks again.
>
> Regards
> Marshall
>
>> Gorry
>