* [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included).
@ 2004-06-17 18:57 Mark Ruijter
2004-06-17 20:28 ` Stephen Hemminger
2004-07-16 23:28 ` Stephen Hemminger
0 siblings, 2 replies; 6+ messages in thread
From: Mark Ruijter @ 2004-06-17 18:57 UTC (permalink / raw)
To: bridge
[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]
1. Link state monitoring added.
We discovered the following problem with the existing bridge code.
The code doesn't monitor the link state of the interfaces. When using
two cisco switches with two Linux bridging firewalls the following
problem exists. When the Cisco switch that has the blocking (stp) Linux
bridge attached dies, the Linux bridge no longer receives stp packets
and changes it's state from blocking to forwarding. As soon as the Cisco
switch comes back alive a loop is created in the network for a short but
long enough period of time. :-(
To fix this I added link state monitoring to the code. This is only
active when stp is turned on. When a link fails the bridge port goes to
blocked and then to the new 'nolink' status. The bridge-utils patch
makes it possible to see the status with brctl showstp.
I did encounter a second problem when writing the link monitoring code.
When you add a vlan interface like eth0.10 then it's not possible to
obtain link state information from this interface.
The fix I wrote is that brctl now allows you to specify the interface
that contains the link state.
Example : brctl addif NUM1 eth0.10 eth0
The old syntax: "brctl addif NUM1 eth0.10" still works but the link will
always appear to be up.
"brctl addif NUM1 eth0" works with link detection since this is a valid
'link state providing' device.
If anyone is wants to know how to use the linux bridges with Cisco
switches running rstp feel free to ask. It involves a few tricks to get
it running.....
I also discovered a small bug in de bridge code shipped in 2.6.6.
2. Bug fix...
Creating a bridge like this is no longer possible:
vconfig add eth0 10
vconfig add eth0 20
ifconfig eth0.10 0.0.0.0 up
ifconfig eth0.20 0.0.0.0 up
brctl addbr NUM1
brctl addif NUM1 eth0.10
brctl addif NUM1 eth0.20
The last command produces the following error:
br_add_interface: File exists
Older bridge code did accept this although it would log a warning.
I hope the code can make it to mainstream. Please let me know how you
feel about it.
Mark Ruijter.
[-- Attachment #2: bridge-utils-1.0.4.patch --]
[-- Type: text/x-patch, Size: 5754 bytes --]
diff -urN ./bridge-utils-1.0.4-old/brctl/brctl.c ./bridge-utils-1.0.4/brctl/brctl.c
--- ./bridge-utils-1.0.4-old/brctl/brctl.c 2004-05-21 19:41:49.000000000 +0200
+++ ./bridge-utils-1.0.4/brctl/brctl.c 2004-06-16 12:10:10.000000000 +0200
@@ -54,7 +54,7 @@
goto help;
}
- if (argc < cmd->nargs + 2) {
+ if (argc < cmd->nargs + 2 ) {
fprintf(stderr, "incorrect number of arguments for command\n");
goto help;
}
diff -urN ./bridge-utils-1.0.4-old/brctl/brctl_cmd.c ./bridge-utils-1.0.4/brctl/brctl_cmd.c
--- ./bridge-utils-1.0.4-old/brctl/brctl_cmd.c 2004-06-04 20:03:40.000000000 +0200
+++ ./bridge-utils-1.0.4/brctl/brctl_cmd.c 2004-06-16 00:17:21.000000000 +0200
@@ -83,7 +83,7 @@
{
int err;
- switch (err = br_add_interface(argv[1], argv[2])) {
+ switch (err = br_add_interface(argv[1], argv[2], argv[3])) {
case 0:
return 0;
case ENODEV:
@@ -369,33 +369,33 @@
}
static const struct command commands[] = {
- { 1, "addbr", br_cmd_addbr, "<bridge>\t\tadd bridge" },
- { 1, "delbr", br_cmd_delbr, "<bridge>\t\tdelete bridge" },
+ { 1, "addbr", br_cmd_addbr, "<bridge>\t\t\tadd bridge" },
+ { 1, "delbr", br_cmd_delbr, "<bridge>\t\t\tdelete bridge" },
{ 2, "addif", br_cmd_addif,
- "<bridge> <device>\tadd interface to bridge" },
+ "<bridge> <dev> [<realdev>]\tadd interface to bridge, <dev> is the bridge device,\n\t\t\t\t\t\t\trealdev (optional) is the device with link information\n\t\t\t\t\t\t\tExample : brctl addif eth0.10 eth0" },
{ 2, "delif", br_cmd_delif,
- "<bridge> <device>\tdelete interface from bridge" },
+ "<bridge> <device>\t\tdelete interface from bridge" },
{ 2, "setageing", br_cmd_setageing,
- "<bridge> <time>\t\tset ageing time" },
+ "<bridge> <time>\t\t\tset ageing time" },
{ 2, "setbridgeprio", br_cmd_setbridgeprio,
- "<bridge> <prio>\t\tset bridge priority" },
+ "<bridge> <prio>\t\t\tset bridge priority" },
{ 2, "setfd", br_cmd_setfd,
- "<bridge> <time>\t\tset bridge forward delay" },
+ "<bridge> <time>\t\t\tset bridge forward delay" },
{ 2, "sethello", br_cmd_sethello,
- "<bridge> <time>\t\tset hello time" },
+ "<bridge> <time>\t\t\tset hello time" },
{ 2, "setmaxage", br_cmd_setmaxage,
- "<bridge> <time>\t\tset max message age" },
+ "<bridge> <time>\t\t\tset max message age" },
{ 3, "setpathcost", br_cmd_setpathcost,
- "<bridge> <port> <cost>\tset path cost" },
+ "<bridge> <port> <cost>\t\tset path cost" },
{ 3, "setportprio", br_cmd_setportprio,
- "<bridge> <port> <prio>\tset port priority" },
- { 0, "show", br_cmd_show, "\t\t\tshow a list of bridges" },
+ "<bridge> <port> <prio>\t\tset port priority" },
+ { 0, "show", br_cmd_show, "\t\t\t\tshow a list of bridges" },
{ 1, "showmacs", br_cmd_showmacs,
- "<bridge>\t\tshow a list of mac addrs"},
+ "<bridge>\t\t\tshow a list of mac addrs"},
{ 1, "showstp", br_cmd_showstp,
- "<bridge>\t\tshow bridge stp info"},
+ "<bridge>\t\t\tshow bridge stp info"},
{ 1, "stp", br_cmd_stp,
- "<bridge> <state>\tturn stp on/off" },
+ "<bridge> <state>\t\tturn stp on/off" },
};
const struct command *command_lookup(const char *cmd)
diff -urN ./bridge-utils-1.0.4-old/libbridge/libbridge.h ./bridge-utils-1.0.4/libbridge/libbridge.h
--- ./bridge-utils-1.0.4-old/libbridge/libbridge.h 2004-06-08 17:57:49.000000000 +0200
+++ ./bridge-utils-1.0.4/libbridge/libbridge.h 2004-06-15 23:50:23.000000000 +0200
@@ -93,7 +93,7 @@
struct port_info *info);
extern int br_add_bridge(const char *brname);
extern int br_del_bridge(const char *brname);
-extern int br_add_interface(const char *br, const char *dev);
+extern int br_add_interface(const char *br, const char *dev, const char *rdev);
extern int br_del_interface(const char *br, const char *dev);
extern int br_set_bridge_forward_delay(const char *br, struct timeval *tv);
extern int br_set_bridge_hello_time(const char *br, struct timeval *tv);
diff -urN ./bridge-utils-1.0.4-old/libbridge/libbridge_if.c ./bridge-utils-1.0.4/libbridge/libbridge_if.c
--- ./bridge-utils-1.0.4-old/libbridge/libbridge_if.c 2004-06-04 20:03:42.000000000 +0200
+++ ./bridge-utils-1.0.4/libbridge/libbridge_if.c 2004-06-15 23:49:47.000000000 +0200
@@ -66,14 +66,16 @@
return ret < 0 ? errno : 0;
}
-int br_add_interface(const char *bridge, const char *dev)
+int br_add_interface(const char *bridge, const char *dev, const char *rdev )
{
struct ifreq ifr;
int err;
int ifindex = if_nametoindex(dev);
+ int master_ifindex = if_nametoindex(rdev);
if (ifindex == 0)
return ENODEV;
+ if (master_ifindex == 0 ) master_ifindex=ifindex;
strncpy(ifr.ifr_name, bridge, IFNAMSIZ);
#ifdef SIOCBRADDIF
@@ -82,7 +84,7 @@
if (err < 0)
#endif
{
- unsigned long args[4] = { BRCTL_ADD_IF, ifindex, 0, 0 };
+ unsigned long args[4] = { BRCTL_ADD_IF, ifindex, master_ifindex, 0 };
ifr.ifr_data = (char *) args;
err = ioctl(br_socket_fd, SIOCDEVPRIVATE, &ifr);
diff -urN ./bridge-utils-1.0.4-old/libbridge/libbridge_misc.c ./bridge-utils-1.0.4/libbridge/libbridge_misc.c
--- ./bridge-utils-1.0.4-old/libbridge/libbridge_misc.c 2004-05-21 19:41:49.000000000 +0200
+++ ./bridge-utils-1.0.4/libbridge/libbridge_misc.c 2004-06-15 23:33:30.000000000 +0200
@@ -25,17 +25,18 @@
#include "libbridge_private.h"
-static const char *state_names[5] = {
+static const char *state_names[6] = {
[BR_STATE_DISABLED] = "disabled",
[BR_STATE_LISTENING] = "listening",
[BR_STATE_LEARNING] = "learning",
[BR_STATE_FORWARDING] = "forwarding",
[BR_STATE_BLOCKING] ="blocking",
+ [BR_STATE_NOLINK] ="nolink",
};
const char *br_get_state_name(int state)
{
- if (state >= 0 && state <= 4)
+ if (state >= 0 && state <= 5)
return state_names[state];
return "<INVALID STATE>";
[-- Attachment #3: linux-2.6.6-bridge_linkstate.patch --]
[-- Type: text/x-patch, Size: 8549 bytes --]
diff -urN ./linux-2.6.6-old/include/linux/if_bridge.h ./linux-2.6.6/include/linux/if_bridge.h
--- ./linux-2.6.6-old/include/linux/if_bridge.h 2004-05-10 04:32:37.000000000 +0200
+++ ./linux-2.6.6/include/linux/if_bridge.h 2004-06-15 22:55:46.000000000 +0200
@@ -44,6 +44,7 @@
#define BR_STATE_LEARNING 2
#define BR_STATE_FORWARDING 3
#define BR_STATE_BLOCKING 4
+#define BR_STATE_NOLINK 5
struct __bridge_info
{
diff -urN ./linux-2.6.6-old/net/bridge/br_fdb.c ./linux-2.6.6/net/bridge/br_fdb.c
--- ./linux-2.6.6-old/net/bridge/br_fdb.c 2004-05-10 04:32:00.000000000 +0200
+++ ./linux-2.6.6/net/bridge/br_fdb.c 2004-06-15 22:17:16.000000000 +0200
@@ -281,12 +281,13 @@
printk(KERN_INFO "%s: attempt to add"
" interface with same source address.\n",
source->dev->name);
- else if (net_ratelimit())
+ else if (net_ratelimit()) {
printk(KERN_WARNING "%s: received packet with "
" own address as source address\n",
source->dev->name);
- ret = -EEXIST;
- goto out;
+ ret = -EEXIST;
+ goto out;
+ }
}
diff -urN ./linux-2.6.6-old/net/bridge/br_if.c ./linux-2.6.6/net/bridge/br_if.c
--- ./linux-2.6.6-old/net/bridge/br_if.c 2004-05-10 04:33:21.000000000 +0200
+++ ./linux-2.6.6/net/bridge/br_if.c 2004-06-15 23:12:30.000000000 +0200
@@ -189,6 +189,7 @@
/* called under bridge lock */
static struct net_bridge_port *new_nbp(struct net_bridge *br,
struct net_device *dev,
+ struct net_device *rdev,
unsigned long cost)
{
int index;
@@ -206,6 +207,7 @@
p->br = br;
dev_hold(dev);
p->dev = dev;
+ p->rdev = rdev;
p->path_cost = cost;
p->priority = 0x8000 >> BR_PORT_BITS;
dev->br_port = p;
@@ -257,7 +259,7 @@
return ret;
}
-int br_add_if(struct net_bridge *br, struct net_device *dev)
+int br_add_if(struct net_bridge *br, struct net_device *dev , struct net_device *rdev)
{
struct net_bridge_port *p;
unsigned long cost;
@@ -275,7 +277,7 @@
if (dev->br_port != NULL)
err = -EBUSY;
- else if (IS_ERR(p = new_nbp(br, dev, cost)))
+ else if (IS_ERR(p = new_nbp(br, dev, rdev, cost)))
err = PTR_ERR(p);
else if ((err = br_fdb_insert(br, p, dev->dev_addr, 1)))
diff -urN ./linux-2.6.6-old/net/bridge/br_ioctl.c ./linux-2.6.6/net/bridge/br_ioctl.c
--- ./linux-2.6.6-old/net/bridge/br_ioctl.c 2004-05-10 04:32:01.000000000 +0200
+++ ./linux-2.6.6/net/bridge/br_ioctl.c 2004-06-15 22:36:32.000000000 +0200
@@ -39,17 +39,19 @@
case BRCTL_DEL_IF:
{
struct net_device *dev;
+ struct net_device *rdev;
int ret;
if (!capable(CAP_NET_ADMIN))
return -EPERM;
dev = dev_get_by_index(arg0);
+ rdev = dev_get_by_index(arg1);
if (dev == NULL)
return -EINVAL;
if (cmd == BRCTL_ADD_IF)
- ret = br_add_if(br, dev);
+ ret = br_add_if(br, dev, rdev);
else
ret = br_del_if(br, dev);
diff -urN ./linux-2.6.6-old/net/bridge/br_private.h ./linux-2.6.6/net/bridge/br_private.h
--- ./linux-2.6.6-old/net/bridge/br_private.h 2004-05-10 04:32:26.000000000 +0200
+++ ./linux-2.6.6/net/bridge/br_private.h 2004-06-15 22:47:18.000000000 +0200
@@ -58,6 +58,7 @@
{
struct net_bridge *br;
struct net_device *dev;
+ struct net_device *rdev;
struct list_head list;
/* STP */
@@ -84,6 +85,7 @@
spinlock_t lock;
struct list_head port_list;
struct net_device *dev;
+ struct net_device *rdev;
struct net_device_stats statistics;
rwlock_t hash_lock;
struct hlist_head hash[BR_HASH_SIZE];
@@ -165,7 +167,7 @@
extern int br_del_bridge(const char *name);
extern void br_cleanup_bridges(void);
extern int br_add_if(struct net_bridge *br,
- struct net_device *dev);
+ struct net_device *dev, struct net_device *rdev);
extern int br_del_if(struct net_bridge *br,
struct net_device *dev);
extern int br_get_bridge_ifindices(int *indices,
@@ -195,6 +197,7 @@
u16 port_no);
extern void br_init_port(struct net_bridge_port *p);
extern void br_become_designated_port(struct net_bridge_port *p);
+void check_link( struct net_bridge_port *p );
/* br_stp_if.c */
extern void br_stp_enable_bridge(struct net_bridge *br);
diff -urN ./linux-2.6.6-old/net/bridge/br_stp.c ./linux-2.6.6/net/bridge/br_stp.c
--- ./linux-2.6.6-old/net/bridge/br_stp.c 2004-05-10 04:32:25.000000000 +0200
+++ ./linux-2.6.6/net/bridge/br_stp.c 2004-06-16 12:29:25.000000000 +0200
@@ -14,6 +14,7 @@
*/
#include <linux/kernel.h>
#include <linux/smp_lock.h>
+#include <linux/ethtool.h>
#include "br_private.h"
#include "br_private_stp.h"
@@ -24,6 +25,7 @@
[BR_STATE_LEARNING] = "learning",
[BR_STATE_FORWARDING] = "forwarding",
[BR_STATE_BLOCKING] = "blocking",
+ [BR_STATE_NOLINK] = "nolink",
};
void br_log_state(const struct net_bridge_port *p)
@@ -351,10 +353,11 @@
if (p->state != BR_STATE_DISABLED &&
p->state != BR_STATE_BLOCKING) {
if (p->state == BR_STATE_FORWARDING ||
- p->state == BR_STATE_LEARNING)
+ p->state == BR_STATE_LEARNING ||
+ p->state == BR_STATE_NOLINK )
br_topology_change_detection(p->br);
- p->state = BR_STATE_BLOCKING;
+ if ( p->state != BR_STATE_NOLINK ) p->state = BR_STATE_BLOCKING;
br_log_state(p);
del_timer(&p->forward_delay_timer);
}
@@ -363,14 +366,55 @@
/* called under bridge lock */
static void br_make_forwarding(struct net_bridge_port *p)
{
- if (p->state == BR_STATE_BLOCKING) {
- if (p->br->stp_enabled) {
- p->state = BR_STATE_LISTENING;
- } else {
- p->state = BR_STATE_LEARNING;
- }
- br_log_state(p);
- mod_timer(&p->forward_delay_timer, jiffies + p->br->forward_delay); }
+ if (p->state == BR_STATE_BLOCKING ) {
+ if (p->br->stp_enabled) {
+ p->state = BR_STATE_LISTENING;
+ } else {
+ p->state = BR_STATE_LEARNING;
+ }
+ br_log_state(p);
+ mod_timer(&p->forward_delay_timer, jiffies + p->br->forward_delay); }
+}
+
+void check_link( struct net_bridge_port *p )
+{
+ if ( p->rdev != NULL ) {
+ if ( ethtool_op_get_link( p->rdev ) ) {
+ if ( p->state == BR_STATE_NOLINK ) {
+ spin_lock_bh(&p->br->lock);
+ p->state=BR_STATE_BLOCKING;
+ p->config_pending = 0;
+ p->topology_change_ack = 0;
+ br_log_state(p);
+ br_make_forwarding(p);
+ spin_unlock_bh(&p->br->lock);
+ }
+ return;
+ }
+ } else {
+ if ( ethtool_op_get_link( p->dev ) ) {
+ if ( p->state == BR_STATE_NOLINK ) {
+ spin_lock_bh(&p->br->lock);
+ p->state=BR_STATE_BLOCKING;
+ p->config_pending = 0;
+ p->topology_change_ack = 0;
+ br_log_state(p);
+ br_make_forwarding(p);
+ spin_unlock_bh(&p->br->lock);
+ }
+ return;
+ }
+ }
+ if ( p->state != BR_STATE_NOLINK ) {
+ spin_lock_bh(&p->br->lock);
+ p->config_pending = 0;
+ p->topology_change_ack = 0;
+ br_make_blocking(p);
+ p->state=BR_STATE_NOLINK;
+ spin_unlock_bh(&p->br->lock);
+ br_log_state(p);
+ }
+ return;
}
/* called under bridge lock */
@@ -379,11 +423,11 @@
struct net_bridge_port *p;
list_for_each_entry(p, &br->port_list, list) {
- if (p->state != BR_STATE_DISABLED) {
+ if (p->state != BR_STATE_DISABLED ) {
if (p->port_no == br->root_port) {
p->config_pending = 0;
p->topology_change_ack = 0;
- br_make_forwarding(p);
+ br_make_forwarding(p);
} else if (br_is_designated_port(p)) {
del_timer(&p->message_age_timer);
br_make_forwarding(p);
diff -urN ./linux-2.6.6-old/net/bridge/br_stp_timer.c ./linux-2.6.6/net/bridge/br_stp_timer.c
--- ./linux-2.6.6-old/net/bridge/br_stp_timer.c 2004-05-10 04:32:37.000000000 +0200
+++ ./linux-2.6.6/net/bridge/br_stp_timer.c 2004-06-16 13:42:00.000000000 +0200
@@ -136,7 +136,9 @@
pr_debug("%s: %d(%s) hold timer expired\n",
p->br->dev->name, p->port_no, p->dev->name);
-
+ if (p->br->stp_enabled) {
+ check_link(p);
+ }
spin_lock_bh(&p->br->lock);
if (p->config_pending)
br_transmit_config(p);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included).
2004-06-17 18:57 [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included) Mark Ruijter
@ 2004-06-17 20:28 ` Stephen Hemminger
2004-06-17 21:08 ` Mark Ruijter
2004-07-16 23:28 ` Stephen Hemminger
1 sibling, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2004-06-17 20:28 UTC (permalink / raw)
To: Mark Ruijter; +Cc: bridge
On Thu, 17 Jun 2004 20:57:23 +0200
Mark Ruijter <bridge@siennax.com> wrote:
>
> 1. Link state monitoring added.
> We discovered the following problem with the existing bridge code.
> The code doesn't monitor the link state of the interfaces. When using
> two cisco switches with two Linux bridging firewalls the following
> problem exists. When the Cisco switch that has the blocking (stp) Linux
> bridge attached dies, the Linux bridge no longer receives stp packets
> and changes it's state from blocking to forwarding. As soon as the Cisco
> switch comes back alive a loop is created in the network for a short but
> long enough period of time. :-(
>
Rather than add another state, it should just go to DISABLED state.
> To fix this I added link state monitoring to the code. This is only
> active when stp is turned on. When a link fails the bridge port goes to
> blocked and then to the new 'nolink' status. The bridge-utils patch
> makes it possible to see the status with brctl showstp.
>
> I did encounter a second problem when writing the link monitoring code.
> When you add a vlan interface like eth0.10 then it's not possible to
> obtain link state information from this interface.
That is a VLAN problem, fix it there. Sorry.
> The fix I wrote is that brctl now allows you to specify the interface
> that contains the link state.
>
> Example : brctl addif NUM1 eth0.10 eth0
I prefer to eschew needless complexity.
That doesn't mean I won't consider adding it, but let me look at the problem
a little more before accepting your solution as is.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included).
2004-06-17 20:28 ` Stephen Hemminger
@ 2004-06-17 21:08 ` Mark Ruijter
0 siblings, 0 replies; 6+ messages in thread
From: Mark Ruijter @ 2004-06-17 21:08 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: bridge
> Rather than add another state, it should just go to DISABLED state.
I did think about doing just that. But the problem then is that the
bridge needs to be reenabled by hand when the link comes back.
You can't automatically enable a disabled interface when the link goes
down/up. It would also not be clear why a port is disabled. Link problem
or human intervention?
So that's the reason why I made up the nolink state.
>
>>I did encounter a second problem when writing the link monitoring code.
>>When you add a vlan interface like eth0.10 then it's not possible to
>>obtain link state information from this interface.
>
> That is a VLAN problem, fix it there. Sorry.
Hmmm.... I'll investigate the amount of work that needs to be done for this.
>
>
>
>>The fix I wrote is that brctl now allows you to specify the interface
>>that contains the link state.
>>
>>Example : brctl addif NUM1 eth0.10 eth0
>
>
> I prefer to eschew needless complexity.
It might be complex but this is also true for a swiss army knife. :-)
If the vlan interfaces are the only ones that don't allow link state
monitoring then I do agree that it would be best to fix the vlan
interface status.
If the are more types of interfaces that don't support state checking
then the current solution might be better. It is highly flexible....
>
> That doesn't mean I won't consider adding it, but let me look at the problem
> a little more before accepting your solution as is.
Thanks for considering it. It probably needs some work. I hope that you
do agree that some form of link state monitoring is useful.
Regards,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included).
2004-06-17 18:57 [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included) Mark Ruijter
2004-06-17 20:28 ` Stephen Hemminger
@ 2004-07-16 23:28 ` Stephen Hemminger
2004-07-17 19:31 ` Mark Ruijter
2004-07-20 11:24 ` Mark Ruijter
1 sibling, 2 replies; 6+ messages in thread
From: Stephen Hemminger @ 2004-07-16 23:28 UTC (permalink / raw)
To: Mark Ruijter; +Cc: bridge
On Thu, 17 Jun 2004 20:57:23 +0200
Mark Ruijter <bridge@siennax.com> wrote:
>
> 1. Link state monitoring added.
> We discovered the following problem with the existing bridge code.
> The code doesn't monitor the link state of the interfaces. When using
> two cisco switches with two Linux bridging firewalls the following
> problem exists. When the Cisco switch that has the blocking (stp) Linux
> bridge attached dies, the Linux bridge no longer receives stp packets
> and changes it's state from blocking to forwarding. As soon as the Cisco
> switch comes back alive a loop is created in the network for a short but
> long enough period of time. :-(
>
> To fix this I added link state monitoring to the code. This is only
> active when stp is turned on. When a link fails the bridge port goes to
> blocked and then to the new 'nolink' status. The bridge-utils patch
> makes it possible to see the status with brctl showstp.
>
Could you split out the link state monitoring, and look at bonding to see
how they are doing it?
I have a fix for vlan code that passes MII and ethtool requests from the virtual
to physical device, so the bridge code wouldn't need to the hack about root device.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included).
2004-07-16 23:28 ` Stephen Hemminger
@ 2004-07-17 19:31 ` Mark Ruijter
2004-07-20 11:24 ` Mark Ruijter
1 sibling, 0 replies; 6+ messages in thread
From: Mark Ruijter @ 2004-07-17 19:31 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: bridge
>
>
> Could you split out the link state monitoring, and look at bonding to see
> how they are doing it?
Yes, no problem.
>
> I have a fix for vlan code that passes MII and ethtool requests from the virtual
> to physical device, so the bridge code wouldn't need to the hack about root device.
>
I'll change the code so it doesn't include the root device hack anymore.
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included).
2004-07-16 23:28 ` Stephen Hemminger
2004-07-17 19:31 ` Mark Ruijter
@ 2004-07-20 11:24 ` Mark Ruijter
1 sibling, 0 replies; 6+ messages in thread
From: Mark Ruijter @ 2004-07-20 11:24 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: bridge
[-- Attachment #1: Type: text/plain, Size: 521 bytes --]
Stephen Hemminger wrote:
>
> Could you split out the link state monitoring, and look at bonding to see
> how they are doing it?
>
> I have a fix for vlan code that passes MII and ethtool requests from the virtual
> to physical device, so the bridge code wouldn't need to the hack about root device.
>
Stephen,
I've attached two patches that implement link state monitoring as
described above.
De bridge-utils patch also fixes a small bug:
./brctl stp TEST
Segmentation fault
--
Mark Ruijter.
bridge@siennax.com
[-- Attachment #2: linux-2.6.7-bridge_linkstate.patch --]
[-- Type: text/x-patch, Size: 8578 bytes --]
diff -urN ./linux-2.6.7-old/include/linux/if_bridge.h linux-2.6.7/include/linux/if_bridge.h
--- ./linux-2.6.7-old/include/linux/if_bridge.h 2004-06-16 07:19:23.000000000 +0200
+++ linux-2.6.7/include/linux/if_bridge.h 2004-07-19 14:59:56.000000000 +0200
@@ -44,12 +44,14 @@
#define BRCTL_SET_PORT_PRIORITY 16
#define BRCTL_SET_PATH_COST 17
#define BRCTL_GET_FDB_ENTRIES 18
+#define BRCTL_SET_BRIDGE_STP_CARRIER 19
#define BR_STATE_DISABLED 0
#define BR_STATE_LISTENING 1
#define BR_STATE_LEARNING 2
#define BR_STATE_FORWARDING 3
#define BR_STATE_BLOCKING 4
+#define BR_STATE_NOLINK 5
struct __bridge_info
{
diff -urN ./linux-2.6.7-old/net/bridge/br_if.c linux-2.6.7/net/bridge/br_if.c
--- ./linux-2.6.7-old/net/bridge/br_if.c 2004-06-16 07:20:26.000000000 +0200
+++ linux-2.6.7/net/bridge/br_if.c 2004-07-19 17:04:54.000000000 +0200
@@ -148,7 +148,8 @@
br->bridge_id.prio[0] = 0x80;
br->bridge_id.prio[1] = 0x00;
memset(br->bridge_id.addr, 0, ETH_ALEN);
-
+
+ br->stp_check_carrier = 0;
br->stp_enabled = 0;
br->designated_root = br->bridge_id;
br->root_path_cost = 0;
diff -urN ./linux-2.6.7-old/net/bridge/br_ioctl.c linux-2.6.7/net/bridge/br_ioctl.c
--- ./linux-2.6.7-old/net/bridge/br_ioctl.c 2004-06-16 07:18:58.000000000 +0200
+++ linux-2.6.7/net/bridge/br_ioctl.c 2004-07-20 12:09:50.000000000 +0200
@@ -246,6 +246,12 @@
return 0;
}
+ case BRCTL_SET_BRIDGE_STP_CARRIER:
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
+ br->stp_check_carrier = args[1]?args[1]:0;
+ if ( ! br->stp_enabled && br->stp_check_carrier != 0 ) return -EOPNOTSUPP;
+ return 0;
case BRCTL_SET_BRIDGE_STP_STATE:
if (!capable(CAP_NET_ADMIN))
diff -urN ./linux-2.6.7-old/net/bridge/br_private.h linux-2.6.7/net/bridge/br_private.h
--- ./linux-2.6.7-old/net/bridge/br_private.h 2004-06-16 07:19:01.000000000 +0200
+++ linux-2.6.7/net/bridge/br_private.h 2004-07-19 17:02:07.000000000 +0200
@@ -104,6 +104,7 @@
u16 root_port;
unsigned char stp_enabled;
+ unsigned char stp_check_carrier;
unsigned char topology_change;
unsigned char topology_change_detected;
@@ -187,6 +188,8 @@
u16 port_no);
extern void br_init_port(struct net_bridge_port *p);
extern void br_become_designated_port(struct net_bridge_port *p);
+extern void check_link( struct net_bridge_port *p );
+extern int check_dev_link( struct net_device *netdev, int use_carrier );
/* br_stp_if.c */
extern void br_stp_enable_bridge(struct net_bridge *br);
diff -urN ./linux-2.6.7-old/net/bridge/br_private_stp.h linux-2.6.7/net/bridge/br_private_stp.h
--- ./linux-2.6.7-old/net/bridge/br_private_stp.h 2004-06-16 07:19:36.000000000 +0200
+++ linux-2.6.7/net/bridge/br_private_stp.h 2004-07-19 22:33:08.000000000 +0200
@@ -55,4 +55,13 @@
extern void br_send_config_bpdu(struct net_bridge_port *, struct br_config_bpdu *);
extern void br_send_tcn_bpdu(struct net_bridge_port *);
+#define IOCTL(dev, arg, cmd) ({ \
+ int res = 0; \
+ mm_segment_t fs = get_fs(); \
+ set_fs(get_ds()); \
+ res = ioctl(dev, arg, cmd); \
+ set_fs(fs); \
+ res; })
+
+
#endif
diff -urN ./linux-2.6.7-old/net/bridge/br_stp.c linux-2.6.7/net/bridge/br_stp.c
--- ./linux-2.6.7-old/net/bridge/br_stp.c 2004-06-16 07:19:01.000000000 +0200
+++ linux-2.6.7/net/bridge/br_stp.c 2004-07-20 09:36:41.000000000 +0200
@@ -14,6 +14,8 @@
*/
#include <linux/kernel.h>
#include <linux/smp_lock.h>
+#include <linux/mii.h>
+#include <linux/ethtool.h>
#include "br_private.h"
#include "br_private_stp.h"
@@ -24,6 +26,7 @@
[BR_STATE_LEARNING] = "learning",
[BR_STATE_FORWARDING] = "forwarding",
[BR_STATE_BLOCKING] = "blocking",
+ [BR_STATE_NOLINK] = "nolink",
};
void br_log_state(const struct net_bridge_port *p)
@@ -351,10 +354,11 @@
if (p->state != BR_STATE_DISABLED &&
p->state != BR_STATE_BLOCKING) {
if (p->state == BR_STATE_FORWARDING ||
- p->state == BR_STATE_LEARNING)
+ p->state == BR_STATE_LEARNING ||
+ p->state == BR_STATE_NOLINK )
br_topology_change_detection(p->br);
- p->state = BR_STATE_BLOCKING;
+ if ( p->state != BR_STATE_NOLINK ) p->state = BR_STATE_BLOCKING;
br_log_state(p);
del_timer(&p->forward_delay_timer);
}
@@ -451,3 +455,101 @@
br_topology_change_acknowledge(p);
}
}
+
+extern void check_link( struct net_bridge_port *p )
+{
+ int link;
+
+ if ( p->br->stp_check_carrier == 1 )
+ link=check_dev_link(p->dev, 1);
+ else
+ link=check_dev_link(p->dev, 0);
+
+ if ( link == 2 ) {
+ pr_info("%s: port %i(%s) does not support link state monitoring with mii/ethtool.\n",
+ p->br->dev->name, p->port_no, p->dev->name);
+ pr_info("%s: Please use net_if_carrier ( nfc ) or disable link state monitoring.\n",
+ p->br->dev->name);
+ return;
+ }
+
+ if ( link == 0 ) {
+ if ( p->state != BR_STATE_NOLINK ) {
+ spin_lock_bh(&p->br->lock);
+ p->config_pending = 0;
+ p->topology_change_ack = 0;
+ br_make_blocking(p);
+ p->state=BR_STATE_NOLINK;
+ spin_unlock_bh(&p->br->lock);
+ br_log_state(p);
+ }
+ return;
+ }
+
+ if ( p->state == BR_STATE_NOLINK ) {
+ spin_lock_bh(&p->br->lock);
+ p->state=BR_STATE_BLOCKING;
+ p->config_pending = 0;
+ p->topology_change_ack = 0;
+ br_log_state(p);
+ br_make_forwarding(p);
+ spin_unlock_bh(&p->br->lock);
+ }
+}
+
+/* Check the linkstate of the port using either ETHTOOL,
+ * or check netif_carrier_ok(), depening upon the setting of the
+ * use_carrier parameter.
+ *
+ * Return either BMSR_LSTATUS (4), meaning that the link is up (or we
+ * can't tell and just pretend it is), or 0, meaning that the link is
+ * down, or 2 meaning that link state check has failed.
+ *
+ * This code is shamelessly copied from the bonding driver.
+*/
+extern int check_dev_link(struct net_device *netdev, int use_carrier )
+{
+ static int (* ioctl)(struct net_device *, struct ifreq *, int);
+ struct ifreq ifr;
+ struct mii_ioctl_data *mii;
+ struct ethtool_value etool;
+
+
+ if (use_carrier) {
+ return netif_carrier_ok(netdev) ? BMSR_LSTATUS : 0;
+ }
+
+ ioctl = netdev->do_ioctl;
+ if (ioctl) {
+ strncpy(ifr.ifr_name, netdev->name, IFNAMSIZ);
+ mii = (struct mii_ioctl_data *)&ifr.ifr_data;
+ if (IOCTL(netdev, &ifr, SIOCGMIIPHY) == 0) {
+ mii->reg_num = MII_BMSR;
+ if (IOCTL(netdev, &ifr, SIOCGMIIREG) == 0) {
+ return (mii->val_out & BMSR_LSTATUS );
+ }
+ }
+ }
+
+ if (netdev->ethtool_ops) {
+ pr_info("Use get_link=%i\n",use_carrier);
+ if (netdev->ethtool_ops->get_link) {
+ u32 link;
+ link = netdev->ethtool_ops->get_link(netdev);
+ return link ? BMSR_LSTATUS : 0;
+ }
+ }
+
+ if (ioctl) {
+ strncpy(ifr.ifr_name, netdev->name, IFNAMSIZ);
+ etool.cmd = ETHTOOL_GLINK;
+ ifr.ifr_data = (char*)&etool;
+ if (IOCTL(netdev, &ifr, SIOCETHTOOL) == 0) {
+ if (etool.data == 1 )
+ return BMSR_LSTATUS;
+ else
+ return 0;
+ }
+ }
+ return 2;
+}
diff -urN ./linux-2.6.7-old/net/bridge/br_stp_timer.c linux-2.6.7/net/bridge/br_stp_timer.c
--- ./linux-2.6.7-old/net/bridge/br_stp_timer.c 2004-06-16 07:19:23.000000000 +0200
+++ linux-2.6.7/net/bridge/br_stp_timer.c 2004-07-19 17:29:33.000000000 +0200
@@ -137,7 +137,9 @@
pr_debug("%s: %d(%s) hold timer expired\n",
p->br->dev->name, p->port_no, p->dev->name);
-
+ if (p->br->stp_enabled && p->br->stp_check_carrier != 0 ) {
+ check_link(p);
+ }
spin_lock_bh(&p->br->lock);
if (p->config_pending)
br_transmit_config(p);
diff -urN ./linux-2.6.7-old/net/bridge/stp_enabled linux-2.6.7/net/bridge/stp_enabled
--- ./linux-2.6.7-old/net/bridge/stp_enabled 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.7/net/bridge/stp_enabled 2004-07-19 14:47:20.000000000 +0200
@@ -0,0 +1,2 @@
+br_private.h: return !memcmp(&br->bridge_id, &br->designated_root, 8);
+br_private_stp.h: return !memcmp(&p->designated_bridge, &p->br->bridge_id, 8) &&
[-- Attachment #3: bridge-utils-1.0.4-2.patch --]
[-- Type: text/x-patch, Size: 5786 bytes --]
diff -urN ./bridge-utils-1.0.4-old/brctl/brctl_cmd.c bridge-utils-1.0.4/brctl/brctl_cmd.c
--- ./bridge-utils-1.0.4-old/brctl/brctl_cmd.c 2004-06-04 20:03:40.000000000 +0200
+++ bridge-utils-1.0.4/brctl/brctl_cmd.c 2004-07-20 12:23:55.000000000 +0200
@@ -257,13 +257,16 @@
{
int stp, err;
- if (!strcmp(argv[2], "on") || !strcmp(argv[2], "yes")
- || !strcmp(argv[2], "1"))
- stp = 1;
- else if (!strcmp(argv[2], "off") || !strcmp(argv[2], "no")
- || !strcmp(argv[2], "0"))
- stp = 0;
- else {
+ stp=-1;
+ if ( argv[1] && argv[2] ){
+ if (!strcmp(argv[2], "on") || !strcmp(argv[2], "yes")
+ || !strcmp(argv[2], "1"))
+ stp = 1;
+ else if (!strcmp(argv[2], "off") || !strcmp(argv[2], "no")
+ || !strcmp(argv[2], "0"))
+ stp = 0;
+ }
+ if ( stp == -1 ){
fprintf(stderr, "expect on/off for argument\n");
return 1;
}
@@ -275,6 +278,36 @@
return err != 0;
}
+static int br_cmd_stp_carrier(char** argv)
+{
+ int carrier, err;
+
+ carrier=-1;
+ if ( argv[2] && argv[1] ) {
+ if (!strcmp(argv[2], "nfc"))
+ carrier = 1;
+ else if (!strcmp(argv[2], "off"))
+ carrier = 0;
+ else if (!strcmp(argv[2], "mii"))
+ carrier = 2;
+ }
+
+ if ( carrier == -1 ) {
+ fprintf(stderr, "expect off to disable, nfc for net_if_carrier (recommended), mii for mii/ethtool linkstate detection\n");
+ return 1;
+ }
+
+ err = br_set_stp_carrier(argv[1], carrier);
+ if (err){
+ if ( err == EOPNOTSUPP )
+ fprintf(stderr,"Don't forget to enable stp.\n");
+ else
+ fprintf(stderr, "set linkstate detection failed: %s\n",
+ strerror(errno));
+ }
+ return err != 0;
+}
+
static int br_cmd_showstp(char** argv)
{
struct bridge_info info;
@@ -396,6 +429,8 @@
"<bridge>\t\tshow bridge stp info"},
{ 1, "stp", br_cmd_stp,
"<bridge> <state>\tturn stp on/off" },
+ { 1, "linkstate", br_cmd_stp_carrier,
+ "<bridge> <method>\tturn linkstate detection off / nfc (recommended) / mii" },
};
const struct command *command_lookup(const char *cmd)
diff -urN ./bridge-utils-1.0.4-old/doc/brctl.8 bridge-utils-1.0.4/doc/brctl.8
--- ./bridge-utils-1.0.4-old/doc/brctl.8 2001-11-07 18:16:20.000000000 +0100
+++ bridge-utils-1.0.4/doc/brctl.8 2004-07-20 11:23:45.000000000 +0200
@@ -159,6 +159,18 @@
dimension. This metric is used in the designated port and root port
selection algorithms.
+.B brctl linkstate <bridge> <method>
+enables link state monitoring of the bridge ports. We support two
+types of link state monitoring. The first type <nfc> uses the
+net_if_carrier_ok() call to determine interface status.
+
+The second type <mii> uses MII or ETHTOOL ioctls that
+are less efficient and utilize a deprecated calling sequence within the kernel.
+The netif_carrier_ok() relies on the device driver to maintain its state and
+most, but not all, device drivers support this facility. The netif_carrier_ok()
+call returns that the link is up when the state is unknown. So if brctl showstp
+insists that a link is up when it should not be then switch to <mii>. The <mii>
+code will log errors when it does not support the hardware.
.SH NOTES
.BR brctl(8)
diff -urN ./bridge-utils-1.0.4-old/libbridge/libbridge.h bridge-utils-1.0.4/libbridge/libbridge.h
--- ./bridge-utils-1.0.4-old/libbridge/libbridge.h 2004-06-08 17:57:49.000000000 +0200
+++ bridge-utils-1.0.4/libbridge/libbridge.h 2004-07-20 12:03:10.000000000 +0200
@@ -100,6 +100,7 @@
extern int br_set_bridge_max_age(const char *br, struct timeval *tv);
extern int br_set_ageing_time(const char *br, struct timeval *tv);
extern int br_set_stp_state(const char *br, int stp_state);
+extern int br_set_stp_carrier(const char *br, int stp_state);
extern int br_set_bridge_priority(const char *br, int bridge_priority);
extern int br_set_port_priority(const char *br, const char *p,
int port_priority);
diff -urN ./bridge-utils-1.0.4-old/libbridge/libbridge_devif.c bridge-utils-1.0.4/libbridge/libbridge_devif.c
--- ./bridge-utils-1.0.4-old/libbridge/libbridge_devif.c 2004-06-08 17:57:49.000000000 +0200
+++ bridge-utils-1.0.4/libbridge/libbridge_devif.c 2004-07-19 15:28:07.000000000 +0200
@@ -390,6 +390,11 @@
return br_set(br, "stp_state", stp_state, BRCTL_SET_BRIDGE_STP_STATE);
}
+int br_set_stp_carrier(const char *br, int stp_state)
+{
+ return br_set(br, "stp_state", stp_state, BRCTL_SET_BRIDGE_STP_CARRIER);
+}
+
int br_set_bridge_priority(const char *br, int bridge_priority)
{
return br_set(br, "priority", bridge_priority,
diff -urN ./bridge-utils-1.0.4-old/libbridge/libbridge_misc.c bridge-utils-1.0.4/libbridge/libbridge_misc.c
--- ./bridge-utils-1.0.4-old/libbridge/libbridge_misc.c 2004-05-21 19:41:49.000000000 +0200
+++ bridge-utils-1.0.4/libbridge/libbridge_misc.c 2004-07-20 09:51:58.000000000 +0200
@@ -25,17 +25,18 @@
#include "libbridge_private.h"
-static const char *state_names[5] = {
+static const char *state_names[6] = {
[BR_STATE_DISABLED] = "disabled",
[BR_STATE_LISTENING] = "listening",
[BR_STATE_LEARNING] = "learning",
[BR_STATE_FORWARDING] = "forwarding",
[BR_STATE_BLOCKING] ="blocking",
+ [BR_STATE_NOLINK] ="nolink",
};
const char *br_get_state_name(int state)
{
- if (state >= 0 && state <= 4)
+ if (state >= 0 && state <= 5)
return state_names[state];
return "<INVALID STATE>";
diff -urN ./bridge-utils-1.0.4-old/libbridge/stamp-h1 bridge-utils-1.0.4/libbridge/stamp-h1
--- ./bridge-utils-1.0.4-old/libbridge/stamp-h1 1970-01-01 01:00:00.000000000 +0100
+++ bridge-utils-1.0.4/libbridge/stamp-h1 2004-07-19 16:04:59.000000000 +0200
@@ -0,0 +1 @@
+timestamp for libbridge/config.h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-07-20 11:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-06-17 18:57 [Bridge] Bridge code enhancement (link state detection) and bug fix. (patches included) Mark Ruijter
2004-06-17 20:28 ` Stephen Hemminger
2004-06-17 21:08 ` Mark Ruijter
2004-07-16 23:28 ` Stephen Hemminger
2004-07-17 19:31 ` Mark Ruijter
2004-07-20 11:24 ` Mark Ruijter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.