[PATCH 0/3] NIU: Implement discard counters

Previous thread: [PATCH] net: Fix module refcount leak in kernel_accept() by Wei Yongjun on Thursday, December 18, 2008 - 3:20 am. (4 messages)

Next thread: [patch]startup race in hso driver by Oliver Neukum on Thursday, December 18, 2008 - 6:57 am. (9 messages)
From: Jesper Dangaard Brouer
Date: Thursday, December 18, 2008 - 6:52 am

This patch series implements discard counter stats for the NIU driver.

This work has been done together with Robert Olsson.  We have come to a
stage where the patches needs public review, and possible discussion.

Tested with hardware:
 Sun Quad GbE x8 PCIe
 Sun Dual 10GbE XFP PCIe

-- 
Med venlig hilsen / Best regards
  Jesper Brouer
  ComX Networks A/S
  Linux Network developer
  Cand. Scient Datalog / MSc.
  Author of http://adsl-optimizer.dk
  LinkedIn: http://www.linkedin.com/in/brouer

--

From: Jesper Dangaard Brouer
Date: Thursday, December 18, 2008 - 6:54 am

Implementing discard counters for the NIU driver turned out to be more
complicated than first assumed.

The discard counters for the NIU neptune chip is only 16-bit
(eventhough this is a 64-bit chip).  These 16-bit counters can
overflow quickly, especially considering this is a 10Gbit/s ethernet
card.

The overflow indication bit is, unfortunatly, not usable as the
counter value does not wrap, but remains at max value 0xFFFF.
Resulting in lost counts until the counter is reset.

The read and reset scheme also poses a problem. Both in theory and in
praxis counters can be lost in between reading nr64() and clearing the
counter nw64().  For this reason, the number of counter clearings
nw64() is limited/reduced.  On the fash-path the counters are only
syncronized once it exceeds 0x7FFF.  When read by userspace, 
its syncronized fully.

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---

 drivers/net/niu.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 1b6f548..1bd7018 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -3529,6 +3529,51 @@ out:
 	}
 }
 
+static inline void niu_sync_rx_discard_stats(struct niu *np,
+					     struct rx_ring_info *rp,
+					     const int limit)
+{
+	/* This elaborate scheme is needed for reading the RX discard
+	 * counters, as they are only 16-bit and can overflow quickly,
+	 * and because the overflow indication bit is not usable as
+	 * the counter value does not wrap, but remains at max value
+	 * 0xFFFF.
+	 *
+	 * In theory and in praxis counters can be lost in between
+	 * reading nr64() and clearing the counter nw64().  For this
+	 * reason, the number of counter clearings nw64() is
+	 * limited/reduced though the limit parameter.
+	 */
+	int rx_channel = rp->rx_channel;
+	u32 misc, wred;
+
+	/* RXMISC (Receive Miscellaneous Discard Count), covers the
+	 * following discard events: IPP ...
From: Jesper Dangaard Brouer
Date: Thursday, December 18, 2008 - 6:56 am

Discard packet counter debug statements that can be turned on
runtime, by users to assist debugging of the driver code.

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---

 drivers/net/niu.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 1bd7018..9f6a98f 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -3560,6 +3560,9 @@ static inline void niu_sync_rx_discard_stats(struct niu *np,
 		if (unlikely(misc & RXMISC_OFLOW))
 			dev_err(np->device, "rx-%d: Counter overflow "
 				"RXMISC discard\n", rx_channel);
+
+		niudbg(RX_ERR, "%s-rx-%d: MISC drop=%u over=%u\n",
+		       np->dev->name, rx_channel, misc, misc-limit);
 	}
 
 	/* WRED (Weighted Random Early Discard) by hardware */
@@ -3571,6 +3574,9 @@ static inline void niu_sync_rx_discard_stats(struct niu *np,
 		if (unlikely(wred & RED_DIS_CNT_OFLOW))
 			dev_err(np->device, "rx-%d: Counter overflow "
 				"WRED discard\n", rx_channel);
+
+		niudbg(RX_ERR, "%s-rx-%d: WRED drop=%u over=%u\n",
+		       np->dev->name, rx_channel, wred, wred-limit);
 	}
 }
 


--

From: Jesper Dangaard Brouer
Date: Thursday, December 18, 2008 - 6:58 am

Optimize the lightly loaded case, by only syncronizing discards stats
when qlen > 10 indicate potential for drops.

Notice Robert Olsson might disagree with this patch.

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---

 drivers/net/niu.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 9f6a98f..7746230 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -3620,7 +3620,9 @@ static int niu_rx_work(struct niu *np, struct rx_ring_info *rp, int budget)
 
 	nw64(RX_DMA_CTL_STAT(rp->rx_channel), stat);
 
-	niu_sync_rx_discard_stats(np, rp, 0x7FFF);
+	/* Only sync discards stats when qlen indicate potential for drops */
+	if (qlen > 10)
+		niu_sync_rx_discard_stats(np, rp, 0x7FFF);
 
 	return work_done;
 }


--

From: David Miller
Date: Thursday, December 18, 2008 - 8:48 pm

From: Jesper Dangaard Brouer <hawk@comx.dk>

To be honest these patches look great, I'll add them to
net-next-2.6

Thanks!
--

Previous thread: [PATCH] net: Fix module refcount leak in kernel_accept() by Wei Yongjun on Thursday, December 18, 2008 - 3:20 am. (4 messages)

Next thread: [patch]startup race in hso driver by Oliver Neukum on Thursday, December 18, 2008 - 6:57 am. (9 messages)