ofproto-dpif-xlate: Break recirculation actions out from action_set.
[cascardo/ovs.git] / ofproto / ofproto-dpif-xlate.c
index cf184e4..f48c5ac 100644 (file)
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -258,31 +258,28 @@ struct xlate_ctx {
     * members.  When a need for recirculation is identified, the translation
     * process:
     *
-    * 1. Sets 'recirc_action_offset' to the current size of 'action_set'.  The
-    *    action set is part of what needs to be preserved, so this allows the
-    *    action set and the additional state to share the 'action_set' buffer.
-    *    Later steps can tell that setup for recirculation is in progress from
-    *    the nonnegative value of 'recirc_action_offset'.
+    * 1. Sets 'recirculating' to true.
     *
     * 2. Sets 'exit' to true to tell later steps that we're exiting from the
     *    translation process.
     *
-    * 3. Adds an OFPACT_UNROLL_XLATE action to 'action_set'.  This action
-    *    holds the current table ID and cookie so that they can be restored
-    *    during a post-recirculation upcall translation.
+    * 3. Adds an OFPACT_UNROLL_XLATE action to 'recirculate_actions', and
+    *    points recirculate_actions.header to the action to make it easy to
+    *    find it later.  This action holds the current table ID and cookie so
+    *    that they can be restored during a post-recirculation upcall
+    *    translation.
     *
     * 4. Adds the action that prompted recirculation and any actions following
-    *    it within the same flow to 'action_set', so that they can be executed
-    *    during a post-recirculation upcall translation.
+    *    it within the same flow to 'recirculate_actions', so that they can be
+    *    executed during a post-recirculation upcall translation.
     *
     * 5. Returns.
     *
     * 6. The action that prompted recirculation might be nested in a stack of
     *    nested "resubmit"s that have actions remaining.  Each of these notices
-    *    that we're exiting (from 'exit') and that recirculation setup is in
-    *    progress (from 'recirc_action_offset') and responds by adding more
-    *    OFPACT_UNROLL_XLATE actions to 'action_set', as necessary, and any
-    *    actions that were yet unprocessed.
+    *    that we're exiting and recirculating and responds by adding more
+    *    OFPACT_UNROLL_XLATE actions to 'recirculate_actions', as necessary,
+    *    and any actions that were yet unprocessed.
     *
     * The caller stores all the state produced by this process associated with
     * the recirculation ID.  For post-recirculation upcall translation, the
@@ -290,10 +287,8 @@ struct xlate_ctx {
     * process yielded a set of ofpacts that can be translated directly, so it
     * is not much of a special case at that point.
     */
-    int recirc_action_offset;   /* Offset in 'action_set' to actions to be
-                                 * executed after recirculation, or -1. */
-    int last_unroll_offset;     /* Offset in 'action_set' to the latest unroll
-                                 * action, or -1. */
+    bool recirculating;
+    struct ofpbuf recirculate_actions;
 
     /* True if a packet was but is no longer MPLS (due to an MPLS pop action).
      * This is a trigger for recirculation in cases where translating an action
@@ -351,21 +346,23 @@ static void
 ctx_trigger_recirculation(struct xlate_ctx *ctx)
 {
     ctx->exit = true;
-    ctx->recirc_action_offset = ctx->action_set.size;
+    ctx->recirculating = true;
 }
 
 static bool
 ctx_first_recirculation_action(const struct xlate_ctx *ctx)
 {
-    return ctx->recirc_action_offset == ctx->action_set.size;
+    return !ctx->recirculate_actions.size;
 }
 
-static inline bool
-exit_recirculates(const struct xlate_ctx *ctx)
+static void
+ctx_cancel_recirculation(struct xlate_ctx *ctx)
 {
-    /* When recirculating the 'recirc_action_offset' has a non-negative value.
-     */
-    return ctx->recirc_action_offset >= 0;
+    if (ctx->recirculating) {
+        ctx->recirculating = false;
+        ofpbuf_clear(&ctx->recirculate_actions);
+        ctx->recirculate_actions.header = NULL;
+    }
 }
 
 static void compose_recirculate_action(struct xlate_ctx *ctx);
@@ -457,11 +454,11 @@ struct xc_entry {
     } u;
 };
 
-#define XC_ENTRY_FOR_EACH(entry, entries, xcache)               \
-    entries = xcache->entries;                                  \
-    for (entry = ofpbuf_try_pull(&entries, sizeof *entry);      \
-         entry;                                                 \
-         entry = ofpbuf_try_pull(&entries, sizeof *entry))
+#define XC_ENTRY_FOR_EACH(ENTRY, ENTRIES, XCACHE)               \
+    ENTRIES = XCACHE->entries;                                  \
+    for (ENTRY = ofpbuf_try_pull(&ENTRIES, sizeof *ENTRY);      \
+         ENTRY;                                                 \
+         ENTRY = ofpbuf_try_pull(&ENTRIES, sizeof *ENTRY))
 
 struct xlate_cache {
     struct ofpbuf entries;
@@ -506,6 +503,8 @@ static void compose_output_action(struct xlate_ctx *, ofp_port_t ofp_port,
 
 static struct xbridge *xbridge_lookup(struct xlate_cfg *,
                                       const struct ofproto_dpif *);
+static struct xbridge *xbridge_lookup_by_uuid(struct xlate_cfg *,
+                                              const struct uuid *);
 static struct xbundle *xbundle_lookup(struct xlate_cfg *,
                                       const struct ofbundle *);
 static struct xport *xport_lookup(struct xlate_cfg *,
@@ -1233,6 +1232,19 @@ xbridge_lookup(struct xlate_cfg *xcfg, const struct ofproto_dpif *ofproto)
     return NULL;
 }
 
+static struct xbridge *
+xbridge_lookup_by_uuid(struct xlate_cfg *xcfg, const struct uuid *uuid)
+{
+    struct xbridge *xbridge;
+
+    HMAP_FOR_EACH (xbridge, hmap_node, &xcfg->xbridges) {
+        if (uuid_equals(ofproto_dpif_get_uuid(xbridge->ofproto), uuid)) {
+            return xbridge;
+        }
+    }
+    return NULL;
+}
+
 static struct xbundle *
 xbundle_lookup(struct xlate_cfg *xcfg, const struct ofbundle *ofbundle)
 {
@@ -2970,15 +2982,10 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         if (!process_special(ctx, peer) && may_receive(peer, ctx)) {
             if (xport_stp_forward_state(peer) && xport_rstp_forward_state(peer)) {
                 xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true);
-                if (ctx->action_set.size) {
-                    /* Translate action set only if not dropping the packet and
-                     * not recirculating. */
-                    if (!exit_recirculates(ctx)) {
-                        xlate_action_set(ctx);
-                    }
+                if (!ctx->recirculating) {
+                    xlate_action_set(ctx);
                 }
-                /* Check if need to recirculate. */
-                if (exit_recirculates(ctx)) {
+                if (ctx->recirculating) {
                     compose_recirculate_action(ctx);
                 }
             } else {
@@ -2994,11 +3001,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
                 ctx->odp_actions->size = old_size;
 
                 /* Undo changes that may have been done for recirculation. */
-                if (exit_recirculates(ctx)) {
-                    ctx->action_set.size = ctx->recirc_action_offset;
-                    ctx->recirc_action_offset = -1;
-                    ctx->last_unroll_offset = -1;
-                }
+                ctx_cancel_recirculation(ctx);
             }
         }
 
@@ -3312,7 +3315,7 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket)
     ofpbuf_uninit(&action_list);
 
     /* Check if need to recirculate. */
-    if (exit_recirculates(ctx)) {
+    if (ctx->recirculating) {
         compose_recirculate_action(ctx);
     }
 
@@ -3568,49 +3571,47 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
                           enum ofp_packet_in_reason reason,
                           uint16_t controller_id)
 {
-    struct ofproto_packet_in *pin;
     struct dp_packet *packet;
 
     ctx->xout->slow |= SLOW_CONTROLLER;
+    xlate_commit_actions(ctx);
     if (!ctx->xin->packet) {
         return;
     }
 
     packet = dp_packet_clone(ctx->xin->packet);
 
-    xlate_commit_actions(ctx);
-
     odp_execute_actions(NULL, &packet, 1, false,
                         ctx->odp_actions->data, ctx->odp_actions->size, NULL);
 
-    pin = xmalloc(sizeof *pin);
-    pin->up.packet_len = dp_packet_size(packet);
-    pin->up.packet = dp_packet_steal_data(packet);
-    pin->up.reason = reason;
-    pin->up.table_id = ctx->table_id;
-    pin->up.cookie = ctx->rule_cookie;
-
-    flow_get_metadata(&ctx->xin->flow, &pin->up.flow_metadata);
+    /* A packet sent by an action in a table-miss rule is considered an
+     * explicit table miss.  OpenFlow before 1.3 doesn't have that concept so
+     * it will get translated back to OFPR_ACTION for those versions. */
+    if (reason == OFPR_ACTION
+        && ctx->rule && rule_dpif_is_table_miss(ctx->rule)) {
+        reason = OFPR_EXPLICIT_MISS;
+    }
+
+    size_t packet_len = dp_packet_size(packet);
+
+    struct ofproto_async_msg *am = xmalloc(sizeof *am);
+    *am = (struct ofproto_async_msg) {
+        .controller_id = controller_id,
+        .oam = OAM_PACKET_IN,
+        .pin = {
+            .up = {
+                .packet = dp_packet_steal_data(packet),
+                .len = packet_len,
+                .reason = reason,
+                .table_id = ctx->table_id,
+                .cookie = ctx->rule_cookie,
+            },
+            .max_len = len,
+        },
+    };
+    flow_get_metadata(&ctx->xin->flow, &am->pin.up.flow_metadata);
 
-    pin->controller_id = controller_id;
-    pin->send_len = len;
-    /* If a rule is a table-miss rule then this is
-     * a table-miss handled by a table-miss rule.
-     *
-     * Else, if rule is internal and has a controller action,
-     * the later being implied by the rule being processed here,
-     * then this is a table-miss handled without a table-miss rule.
-     *
-     * Otherwise this is not a table-miss. */
-    pin->miss_type = OFPROTO_PACKET_IN_NO_MISS;
-    if (ctx->rule) {
-        if (rule_dpif_is_table_miss(ctx->rule)) {
-            pin->miss_type = OFPROTO_PACKET_IN_MISS_FLOW;
-        } else if (rule_dpif_is_internal(ctx->rule)) {
-            pin->miss_type = OFPROTO_PACKET_IN_MISS_WITHOUT_FLOW;
-        }
-    }
-    ofproto_dpif_send_packet_in(ctx->xbridge->ofproto, pin);
+    ofproto_dpif_send_async_msg(ctx->xbridge->ofproto, am);
     dp_packet_delete(packet);
 }
 
@@ -3622,18 +3623,20 @@ compose_recirculate_action__(struct xlate_ctx *ctx, uint8_t table)
 
     recirc_metadata_from_flow(&md, &ctx->xin->flow);
 
-    ovs_assert(ctx->recirc_action_offset >= 0);
+    ovs_assert(ctx->recirculating);
 
     struct recirc_state state = {
         .table_id = table,
-        .ofproto = ctx->xbridge->ofproto,
+        .ofproto_uuid = *ofproto_dpif_get_uuid(ctx->xbridge->ofproto),
         .metadata = md,
-        .stack = &ctx->stack,
+        .stack = ctx->stack.data,
+        .n_stack = ctx->stack.size / sizeof(union mf_subvalue),
         .mirrors = ctx->mirrors,
         .conntracked = ctx->conntracked,
-        .action_set_len = ctx->recirc_action_offset,
-        .ofpacts_len = ctx->action_set.size,
-        .ofpacts = ctx->action_set.data,
+        .ofpacts = ctx->recirculate_actions.data,
+        .ofpacts_len = ctx->recirculate_actions.size,
+        .action_set = ctx->action_set.data,
+        .action_set_len = ctx->action_set.size,
     };
 
     /* Allocate a unique recirc id for the given metadata state in the
@@ -3652,12 +3655,10 @@ compose_recirculate_action__(struct xlate_ctx *ctx, uint8_t table)
     nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_RECIRC, id);
 
     /* Undo changes done by recirculation. */
-    ctx->action_set.size = ctx->recirc_action_offset;
-    ctx->recirc_action_offset = -1;
-    ctx->last_unroll_offset = -1;
+    ctx_cancel_recirculation(ctx);
 }
 
-/* Called only when ctx->recirc_action_offset is set. */
+/* Called only when we're recirculating. */
 static void
 compose_recirculate_action(struct xlate_ctx *ctx)
 {
@@ -3672,7 +3673,7 @@ compose_recirculate_action(struct xlate_ctx *ctx)
 static void
 compose_recirculate_and_fork(struct xlate_ctx *ctx, uint8_t table)
 {
-    ctx->recirc_action_offset = ctx->action_set.size;
+    ctx->recirculating = true;
     compose_recirculate_action__(ctx, table);
 }
 
@@ -4073,12 +4074,9 @@ may_receive(const struct xport *xport, struct xlate_ctx *ctx)
 }
 
 static void
-xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact *a)
+xlate_write_actions__(struct xlate_ctx *ctx,
+                      const struct ofpact *ofpacts, size_t ofpacts_len)
 {
-    const struct ofpact_nest *on = ofpact_get_WRITE_ACTIONS(a);
-    size_t on_len = ofpact_nest_get_action_len(on);
-    const struct ofpact *inner;
-
     /* Maintain actset_output depending on the contents of the action set:
      *
      *   - OFPP_UNSET, if there is no "output" action.
@@ -4089,10 +4087,11 @@ xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact *a)
      *   - OFPP_UNSET, if there is a "group" action.
      */
     if (!ctx->action_set_has_group) {
-        OFPACT_FOR_EACH (inner, on->actions, on_len) {
-            if (inner->type == OFPACT_OUTPUT) {
-                ctx->xin->flow.actset_output = ofpact_get_OUTPUT(inner)->port;
-            } else if (inner->type == OFPACT_GROUP) {
+        const struct ofpact *a;
+        OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+            if (a->type == OFPACT_OUTPUT) {
+                ctx->xin->flow.actset_output = ofpact_get_OUTPUT(a)->port;
+            } else if (a->type == OFPACT_GROUP) {
                 ctx->xin->flow.actset_output = OFPP_UNSET;
                 ctx->action_set_has_group = true;
                 break;
@@ -4100,8 +4099,13 @@ xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact *a)
         }
     }
 
-    ofpbuf_put(&ctx->action_set, on->actions, on_len);
-    ofpact_pad(&ctx->action_set);
+    ofpbuf_put(&ctx->action_set, ofpacts, ofpacts_len);
+}
+
+static void
+xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact_nest *a)
+{
+    xlate_write_actions__(ctx, a->actions, ofpact_nest_get_action_len(a));
 }
 
 static void
@@ -4123,50 +4127,50 @@ xlate_action_set(struct xlate_ctx *ctx)
 static void
 recirc_put_unroll_xlate(struct xlate_ctx *ctx)
 {
-    struct ofpact_unroll_xlate *unroll;
-
-    unroll = ctx->last_unroll_offset < 0
-        ? NULL
-        : ALIGNED_CAST(struct ofpact_unroll_xlate *,
-                       (char *)ctx->action_set.data + ctx->last_unroll_offset);
+    struct ofpact_unroll_xlate *unroll = ctx->recirculate_actions.header;
 
     /* Restore the table_id and rule cookie for a potential PACKET
      * IN if needed. */
     if (!unroll ||
         (ctx->table_id != unroll->rule_table_id
          || ctx->rule_cookie != unroll->rule_cookie)) {
-
-        ctx->last_unroll_offset = ctx->action_set.size;
-        unroll = ofpact_put_UNROLL_XLATE(&ctx->action_set);
+        unroll = ofpact_put_UNROLL_XLATE(&ctx->recirculate_actions);
         unroll->rule_table_id = ctx->table_id;
         unroll->rule_cookie = ctx->rule_cookie;
+        ctx->recirculate_actions.header = unroll;
     }
 }
 
 
-/* Copy remaining actions to the action_set to be executed after recirculation.
- * UNROLL_XLATE action is inserted, if not already done so, before actions that
- * may generate PACKET_INs from the current table and without matching another
- * rule. */
+/* Copy actions 'a' through 'end' to ctx->recirculate_actions, which will be
+ * executed after recirculation.  UNROLL_XLATE action is inserted, if not
+ * already done so, before actions that may depend on the current table ID or
+ * flow cookie. */
 static void
-recirc_unroll_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
+recirc_unroll_actions(const struct ofpact *a, const struct ofpact *end,
                       struct xlate_ctx *ctx)
 {
-    const struct ofpact *a;
-
-    OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+    for (; a < end; a = ofpact_next(a)) {
         switch (a->type) {
-            /* May generate PACKET INs. */
         case OFPACT_OUTPUT_REG:
         case OFPACT_GROUP:
         case OFPACT_OUTPUT:
         case OFPACT_CONTROLLER:
         case OFPACT_DEC_MPLS_TTL:
         case OFPACT_DEC_TTL:
+            /* These actions may generate asynchronous messages, which include
+             * table ID and flow cookie information. */
             recirc_put_unroll_xlate(ctx);
             break;
 
-            /* These may not generate PACKET INs. */
+        case OFPACT_RESUBMIT:
+            if (ofpact_get_RESUBMIT(a)->table_id == 0xff) {
+                /* This resubmit action is relative to the current table, so we
+                 * need to track what table that is.*/
+                recirc_put_unroll_xlate(ctx);
+            }
+            break;
+
         case OFPACT_SET_TUNNEL:
         case OFPACT_REG_MOVE:
         case OFPACT_SET_FIELD:
@@ -4174,8 +4178,7 @@ recirc_unroll_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         case OFPACT_STACK_POP:
         case OFPACT_LEARN:
         case OFPACT_WRITE_METADATA:
-        case OFPACT_RESUBMIT:        /* May indirectly generate PACKET INs, */
-        case OFPACT_GOTO_TABLE:      /* but from a different table and rule. */
+        case OFPACT_GOTO_TABLE:
         case OFPACT_ENQUEUE:
         case OFPACT_SET_VLAN_VID:
         case OFPACT_SET_VLAN_PCP:
@@ -4209,15 +4212,16 @@ recirc_unroll_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         case OFPACT_DEBUG_RECIRC:
         case OFPACT_CT:
         case OFPACT_NAT:
+            /* These may not generate PACKET INs. */
             break;
 
-            /* These need not be copied for restoration. */
         case OFPACT_NOTE:
         case OFPACT_CONJUNCTION:
+            /* These need not be copied for restoration. */
             continue;
         }
         /* Copy the action over. */
-        ofpbuf_put(&ctx->action_set, a, OFPACT_ALIGN(a->len));
+        ofpbuf_put(&ctx->recirculate_actions, a, OFPACT_ALIGN(a->len));
     }
 }
 
@@ -4412,10 +4416,8 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         if (ctx->exit) {
             /* Check if need to store the remaining actions for later
              * execution. */
-            if (exit_recirculates(ctx)) {
-                recirc_unroll_actions(a, OFPACT_ALIGN(ofpacts_len -
-                                                      ((uint8_t *)a -
-                                                       (uint8_t *)ofpacts)),
+            if (ctx->recirculating) {
+                recirc_unroll_actions(a, ofpact_end(ofpacts, ofpacts_len),
                                       ctx);
             }
             break;
@@ -4549,8 +4551,30 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_RESUBMIT:
+            /* Recirculation complicates resubmit.  There are two cases:
+             *
+             *     - If mpls_pop has been executed, then the flow table lookup
+             *       as part of resubmit might depend on fields that can only
+             *       be obtained via recirculation, so the resubmit itself
+             *       triggers recirculation and we need to make sure that the
+             *       resubmit is executed again after recirculation.
+             *       Therefore, in this case we trigger recirculation and let
+             *       the code following this "switch" append the resubmit to
+             *       the post-recirculation actions.
+             *
+             *     - Otherwise, some action in the flow entry found by resubmit
+             *       might trigger recirculation.  If that happens, then we do
+             *       not want to execute the resubmit again after
+             *       recirculation, so we want to skip back to the head of the
+             *       loop to avoid that, only adding any actions that follow
+             *       the resubmit to the post-recirculation actions.
+             */
+            if (ctx->was_mpls) {
+                ctx_trigger_recirculation(ctx);
+                break;
+            }
             xlate_ofpact_resubmit(ctx, ofpact_get_RESUBMIT(a));
-            break;
+            continue;
 
         case OFPACT_SET_TUNNEL:
             flow->tunnel.tun_id = htonll(ofpact_get_SET_TUNNEL(a)->tun_id);
@@ -4723,7 +4747,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_WRITE_ACTIONS:
-            xlate_write_actions(ctx, a);
+            xlate_write_actions(ctx, ofpact_get_WRITE_ACTIONS(a));
             break;
 
         case OFPACT_WRITE_METADATA:
@@ -4739,12 +4763,8 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         case OFPACT_GOTO_TABLE: {
             struct ofpact_goto_table *ogt = ofpact_get_GOTO_TABLE(a);
 
-            /* Allow ctx->table_id == TBL_INTERNAL, which will be greater
-             * than ogt->table_id. This is to allow goto_table actions that
-             * triggered recirculation: ctx->table_id will be TBL_INTERNAL
-             * after recirculation. */
-            ovs_assert(ctx->table_id == TBL_INTERNAL
-                       || ctx->table_id < ogt->table_id);
+            ovs_assert(ctx->table_id < ogt->table_id);
+
             xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,
                                ogt->table_id, true, true);
             break;
@@ -4773,10 +4793,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         /* Check if need to store this and the remaining actions for later
          * execution. */
         if (!ctx->error && ctx->exit && ctx_first_recirculation_action(ctx)) {
-            recirc_unroll_actions(a, OFPACT_ALIGN(ofpacts_len -
-                                                  ((uint8_t *)a -
-                                                   (uint8_t *)ofpacts)),
-                                  ctx);
+            recirc_unroll_actions(a, ofpact_end(ofpacts, ofpacts_len), ctx);
             break;
         }
     }
@@ -4809,9 +4826,14 @@ xlate_in_init(struct xlate_in *xin, struct ofproto_dpif *ofproto,
     xin->odp_actions = odp_actions;
 
     /* Do recirc lookup. */
-    xin->recirc = flow->recirc_id
-        ? recirc_id_node_find(flow->recirc_id)
-        : NULL;
+    xin->recirc = NULL;
+    if (flow->recirc_id) {
+        const struct recirc_id_node *node
+            = recirc_id_node_find(flow->recirc_id);
+        if (node) {
+            xin->recirc = &node->state;
+        }
+    }
 }
 
 void
@@ -5035,7 +5057,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 {
     *xout = (struct xlate_out) {
         .slow = 0,
-        .fail_open = false,
         .recircs = RECIRC_REFS_EMPTY_INITIALIZER,
     };
 
@@ -5049,6 +5070,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
     union mf_subvalue stack_stub[1024 / sizeof(union mf_subvalue)];
     uint64_t action_set_stub[1024 / 8];
+    uint64_t recirculate_actions_stub[1024 / 8];
     struct flow_wildcards scratch_wc;
     uint64_t actions_stub[256 / 8];
     struct ofpbuf scratch_actions = OFPBUF_STUB_INITIALIZER(actions_stub);
@@ -5078,8 +5100,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         .error = XLATE_OK,
         .mirrors = 0,
 
-        .recirc_action_offset = -1,
-        .last_unroll_offset = -1,
+        .recirculating = false,
+        .recirculate_actions = OFPBUF_STUB_INITIALIZER(recirculate_actions_stub),
 
         .was_mpls = false,
         .conntracked = false,
@@ -5120,7 +5142,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
     COVERAGE_INC(xlate_actions);
 
     if (xin->recirc) {
-        const struct recirc_state *state = &xin->recirc->state;
+        const struct recirc_state *state = xin->recirc;
 
         xlate_report(&ctx, "Restoring state post-recirculation:");
 
@@ -5135,10 +5157,11 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         }
 
         /* Set the bridge for post-recirculation processing if needed. */
-        if (ctx.xbridge->ofproto != state->ofproto) {
+        if (!uuid_equals(ofproto_dpif_get_uuid(ctx.xbridge->ofproto),
+                         &state->ofproto_uuid)) {
             struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
             const struct xbridge *new_bridge
-                = xbridge_lookup(xcfg, state->ofproto);
+                = xbridge_lookup_by_uuid(xcfg, &state->ofproto_uuid);
 
             if (OVS_UNLIKELY(!new_bridge)) {
                 /* Drop the packet if the bridge cannot be found. */
@@ -5167,7 +5190,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
         /* Restore stack, if any. */
         if (state->stack) {
-            ofpbuf_put(&ctx.stack, state->stack->data, state->stack->size);
+            ofpbuf_put(&ctx.stack, state->stack,
+                       state->n_stack * sizeof *state->stack);
         }
 
         /* Restore mirror state. */
@@ -5175,28 +5199,19 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
         /* Restore action set, if any. */
         if (state->action_set_len) {
-            const struct ofpact *a;
-
             xlate_report_actions(&ctx, "- Restoring action set",
-                                 state->ofpacts, state->action_set_len);
+                                 state->action_set, state->action_set_len);
 
-            ofpbuf_put(&ctx.action_set, state->ofpacts, state->action_set_len);
-
-            OFPACT_FOR_EACH(a, state->ofpacts, state->action_set_len) {
-                if (a->type == OFPACT_GROUP) {
-                    ctx.action_set_has_group = true;
-                    break;
-                }
-            }
+            flow->actset_output = OFPP_UNSET;
+            xlate_write_actions__(&ctx, state->action_set,
+                                  state->action_set_len);
         }
 
         /* Restore recirculation actions.  If there are no actions, processing
          * will start with a lookup in the table set above. */
-        if (state->ofpacts_len > state->action_set_len) {
-            xin->ofpacts_len = state->ofpacts_len - state->action_set_len;
-            xin->ofpacts = state->ofpacts +
-                state->action_set_len / sizeof *state->ofpacts;
-
+        xin->ofpacts = state->ofpacts;
+        xin->ofpacts_len = state->ofpacts_len;
+        if (state->ofpacts_len) {
             xlate_report_actions(&ctx, "- Restoring actions",
                                  xin->ofpacts, xin->ofpacts_len);
         }
@@ -5231,7 +5246,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
             ctx.xin->resubmit_hook(ctx.xin, ctx.rule, 0);
         }
     }
-    xout->fail_open = ctx.rule && rule_dpif_is_fail_open(ctx.rule);
 
     /* Get the proximate input port of the packet.  (If xin->recirc,
      * flow->in_port is the ultimate input port of the packet.) */
@@ -5302,33 +5316,25 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
             }
 
             /* We've let OFPP_NORMAL and the learning action look at the
-             * packet, so drop it now if forwarding is disabled. */
+             * packet, so cancel all actions and recirculation if forwarding is
+             * disabled. */
             if (in_port && (!xport_stp_forward_state(in_port) ||
                             !xport_rstp_forward_state(in_port))) {
-                /* Drop all actions added by do_xlate_actions() above. */
                 ctx.odp_actions->size = sample_actions_len;
+                ctx_cancel_recirculation(&ctx);
+                ofpbuf_clear(&ctx.action_set);
+            }
 
-                /* Undo changes that may have been done for recirculation. */
-                if (exit_recirculates(&ctx)) {
-                    ctx.action_set.size = ctx.recirc_action_offset;
-                    ctx.recirc_action_offset = -1;
-                    ctx.last_unroll_offset = -1;
-                }
-            } else if (ctx.action_set.size) {
-                /* Translate action set only if not dropping the packet and
-                 * not recirculating. */
-                if (!exit_recirculates(&ctx)) {
-                    xlate_action_set(&ctx);
-                }
+            if (!ctx.recirculating) {
+                xlate_action_set(&ctx);
             }
-            /* Check if need to recirculate. */
-            if (exit_recirculates(&ctx)) {
+            if (ctx.recirculating) {
                 compose_recirculate_action(&ctx);
             }
         }
 
         /* Output only fully processed packets. */
-        if (!exit_recirculates(&ctx)
+        if (!ctx.recirculating
             && xbridge->has_in_band
             && in_band_must_output_to_local_port(flow)
             && !actions_output_to_local_port(&ctx)) {
@@ -5378,6 +5384,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 exit:
     ofpbuf_uninit(&ctx.stack);
     ofpbuf_uninit(&ctx.action_set);
+    ofpbuf_uninit(&ctx.recirculate_actions);
     ofpbuf_uninit(&scratch_actions);
 
     /* Make sure we return a "drop flow" in case of an error. */