netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] sky2: fix transmit DMA map leakage
@ 2010-02-01 23:41 Stephen Hemminger
  2010-02-01 23:45 ` [PATCH 2/2] sky2: hand receive DMA mapping failures Stephen Hemminger
  2010-02-04  2:34 ` [PATCH 1/2] sky2: fix transmit DMA map leakage David Miller
  0 siblings, 2 replies; 3+ messages in thread
From: Stephen Hemminger @ 2010-02-01 23:41 UTC (permalink / raw)
  To: David Miller; +Cc: Jarek Poplawski, Michael Breuer, netdev

The book keeping structure for transmit always had the flags value
cleared so transmit DMA maps were never released correctly.
Based on patch by Jarek Poplawski, problem observed by Michael Breuer.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--
This patch is against net-next but it should go to 2.6.33 and stable
as well.

--- a/drivers/net/sky2.c	2010-02-01 10:07:42.676296236 -0800
+++ b/drivers/net/sky2.c	2010-02-01 10:18:12.575044064 -0800
@@ -1025,11 +1025,8 @@ static void sky2_prefetch_init(struct sk
 static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
 {
 	struct sky2_tx_le *le = sky2->tx_le + *slot;
-	struct tx_ring_info *re = sky2->tx_ring + *slot;
 
 	*slot = RING_NEXT(*slot, sky2->tx_ring_size);
-	re->flags = 0;
-	re->skb = NULL;
 	le->ctrl = 0;
 	return le;
 }
@@ -1622,8 +1619,7 @@ static unsigned tx_le_req(const struct s
 	return count;
 }
 
-static void sky2_tx_unmap(struct pci_dev *pdev,
-			  const struct tx_ring_info *re)
+static void sky2_tx_unmap(struct pci_dev *pdev, struct tx_ring_info *re)
 {
 	if (re->flags & TX_MAP_SINGLE)
 		pci_unmap_single(pdev, pci_unmap_addr(re, mapaddr),
@@ -1633,6 +1629,7 @@ static void sky2_tx_unmap(struct pci_dev
 		pci_unmap_page(pdev, pci_unmap_addr(re, mapaddr),
 			       pci_unmap_len(re, maplen),
 			       PCI_DMA_TODEVICE);
+	re->flags = 0;
 }
 
 /*
@@ -1839,6 +1836,7 @@ static void sky2_tx_complete(struct sky2
 			dev->stats.tx_packets++;
 			dev->stats.tx_bytes += skb->len;
 
+			re->skb = NULL;
 			dev_kfree_skb_any(skb);
 
 			sky2->tx_next = RING_NEXT(idx, sky2->tx_ring_size);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] sky2: hand receive DMA mapping failures
  2010-02-01 23:41 [PATCH 1/2] sky2: fix transmit DMA map leakage Stephen Hemminger
@ 2010-02-01 23:45 ` Stephen Hemminger
  2010-02-04  2:34 ` [PATCH 1/2] sky2: fix transmit DMA map leakage David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2010-02-01 23:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Jarek Poplawski, Michael Breuer, netdev

If receive buffer mapping failed, then it was possible to get
stuck with unmapped receive buffer in DMA ring. 

This would be an extremely rare condition because the driver had just
released the map for the last receive so it should be able to get
another map again (in soft-irq).

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Patch is against net-next. Not worth the risk for 2.6.33 or stable


--- a/drivers/net/sky2.c	2010-02-01 10:18:12.575044064 -0800
+++ b/drivers/net/sky2.c	2010-02-01 10:18:16.975356378 -0800
@@ -1103,18 +1103,39 @@ static int sky2_rx_map_skb(struct pci_de
 	int i;
 
 	re->data_addr = pci_map_single(pdev, skb->data, size, PCI_DMA_FROMDEVICE);
-	if (unlikely(pci_dma_mapping_error(pdev, re->data_addr)))
-		return -EIO;
+	if (pci_dma_mapping_error(pdev, re->data_addr))
+		goto mapping_error;
 
 	pci_unmap_len_set(re, data_size, size);
 
-	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
-		re->frag_addr[i] = pci_map_page(pdev,
-						skb_shinfo(skb)->frags[i].page,
-						skb_shinfo(skb)->frags[i].page_offset,
-						skb_shinfo(skb)->frags[i].size,
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+
+		re->frag_addr[i] = pci_map_page(pdev, frag->page,
+						frag->page_offset,
+						frag->size,
 						PCI_DMA_FROMDEVICE);
+
+		if (pci_dma_mapping_error(pdev, re->frag_addr[i]))
+			goto map_page_error;
+	}
 	return 0;
+
+map_page_error:
+	while (--i >= 0) {
+		pci_unmap_page(pdev, re->frag_addr[i],
+			       skb_shinfo(skb)->frags[i].size,
+			       PCI_DMA_FROMDEVICE);
+	}
+
+	pci_unmap_single(pdev, re->data_addr, pci_unmap_len(re, data_size),
+			 PCI_DMA_FROMDEVICE);
+
+mapping_error:
+	if (net_ratelimit())
+		dev_warn(&pdev->dev, "%s: rx mapping error\n",
+			 skb->dev->name);
+	return -EIO;
 }
 
 static void sky2_rx_unmap_skb(struct pci_dev *pdev, struct rx_ring_info *re)
@@ -2306,30 +2327,32 @@ static struct sk_buff *receive_new(struc
 				   struct rx_ring_info *re,
 				   unsigned int length)
 {
-	struct sk_buff *skb, *nskb;
+	struct sk_buff *skb;
+	struct rx_ring_info nre;
 	unsigned hdr_space = sky2->rx_data_size;
 
-	/* Don't be tricky about reusing pages (yet) */
-	nskb = sky2_rx_alloc(sky2);
-	if (unlikely(!nskb))
-		return NULL;
+	nre.skb = sky2_rx_alloc(sky2);
+	if (unlikely(!nre.skb))
+		goto nobuf;
+
+	if (sky2_rx_map_skb(sky2->hw->pdev, &nre, hdr_space))
+		goto nomap;
 
 	skb = re->skb;
 	sky2_rx_unmap_skb(sky2->hw->pdev, re);
-
 	prefetch(skb->data);
-	re->skb = nskb;
-	if (sky2_rx_map_skb(sky2->hw->pdev, re, hdr_space)) {
-		dev_kfree_skb(nskb);
-		re->skb = skb;
-		return NULL;
-	}
+	*re = nre;
 
 	if (skb_shinfo(skb)->nr_frags)
 		skb_put_frags(skb, hdr_space, length);
 	else
 		skb_put(skb, length);
 	return skb;
+
+nomap:
+	dev_kfree_skb(nre.skb);
+nobuf:
+	return NULL;
 }
 
 /*

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] sky2: fix transmit DMA map leakage
  2010-02-01 23:41 [PATCH 1/2] sky2: fix transmit DMA map leakage Stephen Hemminger
  2010-02-01 23:45 ` [PATCH 2/2] sky2: hand receive DMA mapping failures Stephen Hemminger
@ 2010-02-04  2:34 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2010-02-04  2:34 UTC (permalink / raw)
  To: shemminger; +Cc: jarkao2, mbreuer, netdev

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 1 Feb 2010 15:41:47 -0800

> The book keeping structure for transmit always had the flags value
> cleared so transmit DMA maps were never released correctly.
> Based on patch by Jarek Poplawski, problem observed by Michael Breuer.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied to net-2.6, and queued for -stable, thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-02-04  2:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-01 23:41 [PATCH 1/2] sky2: fix transmit DMA map leakage Stephen Hemminger
2010-02-01 23:45 ` [PATCH 2/2] sky2: hand receive DMA mapping failures Stephen Hemminger
2010-02-04  2:34 ` [PATCH 1/2] sky2: fix transmit DMA map leakage David Miller

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).