Re: [PATCH net-2.6] ethtool: Remove fallback to old ethtool operations for ETHTOOL_SFEATURES

[Ben Hutchings <ben@decadent.org.uk>]

[Sending this from my home address as solarflare.com mail is under
maintenance and SPF prevents me using that address entirely.]

On Sat, 2011-05-14 at 11:54 +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.
> > 
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> 
> This will disable SFEATURES for drivers which implement changing newer
> features that have no old ethtool ops (e.g.  NETIF_F_LOOPBACK), but are
> not converted, yet. This might matter when bisecting.
> 
> It's easy to fix this.

Yes, I realise that.

> The code is going away for 2.6.40, though.
> Do you want to get rid of ETHTOOL_F_COMPAT bit before 2.6.39?

Right, I don't want this to ever be in a stable release.

> BTW, what are the complications for userspace?

With 2.6.38 and earlier, ETHTOOL_SFEATURES will fail; ethtool -K will
fall back to the individual operations can can report which of them
failed and why.

With 2.6.39 and a converted driver, ethtool -K can find out exactly
which features are changeable and report whether any of the individual
changes are not supported at all.  If there's something wrong with the
combination of features then ETHTOOL_SFEATURES will return
ETHTOOL_F_WISH and it can report that the combination is not supported.

With 2.6.39 and an unconverted driver, ETHTOOL_SFEATURES can return
ETHTOOL_F_COMPAT which just tells us some change failed, but not why.
We can try to guess what went wrong by reading back the features, but
it's unclear.  This is particularly problematic with set_flags (again)
where the ethtool core can't tell which flags are settable in an
unconverted driver.

I would much prefer not to have this third case existing in a single
stable release.

> This change misses ethtool_get_features_compat() setting available bits.

Yes, but that is less of a problem in practice.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.