stp: Fix bpdu tx problem in listening state

[cascardo/ovs.git] / ofproto / ofproto-dpif-xlate.c

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e6bf34f..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) {
@@ -3014,6 +3030,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
     struct xlate_ctx ctx;
     size_t ofpacts_len;
     bool tnl_may_send;
+    bool is_icmp;
 
     COVERAGE_INC(xlate_actions);
 
@@ -3068,6 +3085,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
     if (is_ip_any(flow)) {
         wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
     }
+    is_icmp = is_icmpv4(flow) || is_icmpv6(flow);
 
     tnl_may_send = tnl_xlate_init(&ctx.base_flow, flow, wc);
     if (ctx.xbridge->netflow) {
@@ -3228,6 +3246,21 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
      * use non-header fields as part of the cache. */
     flow_wildcards_clear_non_packet_fields(wc);
 
+    /* ICMPv4 and ICMPv6 have 8-bit "type" and "code" fields.  struct flow uses
+     * the low 8 bits of the 16-bit tp_src and tp_dst members to represent
+     * these fields.  The datapath interface, on the other hand, represents
+     * them with just 8 bits each.  This means that if the high 8 bits of the
+     * masks for these fields somehow become set, then they will get chopped
+     * off by a round trip through the datapath, and revalidation will spot
+     * that as an inconsistency and delete the flow.  Avoid the problem here by
+     * making sure that only the low 8 bits of either field can be unwildcarded
+     * for ICMP.
+     */
+    if (is_icmp) {
+        wc->masks.tp_src &= htons(UINT8_MAX);
+        wc->masks.tp_dst &= htons(UINT8_MAX);
+    }
+
 out:
     rule_actions_unref(actions);
     rule_dpif_unref(rule);
@@ -3243,7 +3276,6 @@ xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
     struct ofpact_output output;
     struct flow flow;
     union flow_in_port in_port_;
-    int error;
 
     ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
     /* Use OFPP_NONE as the in_port to avoid special packet processing. */
@@ -3258,9 +3290,9 @@ xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
     }
     output.port = xport->ofp_port;
     output.max_len = 0;
-    error =  ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow, NULL,
-                                          &output.ofpact, sizeof output,
-                                          packet);
     ovs_rwlock_unlock(&xlate_rwlock);
-    return error;
+
+    return ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow, NULL,
+                                        &output.ofpact, sizeof output,
+                                        packet);
 }