On Wed, 11 Oct 2006 10:44:12 -0700 "Jesse Brandeburg" <[EMAIL PROTECTED]> wrote:
> 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 You might want to put the tx values in a per-cpu structure and sum later. Incrementing statistics can actually be a performance bottleneck on SMP tests, because it causes lots of cache thrashing. -- Stephen Hemminger <[EMAIL PROTECTED]> - 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
