From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jarek Poplawski Subject: Re: [Bug #16626] Machine hangs with EIP at skb_copy_and_csum_dev Date: Wed, 1 Sep 2010 17:05:04 +0200 Message-ID: <20100901150504.GA3091@del.dom.local> References: <4LwrqITm-eJ.A.r8G.eFueMB@chimera> <4AUWBNzTkbD.A.ey.cGueMB@chimera> <20100831192659.GA3093@del.dom.local> <1283338251.2556.124.camel@edumazet-laptop> <20100901112026.GA9468@ff.dom.local> <1283349461.2556.333.camel@edumazet-laptop> Mime-Version: 1.0 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: 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:references:mime-version:content-type:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=1dDP2V10iHNAI5BS77h+uZPGmV186aRroxNUXDK6fTE=; b=r13GY5Nqyr71fQE364xJTWOEMpENS5eWJcxUo25UYhSy62PoLKAiOTzUzUILZQDxyv M4hXa/etj4Ckz0uoVyaFmiIomyCUgv+JgXvfr5Nv/9NO6OXHswKeznZRv6rDKfDcVM4G +ul5sqw7SC5cIH1AVWteNBuVyGMW43rRqDLD4= Content-Disposition: inline In-Reply-To: <1283349461.2556.333.camel@edumazet-laptop> Sender: kernel-testers-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="iso-8859-1" To: Eric Dumazet Cc: Plamen Petrov , Herbert Xu , "Rafael J. Wysocki" , Kernel Testers List , Maciej Rutecki , "David S. Miller" , Linux Kernel Mailing List , netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Wed, Sep 01, 2010 at 03:57:41PM +0200, Eric Dumazet wrote: > Le mercredi 01 septembre 2010 ?? 11:20 +0000, Jarek Poplawski a =E9cr= it : > > On Wed, Sep 01, 2010 at 12:50:51PM +0200, Eric Dumazet wrote: > > > Plamen, could you test following patch ? > > >=20 > > > I reproduced problem on a dev machine and following patch cured i= t. > > >=20 > > > Thanks > > >=20 > > > [PATCH] gro: fix different skb headrooms > > >=20 > > > packets entering GRO might have different headrooms, even for a g= iven > > > flow (because of implementation details in drivers, like copybrea= k). > > > We cant force drivers to deliver packets with a fixed headroom. > > >=20 > > > 1) fix skb_segment() > > >=20 > > > skb_segment() makes the false assumption headrooms of fragments a= re same > > > than the head. When CHECKSUM_PARTIAL is used, this can give csum_= start > > > errors, and crash later in skb_copy_and_csum_dev() > >=20 > > Eric, probably I missed something, but since the same test as in > > skb_copy_and_csum_dev() gave different result a bit earlier on exac= tly > > the same skb, I've suspected some sharing (or use after free) > > problems, so I'm not sure your current diagnose can explain this. > > (Unless this old test was dismissed later.) >=20 > Oh, this is because your patch had an error for the gso part that rea= d : >=20 > - rc =3D ops->ndo_start_xmit(nskb, dev); > + if (skb_csum_start_bug(skb, 50)) { > + kfree_skb(skb); > + rc =3D NETDEV_TX_OK; > + } else > + rc =3D ops->ndo_start_xmit(nskb, dev); > + > if (unlikely(rc !=3D NETDEV_TX_OK)) { > if (rc & ~NETDEV_TX_MASK) > goto out_kfree_gso_skb; >=20 > You called skb_csum_start_bug(skb, 50) instead of > skb_csum_start_bug(nskb, 50) >=20 > Hope this clarify a bit ;) All clear! Sorry for the false alarm! Thanks, Jarek P. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755564Ab0IAPFO (ORCPT ); Wed, 1 Sep 2010 11:05:14 -0400 Received: from mail-ew0-f46.google.com ([209.85.215.46]:44479 "EHLO mail-ew0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753731Ab0IAPFL (ORCPT ); Wed, 1 Sep 2010 11:05:11 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:content-transfer-encoding :in-reply-to:user-agent; b=sLmgniQ3pnUgY206FVAERrOsj+NJ61dfOGwDeT2TGqqjKDH2NIeh4Q+zSpetnumIkq EsmvHo1h6a2hicDCyVuLgC5l8VY9i7A2KCcuFsV0OCRTvydrx1xbmuzT6nFdYoJGhotP 27qpYOZAVYonr1p4xa/9pfxnJYF8PoUsD3Rj8= Date: Wed, 1 Sep 2010 17:05:04 +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: <20100901150504.GA3091@del.dom.local> References: <4LwrqITm-eJ.A.r8G.eFueMB@chimera> <4AUWBNzTkbD.A.ey.cGueMB@chimera> <20100831192659.GA3093@del.dom.local> <1283338251.2556.124.camel@edumazet-laptop> <20100901112026.GA9468@ff.dom.local> <1283349461.2556.333.camel@edumazet-laptop> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1283349461.2556.333.camel@edumazet-laptop> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 01, 2010 at 03:57:41PM +0200, Eric Dumazet wrote: > Le mercredi 01 septembre 2010 ?? 11:20 +0000, Jarek Poplawski a écrit : > > On Wed, Sep 01, 2010 at 12:50:51PM +0200, Eric Dumazet wrote: > > > Plamen, could you test following patch ? > > > > > > I reproduced problem on a dev machine and following patch cured it. > > > > > > Thanks > > > > > > [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() > > > > Eric, probably I missed something, but since the same test as in > > skb_copy_and_csum_dev() gave different result a bit earlier on exactly > > the same skb, I've suspected some sharing (or use after free) > > problems, so I'm not sure your current diagnose can explain this. > > (Unless this old test was dismissed later.) > > Oh, this is because your patch had an error for the gso part that read : > > - rc = ops->ndo_start_xmit(nskb, dev); > + if (skb_csum_start_bug(skb, 50)) { > + kfree_skb(skb); > + rc = NETDEV_TX_OK; > + } else > + rc = ops->ndo_start_xmit(nskb, dev); > + > if (unlikely(rc != NETDEV_TX_OK)) { > if (rc & ~NETDEV_TX_MASK) > goto out_kfree_gso_skb; > > You called skb_csum_start_bug(skb, 50) instead of > skb_csum_start_bug(nskb, 50) > > Hope this clarify a bit ;) All clear! Sorry for the false alarm! Thanks, Jarek P.