From patchwork Sat Sep 4 20:34:29 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jarek Poplawski X-Patchwork-Id: 63806 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 DF34BB713E for ; Sun, 5 Sep 2010 06:35:30 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753157Ab0IDUej (ORCPT ); Sat, 4 Sep 2010 16:34:39 -0400 Received: from mail-bw0-f46.google.com ([209.85.214.46]:52793 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751966Ab0IDUeh (ORCPT ); Sat, 4 Sep 2010 16:34:37 -0400 Received: by bwz11 with SMTP id 11so2455513bwz.19 for ; Sat, 04 Sep 2010 13:34:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:cc:subject :message-id:mime-version:content-type:content-disposition :in-reply-to:user-agent; bh=IVMnv6yeMHoohTBfhES+X90IXZyAvwKpWdLPeDZlj34=; b=l+RbeQRfdv69xDZ9rrvDtjlDQXLzX5XIdOct5QSAoB2nlzi2eMbOGYUFzuBnTUQAFq ek1YJRtNEcSq/su4pK10UdDXjyav+kZids9S2z0b8qhQ9vdybRTMWGfNSn2Srelyznr6 qpOyDnCWT1vak38YEfiwuJyCINKbOXd3he4NA= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:mime-version:content-type :content-disposition:in-reply-to:user-agent; b=BaJwfUmPLBCIAu0xigV9mU8DS0FYRlgcu5jT69xqMhfAvLzIlQYUg5DPF5cHxKepnK orSLtn8k/bKmWYvwfwy9BI/VSSW22D8tYJHCFg6KNEXrH+h9wIC9amzj0rI+p1rPXEP4 NfRyereXXsrhAmJQm1aEm4nS6enqJ2BNAp6+w= Received: by 10.204.85.90 with SMTP id n26mr1634056bkl.109.1283632476150; Sat, 04 Sep 2010 13:34:36 -0700 (PDT) Received: from del.dom.local (public68459.xdsl.centertel.pl [79.162.139.107]) by mx.google.com with ESMTPS id 11sm2733753bkj.23.2010.09.04.13.34.32 (version=SSLv3 cipher=RC4-MD5); Sat, 04 Sep 2010 13:34:34 -0700 (PDT) Date: Sat, 4 Sep 2010 22:34:29 +0200 From: Jarek Poplawski To: Eric Dumazet Cc: Plamen Petrov , Herbert Xu , "Rafael J. Wysocki" , Kernel Testers List , Maciej Rutecki , "David S. Miller" , Linux Kernel Mailing List , netdev@vger.kernel.org Subject: Re: [Bug #16626] Machine hangs with EIP at skb_copy_and_csum_dev Message-ID: <20100904203429.GA4891@del.dom.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1283338251.2556.124.camel@edumazet-laptop> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Eric Dumazet wrote, On 09/01/2010 12:50 PM: > [PATCH] gro: fix different skb headrooms > > packets entering GRO might have different headrooms, even for a given > flow (because of implementation details in drivers, like copybreak). > We cant force drivers to deliver packets with a fixed headroom. > > 1) fix skb_segment() > > skb_segment() makes the false assumption headrooms of fragments are same > than the head. When CHECKSUM_PARTIAL is used, this can give csum_start > errors, and crash later in skb_copy_and_csum_dev() > > 2) allocate a minimal skb for head of frag_list > > skb_gro_receive() uses netdev_alloc_skb(headroom + skb_gro_offset(p)) to > allocate a fresh skb. This adds NET_SKB_PAD to a padding already > provided by netdevice, depending on various things, like copybreak. > > Use alloc_skb() to allocate an exact padding, to reduce cache line > needs: > NET_SKB_PAD + NET_IP_ALIGN > > bugzilla : https://bugzilla.kernel.org/show_bug.cgi?id=16626 > > Many thanks to Plamen Petrov, testing many debugging patches ! > With help of Jarek Poplawski. > > Reported-by: Plamen Petrov > Signed-off-by: Eric Dumazet > CC: Jarek Poplawski > --- > patch against linux-2.6 current tree > > net/core/skbuff.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c ... > @@ -2702,8 +2706,8 @@ int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb) > } else if (skb_gro_len(p) != pinfo->gso_size) > return -E2BIG; > > - headroom = skb_headroom(p); > - nskb = netdev_alloc_skb(p->dev, headroom + skb_gro_offset(p)); > + headroom = NET_SKB_PAD + NET_IP_ALIGN; > + nskb = alloc_skb(headroom + skb_gro_offset(p), GFP_ATOMIC); > if (unlikely(!nskb)) > return -ENOMEM; Hi again, Just had a second look, and unless I miss something... Plamen, could you test this patch, too? (Without removing the previous one.) Thanks, Jarek P. -------------------> [PATCH] gro: Re-fix different skb headrooms The patch: "gro: fix different skb headrooms" in its part: "2) allocate a minimal skb for head of frag_list" is buggy. The copied skb has p->data set at the ip header at the moment, and skb_gro_offset is the length of ip + tcp headers. So, after the change the length of mac header is skipped. Later skb_set_mac_header() sets it into the NET_SKB_PAD area (if it's long enough) and ip header is misaligned at NET_SKB_PAD + NET_IP_ALIGN offset. There is no reason to assume the original skb was wrongly allocated, so let's copy it as it was. bugzilla : https://bugzilla.kernel.org/show_bug.cgi?id=16626 fixes commit: 3d3be4333fdf6faa080947b331a6a19bce1a4f57 Reported-by: Plamen Petrov Signed-off-by: Jarek Poplawski CC: Eric Dumazet Acked-by: Eric Dumazet Tested-by: Plamen Petrov --- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 26396ff..c83b421 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -2706,7 +2706,7 @@ int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb) } else if (skb_gro_len(p) != pinfo->gso_size) return -E2BIG; - headroom = NET_SKB_PAD + NET_IP_ALIGN; + headroom = skb_headroom(p); nskb = alloc_skb(headroom + skb_gro_offset(p), GFP_ATOMIC); if (unlikely(!nskb)) return -ENOMEM;