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

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

[Koki Sanagi]

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 <sanagi.koki@jp.fujitsu.com>
---
 net/bridge/br_input.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 5a31731..4e5c862 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -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;

Re: [PATCH] bridge: Cannot communicate with brX when its MAC address is changed

[David Miller]

From: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
Date: Wed, 30 Nov 2011 17:31:18 +0900

> 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 <sanagi.koki@jp.fujitsu.com>

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?

> ---
>  net/bridge/br_input.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 5a31731..4e5c862 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -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;

Re: [PATCH] bridge: Cannot communicate with brX when its MAC address is changed

[Stephen Hemminger]

On Wed, 30 Nov 2011 17:31:18 +0900
Koki Sanagi <sanagi.koki@jp.fujitsu.com> wrote:

> 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 <sanagi.koki@jp.fujitsu.com>
> ---
>  net/bridge/br_input.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 5a31731..4e5c862 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -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.

Re: [PATCH] bridge: Cannot communicate with brX when its MAC address is changed

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 1 Dec 2011 09:21:06 -0800

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