From patchwork Tue Feb 2 17:36:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kamal Mostafa X-Patchwork-Id: 577279 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 0C5D6140C57 for ; Wed, 3 Feb 2016 04:36:44 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756537AbcBBRg3 (ORCPT ); Tue, 2 Feb 2016 12:36:29 -0500 Received: from youngberry.canonical.com ([91.189.89.112]:36442 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756490AbcBBRg2 (ORCPT ); Tue, 2 Feb 2016 12:36:28 -0500 Received: from 1.general.kamal.us.vpn ([10.172.68.52] helo=fourier) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1aQesH-0007Ry-9i; Tue, 02 Feb 2016 17:36:21 +0000 Received: from kamal by fourier with local (Exim 4.82) (envelope-from ) id 1aQesE-0003Eb-Io; Tue, 02 Feb 2016 09:36:18 -0800 From: Kamal Mostafa To: Vijay Pandurangan Cc: Evan Jones , Nicolas Dichtel , Phil Sutter , Toshiaki Makita , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Cong Wang , "David S. Miller" , Kamal Mostafa , kernel-team@lists.ubuntu.com Subject: =?UTF-8?q?=5B3=2E13=2Ey-ckt=20stable=5D=20Patch=20=22=3D=3FUTF-8=3Fq=3Fveth=3A=3D20don=3DE2=3D80=3D99t=3D20modify=3D20ip=3D5Fsummed=3B=3D20doing=3F=3D=20=3D=3FUTF-8=3Fq=3F=3D20so=3D20treats=3D20packets=3D20with=3D20bad=3D20checksums=3D20as=3D20good=2E=3F=3D=22=20has=20been=20added=20to=20the=203=2E13=2Ey-ckt=20tree?= Date: Tue, 2 Feb 2016 09:36:17 -0800 Message-Id: <1454434577-12403-1-git-send-email-kamal@canonical.com> X-Mailer: git-send-email 1.9.1 X-Extended-Stable: 3.13 MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org This is a note to let you know that I have just added a patch titled =?UTF-8?q?veth:=20don=E2=80=99t=20modify=20ip=5Fsummed;=20doing?= =?UTF-8?q?=20so=20treats=20packets=20with=20bad=20checksums=20as=20good.?= to the linux-3.13.y-queue branch of the 3.13.y-ckt extended stable tree which can be found at: http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.13.y-queue This patch is scheduled to be released in version 3.13.11-ckt34. If you, or anyone else, feels it should not be added to this tree, please reply to this email. For more information about the 3.13.y-ckt tree, see https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable Thanks. -Kamal ---8<------------------------------------------------------------ From 707eea8be46c3bd8ef61cea1d105dbecdb937e8b Mon Sep 17 00:00:00 2001 From: Vijay Pandurangan Date: Fri, 18 Dec 2015 14:34:59 -0500 Subject: =?UTF-8?q?veth:=20don=E2=80=99t=20modify=20ip=5Fsummed;=20doing?= =?UTF-8?q?=20so=20treats=20packets=20with=20bad=20checksums=20as=20good.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ Upstream commit ce8c839b74e3017996fad4e1b7ba2e2625ede82f ] 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. We believe this was added as an optimization to skip computing and verifying checksums for communication between containers. However, locally generated packets have ip_summed == CHECKSUM_PARTIAL, so the code as written does nothing for them. As far as we can tell, after removing this code, these packets are transmitted from one stack to another unmodified (tcpdump shows invalid checksums on both sides, as expected), and they are delivered correctly to applications. We didn’t test every possible network configuration, but we tried a few common ones such as bridging containers, using NAT between the host and a container, and routing from hardware devices to containers. We have effectively deployed this in production at Twitter (by disabling RX checksum offloading on veth devices). This code dates back to the first version of the driver, commit ("[NET]: Virtual ethernet device driver"), so I suspect this bug occurred mostly because the driver API has evolved significantly since then. Commit <0b7967503dc97864f283a> ("net/veth: Fix packet checksumming") (in December 2010) fixed this for packets that get created locally and sent to hardware devices, by not changing CHECKSUM_PARTIAL. However, the same issue still occurs for packets coming in from hardware devices. Co-authored-by: Evan Jones Signed-off-by: Evan Jones Cc: Nicolas Dichtel Cc: Phil Sutter Cc: Toshiaki Makita Cc: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Vijay Pandurangan Acked-by: Cong Wang Signed-off-by: David S. Miller Signed-off-by: Kamal Mostafa --- drivers/net/veth.c | 6 ------ 1 file changed, 6 deletions(-) -- 1.9.1 diff --git a/drivers/net/veth.c b/drivers/net/veth.c index 5b37437..887e698 100644 --- a/drivers/net/veth.c +++ b/drivers/net/veth.c @@ -116,12 +116,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);