From patchwork Mon Jan 16 22:43:38 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Hutchings X-Patchwork-Id: 136376 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 C4F2E1007D4 for ; Tue, 17 Jan 2012 09:43:47 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751598Ab2APWnm (ORCPT ); Mon, 16 Jan 2012 17:43:42 -0500 Received: from mail.solarflare.com ([216.237.3.220]:6817 "EHLO ocex02.SolarFlarecom.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751239Ab2APWnl (ORCPT ); Mon, 16 Jan 2012 17:43:41 -0500 Received: from [10.17.20.137] (10.17.20.137) by ocex02.SolarFlarecom.com (10.20.40.31) with Microsoft SMTP Server (TLS) id 14.1.355.2; Mon, 16 Jan 2012 14:43:40 -0800 Message-ID: <1326753818.3065.35.camel@bwh-desktop> Subject: [PATCH net 2/2] net: WARN if skb_checksum_help() is called on skb requiring segmentation From: Ben Hutchings To: David Miller CC: Herbert Xu , Date: Mon, 16 Jan 2012 22:43:38 +0000 In-Reply-To: <1326753539.3065.31.camel@bwh-desktop> References: <1326753539.3065.31.camel@bwh-desktop> Organization: Solarflare Communications X-Mailer: Evolution 3.2.2 (3.2.2-1.fc16) MIME-Version: 1.0 X-Originating-IP: [10.17.20.137] X-TM-AS-Product-Ver: SMEX-10.0.0.1412-6.800.1017-18648.005 X-TM-AS-Result: No--14.676800-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org skb_checksum_help() has never done anything useful with skbs that require segmentation. Setting skb->ip_summed = CHECKSUM_NONE makes them invalid and provokes a later WARNing in skb_gso_segment(). Passing such an skb to skb_checksum_help() indicates a bug, so we should warn about it immediately. Move the warning from skb_gso_segment() into a shared function, and add the calling function name, gso_type and gso_size to it. Signed-off-by: Ben Hutchings --- The price for writing the warning format only once is having to pass in the calling function name. Not sure whether it's a good trade-off. Ben. net/core/dev.c | 31 +++++++++++++++++++------------ 1 files changed, 19 insertions(+), 12 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 7e6b7dc..6824393 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1887,6 +1887,22 @@ void skb_set_dev(struct sk_buff *skb, struct net_device *dev) EXPORT_SYMBOL(skb_set_dev); #endif /* CONFIG_NET_NS */ +static void skb_warn_bad_offload(const char *func, const struct sk_buff *skb) +{ + struct net_device *dev = skb->dev; + const char *driver = ""; + + if (dev && dev->dev.parent) + driver = dev_driver_string(dev->dev.parent); + + WARN(1, "%s: in %s caps=(%pNF, %pNF) len=%d data_len=%d gso_size=%d " + "gso_type=%d ip_summed=%d\n", + driver, func, dev ? &dev->features : NULL, + skb->sk ? &skb->sk->sk_route_caps : NULL, + skb->len, skb->data_len, skb_shinfo(skb)->gso_size, + skb_shinfo(skb)->gso_type, skb->ip_summed); +} + /* * Invalidate hardware checksum when packet is to be mangled, and * complete checksum manually on outgoing path. @@ -1900,8 +1916,8 @@ int skb_checksum_help(struct sk_buff *skb) goto out_set_summed; if (unlikely(skb_shinfo(skb)->gso_size)) { - /* Let GSO fix up the checksum. */ - goto out_set_summed; + skb_warn_bad_offload(__func__, skb); + return -EINVAL; } offset = skb_checksum_start_offset(skb); @@ -1961,16 +1977,7 @@ struct sk_buff *skb_gso_segment(struct sk_buff *skb, __skb_pull(skb, skb->mac_len); if (unlikely(skb->ip_summed != CHECKSUM_PARTIAL)) { - struct net_device *dev = skb->dev; - const char *driver = ""; - - if (dev && dev->dev.parent) - driver = dev_driver_string(dev->dev.parent); - - WARN(1, "%s: caps=(%pNF, %pNF) len=%d data_len=%d ip_summed=%d\n", - driver, dev ? &dev->features : NULL, - skb->sk ? &skb->sk->sk_route_caps : NULL, - skb->len, skb->data_len, skb->ip_summed); + skb_warn_bad_offload(__func__, skb); if (skb_header_cloned(skb) && (err = pskb_expand_head(skb, 0, 0, GFP_ATOMIC)))