Skip to contentSkip to conversation

Bridge patches for 2.6.22

PATCH Applied Stephen Hemminger 2 participants 8 messages Apr 25, 2007 – Apr 26, 2007

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

Stephen Hemminger

author
#0 lore raw

cover Bridge patches for 2.6.22

Here are some patches for bridge code in 2.6.22. The user mode RSTP from Aji is working. Anyone who wants to test it can get it from:
git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/rstp.git

Thanks

author
#1 lore raw

PATCH 1/4 bridge: don't change packet type

The change to forward STP bpdu's (for usermode STP) through normal path, changed the packet type in the process. Since link local stuff is multicast, it should stay pkt_type = PACKET_MULTICAST. The code was probably copy/pasted incorrectly from the bridge pseudo-device receive path.

Signed-off-by Stephen Hemminger <[email protected]>
net/bridge/br_input.c +1 -4
@@ -131,12 +131,9 @@ struct sk_buff *br_handle_frame(struct n
 	if (!is_valid_ether_addr(eth_hdr(skb)->h_source))
 		goto drop;
 
-	if (unlikely(is_link_local(dest))) {
-		skb->pkt_type = PACKET_HOST;
-
+	if (unlikely(is_link_local(dest)))
 		return (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
 				NULL, br_handle_local_finish) == 0) ? skb : NULL;
-	}
 
 	switch (p->state) {
 	case BR_STATE_FORWARDING:
maintainer Applied
#2 lore raw

Re: [PATCH 1/4] bridge: don't change packet type

From: Stephen Hemminger <[email protected]>
Date: Wed, 25 Apr 2007 16:47:38 -0700

Quoted 6 lines

The change to forward STP bpdu's (for usermode STP) through normal path, changed the packet type in the process. Since link local stuff is multicast, it should stay pkt_type = PACKET_MULTICAST. The code was probably copy/pasted incorrectly from the bridge pseudo-device receive path.

Signed-off-by Stephen Hemminger <[email protected]>

Applied.

author
#3 lore raw

PATCH 2/4 bridge: drop PAUSE frames

In reply to Stephen Hemminger

Pause frames should never make it out of the network device into the stack. But if a device was misconfigured, it might happen. So drop pause frames in bridge.

Signed-off-by Stephen Hemminger <[email protected]>
include/linux/if_ether.h +1 -0
@@ -61,6 +61,7 @@
 #define ETH_P_8021Q	0x8100          /* 802.1Q VLAN Extended Header  */
 #define ETH_P_IPX	0x8137		/* IPX over DIX			*/
 #define ETH_P_IPV6	0x86DD		/* IPv6 over bluebook		*/
+#define ETH_P_PAUSE	0x8808		/* IEEE Pause frames. See 802.3 31B */
 #define ETH_P_SLOW	0x8809		/* Slow Protocol. See 802.3ad 43B */
 #define ETH_P_WCCP	0x883E		/* Web-cache coordination protocol
 					 * defined in draft-wilson-wrec-wccp-v2-00.txt */
net/bridge/br_input.c +6 -1
@@ -131,9 +131,14 @@ struct sk_buff *br_handle_frame(struct n
 	if (!is_valid_ether_addr(eth_hdr(skb)->h_source))
 		goto drop;
 
-	if (unlikely(is_link_local(dest)))
+	if (unlikely(is_link_local(dest))) {
+		/* Pause frames shouldn't be passed up by driver anyway */
+		if (skb->protocol == htons(ETH_P_PAUSE))
+			goto drop;
+
 		return (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
 				NULL, br_handle_local_finish) == 0) ? skb : NULL;
+	}
 
 	switch (p->state) {
 	case BR_STATE_FORWARDING:
maintainer
#4 lore raw

Re: [PATCH 2/4] bridge: drop PAUSE frames

From: Stephen Hemminger <[email protected]>
Date: Wed, 25 Apr 2007 16:47:39 -0700

Quoted 5 lines

Pause frames should never make it out of the network device into the stack. But if a device was misconfigured, it might happen. So drop pause frames in bridge.

Signed-off-by Stephen Hemminger <[email protected]>

It can happen when in promiscuous mode, but that's the only legal case I can think of.

But that case shouldn't be hitting this path I don't think.

So this change is borderline and if anything we should put an assertion somewhere maybe, but applied for now.

author
#5 lore raw

PATCH 3/4 bridge: if no STP then forward all BPDUs

In reply to Stephen Hemminger

If a bridge is not running STP, then it has no way to detect a cycle in the network. But if it is not running STP and some other machine or device is running STP, then if STP BPDU's get forwarded to it can detect the cycle.

This is how the old 2.4 and early 2.6 code worked.

Signed-off-by Stephen Hemminger <[email protected]>
---
 net/bridge/br_input.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
net/bridge/br_input.c +8 -2
@@ -136,8 +136,14 @@ struct sk_buff *br_handle_frame(struct n
 		if (skb->protocol == htons(ETH_P_PAUSE))
 			goto drop;
 
-		return (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
-				NULL, br_handle_local_finish) == 0) ? skb : NULL;
+		/* Process STP BPDU's through normal netif_receive_skb() path */
+		if (p->br->stp_enabled != BR_NO_STP) {
+			if (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
+				    NULL, br_handle_local_finish))
+				return NULL;
+			else
+				return skb;
+		}
 	}
 
 	switch (p->state) {
author
#6 lore raw

PATCH 4/4 bridge: missing rtnl

In reply to Stephen Hemminger

Writing to /sys/class/net/brX/bridge/stp_state causes a warning because RTNL is not held when call br_stp_if.c

Signed-off-by Stephen Hemminger <[email protected]>
---
 net/bridge/br_sysfs_br.c |    2 ++
 1 file changed, 2 insertions(+)
net/bridge/br_sysfs_br.c +2 -0
@@ -149,9 +149,11 @@ static ssize_t show_stp_state(struct dev
 
 static void set_stp_state(struct net_bridge *br, unsigned long val)
 {
+	rtnl_lock();
 	spin_unlock_bh(&br->lock);
 	br_stp_set_enabled(br, val);
 	spin_lock_bh(&br->lock);
+	rtnl_unlock();
 }
 
 static ssize_t store_stp_state(struct device *d,
maintainer Applied
#7 lore raw

Re: [PATCH 4/4] bridge: missing rtnl

From: Stephen Hemminger <[email protected]>
Date: Wed, 25 Apr 2007 16:47:41 -0700

Quoted 4 lines

Writing to /sys/class/net/brX/bridge/stp_state causes a warning because RTNL is not held when call br_stp_if.c

Signed-off-by Stephen Hemminger <[email protected]>

Applied, thanks Stephen.