netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Igor Gnatenko <i.gnatenko.brain@gmail.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: David Miller <davem@davemloft.net>,
	stephen@networkplumber.org, netdev@vger.kernel.org
Subject: Re: [PATCH] skge: fix broken driver
Date: Thu, 19 Sep 2013 22:16:48 +0400	[thread overview]
Message-ID: <1379614608.2331.0.camel@ThinkPad-X230.localdomain> (raw)
In-Reply-To: <alpine.LRH.2.02.1309191359300.12162@file01.intranet.prod.int.rdu2.redhat.com>

On Thu, 2013-09-19 at 14:04 -0400, Mikulas Patocka wrote:
> 
> 
> On Thu, 19 Sep 2013, David Miller wrote:
> 
> > From: Mikulas Patocka <mpatocka@redhat.com>
> > Date: Thu, 19 Sep 2013 12:33:30 -0400 (EDT)
> > 
> > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > > Cc: stable@kernel.org # 3.11
> > 
> > First, this is missing the reported-by and tested-by tags provided
> > by people who tested this patch.
> 
> I noticed the problem and tested the patch on my own machine. So I added 
> myself to these tags.
> 
> > Secondly, CC:'ing stable is not the correct way to submit networking
> > patches for -stable inclusion.  You simply ask me to queue them up
> > for -stable explicitly instead.
> > 
> > Please re-submit this with the currect signoffs, thank you.
> 
> Here I'm re-submitting it.
> 
> ---
> 
> skge: fix broken driver
> 
> The patch 136d8f377e1575463b47840bc5f1b22d94bf8f63 broke the skge driver.
> Note this part of the patch:
> +               if (skge_rx_setup(skge, e, nskb, skge->rx_buf_size) < 0) {
> +                       dev_kfree_skb(nskb);
> +                       goto resubmit;
> +               }
> +
>                 pci_unmap_single(skge->hw->pdev,
>                                  dma_unmap_addr(e, mapaddr),
>                                  dma_unmap_len(e, maplen),
>                                  PCI_DMA_FROMDEVICE);
>                 skb = e->skb;
>                 prefetch(skb->data);
> -               skge_rx_setup(skge, e, nskb, skge->rx_buf_size);
> 
> The function skge_rx_setup modifies e->skb to point to the new skb. Thus,
> after this change, the new buffer, not the old, is returned to the
> networking stack.
> 
> This bug is present in kernels 3.11, 3.11.1 and 3.12-rc1. The patch should 
> be queued for 3.11-stable.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Tested-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  drivers/net/ethernet/marvell/skge.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> Index: linux-3.11.1-fast/drivers/net/ethernet/marvell/skge.c
> ===================================================================
> --- linux-3.11.1-fast.orig/drivers/net/ethernet/marvell/skge.c  2013-09-10 19:46:58.000000000 +0200
> +++ linux-3.11.1-fast/drivers/net/ethernet/marvell/skge.c       2013-09-19 18:20:43.000000000 +0200
> @@ -3092,6 +3092,9 @@ static struct sk_buff *skge_rx_get(struc
>                 if (!nskb)
>                         goto resubmit;
>  
> +               skb = e->skb;
> +               prefetch(skb->data);
> +
>                 if (skge_rx_setup(skge, e, nskb, skge->rx_buf_size) < 0) {
>                         dev_kfree_skb(nskb);
>                         goto resubmit;
> @@ -3101,8 +3104,6 @@ static struct sk_buff *skge_rx_get(struc
>                                  dma_unmap_addr(e, mapaddr),
>                                  dma_unmap_len(e, maplen),
>                                  PCI_DMA_FROMDEVICE);
> -               skb = e->skb;
> -               prefetch(skb->data);
>         }
>  
>         skb_put(skb, len);
> --
> 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

Hey Mikulas! See https://lkml.org/lkml/2013/9/19/38 , plz.
-- 
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.1-300.fc20.x86_64

  parent reply	other threads:[~2013-09-19 18:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-19 16:33 [PATCH] skge: fix broken driver Mikulas Patocka
2013-09-19 17:56 ` David Miller
2013-09-19 18:04   ` Mikulas Patocka
2013-09-19 18:07     ` David Miller
2013-09-19 18:16     ` Igor Gnatenko [this message]
2013-09-19 18:29       ` Mikulas Patocka
2013-09-19 21:32         ` Francois Romieu
2013-09-20 14:32           ` Mikulas Patocka
2013-09-20 15:35             ` Igor Gnatenko
2013-09-20 21:38             ` Francois Romieu
2013-09-23 14:58               ` Mikulas Patocka
2013-09-19 18:31     ` [PATCH] stable_kernel_rules.txt: Exclude networking from stable rules Joe Perches
2013-09-19 21:32       ` Francois Romieu
2013-09-19 21:45         ` Joe Perches
2013-09-19 22:37           ` Francois Romieu
2013-09-20 14:54       ` Joe Perches
2013-09-20 15:59         ` David Miller
2013-09-22 18:51       ` Christoph Hellwig
2013-09-23 20:34         ` Joe Perches
2013-09-24  8:48           ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1379614608.2331.0.camel@ThinkPad-X230.localdomain \
    --to=i.gnatenko.brain@gmail.com \
    --cc=davem@davemloft.net \
    --cc=mpatocka@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).