]>
Commit | Line | Data |
---|---|---|
0718c1e0 DM |
1 | From ce8c839b74e3017996fad4e1b7ba2e2625ede82f Mon Sep 17 00:00:00 2001 |
2 | From: Vijay Pandurangan <vijayp@vijayp.ca> | |
3 | Date: Fri, 18 Dec 2015 14:34:59 -0500 | |
4 | Subject: =?UTF-8?q?veth:=20don=E2=80=99t=20modify=20ip=5Fsummed;=20doing?= | |
5 | =?UTF-8?q?=20so=20treats=20packets=20with=20bad=20checksums=20as=20good.?= | |
6 | MIME-Version: 1.0 | |
7 | Content-Type: text/plain; charset=UTF-8 | |
8 | Content-Transfer-Encoding: 8bit | |
9 | ||
10 | Packets that arrive from real hardware devices have ip_summed == | |
11 | CHECKSUM_UNNECESSARY if the hardware verified the checksums, or | |
12 | CHECKSUM_NONE if the packet is bad or it was unable to verify it. The | |
13 | current version of veth will replace CHECKSUM_NONE with | |
14 | CHECKSUM_UNNECESSARY, which causes corrupt packets routed from hardware to | |
15 | a veth device to be delivered to the application. This caused applications | |
16 | at Twitter to receive corrupt data when network hardware was corrupting | |
17 | packets. | |
18 | ||
19 | We believe this was added as an optimization to skip computing and | |
20 | verifying checksums for communication between containers. However, locally | |
21 | generated packets have ip_summed == CHECKSUM_PARTIAL, so the code as | |
22 | written does nothing for them. As far as we can tell, after removing this | |
23 | code, these packets are transmitted from one stack to another unmodified | |
24 | (tcpdump shows invalid checksums on both sides, as expected), and they are | |
25 | delivered correctly to applications. We didn’t test every possible network | |
26 | configuration, but we tried a few common ones such as bridging containers, | |
27 | using NAT between the host and a container, and routing from hardware | |
28 | devices to containers. We have effectively deployed this in production at | |
29 | Twitter (by disabling RX checksum offloading on veth devices). | |
30 | ||
31 | This code dates back to the first version of the driver, commit | |
32 | <e314dbdc1c0dc6a548ecf> ("[NET]: Virtual ethernet device driver"), so I | |
33 | suspect this bug occurred mostly because the driver API has evolved | |
34 | significantly since then. Commit <0b7967503dc97864f283a> ("net/veth: Fix | |
35 | packet checksumming") (in December 2010) fixed this for packets that get | |
36 | created locally and sent to hardware devices, by not changing | |
37 | CHECKSUM_PARTIAL. However, the same issue still occurs for packets coming | |
38 | in from hardware devices. | |
39 | ||
40 | Co-authored-by: Evan Jones <ej@evanjones.ca> | |
41 | Signed-off-by: Evan Jones <ej@evanjones.ca> | |
42 | Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com> | |
43 | Cc: Phil Sutter <phil@nwl.cc> | |
44 | Cc: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> | |
45 | Cc: netdev@vger.kernel.org | |
46 | Cc: linux-kernel@vger.kernel.org | |
47 | Signed-off-by: Vijay Pandurangan <vijayp@vijayp.ca> | |
48 | Acked-by: Cong Wang <cwang@twopensource.com> | |
49 | Signed-off-by: David S. Miller <davem@davemloft.net> | |
50 | --- | |
51 | drivers/net/veth.c | 6 ------ | |
52 | 1 file changed, 6 deletions(-) | |
53 | ||
54 | diff --git a/drivers/net/veth.c b/drivers/net/veth.c | |
55 | index 0ef4a5a..ba21d07 100644 | |
56 | --- a/drivers/net/veth.c | |
57 | +++ b/drivers/net/veth.c | |
58 | @@ -117,12 +117,6 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev) | |
59 | kfree_skb(skb); | |
60 | goto drop; | |
61 | } | |
62 | - /* don't change ip_summed == CHECKSUM_PARTIAL, as that | |
63 | - * will cause bad checksum on forwarded packets | |
64 | - */ | |
65 | - if (skb->ip_summed == CHECKSUM_NONE && | |
66 | - rcv->features & NETIF_F_RXCSUM) | |
67 | - skb->ip_summed = CHECKSUM_UNNECESSARY; | |
68 | ||
69 | if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) { | |
70 | struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats); | |
71 | -- | |
72 | cgit v0.12 | |
73 |