Date:	Wed, 06 May 2009 15:09:11 +0200
From:	Jesper Dangaard Brouer <hawk@...x.dk>
To:	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>
Cc:	David Miller <davem@...emloft.net>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"hawk@...u.dk" <hawk@...u.dk>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>
Subject: Re: [PATCH] igb: Record hardware RX overruns in net_stats

On Wed, 2009-05-06 at 01:11 -0700, Waskiewicz Jr, Peter P wrote:
> On Wed, 6 May 2009, Jesper Dangaard Brouer wrote:
> 
> > On Tue, 2009-05-05 at 14:35 -0700, David Miller wrote:
> > > From: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> > > Date: Tue, 5 May 2009 14:32:04 -0700
> > > 
> > > > the manual[1] for the hardware says:
> > > > RNBC:
> > > > This register counts the number of times that frames were received
> > > > when there were no available buffers in host memory to store those
> > > > frames (receive descriptor head and tail pointers were equal). The
> > > > packet is still received if there is space in the FIFO. This register
> > > > only increments if receives are enabled. This register does not
> > > > increment when flow control packets are received.
> > > > 
> > > > The critical bit "The packet is still received if there is space in
> > > > the FIFO" (AND a host memory buffer becomes available) So the reason
> > > > we don't want to put it in the net_stats stats for drops is that the
> > > > packet
> > > > *wasn't* necessarily dropped.
> > > > 
> > > > The rx_missed errors is for packets that were definitely dropped, and
> > > > is already stored in the net_stats structure.
> > > 
> > > While not an "rx_missed" because we do eventually take the
> > > packet, conceptually it is a "fifo overflow" in the sense
> > > that we exceeded available receive resources at the time that
> > > the packet arrived.
> > 
> > Yes, with this argumentation, the MPC should then be kept as "rx_missed"
> > packets.  And the RNBC stored as "rx_fifo_errors" as its an overflow
> > indication, not a number of packets dropped.
> 
> The way RNBC works depends on how the queues themselves are configured.  
> Specifically, if you have packet drop enabled per queue or not will affect 
> RNBC.

Very good description, thank you Peter.
But I could not resist to actually verify/test it, and my observations
differ some! ;-)  (patch in bottom indicate where I set it in the code)

> In the SRRCTL registers, there is a DROP_EN bit, bit 31.  If this 
> bit is set to 1b for the queue in question, then the packet will be 
> dropped when there are no buffers in the packet buffer.  This does not 
> mean the FIFO is full or has been overrun, it just means there's no more
> descriptors available in the Rx ring for that queue.  In this case, RNBC 
> is incremented, MPC is not.

My experience is that if DROP_EN bit is *set*. Then I cannot find the
drop count anywhere... not in RNBC and not in MPC ... and I can still
see the drops with my netfilter module mp2t.

ethtool -S eth21 | egrep 'rx_no_buffer_count|rx_miss'
     rx_no_buffer_count: 0
     rx_missed_errors: 0

I'm guessing that the drop counters are now in the per queue RQDPC
register (Receive Queue Drop Packet Count), but reading that is not
implemented in the driver.

(kernel: [438792.665028] Hawk hack -- Register: srrctl:[0x82000002])

> If bit 31 in SRRCTL is 0b, then if there's no room in the packet buffer
> (no more descriptors available), the device tries to store the packet in
> the FIFO.  RNBC will *not* be incremented in this case.  If there's no space
> in the FIFO, then the packet is dropped.  RNBC still is not incremented in this
> case, rather MPC will be incremented, since the packet was dropped due to the FIFO 
> being full.

My experience is that if DROP_EN bit is *NOT* set. Then the RNBC *is*
incremented...

 ethtool -S eth21 | egrep 'rx_no_buffer_count|rx_miss'
     rx_no_buffer_count: 26436
     rx_missed_errors: 0

(kernel: [439261.463628] Hawk hack -- Register: srrctl:[0x2000002])


> In 82576, according to the manual, SRRCTL bit 31 is 0b for queue 0 by 
> default, and is 1b for all other queues by default.

Funny default...

> I hope this helps explain what the hardware is doing, and how these two 
> counters get used in overrun cases.

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


Testing the SRRCTL_DROP_EN bit behavior.

From: Jesper Dangaard Brouer <hawk@...x.dk>
---

 drivers/net/igb/igb_main.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)


diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 3ee00a5..20117ce 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -49,7 +49,7 @@
 #endif
 #include "igb.h"
 
-#define DRV_VERSION "1.3.16-k2"
+#define DRV_VERSION "1.3.16-k2-test-drop-bit"
 char igb_driver_name[] = "igb";
 char igb_driver_version[] = DRV_VERSION;
 static const char igb_driver_string[] =
@@ -2091,6 +2091,11 @@ static void igb_setup_rctl(struct igb_adapter *adapter)
 		wr32(E1000_VMOLR(j), vmolr);
 	}
 
+	/* Hawk: Hack to test the SRRCTL_DROP_EN bit behavior */
+	srrctl &= ~E1000_SRRCTL_DROP_EN; /* Unset bit */
+	//srrctl |= E1000_SRRCTL_DROP_EN; /* Set bit */
+	printk(KERN_INFO "Hawk hack -- Register: srrctl:[0x%X]\n", srrctl);
+
 	for (i = 0; i < adapter->num_rx_queues; i++) {
 		j = adapter->rx_ring[i].reg_idx;
 		wr32(E1000_SRRCTL(j), srrctl);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists