[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110516121339.GA1094@rere.qmqm.pl>
Date: Mon, 16 May 2011 14:13:39 +0200
From: Michał Mirosław <mirq-linux@...e.qmqm.pl>
To: Ben Hutchings <bhutchings@...arflare.com>
Cc: David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net-2.6] ethtool: Remove fallback to old ethtool
operations for ETHTOOL_SFEATURES
On Mon, May 16, 2011 at 03:45:23AM +0100, Ben Hutchings wrote:
> On Sat, 2011-05-14 at 12:35 +0200, Michał Mirosław wrote:
> > On Sat, May 14, 2011 at 02:05:42AM +0100, Ben Hutchings wrote:
> > > ethtool_set_feature_compat() squashes the feature mask into a boolean,
> > > which is not correct for ethtool_ops::set_flags.
> > >
> > > We could fix this, but the fallback code for ETHTOOL_SFEATURES actually
> > > makes things more complicated for the ethtool utility and any other
> > > application using the ethtool API. They will still need to fall back to
> > > the old offload control commands in order to support older kernel
> > > versions. The fallback code in the kernel adds a third possibility for
> > > them to handle. So make ETHTOOL_SFEATURES fail when the driver
> > > implements the old offload control operations, and let userland do the
> > > fallback.
> > BTW, the idea behind the compat code is that if ETHTOOL_[GS]FEATURES is
> > available, then there should be no need to fallback to old ops. For
> > a userspace tool that targets only kernels >= 2.6.39 there's no need
> > to care about old ops at all.
> Well that's not true, because those tools still have to deal with
> ETHTOOL_F_COMPAT. And we're supposed to have all drivers converted for
> 2.6.40, so the hypothetical new tool only has to wait one more release.
I think that handling ETHTOOL_F_COMPAT is like ETHTOOL_F_WISH - the tool
needs to reread features and check what was changed. So we can get rid
of the former by replacing it with the latter.
Changing some feature state may change others that were not requested
to be changed (not present in .valid). This probably should be signalled
as well. Extend ETHTOOL_F_WISH meaning (I prefer this one) or introduce
new bit?
Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists