configure: Include pthread-win32 libraries for Windows build.
[sliver-openvswitch.git] / ofproto / ofproto-dpif-xlate.c
index 558598c..eb4931e 100644 (file)
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -51,6 +51,7 @@
 
 COVERAGE_DEFINE(xlate_actions);
 COVERAGE_DEFINE(xlate_actions_oversize);
+COVERAGE_DEFINE(xlate_actions_mpls_overflow);
 
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_xlate);
 
@@ -92,6 +93,10 @@ struct xbridge {
      * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
      * False if the datapath supports only 8-byte (or shorter) userdata. */
     bool variable_length_userdata;
+
+    /* Number of MPLS label stack entries that the datapath supports
+     * in matches. */
+    size_t max_mpls_depth;
 };
 
 struct xbundle {
@@ -170,16 +175,10 @@ struct xlate_ctx {
     /* The rule that we are currently translating, or NULL. */
     struct rule_dpif *rule;
 
-    int mpls_depth_delta;       /* Delta of the mpls stack depth since
-                                 * actions were last committed.
-                                 * Must be between -1 and 1 inclusive. */
-    ovs_be32 pre_push_mpls_lse; /* Used to record the top-most MPLS LSE
-                                 * prior to an mpls_push so that it may be
-                                 * used for a subsequent mpls_pop. */
-
     /* Resubmit statistics, via xlate_table_action(). */
     int recurse;                /* Current resubmit nesting depth. */
     int resubmits;              /* Total number of resubmits. */
+    bool in_group;              /* Currently translating ofgroup, if true. */
 
     uint32_t orig_skb_priority; /* Priority when packet arrived. */
     uint8_t table_id;           /* OpenFlow table ID where flow was found. */
@@ -255,7 +254,8 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
                   const struct dpif_ipfix *ipfix,
                   const struct netflow *netflow, enum ofp_config_flags frag,
                   bool forward_bpdu, bool has_in_band,
-                  bool variable_length_userdata)
+                  bool variable_length_userdata,
+                  size_t max_mpls_depth)
 {
     struct xbridge *xbridge = xbridge_lookup(ofproto);
 
@@ -308,6 +308,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
     xbridge->miss_rule = miss_rule;
     xbridge->no_packet_in_rule = no_packet_in_rule;
     xbridge->variable_length_userdata = variable_length_userdata;
+    xbridge->max_mpls_depth = max_mpls_depth;
 }
 
 void
@@ -520,12 +521,10 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
 
 /* Given a datpath, packet, and flow metadata ('backer', 'packet', and 'key'
  * respectively), populates 'flow' with the result of odp_flow_key_to_flow().
- * Optionally, if nonnull, populates 'fitnessp' with the fitness of 'flow' as
- * returned by odp_flow_key_to_flow().  Also, optionally populates 'ofproto'
- * with the ofproto_dpif, 'odp_in_port' with the datapath in_port, that
- * 'packet' ingressed, and 'ipfix', 'sflow', and 'netflow' with the appropriate
- * handles for those protocols if they're enabled.  Caller is responsible for
- * unrefing them.
+ * Optionally populates 'ofproto' with the ofproto_dpif, 'odp_in_port' with
+ * the datapath in_port, that 'packet' ingressed, and 'ipfix', 'sflow', and
+ * 'netflow' with the appropriate handles for those protocols if they're
+ * enabled.  Caller is responsible for unrefing them.
  *
  * If 'ofproto' is nonnull, requires 'flow''s in_port to exist.  Otherwise sets
  * 'flow''s in_port to OFPP_NONE.
@@ -545,19 +544,16 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
  * or some other positive errno if there are other problems. */
 int
 xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
-              const struct nlattr *key, size_t key_len,
-              struct flow *flow, enum odp_key_fitness *fitnessp,
+              const struct nlattr *key, size_t key_len, struct flow *flow,
               struct ofproto_dpif **ofproto, struct dpif_ipfix **ipfix,
               struct dpif_sflow **sflow, struct netflow **netflow,
               odp_port_t *odp_in_port)
 {
-    enum odp_key_fitness fitness;
     const struct xport *xport;
     int error = ENODEV;
 
     ovs_rwlock_rdlock(&xlate_rwlock);
-    fitness = odp_flow_key_to_flow(key, key_len, flow);
-    if (fitness == ODP_FIT_ERROR) {
+    if (odp_flow_key_to_flow(key, key_len, flow) == ODP_FIT_ERROR) {
         error = EINVAL;
         goto exit;
     }
@@ -581,10 +577,8 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
              * an OpenFlow controller properly, so that it looks correct
              * for sFlow, and so that flow_extract() will get the correct
              * vlan_tci if it is called on 'packet'. */
-            eth_push_vlan(packet, flow->vlan_tci);
+            eth_push_vlan(packet, htons(ETH_TYPE_VLAN), flow->vlan_tci);
         }
-        /* We can't reproduce 'key' from 'flow'. */
-        fitness = fitness == ODP_FIT_PERFECT ? ODP_FIT_TOO_MUCH : fitness;
     }
     error = 0;
 
@@ -605,9 +599,6 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
     }
 
 exit:
-    if (fitnessp) {
-        *fitnessp = fitness;
-    }
     ovs_rwlock_unlock(&xlate_rwlock);
     return error;
 }
@@ -1694,7 +1685,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
 
     /* If 'struct flow' gets additional metadata, we'll need to zero it out
      * before traversing a patch port. */
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 23);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 24);
 
     if (!xport) {
         xlate_report(ctx, "Nonexistent output port");
@@ -1790,27 +1781,25 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
                                  &ctx->xout->odp_actions);
         flow->tunnel = flow_tnl; /* Restore tunnel metadata */
     } else {
-        ofp_port_t vlandev_port;
-
         odp_port = xport->odp_port;
+        out_port = odp_port;
         if (ofproto_has_vlan_splinters(ctx->xbridge->ofproto)) {
+            ofp_port_t vlandev_port;
+
             wc->masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
-        }
-        vlandev_port = vsp_realdev_to_vlandev(ctx->xbridge->ofproto, ofp_port,
-                                              flow->vlan_tci);
-        if (vlandev_port == ofp_port) {
-            out_port = odp_port;
-        } else {
-            out_port = ofp_port_to_odp_port(ctx->xbridge, vlandev_port);
-            flow->vlan_tci = htons(0);
+            vlandev_port = vsp_realdev_to_vlandev(ctx->xbridge->ofproto,
+                                                  ofp_port, flow->vlan_tci);
+            if (vlandev_port != ofp_port) {
+                out_port = ofp_port_to_odp_port(ctx->xbridge, vlandev_port);
+                flow->vlan_tci = htons(0);
+            }
         }
     }
 
     if (out_port != ODPP_NONE) {
         ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
                                               &ctx->xout->odp_actions,
-                                              &ctx->xout->wc,
-                                              &ctx->mpls_depth_delta);
+                                              &ctx->xout->wc);
         nl_msg_put_odp_port(&ctx->xout->odp_actions, OVS_ACTION_ATTR_OUTPUT,
                             out_port);
 
@@ -1992,6 +1981,8 @@ xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
 static void
 xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
 {
+    ctx->in_group = true;
+
     switch (group_dpif_get_type(group)) {
     case OFPGT11_ALL:
     case OFPGT11_INDIRECT:
@@ -2007,12 +1998,38 @@ xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
         OVS_NOT_REACHED();
     }
     group_dpif_release(group);
+
+    ctx->in_group = false;
+}
+
+static bool
+xlate_group_resource_check(struct xlate_ctx *ctx)
+{
+    if (!xlate_resubmit_resource_check(ctx)) {
+        return false;
+    } else if (ctx->in_group) {
+        /* Prevent nested translation of OpenFlow groups.
+         *
+         * OpenFlow allows this restriction.  We enforce this restriction only
+         * because, with the current architecture, we would otherwise have to
+         * take a possibly recursive read lock on the ofgroup rwlock, which is
+         * unsafe given that POSIX allows taking a read lock to block if there
+         * is a thread blocked on taking the write lock.  Other solutions
+         * without this restriction are also possible, but seem unwarranted
+         * given the current limited use of groups. */
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+
+        VLOG_ERR_RL(&rl, "cannot recursively translate OpenFlow group");
+        return false;
+    } else {
+        return true;
+    }
 }
 
 static bool
 xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
 {
-    if (xlate_resubmit_resource_check(ctx)) {
+    if (xlate_group_resource_check(ctx)) {
         struct group_dpif *group;
         bool got_group;
 
@@ -2074,7 +2091,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
 {
     struct ofproto_packet_in *pin;
     struct ofpbuf *packet;
-    struct flow key;
+    struct pkt_metadata md = PKT_METADATA_INITIALIZER(0);
 
     ctx->xout->slow |= SLOW_CONTROLLER;
     if (!ctx->xin->packet) {
@@ -2083,17 +2100,12 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
 
     packet = ofpbuf_clone(ctx->xin->packet);
 
-    key.skb_priority = 0;
-    key.pkt_mark = 0;
-    memset(&key.tunnel, 0, sizeof key.tunnel);
-
     ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                           &ctx->xout->odp_actions,
-                                          &ctx->xout->wc,
-                                          &ctx->mpls_depth_delta);
+                                          &ctx->xout->wc);
 
-    odp_execute_actions(NULL, packet, &key, ctx->xout->odp_actions.data,
-                        ctx->xout->odp_actions.size, NULL, NULL);
+    odp_execute_actions(NULL, packet, &md, ctx->xout->odp_actions.data,
+                        ctx->xout->odp_actions.size, NULL);
 
     pin = xmalloc(sizeof *pin);
     pin->up.packet_len = packet->size;
@@ -2114,99 +2126,56 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
     ofpbuf_delete(packet);
 }
 
-static bool
-compose_mpls_push_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
+static void
+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;
+    int n;
 
-    ovs_assert(eth_type_mpls(eth_type));
+    ovs_assert(eth_type_mpls(mpls->ethertype));
 
-    /* If mpls_depth_delta is negative then an MPLS POP action has been
-     * composed and the resulting MPLS label stack is unknown.  This means
-     * an MPLS PUSH action can't be composed as it needs to know either the
-     * top-most MPLS LSE to use as a template for the new MPLS LSE, or that
-     * there is no MPLS label stack present.  Thus, stop processing.
-     *
-     * If mpls_depth_delta is positive then an MPLS PUSH action has been
-     * composed and no further MPLS PUSH action may be performed without
-     * losing MPLS LSE and ether type information held in xtx->xin->flow.
-     * Thus, stop processing.
-     *
-     * If the MPLS LSE of the flow and base_flow differ then the MPLS LSE
-     * has been updated.  Performing a MPLS PUSH action may be would result in
-     * losing MPLS LSE and ether type information held in xtx->xin->flow.
-     * Thus, stop processing.
-     *
-     * It is planned that in the future this case will be handled
-     * by recirculation */
-    if (ctx->mpls_depth_delta ||
-        ctx->xin->flow.mpls_lse != ctx->base_flow.mpls_lse) {
-        return true;
-    }
-
-    memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
-
-    ctx->pre_push_mpls_lse = ctx->xin->flow.mpls_lse;
-
-    if (eth_type_mpls(ctx->xin->flow.dl_type)) {
-        flow->mpls_lse &= ~htonl(MPLS_BOS_MASK);
-    } else {
-        ovs_be32 label;
-        uint8_t tc, ttl;
-
-        if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
-            label = htonl(0x2); /* IPV6 Explicit Null. */
-        } else {
-            label = htonl(0x0); /* IPV4 Explicit Null. */
+    n = flow_count_mpls_labels(flow, wc);
+    if (!n) {
+        ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
+                                              &ctx->xout->odp_actions,
+                                              &ctx->xout->wc);
+    } else if (n >= FLOW_MAX_MPLS_LABELS) {
+        if (ctx->xin->packet != NULL) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_WARN_RL(&rl, "bridge %s: dropping packet on which an "
+                         "MPLS push action can't be performed as it would "
+                         "have more MPLS LSEs than the %d supported.",
+                         ctx->xbridge->name, FLOW_MAX_MPLS_LABELS);
         }
-        wc->masks.nw_tos |= IP_DSCP_MASK;
-        wc->masks.nw_ttl = 0xff;
-        tc = (flow->nw_tos & IP_DSCP_MASK) >> 2;
-        ttl = flow->nw_ttl ? flow->nw_ttl : 0x40;
-        flow->mpls_lse = set_mpls_lse_values(ttl, tc, 1, label);
+        ctx->exit = true;
+        return;
+    } else if (n >= ctx->xbridge->max_mpls_depth) {
+        COVERAGE_INC(xlate_actions_mpls_overflow);
+        ctx->xout->slow |= SLOW_ACTION;
     }
-    flow->dl_type = eth_type;
-    ctx->mpls_depth_delta++;
 
-    return false;
+    flow_push_mpls(flow, n, mpls->ethertype, wc);
 }
 
-static bool
+static void
 compose_mpls_pop_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
 {
     struct flow_wildcards *wc = &ctx->xout->wc;
+    struct flow *flow = &ctx->xin->flow;
+    int n = flow_count_mpls_labels(flow, wc);
 
-    if (!eth_type_mpls(ctx->xin->flow.dl_type)) {
-        return true;
-    }
-
-    /* If mpls_depth_delta is negative then an MPLS POP action has been
-     * composed.  Performing another MPLS POP action
-     * would result in losing ether type that results from
-     * the already composed MPLS POP. Thus, stop processing.
-     *
-     * It is planned that in the future this case will be handled
-     * by recirculation */
-    if (ctx->mpls_depth_delta < 0) {
-        return true;
-    }
-
-    memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
-
-    /* If mpls_depth_delta is positive then an MPLS PUSH action has been
-     * executed and the previous MPLS LSE saved in ctx->pre_push_mpls_lse. The
-     * flow's MPLS LSE should be restored to that value to allow any
-     * subsequent actions that update of the LSE to be executed correctly.
-     */
-    if (ctx->mpls_depth_delta > 0) {
-        ctx->xin->flow.mpls_lse = ctx->pre_push_mpls_lse;
+    if (!flow_pop_mpls(flow, n, eth_type, wc) && n >= FLOW_MAX_MPLS_LABELS) {
+        if (ctx->xin->packet != NULL) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_WARN_RL(&rl, "bridge %s: dropping packet on which an "
+                         "MPLS pop action can't be performed as it has "
+                         "more MPLS LSEs than the %d supported.",
+                         ctx->xbridge->name, FLOW_MAX_MPLS_LABELS);
+        }
+        ctx->exit = true;
+        ofpbuf_clear(&ctx->xout->odp_actions);
     }
-
-    ctx->xin->flow.dl_type = eth_type;
-    ctx->mpls_depth_delta--;
-
-    return false;
 }
 
 static bool
@@ -2235,99 +2204,53 @@ compose_dec_ttl(struct xlate_ctx *ctx, struct ofpact_cnt_ids *ids)
     }
 }
 
-static bool
+static void
 compose_set_mpls_label_action(struct xlate_ctx *ctx, ovs_be32 label)
 {
-    if (!eth_type_mpls(ctx->xin->flow.dl_type)) {
-        return true;
-    }
-
-    /* If mpls_depth_delta is negative then an MPLS POP action has been
-     * executed and the resulting MPLS label stack is unknown.  This means
-     * a SET MPLS LABEL action can't be executed as it needs to manipulate
-     * the top-most MPLS LSE. Thus, stop processing.
-     *
-     * It is planned that in the future this case will be handled
-     * by recirculation.
-     */
-    if (ctx->mpls_depth_delta < 0) {
-        return true;
+    if (eth_type_mpls(ctx->xin->flow.dl_type)) {
+        ctx->xout->wc.masks.mpls_lse[0] |= htonl(MPLS_LABEL_MASK);
+        set_mpls_lse_label(&ctx->xin->flow.mpls_lse[0], label);
     }
-
-    ctx->xout->wc.masks.mpls_lse |= htonl(MPLS_LABEL_MASK);
-    set_mpls_lse_label(&ctx->xin->flow.mpls_lse, label);
-    return false;
 }
 
-static bool
+static void
 compose_set_mpls_tc_action(struct xlate_ctx *ctx, uint8_t tc)
 {
-    if (!eth_type_mpls(ctx->xin->flow.dl_type)) {
-        return true;
-    }
-
-    /* If mpls_depth_delta is negative then an MPLS POP action has been
-     * executed and the resulting MPLS label stack is unknown.  This means
-     * a SET MPLS TC action can't be executed as it needs to manipulate
-     * the top-most MPLS LSE. Thus, stop processing.
-     *
-     * It is planned that in the future this case will be handled
-     * by recirculation.
-     */
-    if (ctx->mpls_depth_delta < 0) {
-        return true;
+    if (eth_type_mpls(ctx->xin->flow.dl_type)) {
+        ctx->xout->wc.masks.mpls_lse[0] |= htonl(MPLS_TC_MASK);
+        set_mpls_lse_tc(&ctx->xin->flow.mpls_lse[0], tc);
     }
-
-    ctx->xout->wc.masks.mpls_lse |= htonl(MPLS_TC_MASK);
-    set_mpls_lse_tc(&ctx->xin->flow.mpls_lse, tc);
-    return false;
 }
 
-static bool
+static void
 compose_set_mpls_ttl_action(struct xlate_ctx *ctx, uint8_t ttl)
 {
-    if (!eth_type_mpls(ctx->xin->flow.dl_type)) {
-        return true;
-    }
-
-    /* If mpls_depth_delta is negative then an MPLS POP action has been
-     * executed and the resulting MPLS label stack is unknown.  This means
-     * a SET MPLS TTL push action can't be executed as it needs to manipulate
-     * the top-most MPLS LSE. Thus, stop processing.
-     *
-     * It is planned that in the future this case will be handled
-     * by recirculation.
-     */
-    if (ctx->mpls_depth_delta < 0) {
-        return true;
+    if (eth_type_mpls(ctx->xin->flow.dl_type)) {
+        ctx->xout->wc.masks.mpls_lse[0] |= htonl(MPLS_TTL_MASK);
+        set_mpls_lse_ttl(&ctx->xin->flow.mpls_lse[0], ttl);
     }
-
-    ctx->xout->wc.masks.mpls_lse |= htonl(MPLS_TTL_MASK);
-    set_mpls_lse_ttl(&ctx->xin->flow.mpls_lse, ttl);
-    return false;
 }
 
 static bool
 compose_dec_mpls_ttl_action(struct xlate_ctx *ctx)
 {
     struct flow *flow = &ctx->xin->flow;
-    uint8_t ttl = mpls_lse_to_ttl(flow->mpls_lse);
+    uint8_t ttl = mpls_lse_to_ttl(flow->mpls_lse[0]);
     struct flow_wildcards *wc = &ctx->xout->wc;
 
     memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
+    if (eth_type_mpls(flow->dl_type)) {
+        if (ttl > 1) {
+            ttl--;
+            set_mpls_lse_ttl(&flow->mpls_lse[0], ttl);
+            return false;
+        } else {
+            execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0);
 
-    if (!eth_type_mpls(flow->dl_type)) {
-        return false;
-    }
-
-    if (ttl > 1) {
-        ttl--;
-        set_mpls_lse_ttl(&flow->mpls_lse, ttl);
-        return false;
+            /* Stop processing for current table. */
+            return true;
+        }
     } else {
-        execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0);
-
-        /* Stop processing for current table. */
         return true;
     }
 }
@@ -2539,8 +2462,7 @@ xlate_sample_action(struct xlate_ctx *ctx,
 
   ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                         &ctx->xout->odp_actions,
-                                        &ctx->xout->wc,
-                                        &ctx->mpls_depth_delta);
+                                        &ctx->xout->wc);
 
   compose_flow_sample_cookie(os->probability, os->collector_set_id,
                              os->obs_domain_id, os->obs_point_id, &cookie);
@@ -2758,7 +2680,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
              * applicable header fields.  Do nothing if no header exists. */
             if ((mf->id != MFF_VLAN_VID || flow->vlan_tci & htons(VLAN_CFI))
                 && ((mf->id != MFF_MPLS_LABEL && mf->id != MFF_MPLS_TC)
-                    || flow->mpls_lse)) {
+                    || eth_type_mpls(flow->dl_type))) {
                 mf_set_flow_value(mf, &set_field->value, flow);
             }
             break;
@@ -2774,38 +2696,24 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_PUSH_MPLS:
-            if (compose_mpls_push_action(ctx,
-                                         ofpact_get_PUSH_MPLS(a)->ethertype)) {
-                return;
-            }
+            compose_mpls_push_action(ctx, ofpact_get_PUSH_MPLS(a));
             break;
 
         case OFPACT_POP_MPLS:
-            if (compose_mpls_pop_action(ctx,
-                                        ofpact_get_POP_MPLS(a)->ethertype)) {
-                return;
-            }
+            compose_mpls_pop_action(ctx, ofpact_get_POP_MPLS(a)->ethertype);
             break;
 
         case OFPACT_SET_MPLS_LABEL:
-            if (compose_set_mpls_label_action(ctx,
-                                              ofpact_get_SET_MPLS_LABEL(a)->label)) {
-                return;
-            }
-            break;
+            compose_set_mpls_label_action(
+                ctx, ofpact_get_SET_MPLS_LABEL(a)->label);
+        break;
 
         case OFPACT_SET_MPLS_TC:
-            if (compose_set_mpls_tc_action(ctx,
-                                           ofpact_get_SET_MPLS_TC(a)->tc)) {
-                return;
-            }
+            compose_set_mpls_tc_action(ctx, ofpact_get_SET_MPLS_TC(a)->tc);
             break;
 
         case OFPACT_SET_MPLS_TTL:
-            if (compose_set_mpls_ttl_action(ctx,
-                                            ofpact_get_SET_MPLS_TTL(a)->ttl)) {
-                return;
-            }
+            compose_set_mpls_ttl_action(ctx, ofpact_get_SET_MPLS_TTL(a)->ttl);
             break;
 
         case OFPACT_DEC_MPLS_TTL:
@@ -3003,6 +2911,7 @@ actions_output_to_local_port(const struct xlate_ctx *ctx)
 /* Thread safe call to xlate_actions__(). */
 void
 xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
+    OVS_EXCLUDED(xlate_rwlock)
 {
     ovs_rwlock_rdlock(&xlate_rwlock);
     xlate_actions__(xin, xout);
@@ -3030,6 +2939,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);
 
@@ -3081,7 +2991,10 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
     memset(&wc->masks.in_port, 0xff, sizeof wc->masks.in_port);
     memset(&wc->masks.skb_priority, 0xff, sizeof wc->masks.skb_priority);
     memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
-    wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
+    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) {
@@ -3090,10 +3003,10 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
 
     ctx.recurse = 0;
     ctx.resubmits = 0;
+    ctx.in_group = false;
     ctx.orig_skb_priority = flow->skb_priority;
     ctx.table_id = 0;
     ctx.exit = false;
-    ctx.mpls_depth_delta = 0;
 
     if (!xin->ofpacts && !ctx.rule) {
         rule_dpif_lookup(ctx.xbridge->ofproto, flow,
@@ -3242,6 +3155,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);
@@ -3256,13 +3184,11 @@ xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
     struct xport *xport;
     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. */
-    in_port_.ofp_port = OFPP_NONE;
-    flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
+    flow_extract(packet, NULL, &flow);
+    flow.in_port.ofp_port = OFPP_NONE;
 
     ovs_rwlock_rdlock(&xlate_rwlock);
     xport = xport_lookup(ofport);
@@ -3272,9 +3198,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);
 }