[PATCH 0/5] bridge patchs

[PATCH 0/5] bridge patchs

[Stephen Hemminger]

Bridge patches. These are order by urgency from important to trivial.

[PATCH 1/5] bridge: send proper message_age in config BPDU

[Stephen Hemminger]

[-- Attachment #1: br-stp-bpdu-messageage.patch --]
[-- Type: text/plain, Size: 2443 bytes --]

A bridge topology with three systems:

      +------+  +------+
      | A(2) |--| B(1) |
      +------+  +------+
           \    /
          +------+
          | C(3) |
          +------+

What is supposed to happen:
 * bridge with the lowest ID is elected root (for example: B)
 * C detects that A->C is higher cost path and puts in blocking state

What happens. Bridge with lowest id (B) is elected correctly as
root and things start out fine initially. But then config BPDU
doesn't get transmitted from A -> C. Because of that 
the link from A-C is transistioned to the forwarding state.

The root cause of this is that the configuration messages
is generated with bogus message age, and dropped before
sending.

In the standardmessage_age is supposed to be:
  the time since the generation of the Configuration BPDU by
  the Root that instigated the generation of this Configuration BPDU.

Reimplement this by recording the timestamp (age + jiffies) when
recording config information. The old code incorrectly used the time
elapsed on the ageing timer which was incorrect.

See also:
  https://bugzilla.vyatta.com/show_bug.cgi?id=7164

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Patch against net-next-2.6.
Obviously needs to go to stable and 3.0 if stil open.


--- a/net/bridge/br_private.h	2011-04-29 16:16:03.000000000 -0700
+++ b/net/bridge/br_private.h	2011-07-21 20:13:31.651989371 -0700
@@ -124,6 +124,7 @@ struct net_bridge_port
 	bridge_id			designated_bridge;
 	u32				path_cost;
 	u32				designated_cost;
+	unsigned long			designated_age;
 
 	struct timer_list		forward_delay_timer;
 	struct timer_list		hold_timer;
--- a/net/bridge/br_stp.c	2011-07-20 09:06:24.519997816 -0700
+++ b/net/bridge/br_stp.c	2011-07-21 20:13:31.651989371 -0700
@@ -164,8 +164,7 @@ void br_transmit_config(struct net_bridg
 	else {
 		struct net_bridge_port *root
 			= br_get_port(br, br->root_port);
-		bpdu.message_age = br->max_age
-			- (root->message_age_timer.expires - jiffies)
+		bpdu.message_age = (jiffies - root->designated_age)
 			+ MESSAGE_AGE_INCR;
 	}
 	bpdu.max_age = br->max_age;
@@ -189,6 +188,7 @@ static inline void br_record_config_info
 	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;
 
 	mod_timer(&p->message_age_timer, jiffies
 		  + (p->br->max_age - bpdu->message_age));

[PATCH 2/5] bridge: ignore bogus STP config packets

[Stephen Hemminger]

[-- Attachment #1: br-ignore-age-bogon.patch --]
[-- Type: text/plain, Size: 890 bytes --]

If the message_age is already greater than the max_age, then the
BPDU is bogus. Linux won't generate BPDU, but conformance tester
or buggy implementation might.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Patch against net-next. Do not apply to stable.


--- a/net/bridge/br_stp_bpdu.c	2011-07-21 20:25:02.991988983 -0700
+++ b/net/bridge/br_stp_bpdu.c	2011-07-21 20:29:11.175988844 -0700
@@ -210,6 +210,17 @@ void br_stp_rcv(const struct stp_proto *
 		bpdu.hello_time = br_get_ticks(buf+28);
 		bpdu.forward_delay = br_get_ticks(buf+30);
 
+		if (bpdu.message_age > bpdu.max_age) {
+			if (net_ratelimit())
+				br_notice(p->br,
+					  "port %u config from %pM"
+					  " (message_age %ul > max_age %ul)\n",
+					  p->port_no,
+					  eth_hdr(skb)->h_source,
+					  bpdu.message_age, bpdu.max_age);
+			goto out;
+		}
+
 		br_received_config_bpdu(p, &bpdu);
 	}
 

[PATCH 3/5] bridge: notifier called with the wrong device

[Stephen Hemminger]

[-- Attachment #1: br-notify-wrong-dev.patch --]
[-- Type: text/plain, Size: 819 bytes --]

If a new device is added to a bridge, the ethernet address of the
bridge network device may change. When the address changes, the
appropriate callback is called, but with the wrong device argument.
The address of the bridge device (ie br0) changes not the address
of the device being passed to add_if (ie eth0).

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Patch against net-next. Should go to stable.


--- a/net/bridge/br_if.c	2011-07-21 20:13:13.539989381 -0700
+++ b/net/bridge/br_if.c	2011-07-21 20:23:12.091989045 -0700
@@ -388,7 +388,7 @@ int br_add_if(struct net_bridge *br, str
 	br_ifinfo_notify(RTM_NEWLINK, p);
 
 	if (changed_addr)
-		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
 
 	dev_set_mtu(br->dev, br_min_mtu(br));
 

[PATCH 4/5] [PATCH] bridge: add notification over netlink when STP changes state

[Stephen Hemminger]

[-- Attachment #1: bridge-notify-on-stp-change.patch --]
[-- Type: text/plain, Size: 2211 bytes --]

When STP changes state of interface need to send a new link
message to reflect that change.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
This is a revised version of earlier patch.

 net/bridge/br_netlink.c   |    2 ++
 net/bridge/br_stp.c       |    4 +++-
 net/bridge/br_stp_if.c    |    3 +++
 net/bridge/br_stp_timer.c |    1 +
 4 files changed, 9 insertions(+), 1 deletion(-)

--- a/net/bridge/br_stp.c	2011-07-21 20:13:31.651989371 -0700
+++ b/net/bridge/br_stp.c	2011-07-21 20:23:14.123989044 -0700
@@ -363,6 +363,8 @@ static void br_make_blocking(struct net_
 
 		p->state = BR_STATE_BLOCKING;
 		br_log_state(p);
+		br_ifinfo_notify(RTM_NEWLINK, p);
+
 		del_timer(&p->forward_delay_timer);
 	}
 }
@@ -386,8 +388,8 @@ static void br_make_forwarding(struct ne
 		p->state = BR_STATE_LEARNING;
 
 	br_multicast_enable_port(p);
-
 	br_log_state(p);
+	br_ifinfo_notify(RTM_NEWLINK, p);
 
 	if (br->forward_delay != 0)
 		mod_timer(&p->forward_delay_timer, jiffies + br->forward_delay);
--- a/net/bridge/br_stp_if.c	2011-07-20 09:06:24.491997816 -0700
+++ b/net/bridge/br_stp_if.c	2011-07-21 20:23:14.123989044 -0700
@@ -88,6 +88,7 @@ void br_stp_enable_port(struct net_bridg
 	br_init_port(p);
 	br_port_state_selection(p->br);
 	br_log_state(p);
+	br_ifinfo_notify(RTM_NEWLINK, p);
 }
 
 /* called under bridge lock */
@@ -104,6 +105,8 @@ void br_stp_disable_port(struct net_brid
 	p->topology_change_ack = 0;
 	p->config_pending = 0;
 
+	br_ifinfo_notify(RTM_NEWLINK, p);
+
 	del_timer(&p->message_age_timer);
 	del_timer(&p->forward_delay_timer);
 	del_timer(&p->hold_timer);
--- a/net/bridge/br_stp_timer.c	2011-07-20 09:06:24.507997816 -0700
+++ b/net/bridge/br_stp_timer.c	2011-07-21 20:23:14.123989044 -0700
@@ -97,6 +97,7 @@ static void br_forward_delay_timer_expir
 		netif_carrier_on(br->dev);
 	}
 	br_log_state(p);
+	br_ifinfo_notify(RTM_NEWLINK, p);
 	spin_unlock(&br->lock);
 }
 
--- a/net/bridge/br_netlink.c	2011-07-20 09:06:24.499997816 -0700
+++ b/net/bridge/br_netlink.c	2011-07-21 20:23:14.123989044 -0700
@@ -188,6 +188,8 @@ static int br_rtm_setlink(struct sk_buff
 
 	p->state = new_state;
 	br_log_state(p);
+	br_ifinfo_notify(RTM_NEWLINK, p);
+
 	return 0;
 }
 

[PATCH 5/5] bridge: minor cleanups

[Stephen Hemminger]

[-- Attachment #1: bridge-deinline.patch --]
[-- Type: text/plain, Size: 4642 bytes --]

Some minor cleanups that won't impact code:
  1. Remove inline from non-critical functions; compiler will most
     likely inline them anyway.
  2. Make function args const where possible.
  3. Whitespace cleanup

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


---
 net/bridge/br_private_stp.h |    3 ++-
 net/bridge/br_stp.c         |   23 +++++++++++------------
 net/bridge/br_stp_bpdu.c    |    4 +---
 3 files changed, 14 insertions(+), 16 deletions(-)

--- a/net/bridge/br_stp.c	2011-07-22 08:23:29.059970391 -0700
+++ b/net/bridge/br_stp.c	2011-07-22 08:32:55.347949086 -0700
@@ -109,7 +109,6 @@ static void br_root_selection(struct net
 	list_for_each_entry(p, &br->port_list, list) {
 		if (br_should_become_root_port(p, root_port))
 			root_port = p->port_no;
-
 	}
 
 	br->root_port = root_port;
@@ -145,7 +144,6 @@ void br_transmit_config(struct net_bridg
 	struct br_config_bpdu bpdu;
 	struct net_bridge *br;
 
-
 	if (timer_pending(&p->hold_timer)) {
 		p->config_pending = 1;
 		return;
@@ -181,8 +179,8 @@ void br_transmit_config(struct net_bridg
 }
 
 /* called under bridge lock */
-static inline void br_record_config_information(struct net_bridge_port *p,
-						const struct br_config_bpdu *bpdu)
+static void br_record_config_information(struct net_bridge_port *p,
+					 const struct br_config_bpdu *bpdu)
 {
 	p->designated_root = bpdu->root;
 	p->designated_cost = bpdu->root_path_cost;
@@ -195,7 +193,7 @@ static inline void br_record_config_info
 }
 
 /* called under bridge lock */
-static inline void br_record_config_timeout_values(struct net_bridge *br,
+static void br_record_config_timeout_values(struct net_bridge *br,
 					    const struct br_config_bpdu *bpdu)
 {
 	br->max_age = bpdu->max_age;
@@ -254,7 +252,8 @@ static void br_designated_port_selection
 }
 
 /* called under bridge lock */
-static int br_supersedes_port_info(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
+static int br_supersedes_port_info(const struct net_bridge_port *p,
+				   const struct br_config_bpdu *bpdu)
 {
 	int t;
 
@@ -285,7 +284,7 @@ static int br_supersedes_port_info(struc
 }
 
 /* called under bridge lock */
-static inline void br_topology_change_acknowledged(struct net_bridge *br)
+static void br_topology_change_acknowledged(struct net_bridge *br)
 {
 	br->topology_change_detected = 0;
 	del_timer(&br->tcn_timer);
@@ -327,7 +326,7 @@ void br_config_bpdu_generation(struct ne
 }
 
 /* called under bridge lock */
-static inline void br_reply(struct net_bridge_port *p)
+static void br_reply(struct net_bridge_port *p)
 {
 	br_transmit_config(p);
 }
@@ -381,8 +380,7 @@ static void br_make_forwarding(struct ne
 		p->state = BR_STATE_FORWARDING;
 		br_topology_change_detection(br);
 		del_timer(&p->forward_delay_timer);
-	}
-	else if (br->stp_enabled == BR_KERNEL_STP)
+	} else if (br->stp_enabled == BR_KERNEL_STP)
 		p->state = BR_STATE_LISTENING;
 	else
 		p->state = BR_STATE_LEARNING;
@@ -433,14 +431,15 @@ void br_port_state_selection(struct net_
 }
 
 /* called under bridge lock */
-static inline void br_topology_change_acknowledge(struct net_bridge_port *p)
+static void br_topology_change_acknowledge(struct net_bridge_port *p)
 {
 	p->topology_change_ack = 1;
 	br_transmit_config(p);
 }
 
 /* called under bridge lock */
-void br_received_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
+void br_received_config_bpdu(struct net_bridge_port *p,
+			     const struct br_config_bpdu *bpdu)
 {
 	struct net_bridge *br;
 	int was_root;
--- a/net/bridge/br_stp_bpdu.c	2011-07-22 08:23:29.043970390 -0700
+++ b/net/bridge/br_stp_bpdu.c	2011-07-22 08:31:50.571951522 -0700
@@ -222,9 +222,7 @@ void br_stp_rcv(const struct stp_proto *
 		}
 
 		br_received_config_bpdu(p, &bpdu);
-	}
-
-	else if (buf[0] == BPDU_TYPE_TCN) {
+	} else if (buf[0] == BPDU_TYPE_TCN) {
 		br_received_tcn_bpdu(p);
 	}
  out:
--- a/net/bridge/br_private_stp.h	2011-07-22 08:23:29.147970387 -0700
+++ b/net/bridge/br_private_stp.h	2011-07-22 08:33:35.611947572 -0700
@@ -56,7 +56,8 @@ extern void br_become_root_bridge(struct
 extern void br_config_bpdu_generation(struct net_bridge *);
 extern void br_configuration_update(struct net_bridge *);
 extern void br_port_state_selection(struct net_bridge *);
-extern void br_received_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu);
+extern void br_received_config_bpdu(struct net_bridge_port *p,
+				    const struct br_config_bpdu *bpdu);
 extern void br_received_tcn_bpdu(struct net_bridge_port *p);
 extern void br_transmit_config(struct net_bridge_port *p);
 extern void br_transmit_tcn(struct net_bridge *br);

Re: [PATCH 0/5] bridge patchs

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 22 Jul 2011 10:47:05 -0700

> Bridge patches. These are order by urgency from important to trivial.

All applied.