From patchwork Thu Feb 18 21:47:26 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 584951 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 7CB5414033E; Fri, 19 Feb 2016 08:47:43 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical-com.20150623.gappssmtp.com header.i=@canonical-com.20150623.gappssmtp.com header.b=hvnRGE7P; dkim-atps=neutral Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1aWWQB-0003Sw-Rv; Thu, 18 Feb 2016 21:47:35 +0000 Received: from mail-ob0-f181.google.com ([209.85.214.181]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.76) (envelope-from ) id 1aWWQ6-0003Sa-TA for kernel-team@lists.ubuntu.com; Thu, 18 Feb 2016 21:47:31 +0000 Received: by mail-ob0-f181.google.com with SMTP id kf7so6523848obb.1 for ; Thu, 18 Feb 2016 13:47:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; bh=Pkxt5FLZNRsrAFaK1YzWKSi8r0sU27C/7h5YKzbYJWk=; b=hvnRGE7Pjb3N48lKgu23EAOGbWTNz2SaUG5Xdz83jpvrtiDqZYk8i1mAH7NCpH4pgz KnW4ZeYYScuajJO9tGZ5dCapA9PtArt8rrBKsQnxWaN4l2gZ3DDTQu0H5EUlwPJSUpS4 nx6qeHMbs7lVwZnbUE0guztiKpTHK/c/guJDIA/aAUxr/Rc+bB3eTiNF5tY2LBveArV7 pAodf0yRaVzPy1S6NHu3miPmyaMNxJlsWzo9QltWS0cOllNZNQFx1BJfCdfZDXqkcncP gqBAHp22VUM/KCRt3MAFKMVJFX0zAuRXklzfBgiBDEUy4w+ob4h1bZ6sCtW2Gjj20PnL kC5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-type:content-transfer-encoding; bh=Pkxt5FLZNRsrAFaK1YzWKSi8r0sU27C/7h5YKzbYJWk=; b=DtOdP/bZGoffPL6ZtgYGEJm8C9SS2yI8P5+IwCB0P51tX86r0RpM/y0LSUvqN6jgm8 VkaoNgz7MQ1sMeklcGwvK53IOz549bTziFOPz7VnmsjXJTMaJ8KTON+BRFnFvOFh4mws abe1Z5ZNBGvuh0rCms0fq+Fn3mzDMcy6LnasWfZV2tSJ8P46+BTrkf1iGMhYyAnifAm6 MmwYg1ASWKV/Qzfvl3LXFZHQYgQWpP4sHmkO6Ny/a74iHo8Ky6v0g/35TIWCtoXco+4t s1TT7W4M50H7UUnvh7cT+NSi6UaJhBzmLlQ2KdDGoJq/L3Kx8xs1SHEIVx8VlKyoPwDw rYog== X-Gm-Message-State: AG10YOT3o3jehP5A/1AizwR5bcB0cVPU0bwhG6TfxTGqEwqYjxFqZYq2l/+uS4YkyxqeECGk X-Received: by 10.60.97.233 with SMTP id ed9mr8340307oeb.17.1455832049587; Thu, 18 Feb 2016 13:47:29 -0800 (PST) Received: from localhost.localdomain (host-98-127-250-84.bln-mt.client.bresnan.net. [98.127.250.84]) by smtp.gmail.com with ESMTPSA id ym6sm5031874obc.23.2016.02.18.13.47.27 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 18 Feb 2016 13:47:28 -0800 (PST) From: tim.gardner@canonical.com To: kernel-team@lists.ubuntu.com Subject: [PATCH Precise SRO] =?UTF-8?q?veth:=20don=E2=80=99t=20modify=20ip=5Fsummed; ?= =?UTF-8?q?=20doing=20so=20treats=20packets=20with=20bad=20checksums=20as?= =?UTF-8?q?=20good.?= Date: Thu, 18 Feb 2016 14:47:26 -0700 Message-Id: <1455832046-14284-1-git-send-email-tim.gardner@canonical.com> X-Mailer: git-send-email 1.9.1 MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com From: Vijay Pandurangan BugLink: http://bugs.launchpad.net/bugs/1547207 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 (cherry picked from commit ce8c839b74e3017996fad4e1b7ba2e2625ede82f) Signed-off-by: Tim Gardner --- drivers/net/veth.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/veth.c b/drivers/net/veth.c index 3634032..9de9dc3 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);