From: Carsten Langgaard <carstenl@mips.com>
To: Jeff Garzik <jgarzik@pobox.com>,
Ralf Baechle <ralf@linux-mips.org>,
linux-mips@linux-mips.org, tsbogend@alpha.franken.de,
linux-net@vger.kernel.org, kevink@mips.com
Subject: Re: BUG in the PCNET32 ethernet driver
Date: Wed, 11 Dec 2002 13:37:57 +0100 [thread overview]
Message-ID: <3DF731A5.14F95F64@mips.com> (raw)
In-Reply-To: 3DD2BA39.E1457D67@mips.com
What happen regarding this issue, did you apply my patch or ?
/Carsten
Carsten Langgaard wrote:
> Jeff Garzik wrote:
>
> > Carsten Langgaard wrote:
> >
> > > Jeff Garzik wrote:
> > >
> > >
> > > >Carsten Langgaard wrote:
> > > >
> > > >
> > > >>@@ -1316,13 +1316,13 @@
> > > >> if ((newskb = dev_alloc_skb (PKT_BUF_SZ))) {
> > > >> skb_reserve (newskb, 2);
> > > >> skb = lp->rx_skbuff[entry];
> > > >>- pci_unmap_single(lp->pci_dev,
> > > lp->rx_dma_addr[entry], skb->len,
> > > >>PCI_DMA_FROMDEVICE);
> > > >>+ pci_unmap_single(lp->pci_dev,
> > > lp->rx_dma_addr[entry], pkt_len +2,
> > > >>PCI_DMA_FROMDEVICE);
> > > >> skb_put (skb, pkt_len);
> > > >> lp->rx_skbuff[entry] = newskb;
> > > >
> > > >Why does this line not reference PKT_BUF_SZ when all the others do?
> > >
> > >
> > > In this case we know the size of the packet and therefore only need to
> > > handle that.
> > > In the other cases we don't know have big the receiving packet is
> > > going to be, so we has to
> > > take care of the whole buffer.
> >
> > Well, it's a seriously bad idea to pass different values to map and
> > unmap steps, because on some platforms you could wind up telling the
> > IOMMU or some other allocator that you are allocating N bytes, but
> > freeing N-M bytes. IOW, a leak.
>
> Ok, fine.
> There is actually another place in the code that also need a fix then.
>
> >
> > Now that that's been clarified, please fix up the patch and resubmit...
> > with this issue fixed, it looks apply-able.
> >
>
> I have made the change and attached a new patch.
>
> >
> > Jeff
>
> --
> _ _ ____ ___ Carsten Langgaard Mailto:carstenl@mips.com
> |\ /|||___)(___ MIPS Denmark Direct: +45 4486 5527
> | \/ ||| ____) Lautrupvang 4B Switch: +45 4486 5555
> TECHNOLOGIES 2750 Ballerup Fax...: +45 4486 5556
> Denmark http://www.mips.com
>
> ------------------------------------------------------------------------
> Index: drivers/net/pcnet32.c
> ===================================================================
> RCS file: /home/cvs/linux/drivers/net/pcnet32.c,v
> retrieving revision 1.33.2.3
> diff -u -r1.33.2.3 pcnet32.c
> --- drivers/net/pcnet32.c 6 Oct 2002 20:49:43 -0000 1.33.2.3
> +++ drivers/net/pcnet32.c 13 Nov 2002 20:38:39 -0000
> @@ -981,7 +981,7 @@
> }
> skb_reserve (rx_skbuff, 2);
> }
> - lp->rx_dma_addr[i] = pci_map_single(lp->pci_dev, rx_skbuff->tail, rx_skbuff->len, PCI_DMA_FROMDEVICE);
> + lp->rx_dma_addr[i] = pci_map_single(lp->pci_dev, rx_skbuff->tail, PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
> lp->rx_ring[i].base = (u32)le32_to_cpu(lp->rx_dma_addr[i]);
> lp->rx_ring[i].buf_length = le16_to_cpu(-PKT_BUF_SZ);
> lp->rx_ring[i].status = le16_to_cpu(0x8000);
> @@ -1316,13 +1316,13 @@
> if ((newskb = dev_alloc_skb (PKT_BUF_SZ))) {
> skb_reserve (newskb, 2);
> skb = lp->rx_skbuff[entry];
> - pci_unmap_single(lp->pci_dev, lp->rx_dma_addr[entry], skb->len, PCI_DMA_FROMDEVICE);
> + pci_unmap_single(lp->pci_dev, lp->rx_dma_addr[entry], PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
> skb_put (skb, pkt_len);
> lp->rx_skbuff[entry] = newskb;
> newskb->dev = dev;
> lp->rx_dma_addr[entry] =
> pci_map_single(lp->pci_dev, newskb->tail,
> - newskb->len, PCI_DMA_FROMDEVICE);
> + PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
> lp->rx_ring[entry].base = le32_to_cpu(lp->rx_dma_addr[entry]);
> rx_in_place = 1;
> } else
> @@ -1349,13 +1349,10 @@
> if (!rx_in_place) {
> skb_reserve(skb,2); /* 16 byte align */
> skb_put(skb,pkt_len); /* Make room */
> - pci_dma_sync_single(lp->pci_dev,
> - lp->rx_dma_addr[entry],
> - pkt_len,
> - PCI_DMA_FROMDEVICE);
> eth_copy_and_sum(skb,
> (unsigned char *)(lp->rx_skbuff[entry]->tail),
> pkt_len,0);
> + lp->rx_dma_addr[entry] = pci_map_single(lp->pci_dev, lp->rx_skbuff[entry]->tail, PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
> }
> lp->stats.rx_bytes += skb->len;
> skb->protocol=eth_type_trans(skb,dev);
> @@ -1406,7 +1403,7 @@
> for (i = 0; i < RX_RING_SIZE; i++) {
> lp->rx_ring[i].status = 0;
> if (lp->rx_skbuff[i]) {
> - pci_unmap_single(lp->pci_dev, lp->rx_dma_addr[i], lp->rx_skbuff[i]->len, PCI_DMA_FROMDEVICE);
> + pci_unmap_single(lp->pci_dev, lp->rx_dma_addr[i], PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
> dev_kfree_skb(lp->rx_skbuff[i]);
> }
> lp->rx_skbuff[i] = NULL;
--
_ _ ____ ___ Carsten Langgaard Mailto:carstenl@mips.com
|\ /|||___)(___ MIPS Denmark Direct: +45 4486 5527
| \/ ||| ____) Lautrupvang 4B Switch: +45 4486 5555
TECHNOLOGIES 2750 Ballerup Fax...: +45 4486 5556
Denmark http://www.mips.com
prev parent reply other threads:[~2002-12-11 12:38 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2002-11-13 13:34 BUG in the PCNET32 ethernet driver Carsten Langgaard
2002-11-13 16:42 ` Jeff Garzik
2002-11-13 20:08 ` Carsten Langgaard
2002-11-14 4:11 ` David S. Miller
2002-11-13 20:20 ` Jeff Garzik
2002-11-13 20:46 ` Carsten Langgaard
2002-12-11 12:37 ` Carsten Langgaard [this message]
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=3DF731A5.14F95F64@mips.com \
--to=carstenl@mips.com \
--cc=jgarzik@pobox.com \
--cc=kevink@mips.com \
--cc=linux-mips@linux-mips.org \
--cc=linux-net@vger.kernel.org \
--cc=ralf@linux-mips.org \
--cc=tsbogend@alpha.franken.de \
/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