[Intel-wired-lan] [PATCH V2] ixgbe: Handle extended IPv6 headers in tx path

[Tom Herbert]

On Tue, Nov 17, 2015 at 10:09 AM, Rustad, Mark D
<mark.d.rustad at intel.com> wrote:
> Alexander Duyck <alexander.duyck at gmail.com> wrote:
>
>> On Tue, Nov 17, 2015 at 8:48 AM, Tom Herbert <tom at herbertland.com> wrote:
>>> On Tue, Nov 17, 2015 at 8:41 AM, Alexander Duyck
>>> <alexander.duyck at gmail.com> wrote:
>>>> On Mon, Nov 16, 2015 at 9:21 PM, Tom Herbert <tom at herbertland.com> wrote:
>>>>>
>>>>> Failures should not result in dev_warn, just call skb_checksum_help
>>>>> instead. Ignoring CHECKSUM_PARTIAL is guaranteed to be sending a bad
>>>>> checksum!
>
> But calling skb_checksum_help I think is a false sense of security. In the case of fragmentation for example, it can't possibly help. I'd rather find out that it is failing and figure out what to do about it.

The _only_ thing that is authoritative in checksum offload are the
values of csum_start and csum_offset. If a driver ensures that
checksum is properly calculated from start and set at the offset, then
driver is implementing correct behavior and has no additional
requirements as far as the stack is concerned. Neither does the stack
does not care if this is done with offload to the device or by calling
skb_checksum_help.

>
>>>> The problem is there are scenarios here where a checksum shouldn't be
>>>> computed.  If for example the frame is fragmented we should be
>>>> displaying some sort of error message and not computing the checksum
>>>> because no matter what we compute it isn't going to be valid.
>>> If the stack is sending fragments with CHECKSUM_PARTIAL set then that
>>> is a bug higher in the stack-- the driver does not need to worry about
>>> that.
>>
>> Right, but it is still something that needs to get fixed.  By
>> triggering the dev_warn here we at least create visibility that
>> something has gone horribly wrong.
>
> Right. It needs to be fixed one way or another. In some cases it would be the stack that needs a fix, in some cases a call to skb_checksum_help might be needed or maybe something else.

Are you actually seeing fragments with CHECKSUM_PARTIAL set?

Tom