Re: [RFC PATCH] Fix false positives in can_checksum_protocol()

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Tue, 2015-09-29 at 15:52 -0700, Tom Herbert wrote:
> Please look at ixgbe_tx_csum in ixgbe driver. This one example of how
> a driver can determine whether the checksum being offloaded is TCP or
> UDP. The bug in this driver is...

I think it serves better as an example of why we don't *want* drivers
doing that kind of thing for themselves... :)

I propose we steal some high bits from csum_offset, as you suggested,
and use them to indicate a 'checksum type', which will include TCP and
UDP.

Then the filter in netif_skb_features() can trivially do the right
thing for NETIF_F_IP{V6,}_CSUM devices, so avoid feeding them packets
they can't handle.

You mentioned that you actually want to deprecate those feature flags —
which works for me, but it's kind of orthogonal. If we do that we'd
still want to provide generic functions that such drivers can use as
their .ndo_features_check() method. And we'd *still* want to do the
check based on a simple flag, rather than grubbing around in the packet
data. (And the drivers if they *are* asked to do the checksum will
sometimes care whether it's TCP vs. UDP too).

I don't think we want drivers calling skb_checksum_help() for
themselves; we want the pre-filter. Mainly because we *definitely*
don't want drivers calling gso_skb_segment() for themselves in the same
situation — see the comment I posted on Friday about the r8169 instance
of that. ('Re: [PATCH net-next 3/3] r8169: support IPv6').

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]