Re: [PATCH] e1000: Real time packets and bytes statistics

[Jean Delvare <khali@linux-fr.org>]

Hi Stephen,

On 10/11/06, Stephen Hemminger wrote:
> On Wed, 11 Oct 2006, Jesse Brandeburg wrote:
> > On 10/11/06, Jean Delvare wrote:
> > > 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.
> > > (...)
> > > --- 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.
> 
> 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.

I don't really see how this would be implemented. Can you please point
me to other drivers which do it that way?

Thanks,
-- 
Jean Delvare