2008-07-24 09:58:22

by Pierre Ossman

[permalink] [raw]
Subject: 22bb1be4d27... breaks hal (and NM)

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.


Attachments:
signature.asc (197.00 B)

2008-07-24 11:40:15

by Johannes Berg

[permalink] [raw]
Subject: Re: 22bb1be4d27... breaks hal (and NM)

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


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-07-24 11:57:30

by Peter Zijlstra

[permalink] [raw]
Subject: Re: 22bb1be4d27... breaks hal (and NM)

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.

2008-07-24 12:09:24

by Johannes Berg

[permalink] [raw]
Subject: Re: 22bb1be4d27... breaks hal (and NM)

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


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-07-24 12:13:18

by Andrew Morton

[permalink] [raw]
Subject: Re: 22bb1be4d27... breaks hal (and NM)

On Thu, 24 Jul 2008 13:57:11 +0200 Peter Zijlstra <[email protected]> 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 <[email protected]>
: 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 <[email protected]>
: Cc: Jean Tourrilhes <[email protected]>
: Signed-off-by: John W. Linville <[email protected]>
:
: 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 <[email protected]>
: +
: +---------------------------
: +
: +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 <[email protected]>
: 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?

2008-07-24 12:22:35

by Peter Zijlstra

[permalink] [raw]
Subject: Re: 22bb1be4d27... breaks hal (and NM)

On Thu, 2008-07-24 at 05:11 -0700, Andrew Morton wrote:
> On Thu, 24 Jul 2008 13:57:11 +0200 Peter Zijlstra <[email protected]> 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.

2008-07-24 12:34:07

by Alistair John Strachan

[permalink] [raw]
Subject: Re: 22bb1be4d27... breaks hal (and NM)

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.

2008-07-24 12:42:57

by Adrian Bunk

[permalink] [raw]
Subject: [RFC: 2.6 patch] clarify the WIRELESS_EXT_SYSFS help text

On Thu, Jul 24, 2008 at 01:38:40PM +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.

Can everyone agree on the patch below?

> johannes

cu
Adrian


<-- snip -->


Current Hal uses the CONFIG_WIRELESS_EXT_SYSFS files, so don't claim
there were no known users and recommend to enable the option.

Signed-off-by: Adrian Bunk <[email protected]>

b685f62718b947264657c298e5ee5b2149200f68
diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index ab015c6..e582432 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -39,4 +39,4 @@ config WIRELESS_EXT_SYSFS
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).
+ If unsure, say Y.

2008-07-24 13:01:22

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC: 2.6 patch] clarify the WIRELESS_EXT_SYSFS help text

On Thu, 2008-07-24 at 15:42 +0300, Adrian Bunk wrote:
> On Thu, Jul 24, 2008 at 01:38:40PM +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.
>
> Can everyone agree on the patch below?
>
> > johannes
>
> cu
> Adrian
>
>
> <-- snip -->
>
>
> Current Hal uses the CONFIG_WIRELESS_EXT_SYSFS files, so don't claim
> there were no known users and recommend to enable the option.
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> b685f62718b947264657c298e5ee5b2149200f68
> diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
> index ab015c6..e582432 100644
> --- a/net/wireless/Kconfig
> +++ b/net/wireless/Kconfig
> @@ -39,4 +39,4 @@ config WIRELESS_EXT_SYSFS
> 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).
> + If unsure, say Y.

Maybe also add:

"HAL <= 0.5.11 depends on this feature."

Where I hope I got the version number right :-)

2008-07-24 20:13:28

by Robert Hancock

[permalink] [raw]
Subject: Re: 22bb1be4d27... breaks hal (and NM)

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).

2008-07-24 20:23:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC: 2.6 patch] clarify the WIRELESS_EXT_SYSFS help text

Sure, since we don't know what the next release will be there's little
point in trying to predict it, this patch is fine with me.

> Current Hal uses the CONFIG_WIRELESS_EXT_SYSFS files, so don't claim
> there were no known users and recommend to enable the option.
>
> Signed-off-by: Adrian Bunk <[email protected]>

Acked-by: Johannes Berg <[email protected]>

>
> b685f62718b947264657c298e5ee5b2149200f68
> diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
> index ab015c6..e582432 100644
> --- a/net/wireless/Kconfig
> +++ b/net/wireless/Kconfig
> @@ -39,4 +39,4 @@ config WIRELESS_EXT_SYSFS
> 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).
> + If unsure, say Y.
>


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-07-24 21:38:14

by Alessandro Suardi

[permalink] [raw]
Subject: Re: 22bb1be4d27... breaks hal (and NM)

On Thu, Jul 24, 2008 at 10:13 PM, Robert Hancock <[email protected]> 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')