Date:	Thu, 23 Apr 2009 13:25:24 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Eric Dumazet <dada1@...mosbay.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] snmp: add missing counters for RFC 4293

On Thu, Apr 23, 2009 at 07:13:44PM +0200, Eric Dumazet wrote:
> Neil Horman a écrit :
> > On Thu, Apr 23, 2009 at 06:37:07PM +0200, Eric Dumazet wrote:
> >> Neil Horman a écrit :
> >>> On Wed, Apr 22, 2009 at 07:39:55PM +0200, Eric Dumazet wrote:
> >>>> <previous contents snipped>
> >>>>
> >>>
> >>> ok, new patch, functionally equivalent, with the following enhancements:
> >>>
> >>> 1) Replaces INRECEIVES/OUTREQUESTS with INPKTS/OUTPKTS.  I did this
> >>>    so that I could use the dual count update in (2) for INPKTS/INOCTETS
> >>>    and OUTPKTS/OUTOCTETS
> >>> 2) Added dual stat update routines for IP/IP6, named *_UPD_PO_STATS[_BH]
> >>>    which updates a pair of stats, a P(acket) count, and an (O)ctet count
> >>> 3) Added BCASTPKS/BCASTOCTETS for completeness
> >>>
> >>>
> >>>
> >>> The IP MIB (RFC 4293) defines stats for InOctets, OutOctets, InMcastOctets and
> >>> OutMcastOctets:
> >>> http://tools.ietf.org/html/rfc4293
> >>> But it seems we don't track those in any way that easy to separate from other
> >>> protocols.  This patch adds those missing counters to the stats file. Tested
> >>> successfully by me
> >>>
> >>> Signed-off-by: Neil Horman <nhorman@...driver.com>
> >>>
> >>
> >>
> >>> +#define SNMP_UPD_PO_STATS(mib, basefield, addend)	\
> >>> +	do { \
> >>> +		int __cpu = get_cpu(); \
> >>> +		per_cpu_ptr(mib[!in_softirq()], __cpu)->mibs[basefield##PKTS]++; \
> >>> +		per_cpu_ptr(mib[!in_softirq()], __cpu)->mibs[basefield##OCTETS] += addend;\
> >>> +		put_cpu(); \
> >>> +	} while (0)
> >> Following should be faster, because per_cpu_ptr() and !in_softirq() factorization.
> >>
> >>> +#define SNMP_UPD_PO_STATS(mib, basefield, addend)	\
> >>> +	do { \
> >>> +		__typeof__(mib[0]) ptr = per_cpu_ptr(mib[!in_softirq()], get_cpu()); \
> >>> +		ptr->mibs[basefield##PKTS]++; \
> >>> +		ptr->mibs[basefield##OCTETS] += addend;\
> >>> +		put_cpu(); \
> >>> +	} while (0)
> >> BTW, we miss HC (64bits) values on 32bit arches, and some RFC (not 4293, but in 4113) mention they
> >>  should be provided if counters can be updated more than 1 million times per second.
> >>
> >> And for Octets counters, this is definitly the case with 100 Mbit networks... Oh well
> > Yeah, I was wondering about this.  Think it would be worthwhile to export all of
> > these counters as 64 bit, rather than 32 bit?  Userspace can then have the HC
> > counters, and downcast to the standard 32 bit flavors.  I'd like this patch to
> > stand as it is, but I think that might be a good subsequent change to make
> 
> Well, maintaining 64bits counters on 32bit is going to be hard, since
> we dont want a reader to get mangled values while folding SNMP values 
> 
> Either we use same tricks than netfilter (so that updates can use regular ptr->field += bytes)
> and look the mess...
> 
> Either we use expensive locking...
> 
> At least, your SNMP_UPD_PO_STATS macros can be changed later if we really want
> HC values.
> 
Perhaps we could define the size of each counter at compile time, so that at
least 64 bit systems can have sane counters?

Btw, I will repost one more time to take in the above enhancement you suggested
regarding the limited use of per_cpu_ptr and in_softirq (my last post made it
sound like I wouldnt)

Neil

> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists