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(). Michal Kubecek