Patchwork Driver-core: Always create network class directories in get_device_parent.

login
register
mail settings
Submitter Eric W. Biederman
Date June 20, 2010, 12:46 p.m.
Message ID <m1ocf5hnwh.fsf_-_@fess.ebiederm.org>
Download mbox | patch
Permalink /patch/56264/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Eric W. Biederman - June 20, 2010, 12:46 p.m.
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(-)
Greg KH - June 21, 2010, 10:20 p.m.
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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman - June 21, 2010, 11 p.m.
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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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;