From: kmindg <kmindg@gmail.com>
Date: Sun, 9 Mar 2014 09:48:52 +0000 (+0800)
Subject: stp: Fix bpdu tx problem in listening state
X-Git-Tag: v2.0.2~24
X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=2ca476b8ae918f6db2af7c147ea0cb807a734939

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 6e1efd07a..e41f8b795 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -692,6 +692,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 89131ebba..5dba2e678 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -627,7 +627,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);
@@ -641,6 +641,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
@@ -1542,9 +1549,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) {