All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Greg KH <greg@kroah.com>, netdev <netdev@vger.kernel.org>
Subject: Re: sysfs class/net/ problem
Date: Wed, 02 Jun 2010 16:09:21 -0700	[thread overview]
Message-ID: <m16321t4ke.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <1275506732.3915.41.camel@jlt3.sipsolutions.net> (Johannes Berg's message of "Wed\, 02 Jun 2010 21\:25\:32 +0200")

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


  reply	other threads:[~2010-06-02 23:09 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-02 13:16 sysfs class/net/ problem Johannes Berg
2010-06-02 15:46 ` Greg KH
2010-06-02 15:48   ` Johannes Berg
2010-06-02 16:17     ` Eric W. Biederman
2010-06-02 16:21       ` Johannes Berg
2010-06-02 16:43         ` Eric W. Biederman
2010-06-02 17:00           ` Johannes Berg
2010-06-02 17:23             ` Eric W. Biederman
2010-06-02 17:52               ` Johannes Berg
2010-06-02 18:05                 ` Eric W. Biederman
2010-06-02 18:55                   ` Johannes Berg
2010-06-02 19:12                     ` Johannes Berg
2010-06-02 19:25                   ` Johannes Berg
2010-06-02 23:09                     ` Eric W. Biederman [this message]
2010-06-03  0:53                       ` [RFC][PATCH] Fix another namespace issue with devices assigned to classes Eric W. Biederman
2010-06-03  9:30                         ` Kay Sievers
2010-06-03 10:00                           ` Eric W. Biederman
2010-06-04  6:54                         ` Johannes Berg
2010-06-04  8:15                           ` Kay Sievers
2010-06-04  8:28                             ` Johannes Berg
2010-06-04  8:34                               ` Kay Sievers
2010-06-06 13:08                                 ` Johannes Berg
2010-06-06 17:17                                   ` Kay Sievers
2010-06-07  9:42                                     ` Johannes Berg
2010-06-07  9:53                                       ` Kay Sievers
2010-06-07 10:14                                         ` Johannes Berg
2010-06-07 11:05                                           ` Kay Sievers
2010-06-07 11:41                                             ` Johannes Berg
2010-06-07 12:26                                               ` Kay Sievers
2010-06-07 12:36                                                 ` Johannes Berg
2010-06-07 12:54                                                   ` Kay Sievers
2010-06-08  9:27                                                     ` Johannes Berg
2010-06-08  9:30                                                       ` Kay Sievers
2010-06-08  9:45                                                         ` Johannes Berg
2010-06-08 11:55                                                           ` Kay Sievers
2010-06-08 12:03                                                             ` Johannes Berg
2010-06-08 13:54                                                               ` Kay Sievers
2010-06-08 14:06                                                                 ` Johannes Berg
2010-06-08 14:21                                                                   ` Kay Sievers
2010-06-08 14:26                                                                     ` Johannes Berg
2010-06-08 14:47                                                                       ` Kay Sievers
2010-06-08 15:06                                                                         ` Johannes Berg
2010-06-08 16:26                                                                           ` Kay Sievers
2010-06-08 16:33                                                                             ` Johannes Berg
2010-06-08 16:39                                                                               ` Kay Sievers
2010-06-11  9:55                                                                                 ` Johannes Berg
2010-06-14  9:13                                                                                   ` Kay Sievers
2010-06-14  9:20                                                                                     ` Johannes Berg
2010-06-14  9:39                                                                                       ` Kay Sievers
2010-06-20  6:20                                                                                         ` [PATCH] Driver-core: Always create class directories fixing the broken network drivers Eric W. Biederman
2010-06-20 10:52                                                                                           ` Kay Sievers
2010-06-20 11:33                                                                                             ` Johannes Berg
2010-06-20 11:46                                                                                               ` Kay Sievers
2010-06-20 12:29                                                                                                 ` Eric W. Biederman
2010-06-20 13:37                                                                                                   ` Kay Sievers
2010-06-20 12:46                                                                                                 ` [PATCH] Driver-core: Always create network class directories in get_device_parent Eric W. Biederman
2010-06-21 22:20                                                                                                   ` Greg KH
2010-06-21 23:00                                                                                                     ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m16321t4ke.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=greg@kroah.com \
    --cc=johannes@sipsolutions.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.