sysfs class/net/ problem

sysfs class/net/ problem

[Johannes Berg]

Hi,

I didn't find anything related in the archives, if I missed it I'd
appreciate a pointer. I'm currently experiencing the following:

# ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN 
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
    link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
# ls -l /sys/class/net/
total 0
lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
# insmod cfg80211.ko ; insmod mac80211.ko ; insmod mac80211_hwsim.ko  radios=3 
# ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN 
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
    link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
5: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
    link/ether 02:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
6: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
    link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
7: wlan2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
    link/ether 02:00:00:00:02:00 brd ff:ff:ff:ff:ff:ff
8: hwsim0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN 
    link/ieee802.11/radiotap 12:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
# ls -l /sys/class/net/
total 0
lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
lrwxrwxrwx 1 root root 0 Jun  2 13:14 hwsim0 -> ../../devices/virtual/net/hwsim0
lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2
# rmmod mac80211_hwsim mac80211 cfg80211
# ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN 
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
    link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
# ls -l /sys/class/net/
total 0
lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2


The same problem doesn't happen with veth, so I'm not sure what's
causing the problem.

Help please?

johannes

Re: sysfs class/net/ problem

[Greg KH]

On Wed, Jun 02, 2010 at 03:16:51PM +0200, Johannes Berg wrote:
> Hi,
> 
> I didn't find anything related in the archives, if I missed it I'd
> appreciate a pointer. I'm currently experiencing the following:
> 
> # ip link
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN 
>     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
>     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
> # ls -l /sys/class/net/
> total 0
> lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
> lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
> # insmod cfg80211.ko ; insmod mac80211.ko ; insmod mac80211_hwsim.ko  radios=3 
> # ip link
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN 
>     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
>     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
> 5: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
>     link/ether 02:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
> 6: wlan1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
>     link/ether 02:00:00:00:01:00 brd ff:ff:ff:ff:ff:ff
> 7: wlan2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
>     link/ether 02:00:00:00:02:00 brd ff:ff:ff:ff:ff:ff
> 8: hwsim0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN 
>     link/ieee802.11/radiotap 12:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
> # ls -l /sys/class/net/
> total 0
> lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
> lrwxrwxrwx 1 root root 0 Jun  2 13:14 hwsim0 -> ../../devices/virtual/net/hwsim0
> lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
> lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
> lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
> lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2
> # rmmod mac80211_hwsim mac80211 cfg80211
> # ip link
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN 
>     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
>     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
> # ls -l /sys/class/net/
> total 0
> lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
> lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
> lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
> lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
> lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2

Ah, so the network devices aren't getting removed?

Do you have network namespaces enabled in your kernel or disabled?

And this is 2.6.35-rc1, right?

Eric, any ideas?

thanks,

greg k-h

Re: sysfs class/net/ problem

[Johannes Berg]

On Wed, 2010-06-02 at 08:46 -0700, Greg KH wrote:

> > # rmmod mac80211_hwsim mac80211 cfg80211
> > # ip link
> > 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN 
> >     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> > 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
> >     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
> > # ls -l /sys/class/net/
> > total 0
> > lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
> > lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
> > lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
> > lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
> > lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2
> 
> Ah, so the network devices aren't getting removed?

Well the netdevs are gone, just the links aren't going away. the
mac80211_hwsim directory is gone too.

> Do you have network namespaces enabled in your kernel or disabled?

enabled

> And this is 2.6.35-rc1, right?

yes.

Come to think of it, maybe somehow it ends up removing
mac80211_hwsim/hwsim0 before wlan0, and thus the link stays around? I
guess I could make it print messages about that somehow?

johannes

Re: sysfs class/net/ problem

[Eric W. Biederman]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2010-06-02 at 08:46 -0700, Greg KH wrote:
>
>> > # rmmod mac80211_hwsim mac80211 cfg80211
>> > # ip link
>> > 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN 
>> >     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> > 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000
>> >     link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
>> > # ls -l /sys/class/net/
>> > total 0
>> > lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
>> > lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
>> > lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
>> > lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
>> > lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2
>> 
>> Ah, so the network devices aren't getting removed?
>
> Well the netdevs are gone, just the links aren't going away. the
> mac80211_hwsim directory is gone too.
>
>> Do you have network namespaces enabled in your kernel or disabled?
>
> enabled
>
>> And this is 2.6.35-rc1, right?
>
> yes.
>
> Come to think of it, maybe somehow it ends up removing
> mac80211_hwsim/hwsim0 before wlan0, and thus the link stays around? I
> guess I could make it print messages about that somehow?

The wireless drivers are a little different, and come to think of it
network namespace support has been added to the wireless drivers since
last I looked closely.  Do you know what creates/deletes these links?
Is it the normal register_netdevice -> device_add path?

I definitely changed the symlink code a little making things network namespace
aware so it is reasonable to assume that something in my changes affected the
wireless drivers.

I took a quick look and with my patches against 2.6.33 I'm not seeing this.

I take a closer look at 2.6.35-rc1 and see if I can figure out what is
going on.  It would definitely be wrong if hwsim0 is removed before
wlan0.  Still on my todo it seems is to fix the lifetime issues of all
of the callers, grr.

Eric

Re: sysfs class/net/ problem

[Johannes Berg]

On Wed, 2010-06-02 at 09:17 -0700, Eric W. Biederman wrote:

> >> Ah, so the network devices aren't getting removed?
> >
> > Well the netdevs are gone, just the links aren't going away. the
> > mac80211_hwsim directory is gone too.
> >
> >> Do you have network namespaces enabled in your kernel or disabled?
> >
> > enabled
> >
> >> And this is 2.6.35-rc1, right?
> >
> > yes.
> >
> > Come to think of it, maybe somehow it ends up removing
> > mac80211_hwsim/hwsim0 before wlan0, and thus the link stays around? I
> > guess I could make it print messages about that somehow?
> 
> The wireless drivers are a little different, and come to think of it
> network namespace support has been added to the wireless drivers since
> last I looked closely. 

Yeah, I now need to go add tagged sysfs support to it too.

>  Do you know what creates/deletes these links?
> Is it the normal register_netdevice -> device_add path?

Yes, they aren't done specially.

> I definitely changed the symlink code a little making things network namespace
> aware so it is reasonable to assume that something in my changes affected the
> wireless drivers.

> I took a quick look and with my patches against 2.6.33 I'm not seeing this.

Hmm. I'm also not seeing it with veth, it would seem that ought to be
similar?

> I take a closer look at 2.6.35-rc1 and see if I can figure out what is
> going on.  It would definitely be wrong if hwsim0 is removed before
> wlan0.

I don't know if that's happening .. just guessing that it might cause
such a problem, and maybe some things are deferred somehow? Since netdev
destruction can be deferred, but the wifi sysfs destruction isn't. But
then that link there should cause the refcount to not go down until the
link goes away>?

johannes

Re: sysfs class/net/ problem

[Eric W. Biederman]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2010-06-02 at 09:17 -0700, Eric W. Biederman wrote:
>
>> >> Ah, so the network devices aren't getting removed?
>> >
>> > Well the netdevs are gone, just the links aren't going away. the
>> > mac80211_hwsim directory is gone too.
>> >
>> >> Do you have network namespaces enabled in your kernel or disabled?
>> >
>> > enabled
>> >
>> >> And this is 2.6.35-rc1, right?
>> >
>> > yes.
>> >
>> > Come to think of it, maybe somehow it ends up removing
>> > mac80211_hwsim/hwsim0 before wlan0, and thus the link stays around? I
>> > guess I could make it print messages about that somehow?
>> 
>> The wireless drivers are a little different, and come to think of it
>> network namespace support has been added to the wireless drivers since
>> last I looked closely. 
>
> Yeah, I now need to go add tagged sysfs support to it too.
>
>>  Do you know what creates/deletes these links?
>> Is it the normal register_netdevice -> device_add path?
>
> Yes, they aren't done specially.



>> I definitely changed the symlink code a little making things network namespace
>> aware so it is reasonable to assume that something in my changes affected the
>> wireless drivers.
>
>> I took a quick look and with my patches against 2.6.33 I'm not seeing this.
>
> Hmm. I'm also not seeing it with veth, it would seem that ought to be
> similar?

Since it is the register_netdev path it should be exactly the same.

The big change I made is I in some instances I replaced
sysfs_remove_link with sysfs_delete_link so I could have enough
information to infer which network namespace the link was in.  Since
wlan0 is a netdevice all of that information should already be there.


>> I take a closer look at 2.6.35-rc1 and see if I can figure out what is
>> going on.  It would definitely be wrong if hwsim0 is removed before
>> wlan0.
>
> I don't know if that's happening .. just guessing that it might cause
> such a problem, and maybe some things are deferred somehow? Since netdev
> destruction can be deferred, but the wifi sysfs destruction isn't. But
> then that link there should cause the refcount to not go down until the
> link goes away>?

unregister_netdevice will defer the final destruction but it does not
defer netdev_unregister_kobject -> device_del.

What happens if in exit_mac80211_hwsim you call unregister_netdev before
mac80211_hwsim_free?

At a quick glance it simply looks like you have the ordering reversed in your
module cleanup, and this is not network namespace related at all.

Eric

Re: sysfs class/net/ problem

[Johannes Berg]

On Wed, 2010-06-02 at 09:43 -0700, Eric W. Biederman wrote:

> > Hmm. I'm also not seeing it with veth, it would seem that ought to be
> > similar?
> 
> Since it is the register_netdev path it should be exactly the same.
> 
> The big change I made is I in some instances I replaced
> sysfs_remove_link with sysfs_delete_link so I could have enough
> information to infer which network namespace the link was in.  Since
> wlan0 is a netdevice all of that information should already be there.

I have no idea :)

> > I don't know if that's happening .. just guessing that it might cause
> > such a problem, and maybe some things are deferred somehow? Since netdev
> > destruction can be deferred, but the wifi sysfs destruction isn't. But
> > then that link there should cause the refcount to not go down until the
> > link goes away>?
> 
> unregister_netdevice will defer the final destruction but it does not
> defer netdev_unregister_kobject -> device_del.
> 
> What happens if in exit_mac80211_hwsim you call unregister_netdev before
> mac80211_hwsim_free?
> 
> At a quick glance it simply looks like you have the ordering reversed in your
> module cleanup, and this is not network namespace related at all.

Nah, the unregister_netdev there removes the "hwsim0" device, while the
mac80211_hwsim_free() will remove all the others, so the ordering of
those two doesn't matter.

johannes

Re: sysfs class/net/ problem

[Eric W. Biederman]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2010-06-02 at 09:43 -0700, Eric W. Biederman wrote:
>
>> > Hmm. I'm also not seeing it with veth, it would seem that ought to be
>> > similar?
>> 
>> Since it is the register_netdev path it should be exactly the same.
>> 
>> The big change I made is I in some instances I replaced
>> sysfs_remove_link with sysfs_delete_link so I could have enough
>> information to infer which network namespace the link was in.  Since
>> wlan0 is a netdevice all of that information should already be there.
>
> I have no idea :)
>
>> > I don't know if that's happening .. just guessing that it might cause
>> > such a problem, and maybe some things are deferred somehow? Since netdev
>> > destruction can be deferred, but the wifi sysfs destruction isn't. But
>> > then that link there should cause the refcount to not go down until the
>> > link goes away>?
>> 
>> unregister_netdevice will defer the final destruction but it does not
>> defer netdev_unregister_kobject -> device_del.
>> 
>> What happens if in exit_mac80211_hwsim you call unregister_netdev before
>> mac80211_hwsim_free?
>> 
>> At a quick glance it simply looks like you have the ordering reversed in your
>> module cleanup, and this is not network namespace related at all.
>
> Nah, the unregister_netdev there removes the "hwsim0" device, while the
> mac80211_hwsim_free() will remove all the others, so the ordering of
> those two doesn't matter.

So far that hypothesis that the target of the symlink is being removed before
the actual actual link looks like it could cause this.

Are there any other left overs in sysfs, besides just /sys/class/net/wlan0?

Eric

Re: sysfs class/net/ problem

[Johannes Berg]

On Wed, 2010-06-02 at 10:23 -0700, Eric W. Biederman wrote:

> So far that hypothesis that the target of the symlink is being removed before
> the actual actual link looks like it could cause this.

Yeah though I'm not sure how that would happen? Wouldn't the symlink
cause the target kobject to still be referenced, and thus stay around
until the symlink goes away?

> Are there any other left overs in sysfs, besides just /sys/class/net/wlan0?

No, not based on find /sys and diffing before/after anyway.

johannes

Re: sysfs class/net/ problem

[Eric W. Biederman]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2010-06-02 at 10:23 -0700, Eric W. Biederman wrote:
>
>> So far that hypothesis that the target of the symlink is being removed before
>> the actual actual link looks like it could cause this.
>
> Yeah though I'm not sure how that would happen? Wouldn't the symlink
> cause the target kobject to still be referenced, and thus stay around
> until the symlink goes away?

The references don't affect visibility in sysfs.  All of that is manual
at the sysfs layer, and there doesn't appear to be a good substitute.  Generally
the device layer manages to handle all of the details automatically but it
appears something is missing.

>> Are there any other left overs in sysfs, besides just /sys/class/net/wlan0?
>
> No, not based on find /sys and diffing before/after anyway.

It is going to be a little bit before I manage to dig into this deeply.

If you want to dig into this look at sysfs_delete_link.  instrument
it so that you can see if it is called for wlan{0,1,2} and see what
ns it is called for.

My current hypothesis is something is causing us to try and delete
the symlink from the wrong namespace, so we just skip that part of it.

Eric

Re: sysfs class/net/ problem

[Johannes Berg]

On Wed, 2010-06-02 at 11:05 -0700, Eric W. Biederman wrote:

> If you want to dig into this look at sysfs_delete_link.  instrument
> it so that you can see if it is called for wlan{0,1,2} and see what
> ns it is called for.
> 
> My current hypothesis is something is causing us to try and delete
> the symlink from the wrong namespace, so we just skip that part of it.

[   78.253128] create link wlan0 ns=ffff88001ce1e600
...
[   93.462268] delete link wlan0 ns=ffff88001ce1e600

looks the same ...

Also note

[  109.872488] netconsole: network logging stopped, interface wlan0 unregistered
[  109.872910] PM: Removing info for No Bus:wlan0
[  109.872941] delete link wlan0 ns=ffff88001e9bd600
[  110.130563] PM: Removing info for No Bus:rfkill0
[  110.130599] delete link rfkill0 ns=ffff88001b61ea80
[  110.131135] PM: Removing info for No Bus:phy0
[  110.131161] delete link phy0 ns=ffff88001b61e240
[  110.131424] PM: Removing info for No Bus:hwsimdev0
[  110.131445] delete link hwsimdev0 ns=ffff88001b67ed80

(I changed the struct device thing in hwsim to be hwsimdev%d rather than
hwsim%d to tell the difference to hwsim0, the monitor netdev) so it's
getting removed from PM way after the wlan0 that links into it...

johannes

Re: sysfs class/net/ problem

[Johannes Berg]

On Wed, 2010-06-02 at 20:55 +0200, Johannes Berg wrote:
> On Wed, 2010-06-02 at 11:05 -0700, Eric W. Biederman wrote:
> 
> > If you want to dig into this look at sysfs_delete_link.  instrument
> > it so that you can see if it is called for wlan{0,1,2} and see what
> > ns it is called for.
> > 
> > My current hypothesis is something is causing us to try and delete
> > the symlink from the wrong namespace, so we just skip that part of it.
> 
> [   78.253128] create link wlan0 ns=ffff88001ce1e600
> ...
> [   93.462268] delete link wlan0 ns=ffff88001ce1e600

No that was the sd, but sd->s_ns is NULL in both cases.

johannes

Re: sysfs class/net/ problem

[Johannes Berg]

On Wed, 2010-06-02 at 11:05 -0700, Eric W. Biederman wrote:

> My current hypothesis is something is causing us to try and delete
> the symlink from the wrong namespace, so we just skip that part of it.

Hmm... ok:

[   70.338274] create link wlan2 ns=(null)
...
[   71.881775] delete link wlan2 ns=(null)
[   71.881777] hash_and_remove ffff88001f9563c0, (null), wlan2
[   71.881782] sd=ffff88001ce2d9c0, sdns=ffffffff8271c260

and thus we skip sysfs_remove_one() in sysfs_hash_and_remove() because
sysfs_find_dirent() return an sd with a different ns than we passed in.
Why is the ns we pass in NULL, shouldn't it be init_ns?

johannes

Re: sysfs class/net/ problem

[Eric W. Biederman]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2010-06-02 at 11:05 -0700, Eric W. Biederman wrote:
>
>> My current hypothesis is something is causing us to try and delete
>> the symlink from the wrong namespace, so we just skip that part of it.
>
> Hmm... ok:
>
> [   70.338274] create link wlan2 ns=(null)

Inside of sysfs_do_create_link we compute sd->s_ns just before
sysfs_addrm_start.

With this sequence:
if (sysfs_ns_type(parent_sd))
		sd->s_ns = target->ktype->namespace(target);

> ...
> [   71.881775] delete link wlan2 ns=(null)
> [   71.881777] hash_and_remove ffff88001f9563c0, (null), wlan2
> [   71.881782] sd=ffff88001ce2d9c0, sdns=ffffffff8271c260
>
> and thus we skip sysfs_remove_one() in sysfs_hash_and_remove() because
> sysfs_find_dirent() return an sd with a different ns than we passed in.
> Why is the ns we pass in NULL, shouldn't it be init_ns?

NULL is what is used in the case where something is not bound to a
namespace (most of sysfs).  It should be init_ns.

If we have a NULL ns in sysfs_delete_link than I expect targ->sd == NULL.
As targ->sd->s_ns should equal init_ns.

So now I just need to figure out if targ->sd is NULL in delete link is
NULL in which case we have an ordering issue or if targ->sd->s_ns is NULL
in which case I have something confused with the network namespaces.

Looking at your report:
# ls -l /sys/class/net/
total 0
lrwxrwxrwx 1 root root 0 Jun  2 13:12 eth0 -> ../../devices/pci0000:00/0000:00:03.0/net/eth0
lrwxrwxrwx 1 root root 0 Jun  2 13:12 lo -> ../../devices/virtual/net/lo
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan0 -> ../../devices/virtual/mac80211_hwsim/hwsim0/wlan0
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan1 -> ../../devices/virtual/mac80211_hwsim/hwsim1/wlan1
lrwxrwxrwx 1 root root 0 Jun  2 13:14 wlan2 -> ../../devices/virtual/mac80211_hwsim/hwsim2/wlan2

It appears that devices/virtual/mac80211_hwsim/hwsim0/wlan0 is not a
normal network device.  The symlink does not point into a net
directory.  Which is done with the normal network devices to ensure
they don't have conflicting names with anything else.

Are your network devices not members of net_class (defined in
net/core/net-sysfs.c)?  There is some odd class_create magic going on
in init_mac80211_hwsim.

Let's see.

netdev_register_kobject unconditionally sets dev->class = &net_class.

device_add calls setup_parent which calls get_device_parent.
get_device_parent calls virtual_device_parent if no parent is present,
or it the parent does not have a class it creates a net directory.

So we are in the case where the parent directory has a class, which I did
not realize was there. Ugh.  Does this matter?

Let's see.

In sysfs_create_link if the parent sysfs_dirent has a namespace type I
assume that the kobject target of the symlink will have a ktype that
returns the namespace the dirent should be in.  Since the target kobject
is a normal network device that assumption is fulfilled.

In sysfs_delete_link I assume that the target kobject dirent has a useful
sd->s_ns, which it will if you are in a class_net subdirectory but hwsim0
seems to be something else.  So the target of the sysfs_dirent does not
appear to meet these requirements, because the target directory is not
name-spacified.

This appears to be specific to the mac80211_hwsim driver I don't think it even
affects other wireless drivers.

What I don't see at the moment is how we get
devices/virtual/mac80211_hwsim/hwsim0/ as our parent directory for
network devices.

Johannes any clues?

If I have read this right this is a bug that only affects mac80211_hwsim because
it does magic creating it's own devices and classes, which ordinary drivers don't
do.

Eric

[RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Eric W. Biederman]

In the last painful restructuring of sysfs we created started
creating class directories under normal devices so we could place
devices such as network devices directly under their the hardware
that implements them instead of in their class directories like
/sys/class/net/.  This creation of class directories avoids the
need to worry about namespace clonflicts if something is renamed.

A special exception was made for devices that were still placed
directly in their class directory.  Looking at how this interacts
with the wireless network devices it appears this special exception
is either completely unneeded or at least needs to be restricted to
a parent device with the same class as the child device.  Certainly
in the case of unrelated classes we very much have the possibility
of namespace classes and we should be creating the subdirectory.

Johannes this should fix your issue with mac80211_hwsim, where
the device symlink were not destroyed when the driver was removed.

Greg, Kay where does that parent->class check come into play?  Do
we need it at all?

> commit 864062457a2e444227bd368ca5f2a2b740de604f
> Author: Kay Sievers <kay.sievers@vrfy.org>
> Date:   Wed Mar 14 03:25:56 2007 +0100
> 
>     driver core: fix namespace issue with devices assigned to classes
>     
>       - uses a kset in "struct class" to keep track of all directories
>         belonging to this class
>       - merges with the /sys/devices/virtual logic.
>       - removes the namespace-dir if the last member of that class
>         leaves the directory.
>     
>     There may be locking or refcounting fixes left, I stopped when it seemed
>     to work with network and sound modules. :)
>     
>     From: Kay Sievers <kay.sievers@vrfy.org>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

---

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 9630fbd..3725f81 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -673,7 +673,7 @@ static struct kobject *get_device_parent(struct device *dev,
 		 */
 		if (parent == NULL)
 			parent_kobj = virtual_device_parent(dev);
-		else if (parent->class)
+		else if (parent->class == dev->class)
 			return &parent->kobj;
 		else
 			parent_kobj = &parent->kobj;

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Thu, Jun 3, 2010 at 02:53, Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> In the last painful restructuring of sysfs we created started
> creating class directories under normal devices so we could place
> devices such as network devices directly under their the hardware
> that implements them instead of in their class directories like
> /sys/class/net/.  This creation of class directories avoids the
> need to worry about namespace clonflicts if something is renamed.
>
> A special exception was made for devices that were still placed
> directly in their class directory.  Looking at how this interacts
> with the wireless network devices it appears this special exception
> is either completely unneeded or at least needs to be restricted to
> a parent device with the same class as the child device.  Certainly
> in the case of unrelated classes we very much have the possibility
> of namespace classes and we should be creating the subdirectory.

The class-glue-directories are only created between a bus-parent and
and a class device. Class devices usually don't have other class
devices as parents, that's why it wasn't done that way.

If people use class devices from other classes as parents, they should
definitely convert the class that acts as a parent to a bus, to fit
into the usual model. All that was really never meant to be used that
way. The current behavior, to not to create the glue-directory is at
least the intended one from the driver core's perspective.

What kind of classes do this, where this change would help or would be needed?

I don't mind trying if that change will work for people, I can't tell
if there are any other users doing things like this which could break
with such a change. Stuff like udev will be fine with directories
inserted, but there are many things out there, that just access their
parents attributes with ../../foo, which might no longer work when we
insert directories.

Thanks,
Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Eric W. Biederman]

Kay Sievers <kay.sievers@vrfy.org> writes:

> On Thu, Jun 3, 2010 at 02:53, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>
>> In the last painful restructuring of sysfs we created started
>> creating class directories under normal devices so we could place
>> devices such as network devices directly under their the hardware
>> that implements them instead of in their class directories like
>> /sys/class/net/.  This creation of class directories avoids the
>> need to worry about namespace clonflicts if something is renamed.
>>
>> A special exception was made for devices that were still placed
>> directly in their class directory.  Looking at how this interacts
>> with the wireless network devices it appears this special exception
>> is either completely unneeded or at least needs to be restricted to
>> a parent device with the same class as the child device.  Certainly
>> in the case of unrelated classes we very much have the possibility
>> of namespace classes and we should be creating the subdirectory.
>
> The class-glue-directories are only created between a bus-parent and
> and a class device. Class devices usually don't have other class
> devices as parents, that's why it wasn't done that way.


> If people use class devices from other classes as parents, they should
> definitely convert the class that acts as a parent to a bus, to fit
> into the usual model. All that was really never meant to be used that
> way. The current behavior, to not to create the glue-directory is at
> least the intended one from the driver core's perspective.
>
> What kind of classes do this, where this change would help or would be needed?


> I don't mind trying if that change will work for people, I can't tell
> if there are any other users doing things like this which could break
> with such a change. Stuff like udev will be fine with directories
> inserted, but there are many things out there, that just access their
> parents attributes with ../../foo, which might no longer work when we
> insert directories.

To the best of my knowledge we are talking a very limited number of
real world cases.

The driver in particular that causes problems is mac80211_hwsim. It
winds up placing network devices in a directory that isn't prepared to
take network namespace tagged members, with the result that when the
module is removed we don't delete the symlinks from /sys/class/net/.
I see no reason to believe we are free of possible namespace conflicts
either, which is why I suggested the patch.

From my perspective not creating the directory in some weird corner case that
appears to practically to never happen looks like an ugly nasty special case.

If the solution winds up being converting mac80211_hwsim to using a
bus instead of a class that seems reasonable to me as well.  More code
in one place to remove the chance of problems elsewhere.

Eric

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Wed, 2010-06-02 at 17:53 -0700, Eric W. Biederman wrote:

> Johannes this should fix your issue with mac80211_hwsim, where
> the device symlink were not destroyed when the driver was removed.

It does, thank you.

FWIW, I'm happy changing hwsim too, but I don't think I quite understand
what you're proposing in your other email so I'll leave it up to you
since you now know what is causing the problem :)

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Fri, Jun 4, 2010 at 08:54, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2010-06-02 at 17:53 -0700, Eric W. Biederman wrote:
>
>> Johannes this should fix your issue with mac80211_hwsim, where
>> the device symlink were not destroyed when the driver was removed.
>
> It does, thank you.
>
> FWIW, I'm happy changing hwsim too, but I don't think I quite understand
> what you're proposing in your other email so I'll leave it up to you
> since you now know what is causing the problem :)

Assuming that hwsim is th parent of the network interface, it should
us a "struct bus_type" not a "struct class" for the subsystem it
assigns the devices to.

Classes should not be used for anything completely simple, at best not
be used at all, they are just too simple. We never know about future
requirements, which usually all go wrong with the non-extendable class
logic.

The difference in the code to switch from class to bus should be minimal.

Cheers,
Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Fri, 2010-06-04 at 10:15 +0200, Kay Sievers wrote:
> On Fri, Jun 4, 2010 at 08:54, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Wed, 2010-06-02 at 17:53 -0700, Eric W. Biederman wrote:
> >
> >> Johannes this should fix your issue with mac80211_hwsim, where
> >> the device symlink were not destroyed when the driver was removed.
> >
> > It does, thank you.
> >
> > FWIW, I'm happy changing hwsim too, but I don't think I quite understand
> > what you're proposing in your other email so I'll leave it up to you
> > since you now know what is causing the problem :)
> 
> Assuming that hwsim is th parent of the network interface, it should
> us a "struct bus_type" not a "struct class" for the subsystem it
> assigns the devices to.

It's all virtual, so yeah, I guess it is the parent? It currently
creates a virtual struct device in the hwsim class and assigns that to
the netdev parent indirectly via the wiphy or something like that.

> Classes should not be used for anything completely simple, at best not
> be used at all, they are just too simple. We never know about future
> requirements, which usually all go wrong with the non-extendable class
> logic.
> 
> The difference in the code to switch from class to bus should be minimal.

Does that mean cfg80211 (net/wireless/) should also not use a struct
class? I'm not familiar with any of these details, mind helping me out?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Fri, Jun 4, 2010 at 10:28, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2010-06-04 at 10:15 +0200, Kay Sievers wrote:
>> On Fri, Jun 4, 2010 at 08:54, Johannes Berg <johannes@sipsolutions.net> wrote:
>> > On Wed, 2010-06-02 at 17:53 -0700, Eric W. Biederman wrote:
>> >
>> >> Johannes this should fix your issue with mac80211_hwsim, where
>> >> the device symlink were not destroyed when the driver was removed.
>> >
>> > It does, thank you.
>> >
>> > FWIW, I'm happy changing hwsim too, but I don't think I quite understand
>> > what you're proposing in your other email so I'll leave it up to you
>> > since you now know what is causing the problem :)
>>
>> Assuming that hwsim is th parent of the network interface, it should
>> us a "struct bus_type" not a "struct class" for the subsystem it
>> assigns the devices to.
>
> It's all virtual, so yeah, I guess it is the parent? It currently
> creates a virtual struct device in the hwsim class and assigns that to
> the netdev parent indirectly via the wiphy or something like that.
>
>> Classes should not be used for anything completely simple, at best not
>> be used at all, they are just too simple. We never know about future
>> requirements, which usually all go wrong with the non-extendable class
>> logic.
>>
>> The difference in the code to switch from class to bus should be minimal.
>
> Does that mean cfg80211 (net/wireless/) should also not use a struct
> class? I'm not familiar with any of these details, mind helping me out?

Everything that might ever have a child device, or might ever need a
sysfs attribute gobal to the subsystem must convert to a bus.

Actually there is not much reason to ever use "struct class" today.
The layout for classes in /sys is not extendable like it is for buses
which put all devices in a devices/ subdir and have the main subsystem
directory to add custom things.

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Fri, 2010-06-04 at 10:34 +0200, Kay Sievers wrote:

> >> Assuming that hwsim is th parent of the network interface, it should
> >> us a "struct bus_type" not a "struct class" for the subsystem it
> >> assigns the devices to.
> >
> > It's all virtual, so yeah, I guess it is the parent? It currently
> > creates a virtual struct device in the hwsim class and assigns that to
> > the netdev parent indirectly via the wiphy or something like that.
> >
> >> Classes should not be used for anything completely simple, at best not
> >> be used at all, they are just too simple. We never know about future
> >> requirements, which usually all go wrong with the non-extendable class
> >> logic.
> >>
> >> The difference in the code to switch from class to bus should be minimal.
> >
> > Does that mean cfg80211 (net/wireless/) should also not use a struct
> > class? I'm not familiar with any of these details, mind helping me out?
> 
> Everything that might ever have a child device, or might ever need a
> sysfs attribute gobal to the subsystem must convert to a bus.
> 
> Actually there is not much reason to ever use "struct class" today.
> The layout for classes in /sys is not extendable like it is for buses
> which put all devices in a devices/ subdir and have the main subsystem
> directory to add custom things.

Ok, I don't get it.

For hwsim, we create entirely virtual "struct device"s. I think we just
need that so userspace like network-manager doesn't get completely
confused. BUT: device_create() needs a struct class parameter as the
first parameter. So should we have a virtual class _and_ a virtual bus??

Similarly for wireless itself (net/wireless/), we use
device_initialize() and set up the dev.class and dev.platform_data (not
sure why platform data?) since this is a virtual abstraction. Basically
we want to show in sysfs that

physical device
  +--- (virtual) wireless phy device 1
     +--- netdev 1
     +--- netdev 2
     +--- netdev 3
  +--- (virtual) wireless phy device 2
     +--- netdev 4

Although the second wireless phy device is only done by ath9k and will
be removed soon. Still we should show that those netdevs all belong to
the given wireless phy device.

So ... what should I do? How can I do device_create/device_initialize
without a struct class? And why a bus instead? It's not like there are
devices that need to be discovered and bound to it etc.

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Sun, Jun 6, 2010 at 15:08, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2010-06-04 at 10:34 +0200, Kay Sievers wrote:
>> Actually there is not much reason to ever use "struct class" today.
>> The layout for classes in /sys is not extendable like it is for buses
>> which put all devices in a devices/ subdir and have the main subsystem
>> directory to add custom things.
>
> Ok, I don't get it.
>
> For hwsim, we create entirely virtual "struct device"s. I think we just
> need that so userspace like network-manager doesn't get completely
> confused. BUT: device_create() needs a struct class parameter as the
> first parameter. So should we have a virtual class _and_ a virtual bus??
>
> Similarly for wireless itself (net/wireless/), we use
> device_initialize() and set up the dev.class and dev.platform_data (not
> sure why platform data?) since this is a virtual abstraction. Basically
> we want to show in sysfs that
>
> physical device
>  +--- (virtual) wireless phy device 1
>     +--- netdev 1
>     +--- netdev 2
>     +--- netdev 3
>  +--- (virtual) wireless phy device 2
>     +--- netdev 4
>
> Although the second wireless phy device is only done by ath9k and will
> be removed soon. Still we should show that those netdevs all belong to
> the given wireless phy device.
>
> So ... what should I do? How can I do device_create/device_initialize
> without a struct class?

There is no real difference between classes and buses. Actually we're
working on merging them completely inside the kernel. Just declare a
"struct bus_type" instead of a "struct class".

> And why a bus instead? It's not like there are
> devices that need to be discovered and bound to it etc.

Because class devices are not meat to ever have children from other
classes. This is just not what they should be used like.

Also the flat directories in /sys/class/<name>/* are not extendable
with other stuff, and they should not be used for new stuff. You can
never put subsystem-wide properties to that directory without
confusing userspace. That is not a problem at all with buses, as they
have a dedicated devices/ subdir in the bus directory. If there are no
legacy reason, or things which are already a class, and they should
look similar, no new classes should be added to the kernel.

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Sun, 2010-06-06 at 19:17 +0200, Kay Sievers wrote:

> There is no real difference between classes and buses. Actually we're
> working on merging them completely inside the kernel. Just declare a
> "struct bus_type" instead of a "struct class".

Can you please tell me then how to device_create() without a class? I
cannot seem to create devices without a class at all, even using manual
allocation (yuck) and device_register crashes the kernel.

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Mon, Jun 7, 2010 at 11:42, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Sun, 2010-06-06 at 19:17 +0200, Kay Sievers wrote:
>
>> There is no real difference between classes and buses. Actually we're
>> working on merging them completely inside the kernel. Just declare a
>> "struct bus_type" instead of a "struct class".
>
> Can you please tell me then how to device_create() without a class? I
> cannot seem to create devices without a class at all, even using manual
> allocation (yuck) and device_register crashes the kernel.

Right, this "convenience API" does not exist for buses. It's not doing
much, just allocates a "struct device" and fills in the few values and
calls device_register().

Does your device create a device node? If not, device_create() should
not be used anyway, because the corresponding device_destroy() will
not do anything.

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Mon, 2010-06-07 at 11:53 +0200, Kay Sievers wrote:

> > Can you please tell me then how to device_create() without a class? I
> > cannot seem to create devices without a class at all, even using manual
> > allocation (yuck) and device_register crashes the kernel.
> 
> Right, this "convenience API" does not exist for buses. It's not doing
> much, just allocates a "struct device" and fills in the few values and
> calls device_register().
> 
> Does your device create a device node? If not, device_create() should
> not be used anyway, because the corresponding device_destroy() will
> not do anything.

No, it doesn't need a dev node. I tried this:

+               data->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
+               if (!data->dev) {
                        err = -ENOMEM;
                        goto failed_drvdata;
                }
+
+               dev_set_name(data->dev, "hwsim%d", i);
+               data->dev->bus = &hwsim_bus;
                data->dev->driver = &mac80211_hwsim_driver;
 
+               err = device_register(data->dev);
+               if (err) {
+                       printk(KERN_DEBUG
+                              "mac80211_hwsim: device_register failed (%d)\n",
+                              err);
+                       goto failed_drvdata;
+               }

(ignore the pluses, snipped from a patch) but it ran into a null ptr
deref?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Mon, Jun 7, 2010 at 12:14, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2010-06-07 at 11:53 +0200, Kay Sievers wrote:
>
>> > Can you please tell me then how to device_create() without a class? I
>> > cannot seem to create devices without a class at all, even using manual
>> > allocation (yuck) and device_register crashes the kernel.
>>
>> Right, this "convenience API" does not exist for buses. It's not doing
>> much, just allocates a "struct device" and fills in the few values and
>> calls device_register().
>>
>> Does your device create a device node? If not, device_create() should
>> not be used anyway, because the corresponding device_destroy() will
>> not do anything.
>
> No, it doesn't need a dev node. I tried this:
>
> +               data->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> +               if (!data->dev) {
>                        err = -ENOMEM;
>                        goto failed_drvdata;
>                }
> +
> +               dev_set_name(data->dev, "hwsim%d", i);
> +               data->dev->bus = &hwsim_bus;
>                data->dev->driver = &mac80211_hwsim_driver;
>
> +               err = device_register(data->dev);
> +               if (err) {
> +                       printk(KERN_DEBUG
> +                              "mac80211_hwsim: device_register failed (%d)\n",
> +                              err);
> +                       goto failed_drvdata;
> +               }
>
> (ignore the pluses, snipped from a patch) but it ran into a null ptr
> deref?

Oh, I see. It's probably something nobody ever did before. You try to
register a bus device which has no parent. Seems that's something
nobody ever expected to happen. :)

Your driver/subsystem is completely virtual, does not depend on any
hardware, right? If we create a virtual parent, like:
  parent = kzalloc(sizeof(struct device), GFP_KERNEL);
  dev_set_name(parent, "mac80211_hwsim");
  device_register(parent);

An in your code:
  data->dev->parent = parent;

That should give you a /sys/devices/mac80211_hwsim/ directory where
all the devices you create should show up.

If that works as expected, we should probably add something like:
   struct device *device_virtual_parent(const char *name);
which will allow you to create such a parent device in the
/sys/device/virtual/ directory.

Let me know if the above hack with the virtual parent works, then we
can check what do add to the core.

Thanks,
Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Mon, 2010-06-07 at 13:05 +0200, Kay Sievers wrote:

> > +               data->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> > +               if (!data->dev) {
> >                        err = -ENOMEM;
> >                        goto failed_drvdata;
> >                }
> > +
> > +               dev_set_name(data->dev, "hwsim%d", i);
> > +               data->dev->bus = &hwsim_bus;
> >                data->dev->driver = &mac80211_hwsim_driver;
> >
> > +               err = device_register(data->dev);
> > +               if (err) {
> > +                       printk(KERN_DEBUG
> > +                              "mac80211_hwsim: device_register failed (%d)\n",
> > +                              err);
> > +                       goto failed_drvdata;
> > +               }
> >
> > (ignore the pluses, snipped from a patch) but it ran into a null ptr
> > deref?
> 
> Oh, I see. It's probably something nobody ever did before. You try to
> register a bus device which has no parent. Seems that's something
> nobody ever expected to happen. :)
> 
> Your driver/subsystem is completely virtual, does not depend on any
> hardware, right? If we create a virtual parent, like:
>   parent = kzalloc(sizeof(struct device), GFP_KERNEL);
>   dev_set_name(parent, "mac80211_hwsim");
>   device_register(parent);
> 
> An in your code:
>   data->dev->parent = parent;
> 
> That should give you a /sys/devices/mac80211_hwsim/ directory where
> all the devices you create should show up.

So that seemed equivalent to my code except for the .bus/.driver
assignments and the two-level hierarchy of course. 

(mind you, I think we probably need to have the bus/driver assignment,
but I wanted to try out your suggestion first)

So I removed bus/driver assignment from the above code just to try it
out, and got

Device 'hwsim0' does not have a release() function, it is broken and
must be fixed.

This has evolved far too much for me right now. Can we just apply the
initial patch from Eric and be happier for a while? I can't justify
spending this much time on it right now. Alternatively, you could look
at hwsim too, since it's all virtual, nothing special is required, I do
testing in a virtual machine ...

current patch is at
http://johannes.sipsolutions.net/patches/kernel/all/2010-06-07-11:41/hwsim-bus.patch

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Mon, Jun 7, 2010 at 13:41, Johannes Berg <johannes@sipsolutions.net> wrote:
> (mind you, I think we probably need to have the bus/driver assignment,
> but I wanted to try out your suggestion first)

Class device can never have a driver. And unregistered drivers can
not be used, what you try to do here. That all should just be removed or
properly registered with the core if needed.

> So I removed bus/driver assignment from the above code just to try it
> out, and got
>
> Device 'hwsim0' does not have a release() function, it is broken and
> must be fixed.

The driver core expects the resources of the device to be freed on
release. You can provide an empty release function because you do this
from a global list of devices.

> This has evolved far too much for me right now. Can we just apply the
> initial patch from Eric and be happier for a while? I can't justify
> spending this much time on it right now. Alternatively, you could look
> at hwsim too, since it's all virtual, nothing special is required, I do
> testing in a virtual machine ...
>
> current patch is at
> http://johannes.sipsolutions.net/patches/kernel/all/2010-06-07-11:41/hwsim-bus.patch

Here is something that seems to work for me.

Cheers,
Kay



diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 6f8cb3e..d210ce6 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -194,7 +194,13 @@ static inline void hwsim_clear_sta_magic(struct ieee80211_sta *sta)
 	sp->magic = 0;
 }
 
-static struct class *hwsim_class;
+static struct bus_type hwsim_bus = {
+	.name = "mac80211_hwsim",
+};
+
+static struct device hwsim_parent = {
+	.init_name = "mac80211_hwsim",
+};
 
 static struct net_device *hwsim_mon; /* global monitor netdev */
 
@@ -1071,14 +1077,10 @@ static void mac80211_hwsim_free(void)
 		device_unregister(data->dev);
 		ieee80211_free_hw(data->hw);
 	}
-	class_destroy(hwsim_class);
+	device_unregister(&hwsim_parent);
+	bus_unregister(&hwsim_bus);
 }
 
-
-static struct device_driver mac80211_hwsim_driver = {
-	.name = "mac80211_hwsim"
-};
-
 static const struct net_device_ops hwsim_netdev_ops = {
 	.ndo_start_xmit 	= hwsim_mon_xmit,
 	.ndo_change_mtu		= eth_change_mtu,
@@ -1231,6 +1233,9 @@ DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_group,
 			hwsim_fops_group_read, hwsim_fops_group_write,
 			"%llx\n");
 
+/* all devices are kept in out own list and the ressources are freed on exit */
+static void hwsim_dev_release(struct device* dev) {}
+
 static int __init init_mac80211_hwsim(void)
 {
 	int i, err = 0;
@@ -1251,9 +1256,15 @@ static int __init init_mac80211_hwsim(void)
 	spin_lock_init(&hwsim_radio_lock);
 	INIT_LIST_HEAD(&hwsim_radios);
 
-	hwsim_class = class_create(THIS_MODULE, "mac80211_hwsim");
-	if (IS_ERR(hwsim_class))
-		return PTR_ERR(hwsim_class);
+	err = bus_register(&hwsim_bus);
+	if (err)
+		return err;
+
+	err = device_register(&hwsim_parent);
+	if (err) {
+		bus_unregister(&hwsim_bus);
+		return err;
+	}
 
 	memset(addr, 0, ETH_ALEN);
 	addr[0] = 0x02;
@@ -1271,16 +1282,24 @@ static int __init init_mac80211_hwsim(void)
 		data = hw->priv;
 		data->hw = hw;
 
-		data->dev = device_create(hwsim_class, NULL, 0, hw,
-					  "hwsim%d", i);
-		if (IS_ERR(data->dev)) {
-			printk(KERN_DEBUG
-			       "mac80211_hwsim: device_create "
-			       "failed (%ld)\n", PTR_ERR(data->dev));
+		data->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
+		if (!data->dev) {
 			err = -ENOMEM;
 			goto failed_drvdata;
 		}
-		data->dev->driver = &mac80211_hwsim_driver;
+
+		dev_set_name(data->dev, "hwsim%d", i);
+		data->dev->parent = &hwsim_parent;
+		data->dev->bus = &hwsim_bus;
+		data->dev->release = hwsim_dev_release;
+
+		err = device_register(data->dev);
+		if (err) {
+			printk(KERN_DEBUG
+			       "mac80211_hwsim: device_register failed (%d)\n",
+			       err);
+			goto failed_drvdata;
+		}
 
 		SET_IEEE80211_DEV(hw, data->dev);
 		addr[3] = i >> 8;

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Mon, 2010-06-07 at 14:26 +0200, Kay Sievers wrote:
> On Mon, Jun 7, 2010 at 13:41, Johannes Berg <johannes@sipsolutions.net> wrote:
> > (mind you, I think we probably need to have the bus/driver assignment,
> > but I wanted to try out your suggestion first)
> 
> Class device can never have a driver. And unregistered drivers can
> not be used, what you try to do here. That all should just be removed or
> properly registered with the core if needed.

Yeah but it shouldn't influence the operation? Anyway I see from your
patch that I should have assigned the release function and that would've
helped I guess.

> > So I removed bus/driver assignment from the above code just to try it
> > out, and got
> >
> > Device 'hwsim0' does not have a release() function, it is broken and
> > must be fixed.
> 
> The driver core expects the resources of the device to be freed on
> release. You can provide an empty release function because you do this
> from a global list of devices.

Ok, makes sense.

> > This has evolved far too much for me right now. Can we just apply the
> > initial patch from Eric and be happier for a while? I can't justify
> > spending this much time on it right now. Alternatively, you could look
> > at hwsim too, since it's all virtual, nothing special is required, I do
> > testing in a virtual machine ...
> >
> > current patch is at
> > http://johannes.sipsolutions.net/patches/kernel/all/2010-06-07-11:41/hwsim-bus.patch
> 
> Here is something that seems to work for me.

I see you remove the driver. Does this mean that in sysfs these devices
wouldn't have a driver symlink? ISTR that we needed that for userspace,
but I'm not entirely sure, and I don't have all the relevant userspace
in my test setup.

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Mon, Jun 7, 2010 at 14:36, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2010-06-07 at 14:26 +0200, Kay Sievers wrote:
>> On Mon, Jun 7, 2010 at 13:41, Johannes Berg <johannes@sipsolutions.net> wrote:
>> > (mind you, I think we probably need to have the bus/driver assignment,
>> > but I wanted to try out your suggestion first)
>>
>> Class device can never have a driver. And unregistered drivers can
>> not be used, what you try to do here. That all should just be removed or
>> properly registered with the core if needed.
>
> Yeah but it shouldn't influence the operation?

It does. You can not use "dead static" drivers. They need to be
properly registered, and can only work for bus devices. Class devices
are not allowed to have drivers.

This worked only because the core ignored the value you assigned
behind its back. :)

>> Here is something that seems to work for me.
>
> I see you remove the driver. Does this mean that in sysfs these devices
> wouldn't have a driver symlink? ISTR that we needed that for userspace,
> but I'm not entirely sure, and I don't have all the relevant userspace
> in my test setup.

Yeah, if you need the driver, you need to use bus device, as you do
now. The driver needs to be registered with the core, and the probe
function needs to tell to bind to the devices while registered. You
can not manually assign this.

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Mon, 2010-06-07 at 14:54 +0200, Kay Sievers wrote:

> > I see you remove the driver. Does this mean that in sysfs these devices
> > wouldn't have a driver symlink? ISTR that we needed that for userspace,
> > but I'm not entirely sure, and I don't have all the relevant userspace
> > in my test setup.
> 
> Yeah, if you need the driver, you need to use bus device, as you do
> now. The driver needs to be registered with the core, and the probe
> function needs to tell to bind to the devices while registered. You
> can not manually assign this.

So I spoke to Jouni and he says that he doesn't remember needing the
driver, so I guess we don't care. Can I get you to submit that patch
properly? Send it to linux-wireless@vger.kernel.org please.

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Tue, Jun 8, 2010 at 11:27, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2010-06-07 at 14:54 +0200, Kay Sievers wrote:
>
>> > I see you remove the driver. Does this mean that in sysfs these devices
>> > wouldn't have a driver symlink? ISTR that we needed that for userspace,
>> > but I'm not entirely sure, and I don't have all the relevant userspace
>> > in my test setup.
>>
>> Yeah, if you need the driver, you need to use bus device, as you do
>> now. The driver needs to be registered with the core, and the probe
>> function needs to tell to bind to the devices while registered. You
>> can not manually assign this.
>
> So I spoke to Jouni and he says that he doesn't remember needing the
> driver, so I guess we don't care. Can I get you to submit that patch
> properly? Send it to linux-wireless@vger.kernel.org please.

Sure, you mean the patch that changes the hwsim code, or the one I
talked about to properly create a virtual bus parent?

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Tue, 2010-06-08 at 11:30 +0200, Kay Sievers wrote:
> On Tue, Jun 8, 2010 at 11:27, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Mon, 2010-06-07 at 14:54 +0200, Kay Sievers wrote:
> >
> >> > I see you remove the driver. Does this mean that in sysfs these devices
> >> > wouldn't have a driver symlink? ISTR that we needed that for userspace,
> >> > but I'm not entirely sure, and I don't have all the relevant userspace
> >> > in my test setup.
> >>
> >> Yeah, if you need the driver, you need to use bus device, as you do
> >> now. The driver needs to be registered with the core, and the probe
> >> function needs to tell to bind to the devices while registered. You
> >> can not manually assign this.
> >
> > So I spoke to Jouni and he says that he doesn't remember needing the
> > driver, so I guess we don't care. Can I get you to submit that patch
> > properly? Send it to linux-wireless@vger.kernel.org please.
> 
> Sure, you mean the patch that changes the hwsim code, or the one I
> talked about to properly create a virtual bus parent?

The hwsim one you pasted earlier.

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Tue, 2010-06-08 at 11:45 +0200, Johannes Berg wrote:
> On Tue, 2010-06-08 at 11:30 +0200, Kay Sievers wrote:
> > On Tue, Jun 8, 2010 at 11:27, Johannes Berg <johannes@sipsolutions.net> wrote:
> > > On Mon, 2010-06-07 at 14:54 +0200, Kay Sievers wrote:
> > >
> > >> > I see you remove the driver. Does this mean that in sysfs these devices
> > >> > wouldn't have a driver symlink? ISTR that we needed that for userspace,
> > >> > but I'm not entirely sure, and I don't have all the relevant userspace
> > >> > in my test setup.
> > >>
> > >> Yeah, if you need the driver, you need to use bus device, as you do
> > >> now. The driver needs to be registered with the core, and the probe
> > >> function needs to tell to bind to the devices while registered. You
> > >> can not manually assign this.
> > >
> > > So I spoke to Jouni and he says that he doesn't remember needing the
> > > driver, so I guess we don't care. Can I get you to submit that patch
> > > properly? Send it to linux-wireless@vger.kernel.org please.
> > 
> > Sure, you mean the patch that changes the hwsim code, or the one I
> > talked about to properly create a virtual bus parent?
> 
> The hwsim one you pasted earlier.

Ok, this needs testing.

Please let me know, if that works for you so far.

Thanks,
Kay


From: Kay Sievers <kay.sievers@vrfy.org>
Subject: mac80211_hwsim - convert class devices to bus devices

Class devices must never have child devices of a different class,
so use bus devices.

Embed the 'struct device' into mac80211_hwsim_data, to do proper
ressource lifetime management at device release time.

Registers a driver, to binds devices to the bus.

Use a virtual root as the parent in /sys/devices/.

Puts the monitor device under the virtual parent.

This creates the following devices (udevadm monitor):
  /module/mac80211_hwsim (module)
  /bus/mac80211_hwsim (bus)
  /bus/mac80211_hwsim/drivers/mac80211_hwsim (drivers)
  /devices/mac80211_hwsim/hwsim0 (mac80211_hwsim)
  /devices/mac80211_hwsim/hwsim0/ieee80211/phy17 (ieee80211)
  /devices/mac80211_hwsim/hwsim0/ieee80211/phy17/rfkill19 (rfkill)
  /devices/mac80211_hwsim/hwsim0/net/wlan9 (net)
  /devices/mac80211_hwsim/hwsim0/net/wlan9/queues/rx-0 (queues)
  /devices/mac80211_hwsim/hwsim0/net/wlan9/queues/rx-1 (queues)
  /devices/mac80211_hwsim/hwsim0/net/wlan9/queues/rx-2 (queues)
  /devices/mac80211_hwsim/hwsim0/net/wlan9/queues/rx-3 (queues)
  /devices/mac80211_hwsim/hwsim1 (mac80211_hwsim)
  /devices/mac80211_hwsim/hwsim1/ieee80211/phy18 (ieee80211)
  /devices/mac80211_hwsim/hwsim1/ieee80211/phy18/rfkill20 (rfkill)
  /devices/mac80211_hwsim/hwsim1/net/wlan10 (net)
  /devices/mac80211_hwsim/hwsim1/net/wlan10/queues/rx-0 (queues)
  /devices/mac80211_hwsim/hwsim1/net/wlan10/queues/rx-1 (queues)
  /devices/mac80211_hwsim/hwsim1/net/wlan10/queues/rx-2 (queues)
  /devices/mac80211_hwsim/hwsim1/net/wlan10/queues/rx-3 (queues)
  /devices/mac80211_hwsim/net/hwsim0 (net)
  /devices/mac80211_hwsim/net/hwsim0/queues/rx-0 (queues)

Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 6f8cb3e..2543e74 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -194,7 +194,16 @@ static inline void hwsim_clear_sta_magic(struct ieee80211_sta *sta)
 	sp->magic = 0;
 }
 
-static struct class *hwsim_class;
+static struct bus_type hwsim_bus = {
+	.name = "mac80211_hwsim",
+};
+
+static struct device_driver hwsim_driver = {
+	.name = "mac80211_hwsim",
+	.bus = &hwsim_bus,
+};
+
+static struct device *hwsim_virtual_parent;
 
 static struct net_device *hwsim_mon; /* global monitor netdev */
 
@@ -280,7 +289,7 @@ static struct list_head hwsim_radios;
 struct mac80211_hwsim_data {
 	struct list_head list;
 	struct ieee80211_hw *hw;
-	struct device *dev;
+	struct device dev;
 	struct ieee80211_supported_band bands[2];
 	struct ieee80211_channel channels_2ghz[ARRAY_SIZE(hwsim_channels_2ghz)];
 	struct ieee80211_channel channels_5ghz[ARRAY_SIZE(hwsim_channels_5ghz)];
@@ -1063,22 +1072,13 @@ static void mac80211_hwsim_free(void)
 		list_move(i, &tmplist);
 	spin_unlock_bh(&hwsim_radio_lock);
 
-	list_for_each_entry_safe(data, tmpdata, &tmplist, list) {
-		debugfs_remove(data->debugfs_group);
-		debugfs_remove(data->debugfs_ps);
-		debugfs_remove(data->debugfs);
-		ieee80211_unregister_hw(data->hw);
-		device_unregister(data->dev);
-		ieee80211_free_hw(data->hw);
-	}
-	class_destroy(hwsim_class);
+	list_for_each_entry_safe(data, tmpdata, &tmplist, list)
+		device_unregister(&data->dev);
+	device_unregister(hwsim_virtual_parent);
+	driver_unregister(&hwsim_driver);
+	bus_unregister(&hwsim_bus);
 }
 
-
-static struct device_driver mac80211_hwsim_driver = {
-	.name = "mac80211_hwsim"
-};
-
 static const struct net_device_ops hwsim_netdev_ops = {
 	.ndo_start_xmit 	= hwsim_mon_xmit,
 	.ndo_change_mtu		= eth_change_mtu,
@@ -1095,6 +1095,7 @@ static void hwsim_mon_setup(struct net_device *dev)
 	dev->type = ARPHRD_IEEE80211_RADIOTAP;
 	memset(dev->dev_addr, 0, ETH_ALEN);
 	dev->dev_addr[0] = 0x12;
+	dev->dev.parent = hwsim_virtual_parent;
 }
 
 
@@ -1231,6 +1232,17 @@ DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_group,
 			hwsim_fops_group_read, hwsim_fops_group_write,
 			"%llx\n");
 
+static void hwsim_dev_release(struct device* hwsim_dev) {
+	struct mac80211_hwsim_data *data =
+	  container_of(hwsim_dev, struct mac80211_hwsim_data, dev);
+
+	debugfs_remove(data->debugfs_group);
+	debugfs_remove(data->debugfs_ps);
+	debugfs_remove(data->debugfs);
+	ieee80211_unregister_hw(data->hw);
+	ieee80211_free_hw(data->hw);
+}
+
 static int __init init_mac80211_hwsim(void)
 {
 	int i, err = 0;
@@ -1251,9 +1263,22 @@ static int __init init_mac80211_hwsim(void)
 	spin_lock_init(&hwsim_radio_lock);
 	INIT_LIST_HEAD(&hwsim_radios);
 
-	hwsim_class = class_create(THIS_MODULE, "mac80211_hwsim");
-	if (IS_ERR(hwsim_class))
-		return PTR_ERR(hwsim_class);
+	err = bus_register(&hwsim_bus);
+	if (err)
+		return err;
+
+	err = driver_register(&hwsim_driver);
+	if (err) {
+		bus_unregister(&hwsim_bus);
+		return err;
+	}
+
+	hwsim_virtual_parent = root_device_register("mac80211_hwsim");
+	if (IS_ERR(hwsim_virtual_parent)) {
+		driver_unregister(&hwsim_driver);
+		bus_unregister(&hwsim_bus);
+		return PTR_ERR(hwsim_virtual_parent);
+	}
 
 	memset(addr, 0, ETH_ALEN);
 	addr[0] = 0x02;
@@ -1271,18 +1296,19 @@ static int __init init_mac80211_hwsim(void)
 		data = hw->priv;
 		data->hw = hw;
 
-		data->dev = device_create(hwsim_class, NULL, 0, hw,
-					  "hwsim%d", i);
-		if (IS_ERR(data->dev)) {
+		dev_set_name(&data->dev, "hwsim%d", i);
+		data->dev.parent = hwsim_virtual_parent;
+		data->dev.bus = &hwsim_bus;
+		data->dev.release = hwsim_dev_release;
+		err = device_register(&data->dev);
+		if (err) {
 			printk(KERN_DEBUG
-			       "mac80211_hwsim: device_create "
-			       "failed (%ld)\n", PTR_ERR(data->dev));
-			err = -ENOMEM;
+			       "mac80211_hwsim: device_register failed (%d)\n",
+			       err);
 			goto failed_drvdata;
 		}
-		data->dev->driver = &mac80211_hwsim_driver;
 
-		SET_IEEE80211_DEV(hw, data->dev);
+		SET_IEEE80211_DEV(hw, &data->dev);
 		addr[3] = i >> 8;
 		addr[4] = i;
 		memcpy(data->addresses[0].addr, addr, ETH_ALEN);
@@ -1513,7 +1539,7 @@ failed_mon:
 	return err;
 
 failed_hw:
-	device_unregister(data->dev);
+	device_unregister(&data->dev);
 failed_drvdata:
 	ieee80211_free_hw(hw);
 failed:

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Tue, 2010-06-08 at 13:55 +0200, Kay Sievers wrote:

> Ok, this needs testing.
> 
> Please let me know, if that works for you so far.

Will do. Just a question: none of this seems to pin the module? So can I
be sure if I rmmod the module that the release function will still be
around etc.?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Tue, Jun 8, 2010 at 14:03, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2010-06-08 at 13:55 +0200, Kay Sievers wrote:
>
>> Ok, this needs testing.
>>
>> Please let me know, if that works for you so far.
>
> Will do. Just a question: none of this seems to pin the module? So can I
> be sure if I rmmod the module that the release function will still be
> around etc.?

Oh, sorry, that's something very specific to network devices, that the
module can be unloaded while the devices it has created are still in
use. I am not sure right now what's needed to make this work in a
single module.

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Tue, 2010-06-08 at 15:54 +0200, Kay Sievers wrote:
> On Tue, Jun 8, 2010 at 14:03, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Tue, 2010-06-08 at 13:55 +0200, Kay Sievers wrote:
> >
> >> Ok, this needs testing.
> >>
> >> Please let me know, if that works for you so far.
> >
> > Will do. Just a question: none of this seems to pin the module? So can I
> > be sure if I rmmod the module that the release function will still be
> > around etc.?
> 
> Oh, sorry, that's something very specific to network devices, that the
> module can be unloaded while the devices it has created are still in
> use. I am not sure right now what's needed to make this work in a
> single module.

Well they will be unregistered and everything, but once all the netdevs
are gone etc. the devices you create in the patch might stick around
because somebody has them open in sysfs, and I see nothing that would
pin the module in that case?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Tue, Jun 8, 2010 at 16:06, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2010-06-08 at 15:54 +0200, Kay Sievers wrote:
>> On Tue, Jun 8, 2010 at 14:03, Johannes Berg <johannes@sipsolutions.net> wrote:
>> > On Tue, 2010-06-08 at 13:55 +0200, Kay Sievers wrote:
>> >
>> >> Ok, this needs testing.
>> >>
>> >> Please let me know, if that works for you so far.
>> >
>> > Will do. Just a question: none of this seems to pin the module? So can I
>> > be sure if I rmmod the module that the release function will still be
>> > around etc.?
>>
>> Oh, sorry, that's something very specific to network devices, that the
>> module can be unloaded while the devices it has created are still in
>> use. I am not sure right now what's needed to make this work in a
>> single module.
>
> Well they will be unregistered and everything, but once all the netdevs
> are gone etc. the devices you create in the patch might stick around
> because somebody has them open in sysfs, and I see nothing that would
> pin the module in that case?

That's what I mean, I have no idea how to solve that with network
devices. I don't think any other subsytem allows to unload the module,
when devices, the module has created, are in use.

With this patch, it is likely to fail, even without sysfs, just when
the netif is still in up, and the module is unloaded.

The current code uses the in-core class_create() logic, which was only
meant for devices with a device node, and which is cleaned up by the
core itself. That's why this issue never appeared before.

But as said, I have no idea how to solve that with a single module. It
might not work at all without moving stuff into the core. That people
use device_create() with no major/minor might indicate that something
else is needed here. :)

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Tue, 2010-06-08 at 16:21 +0200, Kay Sievers wrote:

> > Well they will be unregistered and everything, but once all the netdevs
> > are gone etc. the devices you create in the patch might stick around
> > because somebody has them open in sysfs, and I see nothing that would
> > pin the module in that case?
> 
> That's what I mean, I have no idea how to solve that with network
> devices. I don't think any other subsytem allows to unload the module,
> when devices, the module has created, are in use.

You're right ... the would only be unregistered from your release, which
would happen after the module is long gone ...

> The current code uses the in-core class_create() logic, which was only
> meant for devices with a device node, and which is cleaned up by the
> core itself. That's why this issue never appeared before.
> 
> But as said, I have no idea how to solve that with a single module. It
> might not work at all without moving stuff into the core. That people
> use device_create() with no major/minor might indicate that something
> else is needed here. :)

So ... can we apply Eric's patch for now then?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Tue, Jun 8, 2010 at 16:26, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2010-06-08 at 16:21 +0200, Kay Sievers wrote:
>
>> > Well they will be unregistered and everything, but once all the netdevs
>> > are gone etc. the devices you create in the patch might stick around
>> > because somebody has them open in sysfs, and I see nothing that would
>> > pin the module in that case?
>>
>> That's what I mean, I have no idea how to solve that with network
>> devices. I don't think any other subsytem allows to unload the module,
>> when devices, the module has created, are in use.
>
> You're right ... the would only be unregistered from your release, which
> would happen after the module is long gone ...
>
>> The current code uses the in-core class_create() logic, which was only
>> meant for devices with a device node, and which is cleaned up by the
>> core itself. That's why this issue never appeared before.
>>
>> But as said, I have no idea how to solve that with a single module. It
>> might not work at all without moving stuff into the core. That people
>> use device_create() with no major/minor might indicate that something
>> else is needed here. :)
>
> So ... can we apply Eric's patch for now then?

It might break other stuff we don't know about yet. Just like we did
not know about what things hwsim is doing here. :)

Eric's patch change the sysfs layout and insert directories into the
device path, and stuff which hardcodes things, or accesses devices
with ../<name> will break.

I don't mind trying it, but I wouldn't be surprised if that needs to
be reverted when issues caused by this appear.


The hwsim issues are caused by the current hwsim code, by doing things
it should not do. Class devices of different classes must never be
stacked (the core should not allow that in the first place). Class
devices must never have a driver assigned behind its back. Also
device_create() should not be used for devices without a major/minor
(but that seems to be done in several other places too).

To fix the hwsim driver core interaction, core changes will probably
be needed to allow network modules to be removed while their devices
are active. That's something which seems not to work for bus devices
currently.

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Tue, 2010-06-08 at 16:47 +0200, Kay Sievers wrote:

> > So ... can we apply Eric's patch for now then?
> 
> It might break other stuff we don't know about yet. Just like we did
> not know about what things hwsim is doing here. :)

True.

> The hwsim issues are caused by the current hwsim code, by doing things
> it should not do. Class devices of different classes must never be
> stacked (the core should not allow that in the first place). Class
> devices must never have a driver assigned behind its back. Also
> device_create() should not be used for devices without a major/minor
> (but that seems to be done in several other places too).

Back when hwsim was written that would have been useful feedback to
whoever did ... now, not so much.

> To fix the hwsim driver core interaction, core changes will probably
> be needed to allow network modules to be removed while their devices
> are active. That's something which seems not to work for bus devices
> currently.

Well it just needs to pin the module refcount from the bus and/or device
struct. That seems not too hard?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Tue, Jun 8, 2010 at 17:06, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2010-06-08 at 16:47 +0200, Kay Sievers wrote:
>
>> > So ... can we apply Eric's patch for now then?
>>
>> It might break other stuff we don't know about yet. Just like we did
>> not know about what things hwsim is doing here. :)
>
> True.
>
>> The hwsim issues are caused by the current hwsim code, by doing things
>> it should not do. Class devices of different classes must never be
>> stacked (the core should not allow that in the first place). Class
>> devices must never have a driver assigned behind its back. Also
>> device_create() should not be used for devices without a major/minor
>> (but that seems to be done in several other places too).
>
> Back when hwsim was written that would have been useful feedback to
> whoever did ... now, not so much.

Well, it still needs to be fixed. It will break again, when stuff
changes like now.

>> To fix the hwsim driver core interaction, core changes will probably
>> be needed to allow network modules to be removed while their devices
>> are active. That's something which seems not to work for bus devices
>> currently.
>
> Well it just needs to pin the module refcount from the bus and/or device
> struct. That seems not too hard?

How? If we ever get a reference of the module, it will not be able to
be unloaded while stuff is active, right?

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Tue, 2010-06-08 at 18:26 +0200, Kay Sievers wrote:

> >> To fix the hwsim driver core interaction, core changes will probably
> >> be needed to allow network modules to be removed while their devices
> >> are active. That's something which seems not to work for bus devices
> >> currently.
> >
> > Well it just needs to pin the module refcount from the bus and/or device
> > struct. That seems not too hard?
> 
> How? If we ever get a reference of the module, it will not be able to
> be unloaded while stuff is active, right?

Hmm, true, but how does this work with class devices it has now? It
seems to work fine now. It's because we have a generic release function
there I guess?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Tue, Jun 8, 2010 at 18:33, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2010-06-08 at 18:26 +0200, Kay Sievers wrote:
>
>> >> To fix the hwsim driver core interaction, core changes will probably
>> >> be needed to allow network modules to be removed while their devices
>> >> are active. That's something which seems not to work for bus devices
>> >> currently.
>> >
>> > Well it just needs to pin the module refcount from the bus and/or device
>> > struct. That seems not too hard?
>>
>> How? If we ever get a reference of the module, it will not be able to
>> be unloaded while stuff is active, right?
>
> Hmm, true, but how does this work with class devices it has now? It
> seems to work fine now. It's because we have a generic release function
> there I guess?

Because the code to cleanup is always built into the core, yes. Even
the empty release function thing we can not do. :)

That all works if you have two modules, like almost all buses have.
That's what I meant, that we need to add stuff to the core to be able
to cleanup bus devices internally too, if we use everything in a
single module, which is also supposed to cleanup on unload, like the
network devices like to do.

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Tue, 2010-06-08 at 18:39 +0200, Kay Sievers wrote:

> That all works if you have two modules, like almost all buses have.
> That's what I meant, that we need to add stuff to the core to be able
> to cleanup bus devices internally too, if we use everything in a
> single module, which is also supposed to cleanup on unload, like the
> network devices like to do.

Or some "wait for bus to be cleaned up" we can call in the module exit
maybe?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Fri, Jun 11, 2010 at 11:55, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2010-06-08 at 18:39 +0200, Kay Sievers wrote:
>
>> That all works if you have two modules, like almost all buses have.
>> That's what I meant, that we need to add stuff to the core to be able
>> to cleanup bus devices internally too, if we use everything in a
>> single module, which is also supposed to cleanup on unload, like the
>> network devices like to do.
>
> Or some "wait for bus to be cleaned up" we can call in the module exit
> maybe?

That would block the rmmod process until the resources are cleaned up,
wouldn't it?

The network devices can do this because the cleanup code is always
compiled-in, for a module cleaning up itself, this is kind of
complicated, isn't it?

Kay

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Johannes Berg]

On Mon, 2010-06-14 at 11:13 +0200, Kay Sievers wrote:

> That would block the rmmod process until the resources are cleaned up,
> wouldn't it?

Yes, would that be so bad?

> The network devices can do this because the cleanup code is always
> compiled-in, for a module cleaning up itself, this is kind of
> complicated, isn't it?

It just needs a wait_for_bus_exit() function that the module calls in
_exit?

johannes

Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes

[Kay Sievers]

On Mon, Jun 14, 2010 at 11:20, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2010-06-14 at 11:13 +0200, Kay Sievers wrote:
>
>> That would block the rmmod process until the resources are cleaned up,
>> wouldn't it?
>
> Yes, would that be so bad?

Sounds fine, if we can expect that nothing else can hold a reference
and may block rmmod for a very long time with this logic. Sysfs access
should be fine these days, Tejun changed this a while ago.

> It just needs a wait_for_bus_exit() function that the module calls in
> _exit?

Sounds worth trying, to see if that works as expected.

Kay

[PATCH] Driver-core: Always create class directories fixing the broken network drivers.

[Eric W. Biederman]

In get_device_parent there is a check to not add a class directory
when a class device was put under another class device.  The check was
put in place as a just in case measure to not break old userspace if
any existing code happened to depend on it.  Currently the only known
way that we get a class device under a class device is due to the
rearrangement of devices that happened when the new sysfs layout was
introduced.

With the introduction of tagged sysfs directories for properly
handling network namespace support this omission in creating the class
directories went from a bad thing in terms of namespace pollution, to
actually breaking device_remove.

Currently there are two reported network device drivers that break
because the class directory was not created by the device layer.  The
usb bnep driver and the mac80211_hwsim driver.

Every solution proposed changes the sysfs layout for the affected
devices, and thus has the potential to break userspace.

Since we are changing the sysfs layout anyway, and since we are now
talking about several devices all with the same problem, all caused by
the same over conservative bit of code.  Let's kill that bit of code.

There have been other proposals to fix this but they all have been
more complicated, and none of them have actually resulted in working
code.

Any userspace that works with both the old and the new sysfs layouts
should not be affected by this change, and even if someone depends
on it we are talking a very small number of drivers overall that
are affected.

My apologoies for not fully catching this hole in the logic the
when this code was originally added.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 drivers/base/core.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 9630fbd..7b1c4d4 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -673,8 +673,6 @@ static struct kobject *get_device_parent(struct device *dev,
 		 */
 		if (parent == NULL)
 			parent_kobj = virtual_device_parent(dev);
-		else if (parent->class)
-			return &parent->kobj;
 		else
 			parent_kobj = &parent->kobj;
 
-- 
1.6.5.2.143.g8cc62

Re: [PATCH] Driver-core: Always create class directories fixing the broken network drivers.

[Kay Sievers]

On Sun, Jun 20, 2010 at 08:20, Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> In get_device_parent there is a check to not add a class directory
> when a class device was put under another class device.  The check was
> put in place as a just in case measure to not break old userspace if
> any existing code happened to depend on it.  Currently the only known
> way that we get a class device under a class device is due to the
> rearrangement of devices that happened when the new sysfs layout was
> introduced.
>
> With the introduction of tagged sysfs directories for properly
> handling network namespace support this omission in creating the class
> directories went from a bad thing in terms of namespace pollution, to
> actually breaking device_remove.
>
> Currently there are two reported network device drivers that break
> because the class directory was not created by the device layer.  The
> usb bnep driver and the mac80211_hwsim driver.
>
> Every solution proposed changes the sysfs layout for the affected
> devices, and thus has the potential to break userspace.
>
> Since we are changing the sysfs layout anyway, and since we are now
> talking about several devices all with the same problem, all caused by
> the same over conservative bit of code.  Let's kill that bit of code.
>
> There have been other proposals to fix this but they all have been
> more complicated, and none of them have actually resulted in working
> code.
>
> Any userspace that works with both the old and the new sysfs layouts
> should not be affected by this change, and even if someone depends
> on it we are talking a very small number of drivers overall that
> are affected.
>
> My apologoies for not fully catching this hole in the logic the
> when this code was originally added.

We can not do this. Simply comparing the sysfs tree before and after
shows that it breaks 'input'. inputX and mouseX are now spearated by a
subdirectory, which is wrong.

As mentioned earlier, It's pretty fragile to change things in this
area, and I prefer the broken network driver-core interactions to be
fixed instead - even when they are more complicated.

Thanks,
Kay

Re: [PATCH] Driver-core: Always create class directories fixing the broken network drivers.

[Johannes Berg]

On Sun, 2010-06-20 at 12:52 +0200, Kay Sievers wrote:

> As mentioned earlier, It's pretty fragile to change things in this
> area, and I prefer the broken network driver-core interactions to be
> fixed instead - even when they are more complicated.

Can you _please_ offer a proper way to fix it then?

johannes

Re: [PATCH] Driver-core: Always create class directories fixing the broken network drivers.

[Kay Sievers]

On Sun, Jun 20, 2010 at 13:33, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Sun, 2010-06-20 at 12:52 +0200, Kay Sievers wrote:
>
>> As mentioned earlier, It's pretty fragile to change things in this
>> area, and I prefer the broken network driver-core interactions to be
>> fixed instead - even when they are more complicated.
>
> Can you _please_ offer a proper way to fix it then?

Sorry, I have no real experience with the issues created by the
assumption that network driver need to be able to get unloaded while
in use. That's very special, always requires a
compiled-into-the-kernel part of the subsystem, and makes it hard to
work with, as we can not use any of the usual core infrastructure to
solve that.

The only real simple thing that works is splitting the module in two
modules, which isn't really something I would propose.

Maybe the wait-for in the module-exit like your recent mail suggests
works, but I did not try that. Otherwise we can solve this by changing
the net driver and by adding some needed stuff to the core to allow
in-core bus device cleanup.

The class device hierarchy should be removed for proper network
namespace support, it's nothing we properly support with the current
core code. We better don't fiddle around with stuff nobody really
knows what it breaks. Just like I ran into the 'input' stuff now,
which was a really simple case to find.

Kay

Re: [PATCH] Driver-core: Always create class directories fixing the broken network drivers.

[Eric W. Biederman]

Kay Sievers <kay.sievers@vrfy.org> writes:

> On Sun, Jun 20, 2010 at 13:33, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Sun, 2010-06-20 at 12:52 +0200, Kay Sievers wrote:
>>
>>> As mentioned earlier, It's pretty fragile to change things in this
>>> area, and I prefer the broken network driver-core interactions to be
>>> fixed instead - even when they are more complicated.
>>
>> Can you _please_ offer a proper way to fix it then?
>
> Sorry, I have no real experience with the issues created by the
> assumption that network driver need to be able to get unloaded while
> in use. That's very special, always requires a
> compiled-into-the-kernel part of the subsystem, and makes it hard to
> work with, as we can not use any of the usual core infrastructure to
> solve that.

So please look at https://bugzilla.kernel.org/show_bug.cgi?id=16215

That simply creates and destroys the network device as things come
and go.

I think the bnep case is much more serious because it is real hardware
not a testing simulation, and it is the second instance of this.

Calling the change broken when I can boot up and run X in that
configuration just fine is a vast overstatement.  Especially
when you don't acknowledge that the device layer is broken.

I will agree that insane amounts of backwards compatibility are a good
idea.  So I will cook up a version of my patch that adds a hack to the
device layer to only apply this change to devices of class net.

That should save let us postpone the architectural dreams for another
day.

Eric

Re: [PATCH] Driver-core: Always create class directories fixing the broken network drivers.

[Kay Sievers]

On Sun, Jun 20, 2010 at 14:29, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Kay Sievers <kay.sievers@vrfy.org> writes:
>> On Sun, Jun 20, 2010 at 13:33, Johannes Berg <johannes@sipsolutions.net> wrote:
>>> On Sun, 2010-06-20 at 12:52 +0200, Kay Sievers wrote:
>>>
>>>> As mentioned earlier, It's pretty fragile to change things in this
>>>> area, and I prefer the broken network driver-core interactions to be
>>>> fixed instead - even when they are more complicated.
>>>
>>> Can you _please_ offer a proper way to fix it then?
>>
>> Sorry, I have no real experience with the issues created by the
>> assumption that network driver need to be able to get unloaded while
>> in use. That's very special, always requires a
>> compiled-into-the-kernel part of the subsystem, and makes it hard to
>> work with, as we can not use any of the usual core infrastructure to
>> solve that.
>
> So please look at https://bugzilla.kernel.org/show_bug.cgi?id=16215
>
> That simply creates and destroys the network device as things come
> and go.

I'm still not sure, any help here would be appreciated.

> I think the bnep case is much more serious because it is real hardware
> not a testing simulation, and it is the second instance of this.
>
> Calling the change broken when I can boot up and run X in that
> configuration just fine is a vast overstatement.

Oh, I seriously would love this rule - it would make my work so much
easier. But I need to make it totally clear: "Adding intermediate
directories into 'input' sysfs it absolutely broken, regardless if
your box comes up or not. :)

X is using udev, and udev aggressively hides these details and forbids
matching such details, but many other tools which read sysfs directly,
including ones using the conceptually broken 'device' symlink will for
sure break with such changes.

> Especially
> when you don't acknowledge that the device layer is broken.

Stacking devices from different classes is broken, and not a direct
problem of the core. It is just not supported. The core might just
need to refuse that in the first place, but that's a different issue.

> I will agree that insane amounts of backwards compatibility are a good
> idea.  So I will cook up a version of my patch that adds a hack to the
> device layer to only apply this change to devices of class net.
>
> That should save let us postpone the architectural dreams for another
> day.

It's not a dream, it needs to be fixed where it is used. We can not
allow to stack classes.

Kay

[PATCH] Driver-core: Always create network class directories in get_device_parent.

[Eric W. Biederman]

In get_device_parent there was added check to not add a class
directory when a class device was put under another class device.  The
check was put in place as a just in case measure to not break old
userspace if any existing code happened to depend on it.  Devices in
the input subsystem are affected by this code path so there is a
reasonable chance that some piece of user space will break if we just
remove this kludge.

At the same time there are at least two network drivers that have
potential unnecessary namespace conflicts because class directories
have not been created for their network devices.

With the introduction of tagged sysfs directories for properly
handling network namespace support this omission in creating the class
directories went from a bad thing in terms of namespace pollution, to
actually breaking device_remove.

Currently there are two reported network device drivers that break
because the class directory was not created by the device layer.  The
usb bnep driver and the mac80211_hwsim driver.

Every solution proposed changes the sysfs layout for the affected
devices, and thus has the potential to break userspace.

Since we are changing the sysfs layout anyway, and since we are now
talking about several devices all with the same problem, all caused by
the same over convservative bit of code.  Let's fix the device layer
for network devices.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 drivers/base/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 9630fbd..ffb8443 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -673,7 +673,7 @@ static struct kobject *get_device_parent(struct device *dev,
 		 */
 		if (parent == NULL)
 			parent_kobj = virtual_device_parent(dev);
-		else if (parent->class)
+		else if (parent->class && (strcmp(dev->class->name, "net") != 0))
 			return &parent->kobj;
 		else
 			parent_kobj = &parent->kobj;
-- 
1.6.5.2.143.g8cc62

Re: [PATCH] Driver-core: Always create network class directories in get_device_parent.

[Greg KH]

On Sun, Jun 20, 2010 at 05:46:54AM -0700, Eric W. Biederman wrote:
> 
> In get_device_parent there was added check to not add a class
> directory when a class device was put under another class device.  The
> check was put in place as a just in case measure to not break old
> userspace if any existing code happened to depend on it.  Devices in
> the input subsystem are affected by this code path so there is a
> reasonable chance that some piece of user space will break if we just
> remove this kludge.
> 
> At the same time there are at least two network drivers that have
> potential unnecessary namespace conflicts because class directories
> have not been created for their network devices.
> 
> With the introduction of tagged sysfs directories for properly
> handling network namespace support this omission in creating the class
> directories went from a bad thing in terms of namespace pollution, to
> actually breaking device_remove.
> 
> Currently there are two reported network device drivers that break
> because the class directory was not created by the device layer.  The
> usb bnep driver and the mac80211_hwsim driver.
> 
> Every solution proposed changes the sysfs layout for the affected
> devices, and thus has the potential to break userspace.
> 
> Since we are changing the sysfs layout anyway, and since we are now
> talking about several devices all with the same problem, all caused by
> the same over convservative bit of code.  Let's fix the device layer
> for network devices.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  drivers/base/core.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 9630fbd..ffb8443 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -673,7 +673,7 @@ static struct kobject *get_device_parent(struct device *dev,
>  		 */
>  		if (parent == NULL)
>  			parent_kobj = virtual_device_parent(dev);
> -		else if (parent->class)
> +		else if (parent->class && (strcmp(dev->class->name, "net") != 0))

No, we will not have subsystem specific hacks in the driver core like
this.  What would cause us to ever be able to delete this line?

thanks,

greg k-h

Re: [PATCH] Driver-core: Always create network class directories in get_device_parent.

[Eric W. Biederman]

Greg KH <greg@kroah.com> writes:

>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 9630fbd..ffb8443 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -673,7 +673,7 @@ static struct kobject *get_device_parent(struct device *dev,
>>  		 */
>>  		if (parent == NULL)
>>  			parent_kobj = virtual_device_parent(dev);
>> -		else if (parent->class)
>> +		else if (parent->class && (strcmp(dev->class->name, "net") != 0))
>
> No, we will not have subsystem specific hacks in the driver core like
> this.  What would cause us to ever be able to delete this line?

That entire if clause is a hack.  I have just made it narrower.

If it were actually a serious possibility to convert these drivers with the
existing device core I would be happy to see something else.

Kyle and Johannes went back and forth and could not figure out how to
correctly convert the mac80211_hwsim driver to a bus device, the support
is not in the device core.

Eric