From owner-cvs-src@FreeBSD.ORG Wed Dec 13 02:05:47 2006 Return-Path: X-Original-To: cvs-src@FreeBSD.org Delivered-To: cvs-src@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 2D8D316A403; Wed, 13 Dec 2006 02:05:47 +0000 (UTC) (envelope-from oleg@lath.rinet.ru) Received: from lath.rinet.ru (lath.rinet.ru [195.54.192.90]) by mx1.FreeBSD.org (Postfix) with ESMTP id ADE8C43CAD; Wed, 13 Dec 2006 02:04:19 +0000 (GMT) (envelope-from oleg@lath.rinet.ru) Received: from lath.rinet.ru (localhost [127.0.0.1]) by lath.rinet.ru (8.13.8/8.13.8) with ESMTP id kBD25i74097559 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 13 Dec 2006 05:05:44 +0300 (MSK) (envelope-from oleg@lath.rinet.ru) Received: (from oleg@localhost) by lath.rinet.ru (8.13.8/8.13.8/Submit) id kBD25i0j097558; Wed, 13 Dec 2006 05:05:44 +0300 (MSK) (envelope-from oleg) Date: Wed, 13 Dec 2006 05:05:44 +0300 From: Oleg Bulyzhin To: Jung-uk Kim Message-ID: <20061213020544.GA97240@lath.rinet.ru> References: <200612010108.kB118qxY020349@repoman.freebsd.org> <200612121931.27763.jkim@FreeBSD.org> <20061213004456.GI91560@lath.rinet.ru> <200612122007.47566.jkim@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200612122007.47566.jkim@FreeBSD.org> User-Agent: Mutt/1.5.13 (2006-08-11) Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/bge if_bge.c if_bgereg.h X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Dec 2006 02:05:47 -0000 On Tue, Dec 12, 2006 at 08:07:46PM -0500, Jung-uk Kim wrote: > On Tuesday 12 December 2006 07:44 pm, Oleg Bulyzhin wrote: > > On Tue, Dec 12, 2006 at 07:31:24PM -0500, Jung-uk Kim wrote: > > > 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. > > > > In order to keep statistics across chip resets.( It's required for > > correct ipkt/ierror ratio. Imagine situation: i have bge0 with 1M > > input packets and 1M input errors, so packet loss i 50%. Then I do > > ifconfig bge0 down; iconfig bge0 up and input errors are gone!) > > ifp->if_ierrors does not reset between ifconfig down/up. > > > >As you > > > said, when the controller resets, stat registers are reset as > > > well. > > > > true. > > > > > Therefore, the old offsets must be reset as well. > > >Otherwise, you get bogus stats. > > > > This is not true, old code (prior to rev.1.153) was able to deal > > with it, until you 'simplified' it. > > Well, new code works for me on multiple chips/revisions. > > Jung-uk Kim I have to apologize for waisting your time. I was inattentive while reading code and i should confess i'm wrong and you are right. Thanks for your explanation! P.S. I should have go to bed earlier... -- Oleg. ================================================================ === Oleg Bulyzhin -- OBUL-RIPN -- OBUL-RIPE -- oleg@rinet.ru === ================================================================