[PATCH] Add ability to register class interfaces for network class

[PATCH] Add ability to register class interfaces for network class

[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 <ext-timo.teras@nokia.com>

--- linux-2.6.9-rc4.orig/net/core/net-sysfs.c	2004-10-12 13:23:35.106496312 +0300
+++ linux-2.6.9-rc4/net/core/net-sysfs.c	2004-10-12 13:24:29.177276304 +0300
@@ -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);
--- linux-2.6.9-rc4.orig/include/linux/netdevice.h	2004-10-12 13:23:31.186092304 +0300
+++ linux-2.6.9-rc4/include/linux/netdevice.h	2004-10-12 13:24:29.179276000 +0300
@@ -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 */

Re: [PATCH] Add ability to register class interfaces for network class

[Christoph Hellwig]

On Tue, Oct 26, 2004 at 09:35:26PM +0300, Timo Teräs wrote:
> 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 <ext-timo.teras@nokia.com>

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

Re: [PATCH] Add ability to register class interfaces for network class

[Teras Timo (EXT-YomiGroup/Helsinki)]

On Tue, Oct 26, 2004 at 07:48:38PM +0100, ext Christoph Hellwig wrote:
> On Tue, Oct 26, 2004 at 09:35:26PM +0300, Timo Teräs wrote:
> > 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 <ext-timo.teras@nokia.com>
> 
> 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

Re: [PATCH] Add ability to register class interfaces for network class

[ext Christoph Hellwig]

> 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.

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.

Re: [PATCH] Add ability to register class interfaces for network class

[Timo Teräs]

[-- Attachment #1: Type: text/plain, Size: 1518 bytes --]

ext ext Christoph Hellwig wrote:
>>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.
> 
> 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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH] Add ability to register class interfaces for network class

[Stephen Hemminger]

On Wed, 27 Oct 2004 14:31:04 +0300
Timo Teräs <ext-timo.teras@nokia.com> wrote:

> ext ext Christoph Hellwig wrote:
> >>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.
> > 
> > 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;
	}

Re: [PATCH] Add ability to register class interfaces for network class

[Teras Timo (EXT-YomiGroup/Helsinki)]

On Wed, Oct 27, 2004 at 08:59:06AM -0700, ext Stephen Hemminger wrote:
> 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;
> 	}

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

Re: [PATCH] Add ability to register class interfaces for network class

[Stephen Hemminger]

On Wed, 27 Oct 2004 21:38:04 +0300
"Teras Timo (EXT-YomiGroup/Helsinki)" <Ext-Timo.Teras@nokia.com> wrote:

> On Wed, Oct 27, 2004 at 08:59:06AM -0700, ext Stephen Hemminger wrote:
> > 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;
> > 	}
> 
> 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.
> 

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.

Re: [PATCH] Add ability to register class interfaces for network class

[Teras Timo (EXT-YomiGroup/Helsinki)]

On Wed, Oct 27, 2004 at 11:55:38AM -0700, ext Stephen Hemminger wrote:
> On Wed, 27 Oct 2004 21:38:04 +0300
> "Teras Timo (EXT-YomiGroup/Helsinki)" <Ext-Timo.Teras@nokia.com> wrote:
> > 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.
> > 
> 
> 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