2004-06-16 23:46:16

by Zwane Mwaikambo

[permalink] [raw]
Subject: [PATCH][2.6] fix bridge sysfs improperly initialised knobject

The bridge sysfs interface introduced around 2.6.7-rc1 created a bad
entry in /sys because it didn't initialise the name member of the kobject.

zwane@montezuma /sys {0:0} ls -l
total 0
?--------- ? ? ? ? ?
drwxr-xr-x 17 root root 0 Jun 15 15:47 block
drwxr-xr-x 7 root root 0 Jun 15 15:47 bus
drwxr-xr-x 16 root root 0 Jun 15 15:47 class
drwxr-xr-x 5 root root 0 Jun 15 15:47 devices
drwxr-xr-x 3 root root 0 Jun 15 15:47 firmware
drwxr-xr-x 8 root root 0 Jun 15 19:55 module

Index: linux-2.6.7-rc3-mm2/net/bridge/br_sysfs_br.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.7-rc3-mm2/net/bridge/br_sysfs_br.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 br_sysfs_br.c
--- linux-2.6.7-rc3-mm2/net/bridge/br_sysfs_br.c 14 Jun 2004 12:49:12 -0000 1.1.1.1
+++ linux-2.6.7-rc3-mm2/net/bridge/br_sysfs_br.c 16 Jun 2004 16:45:20 -0000
@@ -305,9 +305,7 @@ static struct bin_attribute bridge_forwa
* This is a dummy kset so bridge objects don't cause
* hotplug events
*/
-struct subsystem bridge_subsys = {
- .kset = { .hotplug_ops = NULL },
-};
+decl_subsys_name(bridge, net_bridge, NULL, NULL);

void br_sysfs_init(void)
{


2004-06-17 20:47:01

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH][2.6] fix bridge sysfs improperly initialised knobject

> The bridge sysfs interface introduced around 2.6.7-rc1 created a bad
> entry in /sys because it didn't initialise the name member of the kobject.
>
> zwane@montezuma /sys {0:0} ls -l
> total 0
> ?--------- ? ? ? ? ?
> drwxr-xr-x 17 root root 0 Jun 15 15:47 block
> drwxr-xr-x 7 root root 0 Jun 15 15:47 bus
> drwxr-xr-x 16 root root 0 Jun 15 15:47 class
> drwxr-xr-x 5 root root 0 Jun 15 15:47 devices
> drwxr-xr-x 3 root root 0 Jun 15 15:47 firmware
> drwxr-xr-x 8 root root 0 Jun 15 19:55 module
>
> Index: linux-2.6.7-rc3-mm2/net/bridge/br_sysfs_br.c
> ===================================================================
> RCS file: /home/cvsroot/linux-2.6.7-rc3-mm2/net/bridge/br_sysfs_br.c,v
> retrieving revision 1.1.1.1
> diff -u -p -B -r1.1.1.1 br_sysfs_br.c
> --- linux-2.6.7-rc3-mm2/net/bridge/br_sysfs_br.c 14 Jun 2004 12:49:12 -0000 1.1.1.1
> +++ linux-2.6.7-rc3-mm2/net/bridge/br_sysfs_br.c 16 Jun 2004 16:45:20 -0000
> @@ -305,9 +305,7 @@ static struct bin_attribute bridge_forwa
> * This is a dummy kset so bridge objects don't cause
> * hotplug events
> */
> -struct subsystem bridge_subsys = {
> - .kset = { .hotplug_ops = NULL },
> -};
> +decl_subsys_name(bridge, net_bridge, NULL, NULL);
>
> void br_sysfs_init(void)
> {


Yes, this would get rid of the name, but then wouldn't bridge show up
as top level subsystem /sys/bridge.

Is there no way to register without causing bogus hotplug events?

I am getting a bad taste about the whole sysfs programming model, since
it seems like programming by side effect. it would be better for sysfs
to handle the case of hidden subsystems, or provide an alternate way
not to generate hotplug events.

2004-06-17 20:52:22

by David Miller

[permalink] [raw]
Subject: Re: [PATCH][2.6] fix bridge sysfs improperly initialised knobject

On Thu, 17 Jun 2004 13:46:36 -0700
Stephen Hemminger <[email protected]> wrote:

> > -struct subsystem bridge_subsys = {
> > - .kset = { .hotplug_ops = NULL },
> > -};
> > +decl_subsys_name(bridge, net_bridge, NULL, NULL);
> >
> > void br_sysfs_init(void)
> > {
>
> Yes, this would get rid of the name, but then wouldn't bridge show up
> as top level subsystem /sys/bridge.
>
> Is there no way to register without causing bogus hotplug events?

Ok, for now I'm putting the change in, since 2.6.8 is not "around
the corner" we'll have time to put a better fix into the tree.

2004-06-17 20:59:51

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH][2.6] fix bridge sysfs improperly initialised knobject

On Thu, Jun 17, 2004 at 01:46:36PM -0700, Stephen Hemminger wrote:
> Yes, this would get rid of the name, but then wouldn't bridge show up
> as top level subsystem /sys/bridge.

If you register it, yes it would. Hm, what happens if you don't
register it...

> Is there no way to register without causing bogus hotplug events?

You are wanting to prevent hotplug events for a subset of a subsystem's
devices, right? You faked out the core by providing a fake subsystem.
How about just using the filter of the subsystem you really want these
entries to show up under? Would that work?

> I am getting a bad taste about the whole sysfs programming model, since
> it seems like programming by side effect. it would be better for sysfs
> to handle the case of hidden subsystems, or provide an alternate way
> not to generate hotplug events.

Well, we never considered that you would want to nest subsystems in such
a wierd way :)

Anyway, take a look at the filter ability to see if that would work out
for you instead of having to create a new subsystem.

thanks,

greg k-h

2004-06-17 21:45:34

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH][2.6] fix bridge sysfs improperly initialised knobject

The whole effort to avoid hotplug was misguided. If it is really a problem
(which it doesn't appear to be) then it can more easily be addressed by smarter
hotplug scripts in user space.

This patch gets rid of the whole subsystem hack for bridge kobjects.

Signed-off-by: Stephen Hemminger <[email protected]>

diff -Nru a/net/bridge/br.c b/net/bridge/br.c
--- a/net/bridge/br.c 2004-06-17 14:14:16 -07:00
+++ b/net/bridge/br.c 2004-06-17 14:14:16 -07:00
@@ -33,8 +33,6 @@
{
br_fdb_init();

- br_sysfs_init();
-
#ifdef CONFIG_BRIDGE_NETFILTER
if (br_netfilter_init())
return 1;
@@ -69,7 +67,6 @@
#endif

br_handle_frame_hook = NULL;
- br_sysfs_fini();
br_fdb_fini();
}

diff -Nru a/net/bridge/br_private.h b/net/bridge/br_private.h
--- a/net/bridge/br_private.h 2004-06-17 14:14:16 -07:00
+++ b/net/bridge/br_private.h 2004-06-17 14:14:16 -07:00
@@ -217,9 +217,6 @@
extern void br_sysfs_freeif(struct net_bridge_port *p);

/* br_sysfs_br.c */
-extern struct subsystem bridge_subsys;
-extern void br_sysfs_init(void);
-extern void br_sysfs_fini(void);
extern int br_sysfs_addbr(struct net_device *dev);
extern void br_sysfs_delbr(struct net_device *dev);

@@ -228,8 +225,6 @@
#define br_sysfs_addif(p) (0)
#define br_sysfs_removeif(p) do { } while(0)
#define br_sysfs_freeif(p) kfree(p)
-#define br_sysfs_init() do { } while(0)
-#define br_sysfs_fini() do { } while(0)
#define br_sysfs_addbr(dev) (0)
#define br_sysfs_delbr(dev) do { } while(0)
#endif /* CONFIG_SYSFS */
diff -Nru a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
--- a/net/bridge/br_sysfs_br.c 2004-06-17 14:14:16 -07:00
+++ b/net/bridge/br_sysfs_br.c 2004-06-17 14:14:16 -07:00
@@ -300,25 +300,6 @@
.read = brforward_read,
};

-
-/*
- * This is a dummy kset so bridge objects don't cause
- * hotplug events
- */
-struct subsystem bridge_subsys = {
- .kset = { .hotplug_ops = NULL },
-};
-
-void br_sysfs_init(void)
-{
- subsystem_register(&bridge_subsys);
-}
-
-void br_sysfs_fini(void)
-{
- subsystem_unregister(&bridge_subsys);
-}
-
/*
* Add entries in sysfs onto the existing network class device
* for the bridge.
@@ -353,7 +334,7 @@

kobject_set_name(&br->ifobj, SYSFS_BRIDGE_PORT_SUBDIR);
br->ifobj.ktype = NULL;
- br->ifobj.kset = &bridge_subsys.kset;
+ br->ifobj.kset = NULL;
br->ifobj.parent = brobj;

err = kobject_register(&br->ifobj);
diff -Nru a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
--- a/net/bridge/br_sysfs_if.c 2004-06-17 14:14:16 -07:00
+++ b/net/bridge/br_sysfs_if.c 2004-06-17 14:14:16 -07:00
@@ -227,7 +227,7 @@
kobject_set_name(&p->kobj, SYSFS_BRIDGE_PORT_ATTR);
p->kobj.ktype = &brport_ktype;
p->kobj.parent = &(p->dev->class_dev.kobj);
- p->kobj.kset = &bridge_subsys.kset;
+ p->kobj.kset = NULL;

err = kobject_add(&p->kobj);
if(err)

2004-06-18 20:24:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH][2.6] fix bridge sysfs improperly initialised knobject

On Thu, 17 Jun 2004 14:45:22 -0700
Stephen Hemminger <[email protected]> wrote:

> The whole effort to avoid hotplug was misguided. If it is really a problem
> (which it doesn't appear to be) then it can more easily be addressed by smarter
> hotplug scripts in user space.
>
> This patch gets rid of the whole subsystem hack for bridge kobjects.

Applied.