Re: [PATCH] snmp: add missing counters for RFC 4293

[Eric Dumazet <dada1@cosmosbay.com>]

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@tuxdriver.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.