All of lore.kernel.org
 help / color / mirror / Atom feed
* Please revert removal of /sys/class/net/*/features
@ 2012-04-10 18:08 Stephen Hemminger
  2012-04-10 18:27 ` David Miller
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Stephen Hemminger @ 2012-04-10 18:08 UTC (permalink / raw)
  To: David Miller; +Cc: Michał Mirosław, netdev

This commit needs to be reverted. It removed an available sysfs file, and sysfs
files are part of the ABI. It caused a bug in our current release of Vyatta because
the shell script was using the sysfs file to see if VLAN was supported on a device.

An API maybe redundant, but you can't just remove it.


commit 974151e6119f20d2af4acb97526c780ae0f18ccb
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date:   Thu Jul 14 14:45:15 2011 -0700

    net: remove /sys/class/net/*/features
    
    The same information and more can be obtained by using ethtool
    with ETHTOOL_GFEATURES.
    
    Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
    Signed-off-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Please revert removal of /sys/class/net/*/features
  2012-04-10 18:08 Please revert removal of /sys/class/net/*/features Stephen Hemminger
@ 2012-04-10 18:27 ` David Miller
  2012-04-10 18:32 ` Ben Greear
  2012-04-10 19:59 ` Michał Mirosław
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-04-10 18:27 UTC (permalink / raw)
  To: shemminger; +Cc: mirq-linux, netdev

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 10 Apr 2012 11:08:51 -0700

> This commit needs to be reverted. It removed an available sysfs file, and sysfs
> files are part of the ABI. It caused a bug in our current release of Vyatta because
> the shell script was using the sysfs file to see if VLAN was supported on a device.
> 
> An API maybe redundant, but you can't just remove it.

I think it's more about representability with an expanded
netdev_features_t type.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Please revert removal of /sys/class/net/*/features
  2012-04-10 18:08 Please revert removal of /sys/class/net/*/features Stephen Hemminger
  2012-04-10 18:27 ` David Miller
@ 2012-04-10 18:32 ` Ben Greear
  2012-04-10 19:59 ` Michał Mirosław
  2 siblings, 0 replies; 6+ messages in thread
From: Ben Greear @ 2012-04-10 18:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Michał Mirosław, netdev

On 04/10/2012 11:08 AM, Stephen Hemminger wrote:
> This commit needs to be reverted. It removed an available sysfs file, and sysfs
> files are part of the ABI. It caused a bug in our current release of Vyatta because
> the shell script was using the sysfs file to see if VLAN was supported on a device.

It's a hack, but you can also make some vlan IOCTL calls to a device
to probe if it's VLAN or not..thats what I've been doing for the last
decade or so :)

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Please revert removal of /sys/class/net/*/features
  2012-04-10 18:08 Please revert removal of /sys/class/net/*/features Stephen Hemminger
  2012-04-10 18:27 ` David Miller
  2012-04-10 18:32 ` Ben Greear
@ 2012-04-10 19:59 ` Michał Mirosław
  2012-04-10 20:25   ` Ben Hutchings
  2 siblings, 1 reply; 6+ messages in thread
From: Michał Mirosław @ 2012-04-10 19:59 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev

On Tue, Apr 10, 2012 at 11:08:51AM -0700, Stephen Hemminger wrote:
> This commit needs to be reverted. It removed an available sysfs file, and sysfs
> files are part of the ABI. It caused a bug in our current release of Vyatta because
> the shell script was using the sysfs file to see if VLAN was supported on a device.
> 
> An API maybe redundant, but you can't just remove it.

There was a discussion about it before the patch was accepted. It was
concluded that due to features flags changes, the /sys/class/net/*/features
was never a stable API.

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Please revert removal of /sys/class/net/*/features
  2012-04-10 19:59 ` Michał Mirosław
@ 2012-04-10 20:25   ` Ben Hutchings
  2012-04-10 20:46     ` Stephen Hemminger
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2012-04-10 20:25 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: Stephen Hemminger, David Miller, netdev

On Tue, 2012-04-10 at 21:59 +0200, Michał Mirosław wrote:
> On Tue, Apr 10, 2012 at 11:08:51AM -0700, Stephen Hemminger wrote:
> > This commit needs to be reverted. It removed an available sysfs file, and sysfs
> > files are part of the ABI. It caused a bug in our current release of Vyatta because
> > the shell script was using the sysfs file to see if VLAN was supported on a device.
> > 
> > An API maybe redundant, but you can't just remove it.
> 
> There was a discussion about it before the patch was accepted. It was
> concluded that due to features flags changes, the /sys/class/net/*/features
> was never a stable API.

For reference:
http://thread.gmane.org/gmane.linux.network/199115/focus=199136

But note that the VLAN-related feature flags never actually changed
value.

Ben.

-- 
Ben Hutchings, Staff 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Please revert removal of /sys/class/net/*/features
  2012-04-10 20:25   ` Ben Hutchings
@ 2012-04-10 20:46     ` Stephen Hemminger
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2012-04-10 20:46 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Michał Mirosław, David Miller, netdev

On Tue, 10 Apr 2012 21:25:47 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Tue, 2012-04-10 at 21:59 +0200, Michał Mirosław wrote:
> > On Tue, Apr 10, 2012 at 11:08:51AM -0700, Stephen Hemminger wrote:
> > > This commit needs to be reverted. It removed an available sysfs file, and sysfs
> > > files are part of the ABI. It caused a bug in our current release of Vyatta because
> > > the shell script was using the sysfs file to see if VLAN was supported on a device.
> > > 
> > > An API maybe redundant, but you can't just remove it.
> > 
> > There was a discussion about it before the patch was accepted. It was
> > concluded that due to features flags changes, the /sys/class/net/*/features
> > was never a stable API.
> 
> For reference:
> http://thread.gmane.org/gmane.linux.network/199115/focus=199136
> 
> But note that the VLAN-related feature flags never actually changed
> value.
> 
> Ben.
> 

It is actually not a big deal for our product since 3.3 is
really early in development cycle and the script can be fixed in
that release. Also, we release an image with utilities, scripts and
kernel in one release, and don't have to support running new kernels with
older userspace.

I am more worried that other people might be hit 
the same thing. What about a patch to keep the old flags values
available for backwards compatiablity. Also, when removing a sysfs
file, shouldn't the documented feature removal process be used?

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-04-10 20:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-10 18:08 Please revert removal of /sys/class/net/*/features Stephen Hemminger
2012-04-10 18:27 ` David Miller
2012-04-10 18:32 ` Ben Greear
2012-04-10 19:59 ` Michał Mirosław
2012-04-10 20:25   ` Ben Hutchings
2012-04-10 20:46     ` Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.