net/core: API to create/destroy /sys/class/net entries

RFC PATCH In review Jay Vosburgh 3 participants 3 messages Apr 17, 2008 – Apr 18, 2008

Subsystems (3)

Networking Network drivers Bonding driver

lore

Share & tools

lore

Jay Vosburgh

author

2008-04-17 01:06 UTC #0 lore raw

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

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

drivers/net/bonding/bond_sysfs.c +4 ~~-18~~

index 979c2d0..b54ab1d 100644
@@ -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);
 }
 
 /*

include/linux/netdevice.h +5 -0

index 7c1d446..85adc92 100644
@@ -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);

net/core/net-sysfs.c +15 -0

index 7635d3f..3ab1dab 100644
@@ -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, [email protected]

Stephen Hemminger

2008-04-17 01:12 UTC #1 lore raw

Quoted 4 lines · On Wed, 16 Apr 2008 18:06:26 -0700 Jay Vosburgh <[email protected]> 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.

David Miller

maintainer

2008-04-18 06:36 UTC #2 lore raw

In reply to Jay Vosburgh

From: Jay Vosburgh <[email protected]>
Date: Wed, 16 Apr 2008 18:06:26 -0700

Quoted 5 lines

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.