[patch 0/3] bridge patches for 2.6.18

[patch 0/3] bridge patches for 2.6.18

[Stephen Hemminger]

Some stuff for 2.6.18. The most important is adding netlink
support for managing interfaces; this allows building STP
as an application.

--

[patch 1/3] bridge: optimize conditional in forward path

[Stephen Hemminger]

[-- Attachment #1: bridge-forward-opt.patch --]
[-- Type: text/plain, Size: 1072 bytes --]

Small optimizations of bridge forwarding path.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>


--- br.orig/net/bridge/br_forward.c
+++ br/net/bridge/br_forward.c
@@ -20,14 +20,11 @@
 #include <linux/netfilter_bridge.h>
 #include "br_private.h"
 
+/* Don't forward packets to originating port or forwarding diasabled */
 static inline int should_deliver(const struct net_bridge_port *p, 
 				 const struct sk_buff *skb)
 {
-	if (skb->dev == p->dev ||
-	    p->state != BR_STATE_FORWARDING)
-		return 0;
-
-	return 1;
+	return (skb->dev != p->dev && p->state == BR_STATE_FORWARDING);
 }
 
 static inline unsigned packet_length(const struct sk_buff *skb)
@@ -55,10 +52,9 @@ int br_dev_queue_push_xmit(struct sk_buf
 
 int br_forward_finish(struct sk_buff *skb)
 {
-	NF_HOOK(PF_BRIDGE, NF_BR_POST_ROUTING, skb, NULL, skb->dev,
-			br_dev_queue_push_xmit);
+	return NF_HOOK(PF_BRIDGE, NF_BR_POST_ROUTING, skb, NULL, skb->dev,
+		       br_dev_queue_push_xmit);
 
-	return 0;
 }
 
 static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)

--

[patch 2/3] bridge: fix module startup error handling

[Stephen Hemminger]

[-- Attachment #1: bridge-startup-errorhandle.patch --]
[-- Type: text/plain, Size: 1826 bytes --]

Return address in use, if some other kernel code has the SAP.
Propogate out error codes from netfilter registration and unwind.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>


--- br.orig/net/bridge/br.c
+++ br/net/bridge/br.c
@@ -30,36 +30,44 @@ static struct llc_sap *br_stp_sap;
 
 static int __init br_init(void)
 {
+	int err;
+
 	br_stp_sap = llc_sap_open(LLC_SAP_BSPAN, br_stp_rcv);
 	if (!br_stp_sap) {
 		printk(KERN_ERR "bridge: can't register sap for STP\n");
-		return -EBUSY;
+		return -EADDRINUSE;
 	}
 
 	br_fdb_init();
 
-#ifdef CONFIG_BRIDGE_NETFILTER
-	if (br_netfilter_init())
-		return 1;
-#endif
+	err = br_netfilter_init();
+	if (err)
+		goto err_out1;
+
+	err = register_netdevice_notifier(&br_device_notifier);
+	if (err)
+		goto err_out2;
+
 	brioctl_set(br_ioctl_deviceless_stub);
 	br_handle_frame_hook = br_handle_frame;
 
 	br_fdb_get_hook = br_fdb_get;
 	br_fdb_put_hook = br_fdb_put;
 
-	register_netdevice_notifier(&br_device_notifier);
-
 	return 0;
+
+err_out2:
+	br_netfilter_fini();
+err_out1:
+	llc_sap_put(br_stp_sap);
+	return err;
 }
 
 static void __exit br_deinit(void)
 {
 	rcu_assign_pointer(br_stp_sap->rcv_func, NULL);
 
-#ifdef CONFIG_BRIDGE_NETFILTER
 	br_netfilter_fini();
-#endif
 	unregister_netdevice_notifier(&br_device_notifier);
 	brioctl_set(NULL);
 
--- br.orig/net/bridge/br_private.h
+++ br/net/bridge/br_private.h
@@ -192,8 +192,13 @@ extern int br_dev_ioctl(struct net_devic
 extern int br_ioctl_deviceless_stub(unsigned int cmd, void __user *arg);
 
 /* br_netfilter.c */
+#ifdef CONFIG_BRIDGE_NETFILTER
 extern int br_netfilter_init(void);
 extern void br_netfilter_fini(void);
+#else
+#define br_netfilter_init()	(0)
+#define br_netfilter_fini()	do { } while(0)
+#endif
 
 /* br_stp.c */
 extern void br_log_state(const struct net_bridge_port *p);

--

[patch 3/3] bridge: netlink interface for link management

[Stephen Hemminger]

[-- Attachment #1: bridge-netlink.patch --]
[-- Type: text/plain, Size: 7922 bytes --]

Add basic netlink support to the Ethernet bridge. Including:
 * dump interfaces in bridges
 * monitor link status changes
 * change state of bridge port

For some demo programs see:
	http://developer.osdl.org/shemminger/prototypes/brnl.tar.gz

These are to allow building a daemon that does alternative implementations
of Spanning Tree Protocol.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>


--- br.orig/net/bridge/Makefile
+++ br/net/bridge/Makefile
@@ -6,7 +6,7 @@ obj-$(CONFIG_BRIDGE) += bridge.o
 
 bridge-y	:= br.o br_device.o br_fdb.o br_forward.o br_if.o br_input.o \
 			br_ioctl.o br_notify.o br_stp.o br_stp_bpdu.o \
-			br_stp_if.o br_stp_timer.o
+			br_stp_if.o br_stp_timer.o br_netlink.o
 
 bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
 
--- /dev/null
+++ br/net/bridge/br_netlink.c
@@ -0,0 +1,199 @@
+/*
+ *	Bridge netlink control interface
+ *
+ *	Authors:
+ *	Stephen Hemminger		<shemminger@osdl.org>
+ *
+ *	This program is free software; you can redistribute it and/or
+ *	modify it under the terms of the GNU General Public License
+ *	as published by the Free Software Foundation; either version
+ *	2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/rtnetlink.h>
+#include "br_private.h"
+
+/*
+ * Create one netlink message for one interface
+ * Contains port and master info as well as carrier and bridge state.
+ */
+static int br_fill_ifinfo(struct sk_buff *skb, const struct net_bridge_port *port,
+			  u32 pid, u32 seq, int event, unsigned int flags)
+{
+	const struct net_bridge *br = port->br;
+	const struct net_device *dev = port->dev;
+	struct ifinfomsg *r;
+	struct nlmsghdr *nlh;
+	unsigned char *b = skb->tail;
+	u32 mtu = dev->mtu;
+	u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN;
+	u8 portstate = port->state;
+
+	pr_debug("br_fill_info event %d port %s master %s\n",
+		 event, dev->name, br->dev->name);
+
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*r), flags);
+	r = NLMSG_DATA(nlh);
+	r->ifi_family = AF_BRIDGE;
+	r->__ifi_pad = 0;
+	r->ifi_type = dev->type;
+	r->ifi_index = dev->ifindex;
+	r->ifi_flags = dev_get_flags(dev);
+	r->ifi_change = 0;
+
+	RTA_PUT(skb, IFLA_IFNAME, strlen(dev->name)+1, dev->name);
+
+	RTA_PUT(skb, IFLA_MASTER, sizeof(int), &br->dev->ifindex);
+
+	if (dev->addr_len)
+		RTA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr);
+
+	RTA_PUT(skb, IFLA_MTU, sizeof(mtu), &mtu);
+	if (dev->ifindex != dev->iflink)
+		RTA_PUT(skb, IFLA_LINK, sizeof(int), &dev->iflink);
+
+
+	RTA_PUT(skb, IFLA_OPERSTATE, sizeof(operstate), &operstate);
+
+	if (event == RTM_NEWLINK)
+		RTA_PUT(skb, IFLA_PROTINFO, sizeof(portstate), &portstate);
+
+	nlh->nlmsg_len = skb->tail - b;
+
+	return skb->len;
+
+nlmsg_failure:
+rtattr_failure:
+
+	skb_trim(skb, b - skb->data);
+	return -EINVAL;
+}
+
+/*
+ * Notify listeners of a change in port information
+ */
+void br_ifinfo_notify(int event, struct net_bridge_port *port)
+{
+	struct sk_buff *skb;
+	int err = -ENOMEM;
+
+	pr_debug("bridge notify event=%d\n", event);
+	skb = alloc_skb(NLMSG_SPACE(sizeof(struct ifinfomsg) + 128),
+			GFP_ATOMIC);
+	if (!skb)
+		goto err_out;
+
+	err = br_fill_ifinfo(skb, port, current->pid, 0, event, 0);
+	if (err)
+		goto err_kfree;
+
+	NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
+	netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_ATOMIC);
+	return;
+
+err_kfree:
+	kfree_skb(skb);
+err_out:
+	netlink_set_err(rtnl, 0, RTNLGRP_LINK, err);
+}
+
+/*
+ * Dump information about all ports, in response to GETLINK
+ */
+static int br_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct net_device *dev;
+	int idx;
+	int s_idx = cb->args[0];
+	int err = 0;
+
+	read_lock(&dev_base_lock);
+	for (dev = dev_base, idx = 0; dev; dev = dev->next) {
+		struct net_bridge_port *p = dev->br_port;
+
+		/* not a bridge port */
+		if (!p)
+			continue;
+
+		if (idx < s_idx)
+			continue;
+
+		err = br_fill_ifinfo(skb, p, NETLINK_CB(cb->skb).pid,
+				     cb->nlh->nlmsg_seq, RTM_NEWLINK, NLM_F_MULTI);
+		if (err <= 0)
+			break;
+		++idx;
+	}
+	read_unlock(&dev_base_lock);
+
+	cb->args[0] = idx;
+
+	return skb->len;
+}
+
+/*
+ * Change state of port (ie from forwarding to blocking etc)
+ * Used by spanning tree in user space.
+ */
+static int br_rtm_setlink(struct sk_buff *skb,  struct nlmsghdr *nlh, void *arg)
+{
+	struct rtattr  **rta = arg;
+	struct ifinfomsg *ifm = NLMSG_DATA(nlh);
+	struct net_device *dev;
+	struct net_bridge_port *p;
+	u8 new_state;
+
+	if (ifm->ifi_family != AF_BRIDGE)
+		return -EPFNOSUPPORT;
+
+	/* Must pass valid state as PROTINFO */
+	if (rta[IFLA_PROTINFO-1]) {
+		u8 *pstate = RTA_DATA(rta[IFLA_PROTINFO-1]);
+		new_state = *pstate;
+	} else
+		return -EINVAL;
+
+	if (new_state > BR_STATE_BLOCKING)
+		return -EINVAL;
+
+	/* Find bridge port */
+	dev = __dev_get_by_index(ifm->ifi_index);
+	if (!dev)
+		return -ENODEV;
+
+	p = dev->br_port;
+	if (!p)
+		return -EINVAL;
+
+	/* if kernel STP is running, don't allow changes */
+	if (p->br->stp_enabled)
+		return -EBUSY;
+
+	if (!netif_running(dev))
+		return -ENETDOWN;
+
+	if (!netif_carrier_ok(dev) && new_state != BR_STATE_DISABLED)
+		return -ENETDOWN;
+
+	p->state = new_state;
+	br_log_state(p);
+	return 0;
+}
+
+
+static struct rtnetlink_link bridge_rtnetlink_table[RTM_NR_MSGTYPES] = {
+	[RTM_GETLINK - RTM_BASE] = { .dumpit	= br_dump_ifinfo, },
+	[RTM_SETLINK - RTM_BASE] = { .doit      = br_rtm_setlink, },
+};
+
+void __init br_netlink_init(void)
+{
+	rtnetlink_links[PF_BRIDGE] = bridge_rtnetlink_table;
+}
+
+void __exit br_netlink_fini(void)
+{
+	rtnetlink_links[PF_BRIDGE] = NULL;
+}
+
--- br.orig/net/bridge/br_private.h
+++ br/net/bridge/br_private.h
@@ -29,7 +29,7 @@
 
 #define BR_PORT_DEBOUNCE (HZ/10)
 
-#define BR_VERSION	"2.1"
+#define BR_VERSION	"2.2"
 
 typedef struct bridge_id bridge_id;
 typedef struct mac_addr mac_addr;
@@ -237,6 +237,11 @@ extern struct net_bridge_fdb_entry *(*br
 extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
 
 
+/* br_netlink.c */
+extern void br_netlink_init(void);
+extern void br_netlink_fini(void);
+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;
--- br.orig/net/bridge/br_notify.c
+++ br/net/bridge/br_notify.c
@@ -14,6 +14,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/rtnetlink.h>
 
 #include "br_private.h"
 
@@ -49,6 +50,7 @@ static int br_device_event(struct notifi
 
 	case NETDEV_CHANGEADDR:
 		br_fdb_changeaddr(p, dev->dev_addr);
+		br_ifinfo_notify(RTM_NEWLINK, p);
 		br_stp_recalculate_bridge_id(br);
 		break;
 
--- br.orig/net/bridge/br_stp_if.c
+++ br/net/bridge/br_stp_if.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/smp_lock.h>
 #include <linux/etherdevice.h>
+#include <linux/rtnetlink.h>
 
 #include "br_private.h"
 #include "br_private_stp.h"
@@ -86,6 +87,7 @@ void br_stp_disable_bridge(struct net_br
 void br_stp_enable_port(struct net_bridge_port *p)
 {
 	br_init_port(p);
+	br_ifinfo_notify(RTM_NEWLINK, p);
 	br_port_state_selection(p->br);
 }
 
@@ -99,6 +101,8 @@ void br_stp_disable_port(struct net_brid
 	printk(KERN_INFO "%s: port %i(%s) entering %s state\n",
 	       br->dev->name, p->port_no, p->dev->name, "disabled");
 
+	br_ifinfo_notify(RTM_DELLINK, p);
+
 	wasroot = br_is_root_bridge(br);
 	br_become_designated_port(p);
 	p->state = BR_STATE_DISABLED;
--- br.orig/net/bridge/br.c
+++ br/net/bridge/br.c
@@ -48,6 +48,7 @@ static int __init br_init(void)
 	if (err)
 		goto err_out2;
 
+	br_netlink_init();
 	brioctl_set(br_ioctl_deviceless_stub);
 	br_handle_frame_hook = br_handle_frame;
 
@@ -67,6 +68,7 @@ static void __exit br_deinit(void)
 {
 	rcu_assign_pointer(br_stp_sap->rcv_func, NULL);
 
+	br_netlink_fini();
 	br_netfilter_fini();
 	unregister_netdevice_notifier(&br_device_notifier);
 	brioctl_set(NULL);

--

Re: [patch 3/3] bridge: netlink interface for link management

[jamal]

Very nice.

On Wed, 2006-24-05 at 10:12 -0700, Stephen Hemminger wrote:
> plain text document attachment (bridge-netlink.patch)
> Add basic netlink support to the Ethernet bridge. Including:
>  * dump interfaces in bridges
>  * monitor link status changes
>  * change state of bridge port


> +static int br_fill_ifinfo(struct sk_buff *skb, const struct net_bridge_port *port,
> +			  u32 pid, u32 seq, int event, unsigned int flags)
> +{
> +	const struct net_bridge *br = port->br;
> +	const struct net_device *dev = port->dev;
> +	struct ifinfomsg *r;
> +	struct nlmsghdr *nlh;
> +	unsigned char *b = skb->tail;
> +	u32 mtu = dev->mtu;
> +	u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN;
> +	u8 portstate = port->state;
> +
> +	pr_debug("br_fill_info event %d port %s master %s\n",
> +		 event, dev->name, br->dev->name);
> +
> +	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*r), flags);
> +	r = NLMSG_DATA(nlh);
> +	r->ifi_family = AF_BRIDGE;
> +	r->__ifi_pad = 0;
> +	r->ifi_type = dev->type;
> +	r->ifi_index = dev->ifindex;
> +	r->ifi_flags = dev_get_flags(dev);
> +	r->ifi_change = 0;
> +
> +	RTA_PUT(skb, IFLA_IFNAME, strlen(dev->name)+1, dev->name);
> +
> +	RTA_PUT(skb, IFLA_MASTER, sizeof(int), &br->dev->ifindex);
> +
> +	if (dev->addr_len)
> +		RTA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr);
> +
> +	RTA_PUT(skb, IFLA_MTU, sizeof(mtu), &mtu);
> +	if (dev->ifindex != dev->iflink)
> +		RTA_PUT(skb, IFLA_LINK, sizeof(int), &dev->iflink);
> +
> +
> +	RTA_PUT(skb, IFLA_OPERSTATE, sizeof(operstate), &operstate);
> +
> +	if (event == RTM_NEWLINK)
> +		RTA_PUT(skb, IFLA_PROTINFO, sizeof(portstate), &portstate);
> +
> +	nlh->nlmsg_len = skb->tail - b;
> +
> +	return skb->len;
> +

Is it desirable to do the above? link events already happen for each
netdevice, no? If not it would probably make more sense to export the
link netlink code (instead of replicating it as above) and just call it
for each netdevice.
I think it is worth reporting all the children (their ifindices) of a
bridge when eventing.

BTW, Where is the bridge state (as in learning, blocking etc) carried? 
Having events for that i would deem as valuable.

> +static int br_rtm_setlink(struct sk_buff *skb,  struct nlmsghdr *nlh, void *arg)
> +{
> +	struct rtattr  **rta = arg;
> +	struct ifinfomsg *ifm = NLMSG_DATA(nlh);
> +	struct net_device *dev;
> +	struct net_bridge_port *p;
> +	u8 new_state;
> +
> +	if (ifm->ifi_family != AF_BRIDGE)
> +		return -EPFNOSUPPORT;
> +
> +	/* Must pass valid state as PROTINFO */
> +	if (rta[IFLA_PROTINFO-1]) {

Why not a noun like IFLA_BRIDGE_STATE ?


> +	/* if kernel STP is running, don't allow changes */
> +	if (p->br->stp_enabled)
> +		return -EBUSY;

Hopefully above to die some day...

> +
> +	p->state = new_state;
> +	br_log_state(p);

Assuming the above will generate an event which reports things like the
children of the bridge and the STP state of the bridge etc i.e things in
struct net_bridge_port mostly.

cheers,
jamal

Re: [patch 3/3] bridge: netlink interface for link management

[Stephen Hemminger]

On Wed, 24 May 2006 14:11:55 -0400
jamal <hadi@cyberus.ca> wrote:

> 
> Very nice.
> 
> On Wed, 2006-24-05 at 10:12 -0700, Stephen Hemminger wrote:
> > plain text document attachment (bridge-netlink.patch)
> > Add basic netlink support to the Ethernet bridge. Including:
> >  * dump interfaces in bridges
> >  * monitor link status changes
> >  * change state of bridge port
> 
> 
> > +static int br_fill_ifinfo(struct sk_buff *skb, const struct net_bridge_port *port,
> > +			  u32 pid, u32 seq, int event, unsigned int flags)
> > +{
> > +	const struct net_bridge *br = port->br;
> > +	const struct net_device *dev = port->dev;
> > +	struct ifinfomsg *r;
> > +	struct nlmsghdr *nlh;
> > +	unsigned char *b = skb->tail;
> > +	u32 mtu = dev->mtu;
> > +	u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN;
> > +	u8 portstate = port->state;
> > +
> > +	pr_debug("br_fill_info event %d port %s master %s\n",
> > +		 event, dev->name, br->dev->name);
> > +
> > +	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*r), flags);
> > +	r = NLMSG_DATA(nlh);
> > +	r->ifi_family = AF_BRIDGE;
> > +	r->__ifi_pad = 0;
> > +	r->ifi_type = dev->type;
> > +	r->ifi_index = dev->ifindex;
> > +	r->ifi_flags = dev_get_flags(dev);
> > +	r->ifi_change = 0;
> > +
> > +	RTA_PUT(skb, IFLA_IFNAME, strlen(dev->name)+1, dev->name);
> > +
> > +	RTA_PUT(skb, IFLA_MASTER, sizeof(int), &br->dev->ifindex);
> > +
> > +	if (dev->addr_len)
> > +		RTA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr);
> > +
> > +	RTA_PUT(skb, IFLA_MTU, sizeof(mtu), &mtu);
> > +	if (dev->ifindex != dev->iflink)
> > +		RTA_PUT(skb, IFLA_LINK, sizeof(int), &dev->iflink);
> > +
> > +
> > +	RTA_PUT(skb, IFLA_OPERSTATE, sizeof(operstate), &operstate);
> > +
> > +	if (event == RTM_NEWLINK)
> > +		RTA_PUT(skb, IFLA_PROTINFO, sizeof(portstate), &portstate);
> > +
> > +	nlh->nlmsg_len = skb->tail - b;
> > +
> > +	return skb->len;
> > +
> 
> Is it desirable to do the above? link events already happen for each
> netdevice, no? If not it would probably make more sense to export the
> link netlink code (instead of replicating it as above) and just call it
> for each netdevice.

The STP daemon only wants to know about interfaces in bridge, and it needs
to know the bridge port state.

> I think it is worth reporting all the children (their ifindices) of a
> bridge when eventing.

can easily track this in daemon.


> BTW, Where is the bridge state (as in learning, blocking etc) carried? 

In IFLA_PROTINFO

> Having events for that i would deem as valuable.
> 
> > +static int br_rtm_setlink(struct sk_buff *skb,  struct nlmsghdr *nlh, void *arg)
> > +{
> > +	struct rtattr  **rta = arg;
> > +	struct ifinfomsg *ifm = NLMSG_DATA(nlh);
> > +	struct net_device *dev;
> > +	struct net_bridge_port *p;
> > +	u8 new_state;
> > +
> > +	if (ifm->ifi_family != AF_BRIDGE)
> > +		return -EPFNOSUPPORT;
> > +
> > +	/* Must pass valid state as PROTINFO */
> > +	if (rta[IFLA_PROTINFO-1]) {
> 
> Why not a noun like IFLA_BRIDGE_STATE ?

Laziness.  Just used "protocol specific portion"

> 
> 
> > +	/* if kernel STP is running, don't allow changes */
> > +	if (p->br->stp_enabled)
> > +		return -EBUSY;
> 
> Hopefully above to die some day...
> 
> > +
> > +	p->state = new_state;
> > +	br_log_state(p);
> 
> Assuming the above will generate an event which reports things like the
> children of the bridge and the STP state of the bridge etc i.e things in
> struct net_bridge_port mostly.


The br_log_state just does console printk's.  

Re: [patch 0/3] bridge patches for 2.6.18

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Wed, 24 May 2006 10:12:16 -0700

> Some stuff for 2.6.18. The most important is adding netlink
> support for managing interfaces; this allows building STP
> as an application.

Applied to net-2.6.18, thanks Stephen.