From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Greear Subject: Re: [RFC] ethtool: Support ETHTOOL_GSTATS2 command. Date: Tue, 20 Mar 2018 11:29:54 -0700 Message-ID: References: <1520452289-14172-1-git-send-email-greearb@candelatech.com> <20180320103747.7kcfdu4yzof6bwxw@unicorn.suse.cz> <37830d46-762b-2a92-4506-5792a65d2ebd@candelatech.com> <20180320182420.u5aa7eny3fbix7fj@unicorn.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Michal Kubecek Return-path: In-Reply-To: <20180320182420.u5aa7eny3fbix7fj-OEaqT8BN2ewCVLCxKZUutA@public.gmane.org> Sender: linux-wireless-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: netdev.vger.kernel.org On 03/20/2018 11:24 AM, Michal Kubecek wrote: > On Tue, Mar 20, 2018 at 08:39:33AM -0700, Ben Greear wrote: >> On 03/20/2018 03:37 AM, Michal Kubecek wrote: >>> >>> IMHO it would be more practical to set "0 means same as GSTATS" as a >>> rule and make ethtool_get_stats() a wrapper for ethtool_get_stats2() to >>> avoid code duplication (or perhaps a use fall-through in the switch). It >>> would also allow drivers to provide only one of the callbacks. >> >> Yes, but that would require changing all drivers at once, and would make backporting >> and out-of-tree drivers harder to manage. I had low hopes that this feature would >> make it upstream, so I didn't want to propose any large changes up front. > > I don't think so. What I mean is: > > (a) driver implements ->get_ethtool_stats2() callback; then we use it > for GSTATS2 > (b) driver does not implement get_ethtool_stats2() but implements > ->get_ethtool_stats(); then we call for GSTATS2 if level is zero, > otherwise GSTATS2 returns -EINVAL > > and GSTATS is always translated to GSTATS2 with level 0, either by > defining ethtool_get_stats() as a wrapper or by fall-through in the > switch statement. > > This way, most drivers could be left untouched and only those which > would implement non-default levels would provide ->get_ethtool_stats2() > callback instead of ->get_ethtool_stats(). OK, that makes sense. I'll wait on feedback from the flags or #defined levels and re-spin the patch accordingly. Thanks, Ben -- Ben Greear Candela Technologies Inc http://www.candelatech.com