[PATCH] bridge: flush forwarding table when device carrier off

[PATCH] bridge: flush forwarding table when device carrier off

[Stephen Hemminger]

Flush the forwarding table when carrier is lost. This helps for
availability because we don't want to forward to a downed device and
new packets may come in on other links.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 net/bridge/br_fdb.c     |    7 ++++++-
 net/bridge/br_if.c      |    4 ++--
 net/bridge/br_private.h |    2 +-
 net/bridge/br_stp_if.c  |    2 ++
 4 files changed, 11 insertions(+), 4 deletions(-)

--- bridge.orig/net/bridge/br_fdb.c
+++ bridge/net/bridge/br_fdb.c
@@ -128,7 +128,10 @@ void br_fdb_cleanup(unsigned long _data)
 	mod_timer(&br->gc_timer, jiffies + HZ/10);
 }
 
-void br_fdb_delete_by_port(struct net_bridge *br, struct net_bridge_port *p)
+
+void br_fdb_delete_by_port(struct net_bridge *br,
+			   const struct net_bridge_port *p,
+			   int do_all)
 {
 	int i;
 
@@ -142,6 +145,8 @@ void br_fdb_delete_by_port(struct net_br
 			if (f->dst != p) 
 				continue;
 
+			if (f->is_static & !do_all)
+				continue;
 			/*
 			 * if multiple ports all have the same device address
 			 * then when one port is deleted, assign
--- bridge.orig/net/bridge/br_if.c
+++ bridge/net/bridge/br_if.c
@@ -163,7 +163,7 @@ static void del_nbp(struct net_bridge_po
 	br_stp_disable_port(p);
 	spin_unlock_bh(&br->lock);
 
-	br_fdb_delete_by_port(br, p);
+	br_fdb_delete_by_port(br, p, 1);
 
 	list_del_rcu(&p->list);
 
@@ -448,7 +448,7 @@ int br_add_if(struct net_bridge *br, str
 
 	return 0;
 err2:
-	br_fdb_delete_by_port(br, p);
+	br_fdb_delete_by_port(br, p, 1);
 err1:
 	kobject_del(&p->kobj);
 err0:
--- bridge.orig/net/bridge/br_private.h
+++ bridge/net/bridge/br_private.h
@@ -143,7 +143,7 @@ extern void br_fdb_changeaddr(struct net
 			      const unsigned char *newaddr);
 extern void br_fdb_cleanup(unsigned long arg);
 extern void br_fdb_delete_by_port(struct net_bridge *br,
-			   struct net_bridge_port *p);
+				  const struct net_bridge_port *p, int do_all);
 extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
 						 const unsigned char *addr);
 extern struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
--- bridge.orig/net/bridge/br_stp_if.c
+++ bridge/net/bridge/br_stp_if.c
@@ -113,6 +113,8 @@ void br_stp_disable_port(struct net_brid
 	del_timer(&p->forward_delay_timer);
 	del_timer(&p->hold_timer);
 
+	br_fdb_delete_by_port(br, p, 0);
+
 	br_configuration_update(br);
 
 	br_port_state_selection(br);

Re: [PATCH] bridge: flush forwarding table when device carrier off

[Andy Gospodarek]

On Thu, Oct 12, 2006 at 11:24:31AM -0700, Stephen Hemminger wrote:
> Flush the forwarding table when carrier is lost. This helps for
> availability because we don't want to forward to a downed device and
> new packets may come in on other links.
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
> 

Stephen,

This is an excellent idea and all looks good except this check

+			if (f->is_static & !do_all)
+				continue;

should be this: 

+			if (f->is_static && !do_all)
+				continue;

I'll ACK a repost with that change.

-andy

Re: [PATCH] bridge: flush forwarding table when device carrier off

[Stephen Hemminger]

On Thu, 12 Oct 2006 16:10:44 -0400
Andy Gospodarek <andy@greyhouse.net> wrote:

> On Thu, Oct 12, 2006 at 11:24:31AM -0700, Stephen Hemminger wrote:
> > Flush the forwarding table when carrier is lost. This helps for
> > availability because we don't want to forward to a downed device and
> > new packets may come in on other links.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
> > 
> 
> Stephen,
> 
> This is an excellent idea and all looks good except this check
> 
> +			if (f->is_static & !do_all)
> +				continue;
> 
> should be this: 
> 
> +			if (f->is_static && !do_all)
> +				continue;
> 

Agreed, but it probably worked during testing because both flags
are strict booleans (ie 0/1)


-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [PATCH] bridge: flush forwarding table when device carrier off

[jamal]

On Thu, 2006-12-10 at 16:10 -0400, Andy Gospodarek wrote:
> On Thu, Oct 12, 2006 at 11:24:31AM -0700, Stephen Hemminger wrote:
> > Flush the forwarding table when carrier is lost. This helps for
> > availability because we don't want to forward to a downed device and
> > new packets may come in on other links.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
> > 
> 
> Stephen,
> 
> This is an excellent idea 

I am on the other extreme - this is problematic if you have a large
table already learnt. Agrevate that with an unstable link and it gets a
lot worse. Both of which dont sound unrealistic in say a wireless AP.

A more sane policy i have seen is a timer that flushes the table after a
programmed period; this way you counter a flipflop-ing link.
IOW, the best place is to have this in some user space daemon. If it has
to be in the kernel, can you add a systcl to disable it?

cheers,
jamal

Re: [PATCH] bridge: flush forwarding table when device carrier off

[Stephen Hemminger]

On Thu, 12 Oct 2006 17:30:33 -0400
jamal <hadi@cyberus.ca> wrote:

> On Thu, 2006-12-10 at 16:10 -0400, Andy Gospodarek wrote:
> > On Thu, Oct 12, 2006 at 11:24:31AM -0700, Stephen Hemminger wrote:
> > > Flush the forwarding table when carrier is lost. This helps for
> > > availability because we don't want to forward to a downed device and
> > > new packets may come in on other links.
> > > 
> > > Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
> > > 
> > 
> > Stephen,
> > 
> > This is an excellent idea 
> 
> I am on the other extreme - this is problematic if you have a large
> table already learnt. Agrevate that with an unstable link and it gets a
> lot worse. Both of which dont sound unrealistic in say a wireless AP.

We don't support bridging wireless, that requires some NDS stuff that
isn't supported, and requires more softmac than the stack has.

> A more sane policy i have seen is a timer that flushes the table after a
> programmed period; this way you counter a flipflop-ing link.

That's already there.

> IOW, the best place is to have this in some user space daemon. If it has
> to be in the kernel, can you add a systcl to disable it?
> 

When RSTP is in userspace, it will do the flushing.

-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [PATCH] bridge: flush forwarding table when device carrier off

[jamal]

On Thu, 2006-12-10 at 14:32 -0700, Stephen Hemminger wrote:

> > I am on the other extreme - this is problematic if you have a large
> > table already learnt. Agrevate that with an unstable link and it gets a
> > lot worse. Both of which dont sound unrealistic in say a wireless AP.
> 
> We don't support bridging wireless, that requires some NDS stuff that
> isn't supported, and requires more softmac than the stack has.
> 

I was more thinking of wireless-to-ethernet bridging; that should still
work, no? i.e say eth1 on wireless with eth0 on the wired side? 
In any case, that may be a bad example (and a digression) of something
that learns large tables. I have however seen 1K entries in bridging.

> > A more sane policy i have seen is a timer that flushes the table after a
> > programmed period; this way you counter a flipflop-ing link.
> 
> That's already there.
> 

ah, ok. So the patch is in an alternative to this then? 

> > IOW, the best place is to have this in some user space daemon. If it has
> > to be in the kernel, can you add a systcl to disable it?
> > 
> 
> When RSTP is in userspace, it will do the flushing.

Cool. And that makes a lot of sense.

cheers,
jamal

Re: [PATCH] bridge: flush forwarding table when device carrier off

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Thu, 12 Oct 2006 11:24:31 -0700

> Flush the forwarding table when carrier is lost. This helps for
> availability because we don't want to forward to a downed device and
> new packets may come in on other links.
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
 ...
> +			if (f->is_static & !do_all)
> +				continue;

Applied with "&" changed to "&&" as mentioned elsewhere :)

Re: [Bridge] [PATCH] bridge: flush forwarding table when device carrier off

[ArcosCom Linux User]

Is this patch submitted into kernel tree? What version of kernel will have
this patch applied (thinking on 2.6.x and 2.4.x branchs)?

Thanks

El Jue, 12 de Octubre de 2006, 20:24, Stephen Hemminger escribió:
> Flush the forwarding table when carrier is lost. This helps for
> availability because we don't want to forward to a downed device and
> new packets may come in on other links.
>
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
> ---
>  net/bridge/br_fdb.c     |    7 ++++++-
>  net/bridge/br_if.c      |    4 ++--
>  net/bridge/br_private.h |    2 +-
>  net/bridge/br_stp_if.c  |    2 ++
>  4 files changed, 11 insertions(+), 4 deletions(-)
>
> --- bridge.orig/net/bridge/br_fdb.c
> +++ bridge/net/bridge/br_fdb.c
> @@ -128,7 +128,10 @@ void br_fdb_cleanup(unsigned long _data)
>  	mod_timer(&br->gc_timer, jiffies + HZ/10);
>  }
>
> -void br_fdb_delete_by_port(struct net_bridge *br, struct net_bridge_port
> *p)
> +
> +void br_fdb_delete_by_port(struct net_bridge *br,
> +			   const struct net_bridge_port *p,
> +			   int do_all)
>  {
>  	int i;
>
> @@ -142,6 +145,8 @@ void br_fdb_delete_by_port(struct net_br
>  			if (f->dst != p)
>  				continue;
>
> +			if (f->is_static & !do_all)
> +				continue;
>  			/*
>  			 * if multiple ports all have the same device address
>  			 * then when one port is deleted, assign
> --- bridge.orig/net/bridge/br_if.c
> +++ bridge/net/bridge/br_if.c
> @@ -163,7 +163,7 @@ static void del_nbp(struct net_bridge_po
>  	br_stp_disable_port(p);
>  	spin_unlock_bh(&br->lock);
>
> -	br_fdb_delete_by_port(br, p);
> +	br_fdb_delete_by_port(br, p, 1);
>
>  	list_del_rcu(&p->list);
>
> @@ -448,7 +448,7 @@ int br_add_if(struct net_bridge *br, str
>
>  	return 0;
>  err2:
> -	br_fdb_delete_by_port(br, p);
> +	br_fdb_delete_by_port(br, p, 1);
>  err1:
>  	kobject_del(&p->kobj);
>  err0:
> --- bridge.orig/net/bridge/br_private.h
> +++ bridge/net/bridge/br_private.h
> @@ -143,7 +143,7 @@ extern void br_fdb_changeaddr(struct net
>  			      const unsigned char *newaddr);
>  extern void br_fdb_cleanup(unsigned long arg);
>  extern void br_fdb_delete_by_port(struct net_bridge *br,
> -			   struct net_bridge_port *p);
> +				  const struct net_bridge_port *p, int do_all);
>  extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
>  						 const unsigned char *addr);
>  extern struct net_bridge_fdb_entry *br_fdb_get(struct net_bridge *br,
> --- bridge.orig/net/bridge/br_stp_if.c
> +++ bridge/net/bridge/br_stp_if.c
> @@ -113,6 +113,8 @@ void br_stp_disable_port(struct net_brid
>  	del_timer(&p->forward_delay_timer);
>  	del_timer(&p->hold_timer);
>
> +	br_fdb_delete_by_port(br, p, 0);
> +
>  	br_configuration_update(br);
>
>  	br_port_state_selection(br);
> _______________________________________________
> Bridge mailing list
> Bridge@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/bridge
>

Re: [Bridge] [PATCH] bridge: flush forwarding table when device carrier off

[David Miller]

From: "ArcosCom Linux User" <linux@arcoscom.com>
Date: Tue, 24 Oct 2006 09:39:08 +0200 (CEST)

> Is this patch submitted into kernel tree? What version of kernel will have
> this patch applied (thinking on 2.6.x and 2.4.x branchs)?

2.6.19 and later will have the patch.