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 <kmindg@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/stp.c b/lib/stp.c
index 8140263..34f5897 100644 (file)
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -694,6 +694,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 affde18..c0175cd 100644 (file)
--- 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 0a71da7..14e8fe2 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -667,7 +667,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);
@@ -681,6 +681,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
@@ -1695,9 +1702,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) {