From: Johannes Berg <johannes@sipsolutions.net> To: Ben Greear <greearb@candelatech.com>, netdev@vger.kernel.org Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org Subject: Re: [PATCH 1/3] ethtool: Support ETHTOOL_GSTATS2 command. Date: Thu, 19 Apr 2018 08:38:30 +0200 [thread overview] Message-ID: <1524119910.3024.12.camel@sipsolutions.net> (raw) In-Reply-To: <f43d7ee4-7f27-b145-125e-32dbc31a27f1@candelatech.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Johannes Berg <johannes@sipsolutions.net> To: Ben Greear <greearb@candelatech.com>, netdev@vger.kernel.org Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org Subject: Re: [PATCH 1/3] ethtool: Support ETHTOOL_GSTATS2 command. Date: Thu, 19 Apr 2018 08:38:30 +0200 [thread overview] Message-ID: <1524119910.3024.12.camel@sipsolutions.net> (raw) In-Reply-To: <f43d7ee4-7f27-b145-125e-32dbc31a27f1@candelatech.com> 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 _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2018-04-19 6:38 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-18 1:49 [PATCH 1/3] ethtool: Support ETHTOOL_GSTATS2 command greearb 2018-04-18 1:49 ` greearb 2018-04-18 1:49 ` greearb-my8/4N5VtI7c+919tysfdA 2018-04-18 1:49 ` [PATCH 2/3] mac80211: Add support for ethtool gstats2 API greearb 2018-04-18 1:49 ` greearb 2018-04-18 1:49 ` [PATCH 3/3] ath10k: Support " greearb 2018-04-18 1:49 ` greearb 2018-04-19 11:19 ` kbuild test robot 2018-04-19 11:19 ` kbuild test robot 2018-04-18 21:07 ` [PATCH 1/3] ethtool: Support ETHTOOL_GSTATS2 command Florian Fainelli 2018-04-18 21:07 ` Florian Fainelli 2018-04-18 21:26 ` Johannes Berg 2018-04-18 21:26 ` Johannes Berg 2018-04-18 21:51 ` Ben Greear 2018-04-18 21:51 ` Ben Greear 2018-04-19 6:38 ` Johannes Berg [this message] 2018-04-19 6:38 ` Johannes Berg 2018-04-19 15:25 ` Ben Greear 2018-04-19 15:25 ` Ben Greear 2018-04-19 15:26 ` Johannes Berg 2018-04-19 15:26 ` Johannes Berg 2018-04-19 15:26 ` Johannes Berg 2018-04-22 18:54 ` David Miller 2018-04-22 18:54 ` David Miller 2018-04-22 18:54 ` David Miller 2018-04-22 21:15 ` Roopa Prabhu 2018-04-22 21:15 ` Roopa Prabhu 2018-04-22 21:15 ` Roopa Prabhu 2018-04-23 15:41 ` Ben Greear 2018-04-23 15:41 ` Ben Greear 2018-04-23 15:38 ` Ben Greear 2018-04-23 15:38 ` Ben Greear 2018-04-23 15:38 ` Ben Greear
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1524119910.3024.12.camel@sipsolutions.net \ --to=johannes@sipsolutions.net \ --cc=ath10k@lists.infradead.org \ --cc=greearb@candelatech.com \ --cc=linux-wireless@vger.kernel.org \ --cc=netdev@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.