Linux Ethernet Bridging - Feb 2012 - [Bridge] [PATCH 1/2] bridge: Adjust min age inc for HZ > 256

min age increment needs to round up its min age tick for all
HZ values to guarantee message age is increasing.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
---
 net/bridge/br_stp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index a04eeb6..9a8aebd 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -17,9 +17,9 @@
 #include "br_private_stp.h"
 
 /* since time values in bpdu are in jiffies and then scaled (1/256)
- * before sending, make sure that is at least one.
+ * before sending, make sure that is at least one STP tick.
  */
-#define MESSAGE_AGE_INCR	((HZ < 256) ? 1 : (HZ/256))
+#define MESSAGE_AGE_INCR	((HZ / 256) + 1)
 
 static const char *const br_port_state_names[] = {
 	[BR_STATE_DISABLED] = "disabled",
-- 
1.7.3.4

commit bridge: send proper message_age in config BPDU
added this gem:
  bpdu.message_age = (jiffies - root->designated_age)
  p->designated_age = jiffies + bpdu->message_age;
Notice how bpdu->message_age is negated when reassigned to
bpdu.message_age. This causes message age to decrease breaking the
STP protocol.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
---
 net/bridge/br_stp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 9a8aebd..45331fe 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -188,7 +188,7 @@ static inline void br_record_config_information(struct
net_bridge_port *p,
 	p->designated_cost = bpdu->root_path_cost;
 	p->designated_bridge = bpdu->bridge_id;
 	p->designated_port = bpdu->port_id;
-	p->designated_age = jiffies + bpdu->message_age;
+	p->designated_age = jiffies - bpdu->message_age;
 
 	mod_timer(&p->message_age_timer, jiffies
 		  + (p->br->max_age - bpdu->message_age));
-- 
1.7.3.4

Ping? I think this a fairly big problem and should go to Linus
before next release.
Same for bridge: Adjust min age inc for HZ > 256

 Jocke

Joakim Tjernlund <Joakim.Tjernlund at transmode.se> wrote on 2012/02/28
19:37:11:
>
> commit bridge: send proper message_age in config BPDU
> added this gem:
>   bpdu.message_age = (jiffies - root->designated_age)
>   p->designated_age = jiffies + bpdu->message_age;
> Notice how bpdu->message_age is negated when reassigned to
> bpdu.message_age. This causes message age to decrease breaking the
> STP protocol.
>
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
> ---
>  net/bridge/br_stp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
> index 9a8aebd..45331fe 100644
> --- a/net/bridge/br_stp.c
> +++ b/net/bridge/br_stp.c
> @@ -188,7 +188,7 @@ static inline void br_record_config_information(struct
net_bridge_port *p,
>     p->designated_cost = bpdu->root_path_cost;
>     p->designated_bridge = bpdu->bridge_id;
>     p->designated_port = bpdu->port_id;
> -   p->designated_age = jiffies + bpdu->message_age;
> +   p->designated_age = jiffies - bpdu->message_age;
>
>     mod_timer(&p->message_age_timer, jiffies
>          + (p->br->max_age - bpdu->message_age));
> --
> 1.7.3.4
>

On Tuesday 28 February 2012 20:37:10 Joakim Tjernlund
wrote:
> min age increment needs to round up its min age tick for all
> HZ values to guarantee message age is increasing.
>
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
> ---
>  net/bridge/br_stp.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
> index a04eeb6..9a8aebd 100644
> --- a/net/bridge/br_stp.c
> +++ b/net/bridge/br_stp.c
> @@ -17,9 +17,9 @@
>  #include "br_private_stp.h"
>
>  /* since time values in bpdu are in jiffies and then scaled (1/256)
> - * before sending, make sure that is at least one.
> + * before sending, make sure that is at least one STP tick.
>   */
> -#define MESSAGE_AGE_INCR	((HZ < 256) ? 1 : (HZ/256))
> +#define MESSAGE_AGE_INCR	((HZ / 256) + 1)
>
>  static const char *const br_port_state_names[] = {
>  	[BR_STATE_DISABLED] = "disabled",
In general I can see the problem which is solved by your patches and it seems 
to me that solution is adequate.
Just a nitpick: I think the following is a little bit more accurate (take into 
account cases when HZ is multiple of 256, which is rare but possible):

-#define MESSAGE_AGE_INCR	((HZ < 256) ? 1 : (HZ/256))
+#define MESSAGE_AGE_INCR	((HZ / 256) + ((HZ % 256) ? 1 : 0))

Also I'd advice to send patches to the netdev list too, people there are
much
more responsive than at the bridge list.

NB: don't you want to try 802.1Q-2005 compatible user-space {m/r}stp 
implementation instead of rusty in-kernel 802.1D? You can find sources here: 
http://sourceforge.net/p/mstpd/code/28/tree/

-- 
Vitalii Demianets