[PATCH 1/2] macb: Handle Retry Limit Exceeded errors

[PATCH 1/2] macb: Handle Retry Limit Exceeded errors

[Haavard Skinnemoen]

From: Erik Waling <erik.waling@konftel.se>

When transfering large amounts of data we sometimes experienced that the
Retry Limit Exceeded (RLE) bit got set in TSR during transmission
attempts. When this happened the driver would stall in a state that
prevented any more data from being sent.

Signed-off-by: Erik Waling <erik.waling@konftel.com>
Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 drivers/net/macb.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index f505010..22a595c 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -316,10 +316,11 @@ static void macb_tx(struct macb *bp)
 	dev_dbg(&bp->pdev->dev, "macb_tx status = %02lx\n",
 		(unsigned long)status);
 
-	if (status & MACB_BIT(UND)) {
+	if (status & (MACB_BIT(UND) | MACB_BIT(TSR_RLE))) {
 		int i;
-		printk(KERN_ERR "%s: TX underrun, resetting buffers\n",
-			bp->dev->name);
+		printk(KERN_ERR "%s: TX %s, resetting buffers\n",
+			bp->dev->name, status & MACB_BIT(UND) ?
+			"underrun" : "retry limit exceeded");
 
 		/* Transfer ongoing, disable transmitter, to avoid confusion */
 		if (status & MACB_BIT(TGO))
@@ -590,7 +591,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			}
 		}
 
-		if (status & (MACB_BIT(TCOMP) | MACB_BIT(ISR_TUND)))
+		if (status & (MACB_BIT(TCOMP) | MACB_BIT(ISR_TUND) |
+			    MACB_BIT(ISR_RLE)))
 			macb_tx(bp);
 
 		/*
-- 
1.6.0.4

[PATCH 2/2] macb: process the RX ring regardless of interrupt status

[Haavard Skinnemoen]

From: Erik Waling <erik.waling@konftel.com>

Suppose that we receive lots of frames, start processing them, but
exhaust our budget so that we return before we had a chance to look
at all of them.

Then, when the network layer calls us again, we will only continue
processing the buffers if the REC bit was set in the mean time, which it
might not be if there was a brief pause in the flow of packets. If this
happens, we'll simply display a warning and call netif_rx_complete()
with potentially lots of unprocessed packets in the RX ring...

Fix this by scanning the ring no matter what flags are set in the
interrupt status register.

Signed-off-by: Erik Waling <erik.waling@konftel.com>
Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 drivers/net/macb.c |   18 ------------------
 1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 22a595c..d473540 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -521,27 +521,10 @@ static int macb_poll(struct napi_struct *napi, int budget)
 	macb_writel(bp, RSR, status);
 
 	work_done = 0;
-	if (!status) {
-		/*
-		 * This may happen if an interrupt was pending before
-		 * this function was called last time, and no packets
-		 * have been received since.
-		 */
-		napi_complete(napi);
-		goto out;
-	}
 
 	dev_dbg(&bp->pdev->dev, "poll: status = %08lx, budget = %d\n",
 		(unsigned long)status, budget);
 
-	if (!(status & MACB_BIT(REC))) {
-		dev_warn(&bp->pdev->dev,
-			 "No RX buffers complete, status = %02lx\n",
-			 (unsigned long)status);
-		napi_complete(napi);
-		goto out;
-	}
-
 	work_done = macb_rx(bp, budget);
 	if (work_done < budget)
 		napi_complete(napi);
@@ -550,7 +533,6 @@ static int macb_poll(struct napi_struct *napi, int budget)
 	 * We've done what we can to clean the buffers. Make sure we
 	 * get notified when new packets arrive.
 	 */
-out:
 	macb_writel(bp, IER, MACB_RX_INT_FLAGS);
 
 	/* TODO: Handle errors */
-- 
1.6.0.4

RE: [PATCH 2/2] macb: process the RX ring regardless of interrupt status

[Gerard Kam]

Hi there

> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> Sent: Thursday, April 16, 2009 2:32 AM

> Fix this by scanning the ring no matter what flags are set in the
> interrupt status register.

I've been using a similar patch to macb, however, it's to fix an issue with
rotting packets.  I'm using kernel 2.6.25, and haven't updated this patch to
a more recent kernel version.  Patch was developed with input from Travis
Stratman, tstratman@emacinc.com.


Fix for RX rotting packets.

diff -pruN a/drivers/net/macb.c b/drivers/net/macb.c
--- a/drivers/net/macb.c	2008-05-18 22:21:53.000000000 -0700
+++ b/drivers/net/macb.c	2008-08-28 11:36:42.000000000 -0700
@@ -508,40 +508,29 @@ static int macb_poll(struct napi_struct 
 	u32 status;
 
 	status = macb_readl(bp, RSR);
-	macb_writel(bp, RSR, status);
-
-	work_done = 0;
-	if (!status) {
-		/*
-		 * This may happen if an interrupt was pending before
-		 * this function was called last time, and no packets
-		 * have been received since.
-		 */
-		netif_rx_complete(dev, napi);
-		goto out;
-	}
+	macb_writel(bp, RSR, status);	/* not atomic read-clear! */
 
 	dev_dbg(&bp->pdev->dev, "poll: status = %08lx, budget = %d\n",
 		(unsigned long)status, budget);
 
-	if (!(status & MACB_BIT(REC))) {
-		dev_warn(&bp->pdev->dev,
-			 "No RX buffers complete, status = %02lx\n",
-			 (unsigned long)status);
-		netif_rx_complete(dev, napi);
-		goto out;
-	}
-
 	work_done = macb_rx(bp, budget);
-	if (work_done < budget)
+	if ((work_done < budget) && !macb_readl(bp, RSR)) {
+		/*
+		 * We've done what we can to clean the buffers.
+		 * Make sure there's no new RX activity during transition
+		 * from poll mode back to interrupt mode.
+		 */
 		netif_rx_complete(dev, napi);
-
-	/*
-	 * We've done what we can to clean the buffers. Make sure we
-	 * get notified when new packets arrive.
-	 */
-out:
-	macb_writel(bp, IER, MACB_RX_INT_FLAGS);
+		macb_writel(bp, IER, MACB_RX_INT_FLAGS);
+		status = macb_readl(bp, RSR);
+		if (status) {
+			/* new RX, undo the polling-done operations */
+			macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
+			netif_rx_reschedule(dev, napi);
+			dev_dbg(&bp->pdev->dev, "poll: undo status =
%08lx\n",
+				(unsigned long)status);
+		}
+	}
 
 	/* TODO: Handle errors */
 
@@ -554,7 +543,7 @@ static irqreturn_t macb_interrupt(int ir
 	struct macb *bp = netdev_priv(dev);
 	u32 status;
 
-	status = macb_readl(bp, ISR);
+	status = macb_readl(bp, ISR) & ~macb_readl(bp, IMR);
 
 	if (unlikely(!status))
 		return IRQ_NONE;
@@ -578,7 +567,9 @@ static irqreturn_t macb_interrupt(int ir
 				dev_dbg(&bp->pdev->dev,
 					"scheduling RX softirq\n");
 				__netif_rx_schedule(dev, &bp->napi);
-			}
+			} else
+				printk(KERN_ERR "%s: sched RX poll
failed\n",
+					dev->name);
 		}
 
 		if (status & (MACB_BIT(TCOMP) | MACB_BIT(ISR_TUND)))
@@ -598,7 +589,7 @@ static irqreturn_t macb_interrupt(int ir
 			       dev->name);
 		}
 
-		status = macb_readl(bp, ISR);
+		status = macb_readl(bp, ISR) & ~macb_readl(bp, IMR);
 	}
 
 	spin_unlock(&bp->lock);
--



 

Re: [PATCH 1/2] macb: Handle Retry Limit Exceeded errors

[David Miller]

From: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
Date: Thu, 16 Apr 2009 11:32:10 +0200

> When transfering large amounts of data we sometimes experienced that the
> Retry Limit Exceeded (RLE) bit got set in TSR during transmission
> attempts. When this happened the driver would stall in a state that
> prevented any more data from being sent.
> 
> Signed-off-by: Erik Waling <erik.waling@konftel.com>
> Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>

Applied.

Re: [PATCH 2/2] macb: process the RX ring regardless of interrupt status

[David Miller]

From: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
Date: Thu, 16 Apr 2009 11:32:11 +0200

> Suppose that we receive lots of frames, start processing them, but
> exhaust our budget so that we return before we had a chance to look
> at all of them.
> 
> Then, when the network layer calls us again, we will only continue
> processing the buffers if the REC bit was set in the mean time, which it
> might not be if there was a brief pause in the flow of packets. If this
> happens, we'll simply display a warning and call netif_rx_complete()
> with potentially lots of unprocessed packets in the RX ring...
> 
> Fix this by scanning the ring no matter what flags are set in the
> interrupt status register.
> 
> Signed-off-by: Erik Waling <erik.waling@konftel.com>
> Signed-off-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>

Applied.