linux-mips
[Top] [All Lists]

Re: BUG in the PCNET32 ethernet driver

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
From: Carsten Langgaard <carstenl@mips.com>
Date: Wed, 11 Dec 2002 13:37:57 +0100
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <3DD254F8.14DE20EA@mips.com> <3DD280FB.7070907@pobox.com> <3DD2B128.62DFB392@mips.com> <3DD2B402.8040508@pobox.com> <3DD2BA39.E1457D67@mips.com>
Sender: linux-mips-bounce@linux-mips.org
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 in Thread] Current Thread [Next in Thread>
  • Re: BUG in the PCNET32 ethernet driver, Carsten Langgaard <=