cvs commit: src/sys/dev/bge if_bge.c if_bgereg.h

[Jung-uk Kim]

On Tuesday 12 December 2006 06:44 pm, Oleg Bulyzhin wrote:
> On Tue, Dec 12, 2006 at 06:09:17PM -0500, Jung-uk Kim wrote:
> > On Tuesday 12 December 2006 05:05 pm, Oleg Bulyzhin wrote:
> > > On Fri, Dec 01, 2006 at 01:08:52AM +0000, Jung-uk Kim wrote:
> > > > jkim        2006-12-01 01:08:52 UTC
> > > >
> > > >   FreeBSD src repository
> > > >
> > > >   Modified files:
> > > >     sys/dev/bge          if_bge.c if_bgereg.h
> > > >   Log:
> > > >   Simplify statistics updates, remove redundant register
> > > > reads, and add discarded RX packets to input error for
> > > > BCM5705 or newer chipset as the others. Unfortunately we
> > > > cannot do the same for output errors because ifOutDiscards
> > > > equivalent register does not exist.  While I am here, replace
> > > > misleading and wrong BGE_RX_STATS/BGE_TX_STATS with
> > > > BGE_MAC_STATS.  They were reversed but worked accidently.
> > > >
> > > >   Revision  Changes    Path
> > > >   1.153     +15 -23    src/sys/dev/bge/if_bge.c
> > > >   1.58      +4 -5      src/sys/dev/bge/if_bgereg.h
> > >
> > > I would say you have simplified it too much. With your change
> > > you will get wrong numbers after ifconfig down/up (since it
> > > implies hardware counters reset while sc->bge_* counters are
> > > not cleared).
> >
> > I did clear sc->bge_* counter:
> >
> > @@ -3368,6 +3357,9 @@ bge_init_locked(struct bge_softc *sc)
> >
> >  	/* Init our RX return ring index. */
> >  	sc->bge_rx_saved_considx = 0;
> > +
> > +	/* Init our RX/TX stat counters. */
> > +	sc->bge_rx_discards = sc->bge_tx_discards =
> > sc->bge_tx_collisions = 0;
> >
> >  	/* Init TX ring. */
> >  	bge_init_tx_ring(sc);
> >
> > While ifconfig down/up, bge_init_locked() should be called.
> > Did I miss something?
>
> Oh, i didnt noticed that, but this makes your change even worse:
> you will loose statistic counters on every interface reset.

I don't understand why you have to keep the old counters.  As you 
said, when the controller resets, stat registers are reset as well.  
Therefore, the old offsets must be reset as well.  Otherwise, you get 
bogus stats.

Jung-uk Kim