Add ability to register class interfaces for network class

PATCH In review Timo Teräs 3 participants 9 messages Oct 26, 2004 – Oct 27, 2004

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Timo Teräs

author

2004-10-26 18:35 UTC #0 lore raw

PATCH 1/1 Add ability to register class interfaces for network class

Add functions to register class interfaces for network class. Similar to the ones found in scsi subsystem.

Originally posted on linux-net and related discussion can be found from http://marc.theaimsgroup.com/?l=linux-net&m=109758592121767&w=2.

Signed-off-by Timo Teräs <[email protected]>

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

@@ -448,3 +448,11 @@
 {
 	return class_register(&net_class);
 }
+
+int netdev_register_interface(struct class_interface *intf)
+{
+	intf->class = &net_class;
+	return class_interface_register(intf);
+}
+
+EXPORT_SYMBOL_GPL(netdev_register_interface);

include/linux/netdevice.h +6 -0

@@ -951,6 +951,12 @@
 extern char *net_sysctl_strdup(const char *s);
 #endif
 
+#ifdef CONFIG_SYSFS
+extern int netdev_register_interface(struct class_interface *intf);
+#define netdev_unregister_interface(intf) \
+	class_interface_unregister(intf)
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif	/* _LINUX_DEV_H */

Christoph Hellwig

2004-10-26 18:48 UTC #1 lore raw

Quoted 7 lines · Timo Teräs

Add functions to register class interfaces for network class. Similar to the ones found in scsi subsystem.
Originally posted on linux-net and related discussion can be found from http://marc.theaimsgroup.com/?l=linux-net&m=109758592121767&w=2.
Signed-off-by Timo Teräs <[email protected]>

And you still haven't told why this makes sense nor shown a user for it.

Teras Timo (EXT-YomiGroup/Helsinki)

author

2004-10-26 20:52 UTC #2 lore raw

Quoted 10 lines · ext Christoph Hellwig

Quoted 7 lines · Timo Teräs
Add functions to register class interfaces for network class. Similar to the ones found in scsi subsystem.
Originally posted on linux-net and related discussion can be found from http://marc.theaimsgroup.com/?l=linux-net&m=109758592121767&w=2.
Signed-off-by Timo Teräs <[email protected]>
And you still haven't told why this makes sense nor shown a user for it.

Did you read my original reply to your earlier mail?

I pointed out the reason why I need it: to add an attribute:
/sys/class/net/<interface>/<new_attribute>. Others might want to do this too.

Also the interface subsystem is used in many places (eg. i2o) to create all the sysfs entries. Maybe it'd be useful to do in net class as well. And if'd make separate interfaces for standard attributes and wireless attributes. That way you'd get rid of the #ifdefs of WIRELESS_EXT in net-sysfs.c.

On the other hand you didn't answer my question: why the interface registration should not be possible? Documentation/driver-model/interface.txt states that this is exactly the thing it should be used for.

Regards,
Timo

ext Christoph Hellwig

2004-10-27 11:13 UTC #3 lore raw

Quoted 10 lines

I pointed out the reason why I need it: to add an attribute:
/sys/class/net/<interface>/<new_attribute>. Others might want to do this too.
Also the interface subsystem is used in many places (eg. i2o) to create all the sysfs entries. Maybe it'd be useful to do in net class as well. And if'd make separate interfaces for standard attributes and wireless attributes. That way you'd get rid of the #ifdefs of WIRELESS_EXT in net-sysfs.c.

Quoted 4 lines

On the other hand you didn't answer my question: why the interface registration should not be possible? Documentation/driver-model/interface.txt states that this is exactly the thing it should be used for.

Because we don't add interfaces for the sake of it. You still haven't show the user that wants this. i you presented a sane user the review would be quite different.

Timo Teräs

author

2004-10-27 11:31 UTC #4 lore raw

Quoted 10 lines · ext ext Christoph Hellwig

Quoted 10 lines
I pointed out the reason why I need it: to add an attribute:
/sys/class/net/<interface>/<new_attribute>. Others might want to do this too.
Also the interface subsystem is used in many places (eg. i2o) to create all the sysfs entries. Maybe it'd be useful to do in net class as well. And if'd make separate interfaces for standard attributes and wireless attributes. That way you'd get rid of the #ifdefs of WIRELESS_EXT in net-sysfs.c.

Quoted 8 lines

Quoted 4 lines
On the other hand you didn't answer my question: why the interface registration should not be possible? Documentation/driver-model/interface.txt states that this is exactly the thing it should be used for.
Because we don't add interfaces for the sake of it. You still haven't show the user that wants this. i you presented a sane user the review would be quite different.

I am the user wanting it :)

If you want to look more detailed what I'm trying to accomplish you could check:
http://seclists.org/lists/linux-kernel/2004/Oct/1373.html
http://seclists.org/lists/linux-kernel/2004/Sep/8685.html
http://lists.netfilter.org/pipermail/netfilter-devel/2004-August/016342.html

Of course the ideal way to do this would be adding a timer to net_device and modifying net-sysfs.c. But I don't think that is going to happen so I want to do as unintrusive implementation as possible. And currently I think it'd be adding an interface to network class device which creates the related attribute.

Cheers,
Timo

Stephen Hemminger

2004-10-27 15:59 UTC #5 lore raw

Quoted 11 lines · On Wed, 27 Oct 2004 14:31:04 +0300 Timo Teräs <[email protected]> wrote:

Quoted 10 lines · ext ext Christoph Hellwig
Quoted 10 lines
I pointed out the reason why I need it: to add an attribute:
/sys/class/net/<interface>/<new_attribute>. Others might want to do this too.
Also the interface subsystem is used in many places (eg. i2o) to create all the sysfs entries. Maybe it'd be useful to do in net class as well. And if'd make separate interfaces for standard attributes and wireless attributes. That way you'd get rid of the #ifdefs of WIRELESS_EXT in net-sysfs.c.

Quoted 8 lines

Quoted 8 lines
Quoted 4 lines
On the other hand you didn't answer my question: why the interface registration should not be possible? Documentation/driver-model/interface.txt states that this is exactly the thing it should be used for.
Because we don't add interfaces for the sake of it. You still haven't show the user that wants this. i you presented a sane user the review would be quite different.

The Ethernet bridge code creates attribute groups in several places without additional interfaces. How hard is it to do:

int br_sysfs_addbr(struct net_device *dev)

{
	struct kobject *brobj = &dev->class_dev.kobj;
	struct net_bridge *br = netdev_priv(dev);
	int err;

	err = sysfs_create_group(brobj, &bridge_group);
	if (err) {
		pr_info("%s: can't create group %s/%s\n",
			__FUNCTION__, dev->name, bridge_group.name);
		goto out1;
	}

Teras Timo (EXT-YomiGroup/Helsinki)

author

2004-10-27 18:38 UTC #6 lore raw

Quoted 4 lines · ext Stephen Hemminger

The Ethernet bridge code creates attribute groups in several places without additional interfaces. How hard is it to do:
int br_sysfs_addbr(struct net_device *dev)

Quoted 11 lines

{
	struct kobject *brobj = &dev->class_dev.kobj;
	struct net_bridge *br = netdev_priv(dev);
	int err;

	err = sysfs_create_group(brobj, &bridge_group);
	if (err) {
		pr_info("%s: can't create group %s/%s\n",
			__FUNCTION__, dev->name, bridge_group.name);
		goto out1;
	}

This way the problem is that I have to know which devices I will add the attributes. But the point is to add attributes to all netdevs.

Using this approach I'd have to enumerate all the interfaces every now and then.

If I have my class interface I get a callback whenever an interface is created or deleted and I can automatically add the attribute to all netdevs.

Cheers,
Timo

Stephen Hemminger

2004-10-27 18:55 UTC #7 lore raw

Quoted 21 lines · On Wed, 27 Oct 2004 21:38:04 +0300 "Teras Timo (EXT-YomiGroup/Helsinki)"…

Quoted 4 lines · ext Stephen Hemminger
The Ethernet bridge code creates attribute groups in several places without additional interfaces. How hard is it to do:
int br_sysfs_addbr(struct net_device *dev)
Quoted 11 lines
{
	struct kobject *brobj = &dev->class_dev.kobj;
	struct net_bridge *br = netdev_priv(dev);
	int err;

	err = sysfs_create_group(brobj, &bridge_group);
	if (err) {
		pr_info("%s: can't create group %s/%s\n",
			__FUNCTION__, dev->name, bridge_group.name);
		goto out1;
	}
This way the problem is that I have to know which devices I will add the attributes. But the point is to add attributes to all netdevs.

Quoted 7 lines

Using this approach I'd have to enumerate all the interfaces every now and then.
If I have my class interface I get a callback whenever an interface is created or deleted and I can automatically add the attribute to all netdevs.

If you are doing something to all interfaces, then it probably should either be part of the common net-sysfs layer, or you can dynamically discover and do it by following device transitions with register_device_notifier.

Teras Timo (EXT-YomiGroup/Helsinki)

author

2004-10-27 19:20 UTC #8 lore raw

Quoted 14 lines · ext Stephen Hemminger

Quoted 11 lines · On Wed, 27 Oct 2004 21:38:04 +0300 "Teras Timo (EXT-YomiGroup/Helsinki)"…
This way the problem is that I have to know which devices I will add the attributes. But the point is to add attributes to all netdevs.
Using this approach I'd have to enumerate all the interfaces every now and then.
If I have my class interface I get a callback whenever an interface is created or deleted and I can automatically add the attribute to all netdevs.

Quoted 4 lines

If you are doing something to all interfaces, then it probably should either be part of the common net-sysfs layer, or you can dynamically discover and do it by following device transitions with register_device_notifier.

I'd like my code to be available as module also so common net-sysfs is not an option. Hmm... I could use device transitions hooks.

But I'm still a bit puzzled why the interface registration should not be public?

After all it is only a mechanism to get callbacks whenever a class device is created or removed. Why to even implement a class interface system if you are not allowed to use it (except in one special place where it is considered to be an ugly hack by Christoph Hellwig)?

I understood that the whole point for interfaces is the ability to extend functionality without core (i.e. common net-sysfs) changes. Why to lose that modularity?

Cheers,
Timo