On 10/11/06, Jean Delvare <[EMAIL PROTECTED]> wrote:
Hi all,

This patch is posted for review and comments.

Let the e1000 driver report the most important statistics (rx/tx_bytes
and rx/tx_packets) in real time, rather than every other second. This
is similar to what the e100 driver is doing.

The current asynchronous statistics refresh model makes it impossible
to monitor the network traffic with an interval which isn't a multiple
of 2 seconds. For example, an interval of 5 seconds would result in a
sawtooth diagram (+20%, -20%) for a constant transfer rate. With a 1
second interval it's even worse (0, 200%) of course. This has been
annoying users for years, but was never actually fixed:

I think the idea is good, however, see below.

<snip
rx/tx_bytes will show slightly lower values than before, because the
hardware appears to include the 4-byte ethernet frame CRC into the
frame length, while the driver doesn't. It's probably OK as the
e100, 3c59x and 8139too drivers don't include it either.

this is okay.

I additionally noted a difference of 6 bytes on some TX frames, which
I am not able to explain. It's probably small and rare enough not to
be considered a problem, but if someone can explain it, I would be
grateful.

now, that sounds odd, however, once again, see below.

Signed-off-by: Jean Delvare <[EMAIL PROTECTED]>
---
 drivers/net/e1000/e1000_main.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

--- linux-2.6.19-rc1.orig/drivers/net/e1000/e1000_main.c        2006-10-11 
10:53:49.000000000 +0200
+++ linux-2.6.19-rc1/drivers/net/e1000/e1000_main.c     2006-10-11 
11:34:41.000000000 +0200
@@ -3118,6 +3118,8 @@
                       e1000_tx_map(adapter, tx_ring, skb, first,
                                    max_per_txd, nr_frags, mss));

+       adapter->net_stats.tx_packets++;
+       adapter->net_stats.tx_bytes += skb->len;
        netdev->trans_start = jiffies;

this is the part I'm most worried about.  as I believe it to be
incorrect for TSO packets.  Maybe something like?
+       if (skb_shinfo(skb)->gso_segs)
+              adapter->net_stats.tx_packets += skb_shinfo(skb)->gso_segs;
+       else
+              adapter->net_stats.tx_packets++;
+       adapter->net_stats.tx_bytes += skb->len;
       netdev->trans_start = jiffies;

skb len will still be off by some amount, because the skb->data
(header) is replicated across each gso segment but only counted once
this way, but hopefully someone will pipe up with a good way to
compute that.

The rest of the patch seems fine, barring any other comments.

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

Reply via email to