stp: Make stp-disabled port forward stp bpdu packets.

Commit 0d1cee123a84 (stp: Fix bpdu tx problem in listening state)
makes ovs drop the stp bpdu packets if stp is not enabled on the
input port.  However, when pif bridge is used and stp is enabled
on the integration bridge.  The flow translation of stp bpdu
packets will go through a level of resubmission which changes
the input port to the corresponding peer port.  Since, the
patch port on the pif bridge does not have stp enabled, the
flow translation will drop the bpdu packets.

This commit fixes the issue by making ovs forward stp bpdu packets
on stp-disabled port.

VMware-BZ: #1284695

Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Acked-by: Joe Stringer <joestringer@nicira.com>

diff --git a/lib/stp.c b/lib/stp.c
index dbe48e8..1d395f3 100644 (file)
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -684,13 +684,18 @@ 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. */
+/* Returns true if 'state' is one in which bpdus should be forwarded on a
+ * port, false otherwise.
+ *
+ * Returns true if 'state' is STP_DISABLED, since in that case the port does
+ * not generate the bpdu and should just forward it (e.g. patch port on pif
+ * bridge). */
 bool
-stp_listen_in_state(enum stp_state state)
+stp_should_forward_bpdu(enum stp_state state)
 {
     return (state &
-            (STP_LISTENING | STP_LEARNING | STP_FORWARDING)) != 0;
+            ( STP_DISABLED | STP_LISTENING | STP_LEARNING
+              | STP_FORWARDING)) != 0;
 }
 
 /* Returns the name for the given 'role' (for use in debugging and log

diff --git a/lib/stp.h b/lib/stp.h
index c0175cd..93c4904 100644 (file)
--- a/lib/stp.h
+++ b/lib/stp.h
@@ -99,13 +99,25 @@ bool stp_get_changed_port(struct stp *, struct stp_port **portp);
  * The following diagram describes the various states and what they are
  * allowed to do in OVS:
  *
- *                     FWD  LRN  TX_BPDU RX_BPDU
- *                     ---  ---  ------- -------
- *        Disabled      Y    -      -       -
- *        Blocking      -    -      -       Y
- *        Listening     -    -      Y       Y
- *        Learning      -    Y      Y       Y
- *        Forwarding    Y    Y      Y       Y
+ *                     FWD  LRN  TX_BPDU RX_BPDU FWD_BPDU
+ *                     ---  ---  ------- ------- --------
+ *        Disabled      Y    -      -       -        Y
+ *        Blocking      -    -      -       Y        -
+ *        Listening     -    -      Y       Y        Y
+ *        Learning      -    Y      Y       Y        Y
+ *        Forwarding    Y    Y      Y       Y        Y
+ *
+ *
+ * FWD:              the port should forward any incoming non-stp-BPDU
+ *                   packets.
+ *
+ * LRN:              the port should conduct MAC learning on packets received.
+ *
+ * TX_BPDU/RX_BPDU:  the port could generate/consume bpdus.
+ *
+ * FWD_BPDU:         the port should should always forward the BPDUS,
+ *                   whether they are generated by the port or received
+ *                   as incoming packets.
  *
  * Once again, note that the disabled state forwards traffic, which is
  * likely different than the spec would indicate.
@@ -120,7 +132,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);
+bool stp_should_forward_bpdu(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 b4a1ed4..a466f3b 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -764,10 +764,10 @@ xport_stp_forward_state(const struct xport *xport)
 }
 
 static bool
-xport_stp_listen_state(const struct xport *xport)
+xport_stp_should_forward_bpdu(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);
+    return stp_should_forward_bpdu(sp ? stp_port_get_state(sp) : STP_DISABLED);
 }
 
 /* Returns true if STP should process 'flow'.  Sets fields in 'wc' that
@@ -1824,7 +1824,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         return;
     } else if (check_stp) {
         if (is_stp(&ctx->base_flow)) {
-            if (!xport_stp_listen_state(xport)) {
+            if (!xport_stp_should_forward_bpdu(xport)) {
                 xlate_report(ctx, "STP not in listening state, "
                              "skipping bpdu output");
                 return;