netdev - Re: [PATCH] r8169: Add values missing in @get

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: <20150818.190537.1494539251161022002.davem@davemloft.net>
Date:	Tue, 18 Aug 2015 19:05:37 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	romieu@...zoreil.com
Cc:	vinschen@...hat.com, netdev@...r.kernel.org, nic_swsd@...ltek.com,
	ivecera@...hat.com
Subject: Re: [PATCH] r8169: Add values missing in @get_stats64 from HW
 counters

From: Francois Romieu <romieu@...zoreil.com>
Date: Tue, 18 Aug 2015 23:40:17 +0200

> Corinna Vinschen <vinschen@...hat.com> :
>> The r8169 driver collects statistical information returned by
>> @get_stats64 by counting them in the driver itself, even though many
>> (but not all) of the values are already collected by tally counters
>> (TCs) in the NIC.  Some of these TC values are not returned by
>> @get_stats64.  Especially the received multicast packages are missing
>> from /proc/net/dev.
>> Rectify this by fetching the TCs and returning them from
>> rtl8169_get_stats64.
> 
> It is not exactly a some / many / not all / confused [*] picture
> - get_stats64 provides software managed values
> - ethtool provides hardware only values
> 
> [*] ok, rx_missed is kinda special.
> 
>> The counters collected in the driver obviously disappear as soon as the
>> driver is unloaded so after a driver is loaded the counters always start
>> at 0.  The TCs are only reset by a power cycle and there's no known way
>> to reset them programatically. Without further considerations the values
>> collected by the driver would not match up against the TC values.
> 
> I'd rather:
> 1. keep software and hardware stats separated
> 2. push to userspace the responsibility to offset device specific hardware
>    values jumps
> 
> My 0.02 €.

Any value that is provided by ->get_stats64() should be provided by whatever
mechanism is available and determined to be "prudent" for the device in
question.  If the device is not tracking the event, this means pure software
counters.

If the device tracks the event, you should attempt to use the hardware
facility to provide the ->get_stats64() values.

If the hardware provides counters not starting at zero on driver load,
one option to handle that is to load the initial values of all of
those counters and calculate diffs at ->get_stats64() time.

Otherwise, if you don't want to calculate diffs, you must use software
computed counters.

Ethtool stats are for values provided by the hardware which are outside of
the collection defined by ->get_stats64().  Some drivers also use it for
providing per-RX-queue and per-TX-queue statistics.

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