Skip to contentSkip to conversation

BRIDGE: Fix faulty check in br_stp_recalculate_bridge_id()

PATCH In review [email protected] 3 participants 5 messages Dec 21, 2005 – Jan 3, 2006

Subsystems (2)
Networking Ethernet bridge
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 BRIDGE: Fix faulty check in br_stp_recalculate_bridge_id()

I did a compile with extra gcc warnings, and found a bug in net/bridge/br_stp_if.c function br_stp_recalculate_bridge_id():
compare_ether_addr() returns 0 if match, positive if not, so checking it for negative is wrong.

Signed-of-by Mika Kukkonen <[email protected]>

---

 net/bridge/br_stp_if.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
net/bridge/br_stp_if.c +1 -1
index ac09b6a..08c52c2 100644
@@ -158,7 +158,7 @@ void br_stp_recalculate_bridge_id(struct
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (addr == br_mac_zero ||
-		    compare_ether_addr(p->dev->dev_addr, addr) < 0)
+		    compare_ether_addr(p->dev->dev_addr, addr))
 			addr = p->dev->dev_addr;
 
 	}
#1 lore raw
Quoted 17 lines · On Wed, 21 Dec 2005 21:55:27 +0200 [email protected] wrote:

I did a compile with extra gcc warnings, and found a bug in net/bridge/br_stp_if.c function br_stp_recalculate_bridge_id():
compare_ether_addr() returns 0 if match, positive if not, so checking it for negative is wrong.

Signed-of-by Mika Kukkonen <[email protected]>

---

 net/bridge/br_stp_if.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
net/bridge/br_stp_if.c +0 -0
index ac09b6a..08c52c2 100644

net/bridge/br_stp_if.c void br_stp_recalculate_bridge_id(struct · line 164

Quoted 8 lines
net/bridge/br_stp_if.c +1 -1
@@ -158,7 +158,7 @@ void br_stp_recalculate_bridge_id(struct
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (addr == br_mac_zero ||
-		    compare_ether_addr(p->dev->dev_addr, addr) < 0)
+		    compare_ether_addr(p->dev->dev_addr, addr))
 			addr = p->dev->dev_addr;
 
 	}

Actually that compare_ether_addr needs to be replaced by memcmp again. Because for bridge id calc it wants the min() of all the device addresses.

Signature · 2 lines
Stephen Hemminger <[email protected]>
OSDL http://developer.osdl.org/~shemminger
maintainer
#2 lore raw

From: Stephen Hemminger <[email protected]>
Date: Wed, 21 Dec 2005 13:25:27 -0800

Quoted 9 lines
Quoted 6 lines · On Wed, 21 Dec 2005 21:55:27 +0200 [email protected] wrote:

I did a compile with extra gcc warnings, and found a bug in net/bridge/br_stp_if.c function br_stp_recalculate_bridge_id():
compare_ether_addr() returns 0 if match, positive if not, so checking it for negative is wrong.

Signed-of-by Mika Kukkonen <[email protected]>

...

Quoted 12 lines
Quoted 12 lines
net/bridge/br_stp_if.c +1 -1
index ac09b6a..08c52c2 100644
@@ -158,7 +158,7 @@ void br_stp_recalculate_bridge_id(struct
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (addr == br_mac_zero ||
-		    compare_ether_addr(p->dev->dev_addr, addr) < 0)
+		    compare_ether_addr(p->dev->dev_addr, addr))
 			addr = p->dev->dev_addr;
Quoted 4 lines

net/bridge/br_stp_if.c void br_stp_recalculate_bridge_id(struct · line 164

Quoted 1 line
net/bridge/br_stp_if.c +0 -0
 	}

Actually that compare_ether_addr needs to be replaced by memcmp again. Because for bridge id calc it wants the min() of all the device addresses.

I'll hold on this patch until that is worked out.

Thanks.

#3 lore raw

PATCH 1/1 BRIDGE: Fix faulty check in br_stp_recalculate_bridge_id()

One of the conversions from memcmp to compare_ether_addr is incorrect. We need to do relative comparison to determine min MAC address to use in bridge id.

Signed-off-by Stephen Hemminger <[email protected]>
net/bridge/br_stp_if.c +1 -1
@@ -158,7 +158,7 @@ void br_stp_recalculate_bridge_id(struct
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (addr == br_mac_zero ||
-		    compare_ether_addr(p->dev->dev_addr, addr) < 0)
+		    memcmp(p->dev->dev_addr, addr, ETH_ALEN) < 0)
 			addr = p->dev->dev_addr;
 
 	}
maintainer Applied
#4 lore raw

From: Stephen Hemminger <[email protected]>
Date: Tue, 3 Jan 2006 10:41:57 -0800

Quoted 5 lines

One of the conversions from memcmp to compare_ether_addr is incorrect. We need to do relative comparison to determine min MAC address to use in bridge id.

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

Applied, thanks.

Is this -stable material?