Received: with ECARTIS (v1.0.0; list netdev); Tue, 28 Sep 2004 19:22:34 -0700 (PDT) Received: from mx02.cybersurf.com (mx02.cybersurf.com [209.197.145.105]) by oss.sgi.com (8.13.0/8.13.0) with ESMTP id i8T2MTMW012178 for ; Tue, 28 Sep 2004 19:22:29 -0700 Received: from mail.cyberus.ca ([209.197.145.21]) by mx02.cybersurf.com with esmtp (Exim 4.30) id 1CCU6j-0000qr-DT for netdev@oss.sgi.com; Tue, 28 Sep 2004 22:22:17 -0400 Received: from cpe0030ab124d2f-cm014500000962.cpe.net.cable.rogers.com ([24.103.99.32] helo=[10.0.0.9]) by mail.cyberus.ca with esmtp (Exim 4.20) id 1CCU6d-0008E7-Tb; Tue, 28 Sep 2004 22:22:12 -0400 Subject: Re: [PATCH 2.6] generic network statistics (was Re: [6/6]: jenkins hash for neigh / Statistics) From: jamal Reply-To: hadi@cyberus.ca To: Thomas Graf Cc: Harald Welte , Robert Olsson , Stephen Hemminger , "David S. Miller" , herbert@gondor.apana.org.au, netdev@oss.sgi.com In-Reply-To: <20040928133334.GW31616@rei.reeler.org> References: <20040925005623.2faf8faf.davem@davemloft.net> <20040927121403.767e2308.davem@davemloft.net> <20040927222613.GE3236@sunbeam.de.gnumonks.org> <20040927160636.7741d973.davem@davemloft.net> <1096327658.1729.19.camel@localhost.localdomain> <16729.9326.93269.422940@robur.slu.se> <20040928111906.GB29961@sunbeam.de.gnumonks.org> <1096375700.8659.235.camel@jzny.localdomain> <20040928133334.GW31616@rei.reeler.org> Content-Type: text/plain Organization: jamalopolous Message-Id: <1096424527.1044.94.camel@jzny.localdomain> Mime-Version: 1.0 X-Mailer: Ximian Evolution 1.2.2 Date: 28 Sep 2004 22:22:07 -0400 Content-Transfer-Encoding: 7bit X-archive-position: 9640 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: hadi@cyberus.ca Precedence: bulk X-list: netdev Content-Length: 1187 Lines: 32 On Tue, 2004-09-28 at 09:33, Thomas Graf wrote: > > Speaking of generic stats; i have a patch netlink ready which may need > > some extensions. I did post it a while back on netdev but didnt get > > feedback. > > The code looks good and I couldn't spot any errors but I'm not > sure if the locking in gen_copy_[x]stats is a good thing. > Shouldn't that be done earlier by the caller? In the netsched code that became a portability issue; Dave fixed it there, so i just replicated here. If you feel like doing something clever you are welcome to submit a patch. > This prevents > corruption but it allows duplicated TLVs in an skb. I suggest > to make the caller have a lock on his data and only allow one > dumper at the same time until the dump is complete, or at least > provide a lockless variant for callers doing the locking on > their own. > Reminds me: gnet_stats needs to have TLVs embedded in it. bytes,drops, packets are generic enough; others are not. So if we add a length field then we can add TLVs for things like QSTATS = { qlen, backlog} etc. This means we could then allow for adding a lot of different stats. A big lesson from current tc_stats. cheers, jamal