Skip to contentSkip to conversation

bridge: fix ordering of NEWLINK and NEWNEIGH events

PATCH Applied Stephen Hemminger 2 participants 4 messages Oct 1, 2011 – Oct 3, 2011 1 message not in the archive

Subsystems (3)
Networking Network drivers Ethernet bridge
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Stephen Hemminger

author
#1 lore raw

PATCH 1/2 bridge: fix ordering of NEWLINK and NEWNEIGH events

When port is added to a bridge, the old code would send the new neighbor netlink message before the subsequent new link message. This bug makes it difficult to use the monitoring API in an application.

This code changes the ordering to add the forwarding entry after the port is setup. One of the error checks (for invalid address) is moved earlier in the process to avoid having to do unwind.

Signed-off-by Stephen Hemminger <[email protected]>
---
 net/bridge/br_if.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
net/bridge/br_if.c +7 -8
@@ -13,6 +13,7 @@
 
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
+#include <linux/etherdevice.h>
 #include <linux/netpoll.h>
 #include <linux/ethtool.h>
 #include <linux/if_arp.h>

@@ -322,7 +323,8 @@ int br_add_if(struct net_bridge *br, str
 
 	/* Don't allow bridging non-ethernet like devices */
 	if ((dev->flags & IFF_LOOPBACK) ||
-	    dev->type != ARPHRD_ETHER || dev->addr_len != ETH_ALEN)
+	    dev->type != ARPHRD_ETHER || dev->addr_len != ETH_ALEN ||
+	    !is_valid_ether_addr(dev->dev_addr))
 		return -EINVAL;
 
 	/* No bridging of bridges */

@@ -350,10 +352,6 @@ int br_add_if(struct net_bridge *br, str
 	err = kobject_init_and_add(&p->kobj, &brport_ktype, &(dev->dev.kobj),
 				   SYSFS_BRIDGE_PORT_ATTR);
 	if (err)
-		goto err0;
-
-	err = br_fdb_insert(br, p, dev->dev_addr);
-	if (err)
 		goto err1;
 
 	err = br_sysfs_addif(p);

@@ -394,6 +392,9 @@ int br_add_if(struct net_bridge *br, str
 
 	dev_set_mtu(br->dev, br_min_mtu(br));
 
+	if (br_fdb_insert(br, p, dev->dev_addr))
+		netdev_err(dev, "failed insert local address bridge forwarding table\n");
+
 	kobject_uevent(&p->kobj, KOBJ_ADD);
 
 	return 0;

@@ -403,11 +404,9 @@ err4:
 err3:
 	sysfs_remove_link(br->ifobj, p->dev->name);
 err2:
-	br_fdb_delete_by_port(br, p, 1);
-err1:
 	kobject_put(&p->kobj);
 	p = NULL; /* kobject_put frees */
-err0:
+err1:
 	dev_set_promiscuity(dev, -1);
 put_back:
 	dev_put(dev);
maintainer Applied
#2 lore raw

From: Stephen Hemminger <[email protected]>
Date: Fri, 30 Sep 2011 17:37:26 -0700

Quoted 9 lines

When port is added to a bridge, the old code would send the new neighbor netlink message before the subsequent new link message. This bug makes it difficult to use the monitoring API in an application.

This code changes the ordering to add the forwarding entry after the port is setup. One of the error checks (for invalid address) is moved earlier in the process to avoid having to do unwind.

Signed-off-by Stephen Hemminger <[email protected]>

Applied.

author
#3 lore raw

PATCH 2/2 bridge: allow updating existing fdb entries

In reply to a message not in the archive

Need to allow application to update existing fdb entries that already exist. This makes bridge netlink neighbor API have same flags and semantics as ip neighbor table.

Signed-off-by Stephen Hemminger <[email protected]>
---
 net/bridge/br_fdb.c |   23 ++++++++++++++++-------
 1 files changed, 16 insertions(+), 7 deletions(-)
net/bridge/br_fdb.c +17 -8
@@ -558,19 +558,28 @@ skip:
 
 /* Create new static fdb entry */
 static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
-			 __u16 state)
+			 __u16 state, __u16 flags)
 {
 	struct net_bridge *br = source->br;
 	struct hlist_head *head = &br->hash[br_mac_hash(addr)];
 	struct net_bridge_fdb_entry *fdb;
 
 	fdb = fdb_find(head, addr);
-	if (fdb)
-		return -EEXIST;
-
-	fdb = fdb_create(head, source, addr);
-	if (!fdb)
-		return -ENOMEM;
+	if (fdb == NULL) {
+		if (!(flags & NLM_F_CREATE))
+			return -ENOENT;
+
+		fdb = fdb_create(head, source, addr);
+		if (!fdb)
+			return -ENOMEM;
+	} else {
+		if (flags & NLM_F_EXCL)
+			return -EEXIST;
+
+		if (flags & NLM_F_REPLACE)
+			fdb->updated = fdb->used = jiffies;
+		fdb->is_local = fdb->is_static = 0;
+	}
 
 	if (state & NUD_PERMANENT)
 		fdb->is_local = fdb->is_static = 1;

@@ -626,7 +635,7 @@ int br_fdb_add(struct sk_buff *skb, stru
 	}
 
 	spin_lock_bh(&p->br->hash_lock);
-	err = fdb_add_entry(p, addr, ndm->ndm_state);
+	err = fdb_add_entry(p, addr, ndm->ndm_state, nlh->nlmsg_flags);
 	spin_unlock_bh(&p->br->hash_lock);
 
 	return err;
maintainer Applied
#4 lore raw

Re: [PATCH 2/2] bridge: allow updating existing fdb entries

From: Stephen Hemminger <[email protected]>
Date: Fri, 30 Sep 2011 17:37:27 -0700

Quoted 5 lines

Need to allow application to update existing fdb entries that already exist. This makes bridge netlink neighbor API have same flags and semantics as ip neighbor table.

Signed-off-by Stephen Hemminger <[email protected]>

Applied.