netdev - Re: [patch net-next 1/4] netdevice: add SW statistics ndo

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]

Date:	Fri, 13 May 2016 11:47:48 -0700
From:	Roopa Prabhu <roopa@...ulusnetworks.com>
To:	Jiri Pirko <jiri@...nulli.us>
CC:	netdev@...r.kernel.org, davem@...emloft.net, nogahf@...lanox.com,
	idosch@...lanox.com, eladr@...lanox.com, yotamg@...lanox.com,
	ogerlitz@...lanox.com, nikolay@...ulusnetworks.com,
	linville@...driver.com, tgraf@...g.ch, gospo@...ulusnetworks.com,
	sfeldma@...il.com, sd@...asysnail.net, eranbe@...lanox.com,
	ast@...mgrid.com, edumazet@...gle.com, hannes@...essinduktion.org
Subject: Re: [patch net-next 1/4] netdevice: add SW statistics ndo

On 5/12/16, 11:03 PM, Jiri Pirko wrote:
> Thu, May 12, 2016 at 11:10:08PM CEST, roopa@...ulusnetworks.com wrote:
>> On 5/12/16, 4:48 AM, Jiri Pirko wrote:
>>> From: Nogah Frankel <nogahf@...lanox.com>
>>>
>>> Till now we had a ndo statistics function that returned SW statistics.
>>> We want to change the "basic" statistics to return HW statistics if
>>> available.
>>> In this case we need to expose a new ndo to return the SW statistics.
>>> Add a new ndo declaration to get SW statistics
>>> Add a function that gets SW statistics if a competible ndo exist
>>>
>>> Signed-off-by: Nogah Frankel <nogahf@...lanox.com>
>>> Reviewed-by: Ido Schimmel <idosch@...lanox.com>
>>> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
>>> ---
>>>
>> To me netdev stats is  combined 'SW + HW' stats for that netdev.
>> ndo_get_stats64 callback into the drivers does the magic of adding HW stats
>> to SW (netdev) stats and returning (see enic_get_stats). HW stats is available for netdevs
>> that are offloaded or are backed by hardware. SW stats is the stats that the driver maintains
>> (logical or physical). HW stats is queried and added to the SW stats.
> I'm not sure I follow. HW stats already contain SW stats. Because on
> slow path every packet that is not offloaded and goes through kernel is
> counted into HW stats as well (because it goes through HW port). 
yes, correct... we don't want to double count those. But since these stats are
generally queried from hw, I am calling them HW stats.
you will not really maintain a software counter for this. But, the driver can maintain its own
 counters for rx and tx errors etc and I call these SW stats. They are counted at the driver.

> If you
> do HW stats + SW stats, what you get makes no sense. Am I missing something?
If you go by my definition of HW and SW stats above, on a ndo_get_stats64() call,
you will add the SW counters + HW counters and return. In my definition, the pkts
that was rx'ed or tx'ed successfully are always in the HW count.

> Btw, looking at enic_get_stats, looks exactly what we introduce for
> mlxsw in this patchset.

In enic_get_stats, the ones counted in software are the ones taken from 'enic->'
     net_stats->rx_over_errors = enic->rq_truncated_pkts;
     net_stats->rx_crc_errors = enic->rq_bad_fcs;

>
> With this patchset, we only allow user to se the actual stats for
> slow-path aka SW stats.
hmm...ok. But i am not sure how many will use this new attribute.
When you do 'ip -s link show' you really want all counters on that port
hardware or software does not matter at that point.

My suggestion to move this to ethtool like attribute is because that is an existing
 way to break down your stats which ever way you want. And the best part is it can be
customized (say rx_pkts_cpu_saw)

Thanks,
Roopa