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

[Eric Dumazet <dada1@cosmosbay.com>]

Neil Horman a écrit :
> On Wed, Apr 22, 2009 at 02:50:03AM -0700, David Miller wrote:
>> From: Eric Dumazet <dada1@cosmosbay.com>
>> Date: Wed, 22 Apr 2009 11:35:45 +0200
>>
>>> So RFC4293 tells corresponding Octets variables should be supported :
>>>
>>>  InOctets, InMcastOctets, OutMcastOctets, OutBcastOctets, OutOctets
>>>
>>> And I dont see them in /proc/net/snmp or /proc/net/netstat
>>>
>>> Mitsuru added :
>>>
>>>        SNMP_MIB_ITEM("InNoRoutes", IPSTATS_MIB_INNOROUTES),
>>>        SNMP_MIB_ITEM("InTruncatedPkts", IPSTATS_MIB_INTRUNCATEDPKTS),
>>>        SNMP_MIB_ITEM("InMcastPkts", IPSTATS_MIB_INMCASTPKTS),
>>>        SNMP_MIB_ITEM("OutMcastPkts", IPSTATS_MIB_OUTMCASTPKTS),
>>>        SNMP_MIB_ITEM("InBcastPkts", IPSTATS_MIB_INBCASTPKTS),
>>>        SNMP_MIB_ITEM("OutBcastPkts", IPSTATS_MIB_OUTBCASTPKTS),
>>>
>>> And Neil adds : InOctets, OutOctets, InMcastOctets and OutMcastOctets
>>>
>>> Neil, you forgot OutBcastOctets :)
>> Fair enough.  Neil I wait for an updated patch :-)
>>
> 
> As promised, new patch, tested and ready to go.  This variant puts all the stats
> at the end of the IpExt stats line in /proc/net/snmp, and at the end of
> /proc/net/dev_snmp6/<iface>, so it shouldn't break any readers of those files.
> 
> Also, Eric, I know you mentioned the need for OutBcastOctets, and intuitively I
> thought it should be there as well, but looking at RFC 4293, [In|Out]BcastOctets
> isn't defined.  I started to wonder if perhaps the RFC assumed that McastOctets
> was assumed to include broadcast frames (i.e. all hosts multicast), but I
> decided that the smartest thing to do was correlate McastPkts and McastOctets.
> If someone wants to make an argument for including broadcast explicitly in both
> counters, then we can deal with that in a subsequent patch.

Yes, I found it on page 7, on diagram.

OutBcastPkts (1)  

Legend (1) is : (1) The HC counters and octet counters are also found at these points
       but have been left out for clarity.

So apparently RFC forgot OutBcastOctets/InBcastOctets somewhere, oh well...

We actually have 

IpExt: InNoRoutes InTruncatedPkts InMcastPkts OutMcastPkts InBcastPkts OutBcastPkts

It would make sense to add : InOctets OutOctets InMcastOctets OutMcastOctets InBcastOctets OutBcastOctets
Even if not fully tagged in RFC...


You could actually use new macros, to update both packet count and bytes count at once.
(less calls to get_cpu()/put_cpu(), and per_cpu_ptr(), thats pretty expensive if CONFIG_PREEMPT


#define SNMP_ADD_PB_STATS(mib, bytesfield, bytes, packetsfield) 	\
	do { 	__typeof__(mib) *ptr = per_cpu_ptr(mib[!in_softirq()], get_cpu()); \
		ptr->mibs[bytesfield] += bytes; \
		ptr->mibs[packetsfield]++; \
		put_cpu(); \
	} while (0)

#define SNMP_ADD_PB_STATS_BH(mib, bytesfield, bytes, packetsfield) 	\
	do { 	__typeof__(mib) *ptr = per_cpu_ptr(mib[0], get_cpu()); \
		ptr->mibs[bytesfield] += bytes; \
		ptr->mibs[packetsfield]++; \
		put_cpu(); \
	} while (0)


PB as Packet & Bytes, maybe name is wrong :)