Re: 22bb1be4d27... breaks hal (and NM)

Re: 22bb1be4d27... breaks hal (and NM)

[Robert Hancock]

Andrew Morton wrote:
> : +config WIRELESS_EXT_SYSFS
> : +	bool "Wireless extensions sysfs files"
> : +	default y
> : +	depends on WIRELESS_EXT && SYSFS
> : +	help
> : +	  This option enables the deprecated wireless statistics
> : +	  files in /sys/class/net/*/wireless/. The same information
> : +	  is available via the ioctls as well.
> : +
> : +	  Say Y if you have programs using it (we don't know of any).
> : 
> 
> So if you've enabled CONFIG_WIRELESS_EXT_SYSFS (and it correctly has
> default y) then things should continue to work OK.  And the deprecation
> date of 2010 sounds reasonable.  In fact generous, for us...
> 
> So where's the problem?

The Kconfig message implies that nobody uses the files. It needs to be 
updated to state that older versions of HAL do use them (and ideally 
what version this was changed).

Re: 22bb1be4d27... breaks hal (and NM)

[Alessandro Suardi]

On Thu, Jul 24, 2008 at 10:13 PM, Robert Hancock <hancockr@shaw.ca> wrote:
> Andrew Morton wrote:
>>
>> : +config WIRELESS_EXT_SYSFS
>> : +     bool "Wireless extensions sysfs files"
>> : +     default y
>> : +     depends on WIRELESS_EXT && SYSFS
>> : +     help
>> : +       This option enables the deprecated wireless statistics
>> : +       files in /sys/class/net/*/wireless/. The same information
>> : +       is available via the ioctls as well.
>> : +
>> : +       Say Y if you have programs using it (we don't know of any).
>> :
>> So if you've enabled CONFIG_WIRELESS_EXT_SYSFS (and it correctly has
>> default y) then things should continue to work OK.  And the deprecation
>> date of 2010 sounds reasonable.  In fact generous, for us...
>>
>> So where's the problem?
>
> The Kconfig message implies that nobody uses the files. It needs to be
> updated to state that older versions of HAL do use them (and ideally what
> version this was changed).

I was also lured by the Kconfig text, so I changed the default, but
 have noticed no breakage so far.

[asuardi@sandman linux-2.6.26-git11]$ grep WIRELESS_EXT .config
CONFIG_WIRELESS_EXT=y
# CONFIG_WIRELESS_EXT_SYSFS is not set
[asuardi@sandman linux-2.6.26-git11]$ rpm -q hal
hal-0.5.11-2.fc9.i386

Is it because Fedora 9 hal is new enough or because I didn't do anything
 to trigger the breakage ? Just curious.

--alessandro

 "Give me love / Or give me hate
 Give me anything that's not just ok"

 (Sophia, 'Weightless')

22bb1be4d27... breaks hal (and NM)

[Pierre Ossman]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

From commit 22bb1be4d27...:

"The /sys/class/net/*/wireless/ direcory is, as far as I know, not used
by anyone."

Can't have been much of an investigation as HAL uses these to determine
if a card is a wireless one. Without this option, you cannot get
wireless support in NetworkManager, stranding most laptops.

Change the default and mark it for future deprecation if you want to
get rid of it.

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: 22bb1be4d27... breaks hal (and NM)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

On Thu, 2008-07-24 at 11:58 +0200, Pierre Ossman wrote:
> From commit 22bb1be4d27...:
> 
> "The /sys/class/net/*/wireless/ direcory is, as far as I know, not used
> by anyone."
> 
> Can't have been much of an investigation as HAL uses these to determine
> if a card is a wireless one. Without this option, you cannot get
> wireless support in NetworkManager, stranding most laptops.

Bzzzzt. You suck. Go check again. Hint: try hal's git tree.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: 22bb1be4d27... breaks hal (and NM)

[Peter Zijlstra]

On Thu, 2008-07-24 at 13:38 +0200, Johannes Berg wrote:
> On Thu, 2008-07-24 at 11:58 +0200, Pierre Ossman wrote:
> > From commit 22bb1be4d27...:
> > 
> > "The /sys/class/net/*/wireless/ direcory is, as far as I know, not used
> > by anyone."
> > 
> > Can't have been much of an investigation as HAL uses these to determine
> > if a card is a wireless one. Without this option, you cannot get
> > wireless support in NetworkManager, stranding most laptops.
> 
> Bzzzzt. You suck. Go check again. Hint: try hal's git tree.

So you're not considering HAL as shipped by most (if not all) distros as
anyone?

If everybody would act like that, the next kernel wouldn't even manage
to get rudimentary user-space up and running. Like we already changed
all those systemcalls in glibc-head, who needs this kernel to boot on
your old stuff anyway..

Common,.. this is rediculous.

Re: 22bb1be4d27... breaks hal (and NM)

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On Thu, 2008-07-24 at 13:57 +0200, Peter Zijlstra wrote:

> So you're not considering HAL as shipped by most (if not all) distros as
> anyone?
> 
> If everybody would act like that, the next kernel wouldn't even manage
> to get rudimentary user-space up and running. Like we already changed
> all those systemcalls in glibc-head, who needs this kernel to boot on
> your old stuff anyway..
> 
> Common,.. this is rediculous.

What the hell is wrong with you guys? I used to think it's accepted
practice to introduce a config option you can change away from the
default if you already know you are using new software that doesn't need
it, and if you are clueless just don't fucking touch the default until I
decide it's time to flip the default/remove the option because distros
are shipping new versions of the dependent software.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: 22bb1be4d27... breaks hal (and NM)

[Andrew Morton]

On Thu, 24 Jul 2008 13:57:11 +0200 Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, 2008-07-24 at 13:38 +0200, Johannes Berg wrote:
> > On Thu, 2008-07-24 at 11:58 +0200, Pierre Ossman wrote:
> > > From commit 22bb1be4d27...:
> > > 
> > > "The /sys/class/net/*/wireless/ direcory is, as far as I know, not used
> > > by anyone."
> > > 
> > > Can't have been much of an investigation as HAL uses these to determine
> > > if a card is a wireless one. Without this option, you cannot get
> > > wireless support in NetworkManager, stranding most laptops.
> > 
> > Bzzzzt. You suck. Go check again. Hint: try hal's git tree.

err,

a) don't be rude

b) what's in various git trees is unuseful for making decisions
   about production kernel features.

otoh the patch seems reasonable.

> So you're not considering HAL as shipped by most (if not all) distros as
> anyone?
>
> If everybody would act like that, the next kernel wouldn't even manage
> to get rudimentary user-space up and running. Like we already changed
> all those systemcalls in glibc-head, who needs this kernel to boot on
> your old stuff anyway..
> 
> Common,.. this is rediculous.

I think you'd have needed to try pretty hard to let this break stuff.

: commit 22bb1be4d271961846cd0889b0f8d671db773080
: Author: Johannes Berg <johannes@sipsolutions.net>
: Date:   Thu Jul 10 11:16:47 2008 +0200
: 
:     wext: make sysfs bits optional and deprecate them
:     
:     The /sys/class/net/*/wireless/ direcory is, as far as I know, not
:     used by anyone. Additionally, the same data is available via wext
:     ioctls. Hence the sysfs files are pretty much useless. This patch
:     makes them optional and schedules them for removal.
:     
:     Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
:     Cc: Jean Tourrilhes <jt@hpl.hp.com>
:     Signed-off-by: John W. Linville <linville@tuxdriver.com>
: 
: diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
: index 8319c46..db300e0 100644
: --- a/Documentation/feature-removal-schedule.txt
: +++ b/Documentation/feature-removal-schedule.txt
: @@ -333,3 +333,13 @@ Why:	This option was introduced just to allow older lm-sensors userspace
:  	to keep working over the upgrade to 2.6.26. At the scheduled time of
:  	removal fixed lm-sensors (2.x or 3.x) should be readily available.
:  Who:	Rene Herman <rene.herman@gmail.com>
: +
: +---------------------------
: +
: +What:	Code that is now under CONFIG_WIRELESS_EXT_SYSFS
: +	(in net/core/net-sysfs.c)
: +When:	After the only user (hal) has seen a release with the patches
: +	for enough time, probably some time in 2010.
: +Why:	Over 1K .text/.data size reduction, data is available in other
: +	ways (ioctls)
: +Who:	Johannes Berg <johannes@sipsolutions.net>
: diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
: index 3f79413..c1f4e0d 100644
: --- a/net/core/net-sysfs.c
: +++ b/net/core/net-sysfs.c
: @@ -318,7 +318,7 @@ static struct attribute_group netstat_group = {
:  	.attrs  = netstat_attrs,
:  };
:  
: -#ifdef CONFIG_WIRELESS_EXT
: +#ifdef CONFIG_WIRELESS_EXT_SYSFS
:  /* helper function that does all the locking etc for wireless stats */
:  static ssize_t wireless_show(struct device *d, char *buf,
:  			     ssize_t (*format)(const struct iw_statistics *,
: @@ -459,7 +459,7 @@ int netdev_register_kobject(struct net_device *net)
:  #ifdef CONFIG_SYSFS
:  	*groups++ = &netstat_group;
:  
: -#ifdef CONFIG_WIRELESS_EXT
: +#ifdef CONFIG_WIRELESS_EXT_SYSFS
:  	if (net->wireless_handlers && net->wireless_handlers->get_wireless_stats)
:  		*groups++ = &wireless_group;
:  #endif
: diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
: index 7927090..ab015c6 100644
: --- a/net/wireless/Kconfig
: +++ b/net/wireless/Kconfig
: @@ -29,3 +29,14 @@ config WIRELESS_EXT
:  
:  	  Say N (if you can) unless you know you need wireless
:  	  extensions for external modules.
: +
: +config WIRELESS_EXT_SYSFS
: +	bool "Wireless extensions sysfs files"
: +	default y
: +	depends on WIRELESS_EXT && SYSFS
: +	help
: +	  This option enables the deprecated wireless statistics
: +	  files in /sys/class/net/*/wireless/. The same information
: +	  is available via the ioctls as well.
: +
: +	  Say Y if you have programs using it (we don't know of any).
: 

So if you've enabled CONFIG_WIRELESS_EXT_SYSFS (and it correctly has
default y) then things should continue to work OK.  And the deprecation
date of 2010 sounds reasonable.  In fact generous, for us...

So where's the problem?

Re: 22bb1be4d27... breaks hal (and NM)

[Peter Zijlstra]

On Thu, 2008-07-24 at 05:11 -0700, Andrew Morton wrote:
> On Thu, 24 Jul 2008 13:57:11 +0200 Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Thu, 2008-07-24 at 13:38 +0200, Johannes Berg wrote:
> > > On Thu, 2008-07-24 at 11:58 +0200, Pierre Ossman wrote:
> > > > From commit 22bb1be4d27...:
> > > > 
> > > > "The /sys/class/net/*/wireless/ direcory is, as far as I know, not used
> > > > by anyone."
> > > > 
> > > > Can't have been much of an investigation as HAL uses these to determine
> > > > if a card is a wireless one. Without this option, you cannot get
> > > > wireless support in NetworkManager, stranding most laptops.
> > > 
> > > Bzzzzt. You suck. Go check again. Hint: try hal's git tree.
> 
> err,
> 
> a) don't be rude
> 
> b) what's in various git trees is unuseful for making decisions
>    about production kernel features.
> 
> otoh the patch seems reasonable.

> So if you've enabled CONFIG_WIRELESS_EXT_SYSFS (and it correctly has
> default y) then things should continue to work OK.  And the deprecation
> date of 2010 sounds reasonable.  In fact generous, for us...
> 
> So where's the problem?

Your two previous points, a and b.

That help text should really have mentioned current software, not future
software. And the tone of the reply.

Re: 22bb1be4d27... breaks hal (and NM)

[Alistair John Strachan]

On Thursday 24 July 2008 13:11:13 Andrew Morton wrote:
> : +	  Say Y if you have programs using it (we don't know of any).

Probably would have been less confusing if the Kconfig message actually lined 
up with the note about older HALs in feature-removal-schedule.txt. Saying "we 
don't know of any" seems inaccurate. This should really be something 
like "old HAL versions" or perhaps even better "HAL <= X.Y.Z".

I'm sure distributors will have the common sense to stop enabling the option 
in the future. In the mean time, removing the deceptive summary would 
hopefully avoid such bug reports.

-- 
Cheers,
Alistair.