Re: [RFC PATCH 1/3] Avoid making inappropriate requests of NETIF_F_V[46]_CSUM devices

[David Woodhouse <dwmw2@infradead.org>]

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

On Wed, 2013-01-16 at 18:00 -0500, David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Wed, 16 Jan 2013 22:34:18 +0000
> 
> > On Wed, 2013-01-16 at 15:54 -0500, David Miller wrote:
> >> 
> >> My opinion on this is that the injectors of packets are responsible
> >> for ensuring checksum types are set on SKBs in an appropriate way.
> >> 
> >> So we ensure this in the local protocol stacks that generate packets,
> >> and if foreign alien entities can inject SKBs with these checksum
> >> settings (like the tun device can) the burdon of verification falls
> >> upon whatever layer allows that to happen.
> >> 
> >> So really, the fix is in the tun device and the virtio layer.
> > 
> > The virtio layer (and the tun device) expose the equivalent of the
> > NETIF_F_HW_CSUM capability to the guest. In the case where we have a
> > real device on the host which *also* has NETIF_F_HW_CSUM capability, are
> > you saying that the tun driver should do the checksum for non-UDP/TCP
> > packets in software *anyway*, just because the packet might end up going
> > out a device *without* that capability, and the check in
> > harmonize_features() isn't sophisticated enough to cope properly?
> 
> I'm saying that tun can't inject unchecked crap into our stack.

Did we ever resolve this? AFAICT from inspecting the code the
virtio_net device still advertises hardware csum capabilities to the
guest. And accepts packets which need checksumming, calling
skb_partial_csum_set() as appropriate. Likewise tun, xen, macvtap and
af_packet.

And that works fine — it's a nice performance win because it means that
VM guests (and other clients) can make full use of the HW csum
capabilities of the real network hardware. And when the outbound
netdevice *doesn't* have HW csum support, we generally do the right
thing and complete the csum in software in the host kernel before
transmitting it.

Perhaps I'm missing something, but I'm not sure why you refer to that
as 'injecting unchecked crap'. Surely it's using CHECKSUM_PARTIAL
precisely as it was designed, and allowing the checksum to be completed
either by hardware or software as appropriate?

The *only* problem is the false positive in harmonize_features(), which
was addressed by my patch which started this thread (in 2013). The
problem is that an IP packet that *isn't* TCP or UDP, being sent out a
device that has only NETIF_F_IP_CSUM capability, ends up being handed
to the device unchecksummed because harmonize_features() fails to clear
the HW csum flag as it (arguably) should.

Original thread at
http://comments.gmane.org/gmane.linux.network/254981

I'm only looking at it again because I'm pondering enabling HW csum in
8139cp (now that I've fixed TSO), and it reminded me of this...


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


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