[PATCH RFC] net/core: API to create/destroy /sys/class/net entries

[PATCH RFC] net/core: API to create/destroy /sys/class/net entries

[Jay Vosburgh]

	Some background: 

	Bonding creates a file in sysfs, /sys/class/net/bonding_masters,
which is used to create and destroy bonding devices.  Currently, when
bonding is loaded, it does some poking through the device structure of
the first bonding device to find the net_class, and then uses that
pointer to create the file (by calling class_create_file).

	Now, I'm working on a patch to permit bonding to load and not
create any devices initially (at the request of a user).  Without the
initial bonding device, there's nothing to backtrack to find the
net_class, and so there's no good way to create the bonding_masters
file, and therefore no way to create any bonding devices.

	I'm attaching the relevant portion of a work-in-progress patch
below to get some input on the best way to create the bonding_masters
file.

	The patch below creates an API in net/core/net-sysfs.c to create
and destroy files within net_class.  Is this the best way to do this, or
would it be preferrable to simply export net_class?

	Comments, please.  This feels cleaner overall to me, but I'd
like some feedback.

	-J


diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 979c2d0..b54ab1d 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -52,7 +52,6 @@ extern struct bond_parm_tbl xmit_hashtype_tbl[];
 extern struct bond_parm_tbl arp_validate_tbl[];
 
 static int expected_refcount = -1;
-static struct class *netdev_class;
 /*--------------------------- Data Structures -----------------------------*/
 
 /* Bonding sysfs lock.  Why can't we just use the subsystem lock?
@@ -1412,19 +1411,9 @@ static struct attribute_group bonding_group = {
  */
 int bond_create_sysfs(void)
 {
-	int ret = 0;
-	struct bonding *firstbond;
-
-	/* get the netdev class pointer */
-	firstbond = container_of(bond_dev_list.next, struct bonding, bond_list);
-	if (!firstbond)
-		return -ENODEV;
+	int ret;
 
-	netdev_class = firstbond->dev->dev.class;
-	if (!netdev_class)
-		return -ENODEV;
-
-	ret = class_create_file(netdev_class, &class_attr_bonding_masters);
+	ret = netdev_class_create_file(&class_attr_bonding_masters);
 	/*
 	 * Permit multiple loads of the module by ignoring failures to
 	 * create the bonding_masters sysfs file.  Bonding devices
@@ -1436,10 +1425,8 @@ int bond_create_sysfs(void)
 	 * initscripts/sysconfig, which load bonding multiple times to
 	 * configure multiple bonding devices.
 	 */
-	if (ret == -EEXIST) {
-		netdev_class = NULL;
+	if (ret == -EEXIST)
 		return 0;
-	}
 
 	return ret;
 
@@ -1450,8 +1437,7 @@ int bond_create_sysfs(void)
  */
 void bond_destroy_sysfs(void)
 {
-	if (netdev_class)
-		class_remove_file(netdev_class, &class_attr_bonding_masters);
+	netdev_class_remove_file(&class_attr_bonding_masters);
 }
 
 /*
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7c1d446..85adc92 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1485,6 +1485,11 @@ extern void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos);
 extern void dev_seq_stop(struct seq_file *seq, void *v);
 #endif
 
+#ifdef CONFIG_SYSFS
+extern int netdev_class_create_file(struct class_attribute *class_attr);
+extern void netdev_class_remove_file(struct class_attribute *class_attr);
+#endif /* CONFIG_SYSFS */
+
 extern void linkwatch_run_queue(void);
 
 extern int netdev_compute_features(unsigned long all, unsigned long one);
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 7635d3f..3ab1dab 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -468,6 +468,21 @@ int netdev_register_kobject(struct net_device *net)
 	return device_add(dev);
 }
 
+#ifdef CONFIG_SYSFS
+int netdev_class_create_file(struct class_attribute *class_attr)
+{
+	return class_create_file(&net_class, class_attr);
+}
+
+void netdev_class_remove_file(struct class_attribute *class_attr)
+{
+	class_remove_file(&net_class, class_attr);
+}
+
+EXPORT_SYMBOL(netdev_class_create_file);
+EXPORT_SYMBOL(netdev_class_remove_file);
+#endif /* CONFIG_SYSFS */
+
 int netdev_kobject_init(void)
 {
 	return class_register(&net_class);


---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

Re: [PATCH RFC] net/core: API to create/destroy /sys/class/net entries

[Stephen Hemminger]

On Wed, 16 Apr 2008 18:06:26 -0700
Jay Vosburgh <fubar@us.ibm.com> wrote:

> 
> 	Some background: 
> 
> 	Bonding creates a file in sysfs, /sys/class/net/bonding_masters,

That is a problem because it confuses scripts that use /sys/class/net/X
to find a list of network devices. I.e. that was a bad API choice.

Re: [PATCH RFC] net/core: API to create/destroy /sys/class/net entries

[David Miller]

From: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed, 16 Apr 2008 18:06:26 -0700

> 	Bonding creates a file in sysfs, /sys/class/net/bonding_masters,
> which is used to create and destroy bonding devices.  Currently, when
> bonding is loaded, it does some poking through the device structure of
> the first bonding device to find the net_class, and then uses that
> pointer to create the file (by calling class_create_file).

As Stephen stated this was a horrible decision, that directory is for
network device instances, that's why the class variable is called
"netdev_class" not "dump_all_yer_networking_shit_here_class"

The only saving grace is that bonding creates a file, not a
directory, so "scan for all directories under /sys/class/net/"
schemes still work.

Water under the bridge I suppose, and we're stuck with this.

I guess your proposal is the best, as if we export netdev_class
people can do other funky, undesirable, things with it.

Please submit the final version of your patch, I'll apply it.