Re: [RFC PATCH] net: clean up rx_copybreak handling

[David Dillow <dave@thedillows.org>]

On Sat, 2011-07-09 at 01:27 +0200, Michał Mirosław wrote:
> diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c

> @@ -1675,25 +1674,14 @@ typhoon_rx(struct typhoon *tp, struct basic_ring *rxRing, volatile __le32 * read
>  
>                 pkt_len = le16_to_cpu(rx->frameLen);
>  
> -               if(pkt_len < rx_copybreak &&
> -                  (new_skb = dev_alloc_skb(pkt_len + 2)) != NULL) {
> -                       skb_reserve(new_skb, 2);
> -                       pci_dma_sync_single_for_cpu(tp->pdev, dma_addr,
> -                                                   PKT_BUF_SZ,
> -                                                   PCI_DMA_FROMDEVICE);
> -                       skb_copy_to_linear_data(new_skb, skb->data, pkt_len);
> -                       pci_dma_sync_single_for_device(tp->pdev, dma_addr,
> -                                                      PKT_BUF_SZ,
> -                                                      PCI_DMA_FROMDEVICE);
> -                       skb_put(new_skb, pkt_len);
> +               new_skb = dev_skb_finish_rx_dma(&rxb->skb,
> +                       pkt_len, rx_copybreak,
> +                       &tp->pdev->dev, dma_addr, PKT_BUF_SZ);

Needs a few more tabs in front of the arguments. It looks like
		new_skb = dev_skb_finish_rx_dma(&rxb->skb, pkt_len,
						rx_copybreak, &tp->pdev->dev,
						dma_addr, PKT_BUF_SZ);

would fit the style around it better.

> +               if (!rxb->skb)
>                         typhoon_recycle_rx_skb(tp, idx);

I think you either meant
		if (rxb->skb)
			typhoon_recycle_rx_skb(tp, idx);

or to swap typhoon_{recycle,alloc}_rx_skb(). As it stands, you'll reload
the NIC with a NULL skb pointer, and it will DMA to the old location
when it eventually uses this descriptor.

> -               } else {
> -                       new_skb = skb;
> -                       skb_put(new_skb, pkt_len);
> -                       pci_unmap_single(tp->pdev, dma_addr, PKT_BUF_SZ,
> -                                      PCI_DMA_FROMDEVICE);
> +               else
>                         typhoon_alloc_rx_skb(tp, idx);