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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49EF566B.70203@cosmosbay.com>
Date:	Wed, 22 Apr 2009 19:39:55 +0200
From:	Eric Dumazet <dada1@...mosbay.com>
To:	Neil Horman <nhorman@...driver.com>
CC:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] snmp: add missing counters for RFC 4293

Neil Horman a écrit :
> On Wed, Apr 22, 2009 at 02:50:03AM -0700, David Miller wrote:
>> From: Eric Dumazet <dada1@...mosbay.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 :)

--
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