veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."

[Ben Greear <greearb@candelatech.com>]

I have an application that creates two pairs of veth devices.

a <-> b       c <-> d

b and c have a raw packet socket opened on them and I 'bridge' frames
between b and c to provide network emulation (ie, configurable delay).


I put IP 1.1.1.1/24 on a, 1.1.1.2/24 on d, and then create a UDP connection
(using policy based routing to ensure frames are sent on the appropriate
interfaces).

This is user-space only app, and kernel in this case is completely unmodified.

The commit below breaks this feature:  UDP frames are sniffed on both a and d ports
(in both directions), but the UDP socket does not receive frames.

Using normal ethernet ports, this network emulation feature works fine, so it is
specific to VETH.

A similar test with just sending UDP between a single veth pair:  e <-> f
works fine.  Maybe it has something to do with raw packets?


The patch below is the culprit:


[greearb@ben-dt3 linux-2.6]$ git bisect bad
ce8c839b74e3017996fad4e1b7ba2e2625ede82f is the first bad commit
commit ce8c839b74e3017996fad4e1b7ba2e2625ede82f
Author: Vijay Pandurangan <vijayp@vijayp.ca>
Date:   Fri Dec 18 14:34:59 2015 -0500

     veth: don’t modify ip_summed; doing so treats packets with bad checksums as good.

     Packets that arrive from real hardware devices have ip_summed ==
     CHECKSUM_UNNECESSARY if the hardware verified the checksums, or
     CHECKSUM_NONE if the packet is bad or it was unable to verify it. The
     current version of veth will replace CHECKSUM_NONE with
     CHECKSUM_UNNECESSARY, which causes corrupt packets routed from hardware to
     a veth device to be delivered to the application. This caused applications
     at Twitter to receive corrupt data when network hardware was corrupting
     packets.

...

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 0ef4a5a..ba21d07 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -117,12 +117,6 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
                 kfree_skb(skb);
                 goto drop;
         }
-       /* don't change ip_summed == CHECKSUM_PARTIAL, as that
-        * will cause bad checksum on forwarded packets
-        */
-       if (skb->ip_summed == CHECKSUM_NONE &&
-           rcv->features & NETIF_F_RXCSUM)
-               skb->ip_summed = CHECKSUM_UNNECESSARY;

         if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
                 struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);


Any suggestions for how to fix this so that I get the old working behaviour and
the bug this patch was trying to fix is also resolved?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com