2012-11-12 21:52:09

by Dave Airlie

[permalink] [raw]
Subject: removal of sysfs wext extension broke on F16

https://bugzilla.redhat.com/show_bug.cgi?id=875328

blamed for breaking wireless.
https://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=35b2a113cb0298d4f9a1263338b456094a414057

apparantly the initscripts used the sysfs directory existance as a
check, so maybe either back this out or bring back just the subdirs.

Dave.


2012-11-13 08:26:40

by Johannes Berg

[permalink] [raw]
Subject: Re: removal of sysfs wext extension broke on F16

On Tue, 2012-11-13 at 07:52 +1000, Dave Airlie wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=875328
>
> blamed for breaking wireless.
> https://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=35b2a113cb0298d4f9a1263338b456094a414057
>
> apparantly the initscripts used the sysfs directory existance as a
> check, so maybe either back this out or bring back just the subdirs.

OK. I really thought nobody was using it, but we can of course revert it
for Fedora's benefit, it's optional anyway.

FWIW, for most drivers you could check if the sys/class/net/$1/phy80211
symlink exists, but that doesn't work for older wext-only drivers (or
say the crap Broadcom binary STA driver.)

Should I revert it and send it in my next pull request?

johannes

2012-11-16 18:20:26

by Linus Torvalds

[permalink] [raw]
Subject: Re: removal of sysfs wext extension broke on F16

On Tue, Nov 13, 2012 at 12:27 AM, Johannes Berg
<[email protected]> wrote:
>
> Should I revert it and send it in my next pull request?

Please do. Or perhaps we could just add the directory back, without
the actual entries, if that is sufficient..

Linus

2012-11-16 20:35:14

by Johannes Berg

[permalink] [raw]
Subject: Re: removal of sysfs wext extension broke on F16

On Fri, 2012-11-16 at 10:20 -0800, Linus Torvalds wrote:
> On Tue, Nov 13, 2012 at 12:27 AM, Johannes Berg
> <[email protected]> wrote:
> >
> > Should I revert it and send it in my next pull request?
>
> Please do. Or perhaps we could just add the directory back, without
> the actual entries, if that is sufficient..

Yeah, that does seem to be sufficient, I'll queue up the patch below.

johannes


>From 8d6422678975e8128904a5a7a9ba02e94ccc1130 Mon Sep 17 00:00:00 2001
From: Johannes Berg <[email protected]>
Date: Fri, 16 Nov 2012 20:46:19 +0100
Subject: [PATCH] wireless: add back sysfs directory

commit 35b2a113cb0298d4f9a1263338b456094a414057 broke (at least)
Fedora's networking scripts, they check for the existence of the
wireless directory. As the files aren't used, add the directory
back and not the files. Also do it for both drivers based on the
old wireless extensions and cfg80211, regardless of whether the
compat code for wext is built into cfg80211 or not.

Cc: [email protected] [3.6]
Reported-by: Dave Airlie <[email protected]>
Reported-by: Bill Nottingham <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
net/core/net-sysfs.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index bcf02f6..4ce301a 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -429,6 +429,12 @@ static struct attribute_group netstat_group = {
.name = "statistics",
.attrs = netstat_attrs,
};
+
+#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211)
+static struct attribute_group wireless_group = {
+ .name = "wireless",
+};
+#endif
#endif /* CONFIG_SYSFS */

#ifdef CONFIG_RPS
@@ -1409,6 +1415,15 @@ int netdev_register_kobject(struct net_device *net)
groups++;

*groups++ = &netstat_group;
+
+#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211)
+ if (net->ieee80211_ptr)
+ *groups++ = &wireless_group;
+#if IS_ENABLED(CONFIG_WIRELESS_EXT)
+ else if (net->wireless_handlers)
+ *groups++ = &wireless_group;
+#endif
+#endif
#endif /* CONFIG_SYSFS */

error = device_add(dev);
--
1.8.0


2012-11-16 20:36:17

by Johannes Berg

[permalink] [raw]
Subject: Re: removal of sysfs wext extension broke on F16

On Fri, 2012-11-16 at 21:35 +0100, Johannes Berg wrote:
> On Fri, 2012-11-16 at 10:20 -0800, Linus Torvalds wrote:
> > On Tue, Nov 13, 2012 at 12:27 AM, Johannes Berg
> > <[email protected]> wrote:
> > >
> > > Should I revert it and send it in my next pull request?
> >
> > Please do. Or perhaps we could just add the directory back, without
> > the actual entries, if that is sufficient..
>
> Yeah, that does seem to be sufficient, I'll queue up the patch below.

Err, no, wishful thinking. I need a *little* bit more for sysfs to work.

johannes

2012-11-16 20:39:11

by Johannes Berg

[permalink] [raw]
Subject: Re: removal of sysfs wext extension broke on F16

On Fri, 2012-11-16 at 21:36 +0100, Johannes Berg wrote:
> On Fri, 2012-11-16 at 21:35 +0100, Johannes Berg wrote:
> > On Fri, 2012-11-16 at 10:20 -0800, Linus Torvalds wrote:
> > > On Tue, Nov 13, 2012 at 12:27 AM, Johannes Berg
> > > <[email protected]> wrote:
> > > >
> > > > Should I revert it and send it in my next pull request?
> > >
> > > Please do. Or perhaps we could just add the directory back, without
> > > the actual entries, if that is sufficient..
> >
> > Yeah, that does seem to be sufficient, I'll queue up the patch below.
>
> Err, no, wishful thinking. I need a *little* bit more for sysfs to work.

This works, sorry about that.

>From 38c1a01cf10c6e4049b4ffbd4a6af655df2a46e1 Mon Sep 17 00:00:00 2001
From: Johannes Berg <[email protected]>
Date: Fri, 16 Nov 2012 20:46:19 +0100
Subject: [PATCH] wireless: add back sysfs directory

commit 35b2a113cb0298d4f9a1263338b456094a414057 broke (at least)
Fedora's networking scripts, they check for the existence of the
wireless directory. As the files aren't used, add the directory
back and not the files. Also do it for both drivers based on the
old wireless extensions and cfg80211, regardless of whether the
compat code for wext is built into cfg80211 or not.

Cc: [email protected] [3.6]
Reported-by: Dave Airlie <[email protected]>
Reported-by: Bill Nottingham <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
net/core/net-sysfs.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index bcf02f6..017a8ba 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -429,6 +429,17 @@ static struct attribute_group netstat_group = {
.name = "statistics",
.attrs = netstat_attrs,
};
+
+#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211)
+static struct attribute *wireless_attrs[] = {
+ NULL
+};
+
+static struct attribute_group wireless_group = {
+ .name = "wireless",
+ .attrs = wireless_attrs,
+};
+#endif
#endif /* CONFIG_SYSFS */

#ifdef CONFIG_RPS
@@ -1409,6 +1420,15 @@ int netdev_register_kobject(struct net_device *net)
groups++;

*groups++ = &netstat_group;
+
+#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211)
+ if (net->ieee80211_ptr)
+ *groups++ = &wireless_group;
+#if IS_ENABLED(CONFIG_WIRELESS_EXT)
+ else if (net->wireless_handlers)
+ *groups++ = &wireless_group;
+#endif
+#endif
#endif /* CONFIG_SYSFS */

error = device_add(dev);
--
1.8.0


2012-11-16 22:32:57

by Sven-Haegar Koch

[permalink] [raw]
Subject: Re: removal of sysfs wext extension broke on F16

On Fri, 16 Nov 2012, Johannes Berg wrote:

> On Fri, 2012-11-16 at 21:36 +0100, Johannes Berg wrote:
> > On Fri, 2012-11-16 at 21:35 +0100, Johannes Berg wrote:
> > > On Fri, 2012-11-16 at 10:20 -0800, Linus Torvalds wrote:
> > > > On Tue, Nov 13, 2012 at 12:27 AM, Johannes Berg
> > > > <[email protected]> wrote:
> > > > >
> > > > > Should I revert it and send it in my next pull request?
> > > >
> > > > Please do. Or perhaps we could just add the directory back, without
> > > > the actual entries, if that is sufficient..
> > >
> > > Yeah, that does seem to be sufficient, I'll queue up the patch below.
> >
> > Err, no, wishful thinking. I need a *little* bit more for sysfs to work.
>
> This works, sorry about that.

> +#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211)
> +static struct attribute *wireless_attrs[] = {
> + NULL
> +};
> +
> +static struct attribute_group wireless_group = {
> + .name = "wireless",
> + .attrs = wireless_attrs,
> +};
> +#endif
> #endif /* CONFIG_SYSFS */
>

How about a small comment in the source why it is there? I suppose
otherwise in a year or two someone will find it, see "WTF? Useless
empty directory?" and try to remove it again...

c'ya
sven-haegar

--
Three may keep a secret, if two of them are dead.
- Ben F.

2012-11-17 10:33:01

by Julian Calaby

[permalink] [raw]
Subject: Re: removal of sysfs wext extension broke on F16

Hi Sven,

On Sat, Nov 17, 2012 at 9:32 AM, Sven-Haegar Koch <[email protected]> wrote:
> On Fri, 16 Nov 2012, Johannes Berg wrote:
>
>> On Fri, 2012-11-16 at 21:36 +0100, Johannes Berg wrote:
>> > On Fri, 2012-11-16 at 21:35 +0100, Johannes Berg wrote:
>> > > On Fri, 2012-11-16 at 10:20 -0800, Linus Torvalds wrote:
>> > > > On Tue, Nov 13, 2012 at 12:27 AM, Johannes Berg
>> > > > <[email protected]> wrote:
>> > > > >
>> > > > > Should I revert it and send it in my next pull request?
>> > > >
>> > > > Please do. Or perhaps we could just add the directory back, without
>> > > > the actual entries, if that is sufficient..
>> > >
>> > > Yeah, that does seem to be sufficient, I'll queue up the patch below.
>> >
>> > Err, no, wishful thinking. I need a *little* bit more for sysfs to work.
>>
>> This works, sorry about that.
>
>> +#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211)
>> +static struct attribute *wireless_attrs[] = {
>> + NULL
>> +};
>> +
>> +static struct attribute_group wireless_group = {
>> + .name = "wireless",
>> + .attrs = wireless_attrs,
>> +};
>> +#endif
>> #endif /* CONFIG_SYSFS */
>>
>
> How about a small comment in the source why it is there? I suppose
> otherwise in a year or two someone will find it, see "WTF? Useless
> empty directory?" and try to remove it again...

Hopefully by then Fedora's networking scripts won't need it =)

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/