ofproto-dpif-xlate: Make flows that match ICMP fields revalidate correctly.

[sliver-openvswitch.git] / ofproto / ofproto-dpif-xlate.c

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ad44582..ffcfdf9 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2103,18 +2103,12 @@ compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls)
 {
     struct flow_wildcards *wc = &ctx->xout->wc;
     struct flow *flow = &ctx->xin->flow;
-    ovs_be16 vlan_tci = flow->vlan_tci;
     int n;
 
     ovs_assert(eth_type_mpls(mpls->ethertype));
 
     n = flow_count_mpls_labels(flow, wc);
     if (!n) {
-        if (mpls->position == OFPACT_MPLS_BEFORE_VLAN) {
-            vlan_tci = 0;
-        } else {
-            flow->vlan_tci = 0;
-        }
         ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
                                               &ctx->xout->odp_actions,
                                               &ctx->xout->wc);
@@ -2134,7 +2128,6 @@ compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls)
     }
 
     flow_push_mpls(flow, n, mpls->ethertype, wc);
-    flow->vlan_tci = vlan_tci;
 }
 
 static void
@@ -2918,6 +2911,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);
 
@@ -2972,6 +2966,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) {
@@ -3131,6 +3126,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);