Skip to contentSkip to conversation

bridge: Cannot communicate with brX when its MAC address is changed

PATCH In review Koki Sanagi 3 participants 4 messages Nov 30, 2011 – Dec 1, 2011

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

Koki Sanagi

author
#0 lore raw

PATCH 1/1 bridge: Cannot communicate with brX when its MAC address is changed

When the MAC address of a bridge interface is changed, it cannot communicate with others. Because Whether or not a packet should be transferred to bridge interface depends on whether or not dst of a packet is in fdb and is_local=y. If we change MAC address of a bridge interface, it isn't in fdb.

This patch adds an condition that dst of a packet matches MAC address of a bridge interface to the conventional condition.

Signed-off-by Koki Sanagi <[email protected]>
---
 net/bridge/br_input.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
net/bridge/br_input.c +2 -1
index 5a31731..4e5c862 100644
@@ -94,7 +94,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
 			skb2 = skb;
 
 		br->dev->stats.multicast++;
-	} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+	} else if ((dst = __br_fdb_get(br, dest) && dst->is_local) ||
+		   !compare_ether_addr(p->br->dev->dev_addr, dest)) {
 		skb2 = skb;
 		/* Do not forward the packet since it's local. */
 		skb = NULL;
maintainer
#1 lore raw

From: Koki Sanagi <[email protected]>
Date: Wed, 30 Nov 2011 17:31:18 +0900

Quoted 9 lines

When the MAC address of a bridge interface is changed, it cannot communicate with others. Because Whether or not a packet should be transferred to bridge interface depends on whether or not dst of a packet is in fdb and is_local=y. If we change MAC address of a bridge interface, it isn't in fdb.

This patch adds an condition that dst of a packet matches MAC address of a bridge interface to the conventional condition.

Signed-off-by Koki Sanagi <[email protected]>

This looks like a patch I've seen before, and in any event it makes more sense to update the FDB when the MAC changes instead of adding a special bypass rule.

Stephen?

Quoted 18 lines
---
 net/bridge/br_input.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
net/bridge/br_input.c +2 -1
index 5a31731..4e5c862 100644
@@ -94,7 +94,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
 			skb2 = skb;
 
 		br->dev->stats.multicast++;
-	} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+	} else if ((dst = __br_fdb_get(br, dest) && dst->is_local) ||
+		   !compare_ether_addr(p->br->dev->dev_addr, dest)) {
 		skb2 = skb;
 		/* Do not forward the packet since it's local. */
 		skb = NULL;
#2 lore raw
In reply to Koki Sanagi
Quoted 19 lines · On Wed, 30 Nov 2011 17:31:18 +0900 Koki Sanagi <[email protected]>…

When the MAC address of a bridge interface is changed, it cannot communicate with others. Because Whether or not a packet should be transferred to bridge interface depends on whether or not dst of a packet is in fdb and is_local=y. If we change MAC address of a bridge interface, it isn't in fdb.

This patch adds an condition that dst of a packet matches MAC address of a bridge interface to the conventional condition.

Signed-off-by Koki Sanagi <[email protected]>
---
 net/bridge/br_input.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
net/bridge/br_input.c +0 -0
index 5a31731..4e5c862 100644

net/bridge/br_input.c int br_handle_frame_finish(struct sk_buff *skb) · line 101

Quoted 9 lines
net/bridge/br_input.c +2 -1
@@ -94,7 +94,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
 			skb2 = skb;
 
 		br->dev->stats.multicast++;
-	} else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+	} else if ((dst = __br_fdb_get(br, dest) && dst->is_local) ||
+		   !compare_ether_addr(p->br->dev->dev_addr, dest)) {
 		skb2 = skb;
 		/* Do not forward the packet since it's local. */
 		skb = NULL;

There is a problem with paren's in this version of the patch, don't apply it!

Looked into using fdb to handle this, but then there would be fdb entries where the destination port entry was either NULL (or a dummy), and that would require a bunch of auditing of all usages and could introduce new bugs.

I am testing a patch that does same thing by moving compare_ether up to where broadcast is tested.

maintainer
#3 lore raw

From: Stephen Hemminger <[email protected]>
Date: Thu, 1 Dec 2011 09:21:06 -0800

Quoted 7 lines

Looked into using fdb to handle this, but then there would be fdb entries where the destination port entry was either NULL (or a dummy), and that would require a bunch of auditing of all usages and could introduce new bugs.

I am testing a patch that does same thing by moving compare_ether up to where broadcast is tested.

Stephen please fix this bug correctly.

The bug is that the FDB gets updated with the initial MAC address, but doesn't get updated when the MAC address changes.

There is no other valid fix than to update the FDB when the MAC changes, and making whatever is necessary for that to work.

I'm not applying a patch that adds a MAC address comparison here, because you might was well not add the FDB entry in the first place if you're going to add a hack like that.