Allow full bridge configuration via sysfs

Right now, you can configure most bridge device parameters via sysfs. However, you cannot either:
- add or remove bridge interfaces
- add or remove physical interfaces from a bridge

The attached patch set rectifies this. With this patch set, brctl (theoretically) becomes completely optional, much like ifenslave is now for bonding. (In fact, the idea for this patch, and the syntax used herein, is inspired by the sysfs bonding configuration.)

Patchset is against net-next-2.6, but should apply to anything reasonably recent with minimal fuzz.

Bill Nottingham (2):
Add a 'bridging_masters' file in sysfs under class/net Add a 'interfaces' file to the bridge device configuration in sysfs

 net/bridge/br.c          |    2 +
 net/bridge/br_private.h  |    4 ++
 net/bridge/br_sysfs_br.c |  136 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 0 deletions(-)

To use this file:
echo "+<bridge device name>" > bridging_masters
to add a new bridge, and:
echo "-<bridge device name>" > bridging masters
to remove a device. Reading the file lists the current bridge devices.

---
 net/bridge/br.c          |    2 +
 net/bridge/br_private.h  |    4 ++
 net/bridge/br_sysfs_br.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 0 deletions(-)

index 573acdf..89c6c7c 100644
@@ -59,6 +59,7 @@ static int __init br_init(void)
 
 	br_fdb_get_hook = br_fdb_get;
 	br_fdb_put_hook = br_fdb_put;
+	br_create_sysfs();
 
 	return 0;
 err_out3:

@@ -83,6 +84,7 @@ static void __exit br_deinit(void)
 	br_cleanup_bridges();
 
 	synchronize_net();
+	br_destroy_sysfs();
 
 	br_netfilter_fini();
 	br_fdb_get_hook = NULL;

index 815ed38..d5ff1c6 100644
@@ -249,6 +249,8 @@ extern void br_ifinfo_notify(int event, struct net_bridge_port *port);
 #ifdef CONFIG_SYSFS
 /* br_sysfs_if.c */
 extern struct sysfs_ops brport_sysfs_ops;
+extern int br_create_sysfs(void);
+extern void br_destroy_sysfs(void);
 extern int br_sysfs_addif(struct net_bridge_port *p);
 
 /* br_sysfs_br.c */

@@ -257,6 +259,8 @@ extern void br_sysfs_delbr(struct net_device *dev);
 
 #else
 
+#define br_create_sysfs()	(0)
+#define br_destroy_sysfs()	(0)
 #define br_sysfs_addif(p)	(0)
 #define br_sysfs_addbr(dev)	(0)
 #define br_sysfs_delbr(dev)	do { } while(0)

1.5.5.1

To use this file:
echo "+<device name>" > /sys/class/net/<name>/bridge/interfaces
to add a new physical interface to bridge device <name>, and:
echo "-<device name>" > interfaces
to remove a device. Reading the file lists the current phyiscal interfaces for that bridge.

---
 net/bridge/br_sysfs_br.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)

index a5d5fef..2dae8f3 100644
@@ -405,6 +405,70 @@ static ssize_t store_flush(struct device *d,
 }
 static DEVICE_ATTR(flush, S_IWUSR, NULL, store_flush);
 
+static ssize_t show_interfaces(struct device *d,
+			       struct device_attribute *attr, char *buf)
+{
+	struct net_bridge *br = to_bridge(d);
+	struct net_bridge_port *p;
+	int res = 0;
+
+	list_for_each_entry(p, &br->port_list, list) {
+		if (res > (PAGE_SIZE - IFNAMSIZ)) {
+			if ((PAGE_SIZE - res) > 10)
+				res = PAGE_SIZE - 10;
+			res += sprintf(buf + res, "++more++ ");
+			break;
+		}
+		res += sprintf(buf + res, "%s ", p->dev->name);
+	}
+	if (res)
+		buf[res-1] = '\n';
+	return res;
+}
+
+static ssize_t store_interfaces(struct device *d,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+{
+	char command[IFNAMSIZ + 1] = { 0, };
+	char *ifname;
+	struct net_bridge *br = to_bridge(d);
+	struct net_device *dev;
+	int ret = 0;
+
+	sscanf(buf, "%16s", command);	/* IFNAMSIZ */
+	ifname = command + 1;
+	if ((strlen(command) <= 1) || !dev_valid_name(ifname))
+		goto err_no_cmd;
+	if (command[0] == '+') {
+		rtnl_lock();
+		dev = __dev_get_by_name(&init_net, ifname);
+		if (dev == NULL)
+			ret = -ENXIO;
+		else
+			ret = br_add_if(br, dev);
+		rtnl_unlock();
+		goto out;
+	}
+	if (command[0] == '-') {
+		rtnl_lock();
+		dev = __dev_get_by_name(&init_net, ifname);
+		if (dev == NULL)
+			ret = -ENXIO;
+		else
+			ret = br_del_if(br, dev);
+		rtnl_unlock();
+		goto out;
+	}
+err_no_cmd:
+	printk(KERN_ERR "bridge: no command found in interfaces file for bridge %s. Use +ifname or -ifname.\n", br->dev->name);
+	ret = -EPERM;
+out:
+	return ret ? ret: len;
+}
+
+static DEVICE_ATTR(interfaces, S_IRUGO | S_IWUSR, show_interfaces, store_interfaces);
+
 static struct attribute *bridge_attrs[] = {
 	&dev_attr_forward_delay.attr,
 	&dev_attr_hello_time.attr,

@@ -424,6 +488,7 @@ static struct attribute *bridge_attrs[] = {
 	&dev_attr_gc_timer.attr,
 	&dev_attr_group_addr.attr,
 	&dev_attr_flush.attr,
+	&dev_attr_interfaces.attr,
 	NULL
 };
 

1.5.5.1

Both should use netlink instead of extending their sysfs interfaces. For bridging I have a patch for the bridge device itself, the API is so far missing support for adding ports though.

How does that improve the situation for bridge devices? Are all bridging parameters (forward_delay, stp, etc.) going to be configurable via netlink, or would we still then have multiple tools/interfaces to configuration? Also, moving bonding configuration to netlink seems like a step backwards.

Bill

Of course its all going to be configurable via netlink, otherwise it really wouldn't make sense.

Please read up on what the standard interface for network configuration is, I'm tired of reiterating this once a week.

OK, let's see.

In the code: Hm, no TODO or FIXME.

In the included documentation:

Documentation/networking/bonding.txt:
Module options, modprobe.conf, or 'distro-specific configuration file', ifenslave, or sysfs.

Documentation/networking/ip-sysctl.txt:
sysctl, obviously.

Documentation/networking/generic_netlink.txt
Hey, here's netlink! Doucmentation points only to a wiki. Referred to by zero other included in-kernel documentation.

Well, that's helpful.

Let's try the OSDL web!

http://www.linuxfoundation.org/en/Net:Bridge
brctl (which uses ioctl and sysfs). And /etc/net.

http://www.linuxfoundation.org/en/Net:Bonding
Module parameters only, including the lovely 'load driver multiple times' method. Doesn't even mention sysfs.

http://www.linuxfoundation.org/en/Net:VLAN
vconfig

I could look at wireless network configuration, but I doubt that's going to help your argument.

Well, if the documentation that described this as the standard existed, or wasn't such crap, perhaps you wouldn't have to.

That being said, how is moving from adding a bonding slave from:
echo "+eth0" > /sys/class/net/bond0/bonding/slaves to:
to:
http://www.linuxfoundation.org/en/Net:Generic_Netlink_HOWTO

a worthwhile improvement for the admin? Let's see, a kernel-userspace protocol with magic message formats. Hey, we reinvented ioctl!

Why, if netlink is the standard (and it's been around for a long damn time), was sysfs configuration for bonding added in 2005? Why was bridge configuration added in 2005, and *extended* in 2006 and 2007? Why were the user-space tools such as brctl ported from ioctl to sysfs?

Bill

From: Bill Nottingham <[email protected]>
Date: Mon, 7 Jul 2008 17:34:20 -0400

Just like any system with age, we have a lot of legacy to convert over. But it will happen.

Because often a lot of shit slips in when someone who understands the ramifications is too busy or on vacation.

We do want everything to be netlink based.

Why?

Because it means that you can run one monitoring tool to listen for netlink events and report them to the user for diagnosis.

It means that network configuration events can be sent over the wire and used remotely at some point.

The latter can never happen as long as we keep adding ad-hoc config stuff.

There are always historical reasons. In this case it was because I knew more about sysfs than netlink, and there was no netlink interface for managing interfaces back in 2005. Sysfs is okay for simple stuff (set forward-delay to 10seconds), but it falls down when anything interesting and transactional happens. Think of sysfs as more an extension of per-device sysctl's or module parameters, rather than a good configuration interface.

Duly noted, will time all patch submissions to land during your vacations in the future.

More seriously, if there's not a mechanism to prevent ABIs the kernel doesn't want like this being added, that's a problem.

Sure, but it does make them more opaque to the normal user, leaving them wrapped in the same old ip/brctl/ifenslave/vconfig tools - for better or worse, people like the discoverability and obviousness of sysfs.

Bill