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 <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
