Date: Wed, 13 Dec 2006 13:28:06 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Oleg Bulyzhin <oleg@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org, Jung-uk Kim <jkim@freebsd.org> Subject: Re: cvs commit: src/sys/dev/bge if_bge.c if_bgereg.h Message-ID: <20061213131442.O833@delplex.bde.org> In-Reply-To: <20061212234424.GG91560@lath.rinet.ru> References: <200612010108.kB118qxY020349@repoman.freebsd.org> <20061212220557.GC91560@lath.rinet.ru> <200612121809.19564.jkim@FreeBSD.org> <20061212234424.GG91560@lath.rinet.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 13 Dec 2006, 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: >>> 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. These counters only record the previous value, for taking differences later. They must be reset to whatever the the harware resets them to. I think this is 0. To be completely correct, the differences should be added on init/reset (just before resetting the hardware). This might be easy to do by calling the stats update function at the start of reset. I think a near equivalent of this happens accidentally for watchdog resets (only), since watchdog timeouts are now synced with bge_tick() and bge_tick() will have updated the stats just before calling the watchdog. Then after resetting the hardware, it's better to depend on the hardware resetting the counters to 0 and not initialize them to the hardware values here, to avoid any losing differences on finishing reset. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061213131442.O833>