| Message ID | alpine.LRH.2.02.1309191359300.12162@file01.intranet.prod.int.rdu2.redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Mikulas Patocka <mpatocka@redhat.com> Date: Thu, 19 Sep 2013 14:04:38 -0400 (EDT) > Here I'm re-submitting it. Please do not hijack an existing discussions to resubmit the patch. Instead, make a fresh, completely new, mailing list posting for the patch submissions. Thank you. -- 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
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.
On Thu, 19 Sep 2013, Igor Gnatenko wrote: > 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 I see. My patch is a bit simpler - it doesn't allocate the structure skge_element on the stack. Mikulas -- 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
Mikulas Patocka <mpatocka@redhat.com> : [...] > I see. My patch is a bit simpler - it doesn't allocate the structure > skge_element on the stack. Both patches don't behave exactly the same wrt pci_unmap_single.
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);