On Mon, Oct 5, 2015 at 4:16 AM, David Woodhouse <dw...@infradead.org> wrote: > 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, here is what I am currently thinking the interface should be:
1) Drivers may advertise NETIF_F_HW_CSUM. The stack will indicate checksum offload exclusively using the CHECKSUM_PARTIAL/csum_start/csum_offset interface. No additional interfaces (bits in skbuff should not be needed) 2) A driver may inspect packets via ndo_check to decide if it wants to offload the checksum, if not cancels NETIF_F_HW_CSUM in the packet. 3) In driver xmit when CHECKSUM_PARTIAL is set the driver MUST correctly resolve the checksum-- either by properly offloading to the device or calling skb_checksum_help. 4) To help drivers for devices with limited offload capabilities we'll define a helper function to check for typical restrictions (.e.g. IPv4 only, TCP/UDP only. no encapsulation, no IPv6 extension headers, etc.). I am working on this helper function and will send RFC shortly. Tom > -- > David Woodhouse Open Source Technology Centre > david.woodho...@intel.com Intel Corporation > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html