From 0d1cee123a84ef8885834c0d086c4a3d5d48355f Mon Sep 17 00:00:00 2001 From: kmindg Date: Sun, 9 Mar 2014 17:48:52 +0800 Subject: [PATCH] stp: Fix bpdu tx problem in listening state The restriction only allows to send bpdu in forwarding state in compose_output_action__. But a port could send bpdu in listening and learning state according to comments in lib/stp.h(State of an STP port). Until this commit, OVS did not send out BPDUs in listening and learning states. But those two states are temporary, the stp port will be in forwarding state and send out BPDUs eventually (In the default configuration listening and learning states last 15+15 second). Therefore, this bug increased convergence time but did not entirely break STP. Signed-off-by: kmindg Signed-off-by: Ben Pfaff --- lib/stp.c | 9 +++++++++ lib/stp.h | 1 + ofproto/ofproto-dpif-xlate.c | 24 ++++++++++++++++++++---- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/stp.c b/lib/stp.c index afe77d082..c5aec57d2 100644 --- a/lib/stp.c +++ b/lib/stp.c @@ -684,6 +684,15 @@ stp_learn_in_state(enum stp_state state) return (state & (STP_DISABLED | STP_LEARNING | STP_FORWARDING)) != 0; } +/* Returns true if 'state' is one in which rx&tx bpdu should be done on + * on a port, false otherwise. */ +bool +stp_listen_in_state(enum stp_state state) +{ + return (state & + (STP_LISTENING | STP_LEARNING | STP_FORWARDING)) != 0; +} + /* Returns the name for the given 'role' (for use in debugging and log * messages). */ const char * diff --git a/lib/stp.h b/lib/stp.h index affde1828..c0175cdbf 100644 --- a/lib/stp.h +++ b/lib/stp.h @@ -120,6 +120,7 @@ enum stp_state { const char *stp_state_name(enum stp_state); bool stp_forward_in_state(enum stp_state); bool stp_learn_in_state(enum stp_state); +bool stp_listen_in_state(enum stp_state); /* Role of an STP port. */ enum stp_role { diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index eb4931ec3..97c4e59ce 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -665,7 +665,7 @@ xport_get_stp_port(const struct xport *xport) : NULL; } -static enum stp_state +static bool xport_stp_learn_state(const struct xport *xport) { struct stp_port *sp = xport_get_stp_port(xport); @@ -679,6 +679,13 @@ xport_stp_forward_state(const struct xport *xport) return stp_forward_in_state(sp ? stp_port_get_state(sp) : STP_DISABLED); } +static bool +xport_stp_listen_state(const struct xport *xport) +{ + struct stp_port *sp = xport_get_stp_port(xport); + return stp_listen_in_state(sp ? stp_port_get_state(sp) : STP_DISABLED); +} + /* Returns true if STP should process 'flow'. Sets fields in 'wc' that * were used to make the determination.*/ static bool @@ -1693,9 +1700,18 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, } else if (xport->config & OFPUTIL_PC_NO_FWD) { xlate_report(ctx, "OFPPC_NO_FWD set, skipping output"); return; - } else if (check_stp && !xport_stp_forward_state(xport)) { - xlate_report(ctx, "STP not in forwarding state, skipping output"); - return; + } else if (check_stp) { + if (eth_addr_equals(ctx->base_flow.dl_dst, eth_addr_stp)) { + if (!xport_stp_listen_state(xport)) { + xlate_report(ctx, "STP not in listening state, " + "skipping bpdu output"); + return; + } + } else if (!xport_stp_forward_state(xport)) { + xlate_report(ctx, "STP not in forwarding state, " + "skipping output"); + return; + } } if (mbridge_has_mirrors(ctx->xbridge->mbridge) && xport->xbundle) { -- 2.20.1