netdev - Re: [PATCH 1/3] ethtool: Support ETHTOOL

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: <1524119910.3024.12.camel@sipsolutions.net>
Date:   Thu, 19 Apr 2018 08:38:30 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Ben Greear <greearb@...delatech.com>, netdev@...r.kernel.org
Cc:     linux-wireless@...r.kernel.org, ath10k@...ts.infradead.org
Subject: Re: [PATCH 1/3] ethtool: Support ETHTOOL_GSTATS2 command.

On Wed, 2018-04-18 at 14:51 -0700, Ben Greear wrote:

> > It'd be pretty hard to know which flags are firmware stats?
> 
> Yes, it is, but ethtool stats are difficult to understand in a generic
> manner anyway, so someone using them is already likely aware of low-level
> details of the driver(s) they are using.

Right. Come to think of it though,

> + * @get_ethtool_stats2: Return extended statistics about the device.
> + *     This is only useful if the device maintains statistics not
> + *     included in &struct rtnl_link_stats64.
> + *      Takes a flags argument:  0 means all (same as get_ethtool_stats),
> + *      0x1 (ETHTOOL_GS2_SKIP_FW) means skip firmware stats.
> + *      Other flags are reserved for now.
> + *      Same number of stats will be returned, but some of them might
> + *      not be as accurate/refreshed.  This is to allow not querying
> + *      firmware or other expensive-to-read stats, for instance.

"skip" vs. "don't refresh" is a bit ambiguous - I'd argue better to
either really skip and not return the non-refreshed ones (also helps
with the identifying), or rename the flag.

Also, wrt. the rest of the patch, I'd argue that it'd be worthwhile to
write the spatch and just add the flags argument to "get_ethtool_stats"
instead of adding a separate method - internally to the kernel it's not
that hard to change.

> I posted the patches to netdev, ath10k and linux-wireless.  If I had only
> posted them individually to different lists I figure I'd be hearing about how
> the netdev patch is useless because it has no driver support, etc.

Sure. I missed netdev, perhaps because it was in To, or more likely
because I was too sleepy. Sorry for the noise.

johannes