From patchwork Mon Jan 14 12:10:31 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 211778 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 6A6152C0096 for ; Mon, 14 Jan 2013 23:10:38 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755388Ab3ANMKe (ORCPT ); Mon, 14 Jan 2013 07:10:34 -0500 Received: from merlin.infradead.org ([205.233.59.134]:35677 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751288Ab3ANMKd (ORCPT ); Mon, 14 Jan 2013 07:10:33 -0500 Received: from shinybook.infradead.org ([2001:8b0:10b:1:e6ce:8fff:fe1f:f2c0]) by merlin.infradead.org with esmtpsa (Exim 4.76 #1 (Red Hat Linux)) id 1Tuis4-0001wl-FU for netdev@vger.kernel.org; Mon, 14 Jan 2013 12:10:33 +0000 Message-ID: <1358165431.27054.62.camel@shinybook.infradead.org> Subject: [RFC PATCH 1/3] Avoid making inappropriate requests of NETIF_F_V[46]_CSUM devices From: David Woodhouse To: netdev@vger.kernel.org Date: Mon, 14 Jan 2013 12:10:31 +0000 X-Mailer: Evolution 3.6.2 (3.6.2-3.fc18) Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Devices with the NETIF_F_V[46]_CSUM feature(s) are *only* required to handle checksumming of UDP and TCP. In netif_skb_features() we attempt to filter out the capabilities which are inappropriate for the device that the skb will actually be sent from... but there we assume that NETIF_F_V4_CSUM devices can handle *all* Legacy IP, and that NETIF_F_V6_CSUM devices can handle *all* IPv6. This may have been OK in the days when CHECKSUM_PARTIAL packets would *only* be produced by the local stack, and we knew the local stack didn't generate them for anything but UDP and TCP. But these days that's not true. When a tun device receives a packet from userspace with VIRTIO_NET_HDR_F_NEEDS_CSUM, that translates fairly directly into setting CHECKSUM_PARTIAL on the resulting skb. Since virtio_net advertises NETIF_F_HW_CSUM to its guests, we should expect to be asked to checksum *anything*. This patch attempts to cope with that by checking skb->csum_offset for such devices. If that doesn't match the offset for UDP or TCP, then we don't use hardware checksum. It won't catch 100% of cases, but a full check of the actual skb contents in the fast path isn't a good idea. It'll probably do well enough for now. This expands the check in can_checksum_protocol() to make it more readable, but doing so shouldn't make the resulting code any *bigger*, except obviously for the additional checks. Signed-off-by: David Woodhouse diff --git a/net/core/dev.c b/net/core/dev.c index 515473e..f1048b6 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2229,22 +2229,39 @@ static int dev_gso_segment(struct sk_buff *skb, netdev_features_t features) return 0; } -static bool can_checksum_protocol(netdev_features_t features, __be16 protocol) +static bool can_checksum_protocol(netdev_features_t features, __be16 protocol, + __u16 csum_offset) { - return ((features & NETIF_F_GEN_CSUM) || - ((features & NETIF_F_V4_CSUM) && - protocol == htons(ETH_P_IP)) || - ((features & NETIF_F_V6_CSUM) && - protocol == htons(ETH_P_IPV6)) || - ((features & NETIF_F_FCOE_CRC) && - protocol == htons(ETH_P_FCOE))); + if (features & NETIF_F_GEN_CSUM) + return 1; + + if ((features & NETIF_F_FCOE_CRC) && protocol == htons(ETH_P_FCOE)) + return 1; + + /* + * Only allow NETIF_F_V[46]_CSUM for UDP/TCP packets. This is an + * overly permissive check, but it's very unlikely to have false + * positives in practice, and actually looking in the packet for + * a proper confirmation would be very slow. + */ + if (csum_offset != offsetof(struct udphdr, check) && + csum_offset != offsetof(struct tcphdr, check)) + return 0; + + if ((features & NETIF_F_V4_CSUM) && protocol == htons(ETH_P_IP)) + return 1; + + if ((features & NETIF_F_V6_CSUM) && protocol == htons(ETH_P_IPV6)) + return 1; + + return 0; } static netdev_features_t harmonize_features(struct sk_buff *skb, __be16 protocol, netdev_features_t features) { if (skb->ip_summed != CHECKSUM_NONE && - !can_checksum_protocol(features, protocol)) { + !can_checksum_protocol(features, protocol, skb->csum_offset)) { features &= ~NETIF_F_ALL_CSUM; features &= ~NETIF_F_SG; } else if (illegal_highdma(skb->dev, skb)) {