meta-flow: Correctly set destination MAC in mf_set_flow_value().
[sliver-openvswitch.git] / ofproto / ofproto-dpif.c
index 020d4ac..f1f93a0 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011 Nicira Networks.
+ * Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -110,7 +110,7 @@ static void flow_push_stats(const struct rule_dpif *, const struct flow *,
                             uint64_t packets, uint64_t bytes,
                             long long int used);
 
-static uint32_t rule_calculate_tag(const struct flow *,
+static tag_type rule_calculate_tag(const struct flow *,
                                    const struct flow_wildcards *,
                                    uint32_t basis);
 static void rule_invalidate(const struct rule_dpif *);
@@ -176,11 +176,23 @@ static void bundle_destroy(struct ofbundle *);
 static void bundle_del_port(struct ofport_dpif *);
 static void bundle_run(struct ofbundle *);
 static void bundle_wait(struct ofbundle *);
-static struct ofport_dpif *lookup_input_bundle(struct ofproto_dpif *,
-                                               uint16_t in_port, bool warn);
+static struct ofbundle *lookup_input_bundle(struct ofproto_dpif *,
+                                            uint16_t in_port, bool warn);
+
+/* A controller may use OFPP_NONE as the ingress port to indicate that
+ * it did not arrive on a "real" port.  'ofpp_none_bundle' exists for
+ * when an input bundle is needed for validation (e.g., mirroring or
+ * OFPP_NORMAL processing).  It is not connected to an 'ofproto' or have
+ * any 'port' structs, so care must be taken when dealing with it. */
+static struct ofbundle ofpp_none_bundle = {
+    .name      = "OFPP_NONE",
+    .vlan_mode = PORT_VLAN_TRUNK
+};
 
 static void stp_run(struct ofproto_dpif *ofproto);
 static void stp_wait(struct ofproto_dpif *ofproto);
+static int set_stp_port(struct ofport *,
+                        const struct ofproto_port_stp_settings *);
 
 static bool ofbundle_includes_vlan(const struct ofbundle *, uint16_t vlan);
 
@@ -198,11 +210,20 @@ struct action_xlate_ctx {
      * revalidating without a packet to refer to. */
     const struct ofpbuf *packet;
 
-    /* Should OFPP_NORMAL MAC learning and NXAST_LEARN actions execute?  We
-     * want to execute them if we are actually processing a packet, or if we
-     * are accounting for packets that the datapath has processed, but not if
-     * we are just revalidating. */
-    bool may_learn;
+    /* Should OFPP_NORMAL update the MAC learning table?  We want to update it
+     * if we are actually processing a packet, or if we are accounting for
+     * packets that the datapath has processed, but not if we are just
+     * revalidating. */
+    bool may_learn_macs;
+
+    /* Should "learn" actions update the flow table?  We want to update it if
+     * we are actually processing a packet, or in most cases if we are
+     * accounting for packets that the datapath has processed, but not if we
+     * are just revalidating.  */
+    bool may_flow_mod;
+
+    /* Cookie of the currently matching rule, or 0. */
+    ovs_be64 cookie;
 
     /* If nonnull, called just before executing a resubmit action.
      *
@@ -227,7 +248,7 @@ struct action_xlate_ctx {
 
     int recurse;                /* Recursion level, via xlate_table_action. */
     struct flow base_flow;      /* Flow at the last commit. */
-    uint32_t original_priority; /* Priority when packet arrived. */
+    uint32_t orig_skb_priority; /* Priority when packet arrived. */
     uint8_t table_id;           /* OpenFlow table ID where flow was found. */
     uint32_t sflow_n_outputs;   /* Number of output ports. */
     uint16_t sflow_odp_port;    /* Output port for composing sFlow action. */
@@ -237,7 +258,8 @@ struct action_xlate_ctx {
 
 static void action_xlate_ctx_init(struct action_xlate_ctx *,
                                   struct ofproto_dpif *, const struct flow *,
-                                  ovs_be16 initial_tci, const struct ofpbuf *);
+                                  ovs_be16 initial_tci, ovs_be64 cookie,
+                                  const struct ofpbuf *);
 static struct ofpbuf *xlate_actions(struct action_xlate_ctx *,
                                     const union ofp_action *in, size_t n_in);
 
@@ -314,19 +336,14 @@ static struct facet *facet_lookup_valid(struct ofproto_dpif *,
                                         const struct flow *);
 static bool facet_revalidate(struct ofproto_dpif *, struct facet *);
 
-static bool execute_controller_action(struct ofproto_dpif *,
-                                      const struct flow *,
-                                      const struct nlattr *odp_actions,
-                                      size_t actions_len,
-                                      struct ofpbuf *packet);
-
 static void facet_flush_stats(struct ofproto_dpif *, struct facet *);
 
 static void facet_update_time(struct ofproto_dpif *, struct facet *,
                               long long int used);
 static void facet_reset_counters(struct facet *);
 static void facet_push_stats(struct facet *);
-static void facet_account(struct ofproto_dpif *, struct facet *);
+static void facet_account(struct ofproto_dpif *, struct facet *,
+                          bool may_flow_mod);
 
 static bool facet_is_controller_flow(struct facet *);
 
@@ -643,7 +660,7 @@ construct(struct ofproto *ofproto_, int *n_tablesp)
     ofproto->sflow = NULL;
     ofproto->stp = NULL;
     hmap_init(&ofproto->bundles);
-    ofproto->ml = mac_learning_create();
+    ofproto->ml = mac_learning_create(MAC_ENTRY_DEFAULT_IDLE_TIME);
     for (i = 0; i < MAX_MIRRORS; i++) {
         ofproto->mirrors[i] = NULL;
     }
@@ -1128,6 +1145,12 @@ set_stp(struct ofproto *ofproto_, const struct ofproto_stp_settings *s)
         stp_set_max_age(ofproto->stp, s->max_age);
         stp_set_forward_delay(ofproto->stp, s->fwd_delay);
     }  else {
+        struct ofport *ofport;
+
+        HMAP_FOR_EACH (ofport, hmap_node, &ofproto->up.ports) {
+            set_stp_port(ofport, NULL);
+        }
+
         stp_destroy(ofproto->stp);
         ofproto->stp = NULL;
     }
@@ -1816,11 +1839,15 @@ bundle_send_learning_packets(struct ofbundle *bundle)
         if (e->port.p != bundle) {
             struct ofpbuf *learning_packet;
             struct ofport_dpif *port;
+            void *port_void;
             int ret;
 
-            learning_packet = bond_compose_learning_packet(bundle->bond, e->mac,
-                                                           e->vlan,
-                                                           (void **)&port);
+            /* The assignment to "port" is unnecessary but makes "grep"ing for
+             * struct ofport_dpif more effective. */
+            learning_packet = bond_compose_learning_packet(bundle->bond,
+                                                           e->mac, e->vlan,
+                                                           &port_void);
+            port = port_void;
             ret = send_packet(port, learning_packet);
             ofpbuf_delete(learning_packet);
             if (ret) {
@@ -2124,6 +2151,13 @@ forward_bpdu_changed(struct ofproto *ofproto_)
     /* Revalidate cached flows whenever forward_bpdu option changes. */
     ofproto->need_revalidate = true;
 }
+
+static void
+set_mac_idle_time(struct ofproto *ofproto_, unsigned int idle_time)
+{
+    struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
+    mac_learning_set_idle_time(ofproto->ml, idle_time);
+}
 \f
 /* Ports. */
 
@@ -2396,50 +2430,30 @@ struct flow_miss_op {
 
 /* Sends an OFPT_PACKET_IN message for 'packet' of type OFPR_NO_MATCH to each
  * OpenFlow controller as necessary according to their individual
- * configurations.
- *
- * If 'clone' is true, the caller retains ownership of 'packet'.  Otherwise,
- * ownership is transferred to this function. */
+ * configurations. */
 static void
 send_packet_in_miss(struct ofproto_dpif *ofproto, struct ofpbuf *packet,
-                    const struct flow *flow, bool clone)
+                    const struct flow *flow)
 {
     struct ofputil_packet_in pin;
 
-    pin.packet = packet;
-    pin.in_port = flow->in_port;
+    pin.packet = packet->data;
+    pin.packet_len = packet->size;
+    pin.total_len = packet->size;
     pin.reason = OFPR_NO_MATCH;
+
+    pin.table_id = 0;
+    pin.cookie = 0;
+
     pin.buffer_id = 0;          /* not yet known */
     pin.send_len = 0;           /* not used for flow table misses */
-    connmgr_send_packet_in(ofproto->up.connmgr, &pin, flow,
-                           clone ? NULL : packet);
-}
 
-/* Sends an OFPT_PACKET_IN message for 'packet' of type OFPR_ACTION to each
- * OpenFlow controller as necessary according to their individual
- * configurations.
- *
- * 'send_len' should be the number of bytes of 'packet' to send to the
- * controller, as specified in the action that caused the packet to be sent.
- *
- * If 'clone' is true, the caller retains ownership of 'upcall->packet'.
- * Otherwise, ownership is transferred to this function. */
-static void
-send_packet_in_action(struct ofproto_dpif *ofproto, struct ofpbuf *packet,
-                      uint64_t userdata, const struct flow *flow, bool clone)
-{
-    struct ofputil_packet_in pin;
-    struct user_action_cookie cookie;
+    flow_get_metadata(flow, &pin.fmd);
 
-    memcpy(&cookie, &userdata, sizeof(cookie));
+    /* Registers aren't meaningful on a miss. */
+    memset(pin.fmd.reg_masks, 0, sizeof pin.fmd.reg_masks);
 
-    pin.packet = packet;
-    pin.in_port = flow->in_port;
-    pin.reason = OFPR_ACTION;
-    pin.buffer_id = 0;          /* not yet known */
-    pin.send_len = cookie.data;
-    connmgr_send_packet_in(ofproto->up.connmgr, &pin, flow,
-                           clone ? NULL : packet);
+    connmgr_send_packet_in(ofproto->up.connmgr, &pin, flow);
 }
 
 static bool
@@ -2526,10 +2540,8 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
                              flow->in_port);
             }
 
-            LIST_FOR_EACH_SAFE (packet, next_packet, list_node,
-                                &miss->packets) {
-                list_remove(&packet->list_node);
-                send_packet_in_miss(ofproto, packet, flow, false);
+            LIST_FOR_EACH (packet, list_node, &miss->packets) {
+                send_packet_in_miss(ofproto, packet, flow);
             }
 
             return;
@@ -2543,7 +2555,10 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
                                miss->initial_tci);
 
     LIST_FOR_EACH_SAFE (packet, next_packet, list_node, &miss->packets) {
-        list_remove(&packet->list_node);
+        struct dpif_flow_stats stats;
+        struct flow_miss_op *op;
+        struct dpif_execute *execute;
+
         ofproto->n_matches++;
 
         if (facet->rule->up.cr.priority == FAIL_OPEN_PRIORITY) {
@@ -2557,29 +2572,37 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
              *
              * See the top-level comment in fail-open.c for more information.
              */
-            send_packet_in_miss(ofproto, packet, flow, true);
+            send_packet_in_miss(ofproto, packet, flow);
         }
 
         if (!facet->may_install || !subfacet->actions) {
             subfacet_make_actions(ofproto, subfacet, packet);
         }
-        if (!execute_controller_action(ofproto, &facet->flow,
-                                       subfacet->actions,
-                                       subfacet->actions_len, packet)) {
-            struct flow_miss_op *op = &ops[(*n_ops)++];
-            struct dpif_execute *execute = &op->dpif_op.execute;
 
-            op->subfacet = subfacet;
-            execute->type = DPIF_OP_EXECUTE;
-            execute->key = miss->key;
-            execute->key_len = miss->key_len;
-            execute->actions
-                = (facet->may_install
-                   ? subfacet->actions
-                   : xmemdup(subfacet->actions, subfacet->actions_len));
-            execute->actions_len = subfacet->actions_len;
-            execute->packet = packet;
+        dpif_flow_stats_extract(&facet->flow, packet, &stats);
+        subfacet_update_stats(ofproto, subfacet, &stats);
+
+        if (flow->vlan_tci != subfacet->initial_tci) {
+            /* This packet was received on a VLAN splinter port.  We added
+             * a VLAN to the packet to make the packet resemble the flow,
+             * but the actions were composed assuming that the packet
+             * contained no VLAN.  So, we must remove the VLAN header from
+             * the packet before trying to execute the actions. */
+            eth_pop_vlan(packet);
         }
+
+        op = &ops[(*n_ops)++];
+        execute = &op->dpif_op.execute;
+        op->subfacet = subfacet;
+        execute->type = DPIF_OP_EXECUTE;
+        execute->key = miss->key;
+        execute->key_len = miss->key_len;
+        execute->actions = (facet->may_install
+                            ? subfacet->actions
+                            : xmemdup(subfacet->actions,
+                                      subfacet->actions_len));
+        execute->actions_len = subfacet->actions_len;
+        execute->packet = packet;
     }
 
     if (facet->may_install && subfacet->key_fitness != ODP_FIT_TOO_LITTLE) {
@@ -2597,10 +2620,27 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
     }
 }
 
+/* Like odp_flow_key_to_flow(), this function converts the 'key_len' bytes of
+ * OVS_KEY_ATTR_* attributes in 'key' to a flow structure in 'flow' and returns
+ * an ODP_FIT_* value that indicates how well 'key' fits our expectations for
+ * what a flow key should contain.
+ *
+ * This function also includes some logic to help make VLAN splinters
+ * transparent to the rest of the upcall processing logic.  In particular, if
+ * the extracted in_port is a VLAN splinter port, it replaces flow->in_port by
+ * the "real" port, sets flow->vlan_tci correctly for the VLAN of the VLAN
+ * splinter port, and pushes a VLAN header onto 'packet' (if it is nonnull).
+ *
+ * Sets '*initial_tci' to the VLAN TCI with which the packet was really
+ * received, that is, the actual VLAN TCI extracted by odp_flow_key_to_flow().
+ * (This differs from the value returned in flow->vlan_tci only for packets
+ * received on VLAN splinters.)
+ */
 static enum odp_key_fitness
 ofproto_dpif_extract_flow_key(const struct ofproto_dpif *ofproto,
                               const struct nlattr *key, size_t key_len,
-                              struct flow *flow, ovs_be16 *initial_tci)
+                              struct flow *flow, ovs_be16 *initial_tci,
+                              struct ofpbuf *packet)
 {
     enum odp_key_fitness fitness;
     uint16_t realdev;
@@ -2618,6 +2658,23 @@ ofproto_dpif_extract_flow_key(const struct ofproto_dpif *ofproto,
          * with the VLAN device's VLAN ID. */
         flow->in_port = realdev;
         flow->vlan_tci = htons((vid & VLAN_VID_MASK) | VLAN_CFI);
+        if (packet) {
+            /* Make the packet resemble the flow, so that it gets sent to 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'.
+             *
+             * The allocated space inside 'packet' probably also contains
+             * 'key', that is, both 'packet' and 'key' are probably part of a
+             * struct dpif_upcall (see the large comment on that structure
+             * definition), so pushing data on 'packet' is in general not a
+             * good idea since it could overwrite 'key' or free it as a side
+             * effect.  However, it's OK in this special case because we know
+             * that 'packet' is inside a Netlink attribute: pushing 4 bytes
+             * will just overwrite the 4-byte "struct nlattr", which is fine
+             * since we don't need that header anymore. */
+            eth_push_vlan(packet, flow->vlan_tci);
+        }
 
         /* Let the caller know that we can't reproduce 'key' from 'flow'. */
         if (fitness == ODP_FIT_PERFECT) {
@@ -2660,12 +2717,13 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall *upcalls,
          * then set 'flow''s header pointers. */
         fitness = ofproto_dpif_extract_flow_key(ofproto,
                                                 upcall->key, upcall->key_len,
-                                                &flow, &initial_tci);
+                                                &flow, &initial_tci,
+                                                upcall->packet);
         if (fitness == ODP_FIT_ERROR) {
             ofpbuf_delete(upcall->packet);
             continue;
         }
-        flow_extract(upcall->packet, flow.priority, flow.tun_id,
+        flow_extract(upcall->packet, flow.skb_priority, flow.tun_id,
                      flow.in_port, &flow);
 
         /* Handle 802.1ag, LACP, and STP specially. */
@@ -2686,14 +2744,10 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall *upcalls,
     /* Process each element in the to-do list, constructing the set of
      * operations to batch. */
     n_ops = 0;
-    HMAP_FOR_EACH_SAFE (miss, next_miss, hmap_node, &todo) {
+    HMAP_FOR_EACH (miss, hmap_node, &todo) {
         handle_flow_miss(ofproto, miss, flow_miss_ops, &n_ops);
-        ofpbuf_list_delete(&miss->packets);
-        hmap_remove(&todo, &miss->hmap_node);
-        free(miss);
     }
     assert(n_ops <= ARRAY_SIZE(flow_miss_ops));
-    hmap_destroy(&todo);
 
     /* Execute batch. */
     for (i = 0; i < n_ops; i++) {
@@ -2713,7 +2767,6 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall *upcalls,
             if (op->subfacet->actions != execute->actions) {
                 free((struct nlattr *) execute->actions);
             }
-            ofpbuf_delete((struct ofpbuf *) execute->packet);
             break;
 
         case DPIF_OP_FLOW_PUT:
@@ -2724,6 +2777,12 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall *upcalls,
             break;
         }
     }
+    HMAP_FOR_EACH_SAFE (miss, next_miss, hmap_node, &todo) {
+        ofpbuf_list_delete(&miss->packets);
+        hmap_remove(&todo, &miss->hmap_node);
+        free(miss);
+    }
+    hmap_destroy(&todo);
 }
 
 static void
@@ -2739,7 +2798,7 @@ handle_userspace_upcall(struct ofproto_dpif *ofproto,
 
     fitness = ofproto_dpif_extract_flow_key(ofproto, upcall->key,
                                             upcall->key_len, &flow,
-                                            &initial_tci);
+                                            &initial_tci, upcall->packet);
     if (fitness == ODP_FIT_ERROR) {
         ofpbuf_delete(upcall->packet);
         return;
@@ -2750,15 +2809,10 @@ handle_userspace_upcall(struct ofproto_dpif *ofproto,
             dpif_sflow_received(ofproto->sflow, upcall->packet, &flow,
                                 &cookie);
         }
-        ofpbuf_delete(upcall->packet);
-    } else if (cookie.type == USER_ACTION_COOKIE_CONTROLLER) {
-        COVERAGE_INC(ofproto_dpif_ctlr_action);
-        send_packet_in_action(ofproto, upcall->packet, upcall->userdata,
-                              &flow, false);
     } else {
         VLOG_WARN_RL(&rl, "invalid user cookie : 0x%"PRIx64, upcall->userdata);
-        ofpbuf_delete(upcall->packet);
     }
+    ofpbuf_delete(upcall->packet);
 }
 
 static int
@@ -2899,7 +2953,7 @@ update_stats(struct ofproto_dpif *p)
             subfacet->dp_byte_count = stats->n_bytes;
 
             subfacet_update_time(p, subfacet, stats->used);
-            facet_account(p, facet);
+            facet_account(p, facet, true);
             facet_push_stats(facet);
         } else {
             if (!VLOG_DROP_WARN(&rl)) {
@@ -3091,33 +3145,6 @@ facet_free(struct facet *facet)
     free(facet);
 }
 
-static bool
-execute_controller_action(struct ofproto_dpif *ofproto,
-                          const struct flow *flow,
-                          const struct nlattr *odp_actions, size_t actions_len,
-                          struct ofpbuf *packet)
-{
-    if (actions_len
-        && odp_actions->nla_type == OVS_ACTION_ATTR_USERSPACE
-        && NLA_ALIGN(odp_actions->nla_len) == actions_len) {
-        /* As an optimization, avoid a round-trip from userspace to kernel to
-         * userspace.  This also avoids possibly filling up kernel packet
-         * buffers along the way.
-         *
-         * This optimization will not accidentally catch sFlow
-         * OVS_ACTION_ATTR_USERSPACE actions, since those are encapsulated
-         * inside OVS_ACTION_ATTR_SAMPLE. */
-        const struct nlattr *nla;
-
-        nla = nl_attr_find_nested(odp_actions, OVS_USERSPACE_ATTR_USERDATA);
-        send_packet_in_action(ofproto, packet, nl_attr_get_u64(nla), flow,
-                              false);
-        return true;
-    } else {
-        return false;
-    }
-}
-
 /* Executes, within 'ofproto', the 'n_actions' actions in 'actions' on
  * 'packet', which arrived on 'in_port'.
  *
@@ -3131,11 +3158,6 @@ execute_odp_actions(struct ofproto_dpif *ofproto, const struct flow *flow,
     struct ofpbuf key;
     int error;
 
-    if (execute_controller_action(ofproto, flow, odp_actions, actions_len,
-                                  packet)) {
-        return true;
-    }
-
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
     odp_flow_key_from_flow(&key, flow);
 
@@ -3158,19 +3180,33 @@ facet_remove(struct ofproto_dpif *ofproto, struct facet *facet)
 {
     struct subfacet *subfacet, *next_subfacet;
 
+    assert(!list_is_empty(&facet->subfacets));
+
+    /* First uninstall all of the subfacets to get final statistics. */
+    LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) {
+        subfacet_uninstall(ofproto, subfacet);
+    }
+
+    /* Flush the final stats to the rule.
+     *
+     * This might require us to have at least one subfacet around so that we
+     * can use its actions for accounting in facet_account(), which is why we
+     * have uninstalled but not yet destroyed the subfacets. */
+    facet_flush_stats(ofproto, facet);
+
+    /* Now we're really all done so destroy everything. */
     LIST_FOR_EACH_SAFE (subfacet, next_subfacet, list_node,
                         &facet->subfacets) {
         subfacet_destroy__(ofproto, subfacet);
     }
-
-    facet_flush_stats(ofproto, facet);
     hmap_remove(&ofproto->facets, &facet->hmap_node);
     list_remove(&facet->list_node);
     facet_free(facet);
 }
 
 static void
-facet_account(struct ofproto_dpif *ofproto, struct facet *facet)
+facet_account(struct ofproto_dpif *ofproto, struct facet *facet,
+              bool may_flow_mod)
 {
     uint64_t n_bytes;
     struct subfacet *subfacet;
@@ -3191,8 +3227,10 @@ facet_account(struct ofproto_dpif *ofproto, struct facet *facet)
         struct action_xlate_ctx ctx;
 
         action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
-                              facet->flow.vlan_tci, NULL);
-        ctx.may_learn = true;
+                              facet->flow.vlan_tci,
+                              facet->rule->up.flow_cookie, NULL);
+        ctx.may_learn_macs = true;
+        ctx.may_flow_mod = may_flow_mod;
         ofpbuf_delete(xlate_actions(&ctx, facet->rule->up.actions,
                                     facet->rule->up.n_actions));
     }
@@ -3265,7 +3303,7 @@ facet_flush_stats(struct ofproto_dpif *ofproto, struct facet *facet)
     }
 
     facet_push_stats(facet);
-    facet_account(ofproto, facet);
+    facet_account(ofproto, facet, false);
 
     if (ofproto->netflow && !facet_is_controller_flow(facet)) {
         struct ofexpired expired;
@@ -3380,7 +3418,8 @@ facet_revalidate(struct ofproto_dpif *ofproto, struct facet *facet)
         bool should_install;
 
         action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
-                              subfacet->initial_tci, NULL);
+                              subfacet->initial_tci, new_rule->up.flow_cookie,
+                              NULL);
         odp_actions = xlate_actions(&ctx, new_rule->up.actions,
                                     new_rule->up.n_actions);
         actions_changed = (subfacet->actions_len != odp_actions->size
@@ -3530,7 +3569,8 @@ flow_push_stats(const struct rule_dpif *rule,
     push.bytes = bytes;
     push.used = used;
 
-    action_xlate_ctx_init(&push.ctx, ofproto, flow, flow->vlan_tci, NULL);
+    action_xlate_ctx_init(&push.ctx, ofproto, flow, flow->vlan_tci,
+                          rule->up.flow_cookie, NULL);
     push.ctx.resubmit_hook = push_resubmit;
     ofpbuf_delete(xlate_actions(&push.ctx,
                                 rule->up.actions, rule->up.n_actions));
@@ -3638,9 +3678,11 @@ subfacet_destroy(struct ofproto_dpif *ofproto, struct subfacet *subfacet)
 {
     struct facet *facet = subfacet->facet;
 
-    subfacet_destroy__(ofproto, subfacet);
-    if (list_is_empty(&facet->subfacets)) {
+    if (list_is_singleton(&facet->subfacets)) {
+        /* facet_remove() needs at least one subfacet (it will remove it). */
         facet_remove(ofproto, facet);
+    } else {
+        subfacet_destroy__(ofproto, subfacet);
     }
 }
 
@@ -3670,7 +3712,7 @@ subfacet_make_actions(struct ofproto_dpif *p, struct subfacet *subfacet,
     struct action_xlate_ctx ctx;
 
     action_xlate_ctx_init(&ctx, p, &facet->flow, subfacet->initial_tci,
-                          packet);
+                          rule->up.flow_cookie, packet);
     odp_actions = xlate_actions(&ctx, rule->up.actions, rule->up.n_actions);
     facet->tags = ctx.tags;
     facet->may_install = ctx.may_set_up_flow;
@@ -3948,7 +3990,8 @@ rule_execute(struct rule *rule_, const struct flow *flow,
     struct ofpbuf *odp_actions;
     size_t size;
 
-    action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, packet);
+    action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci,
+                          rule->up.flow_cookie, packet);
     odp_actions = xlate_actions(&ctx, rule->up.actions, rule->up.n_actions);
     size = packet->size;
     if (execute_odp_actions(ofproto, flow, odp_actions->data,
@@ -4147,7 +4190,7 @@ compose_output_action__(struct action_xlate_ctx *ctx, uint16_t ofp_port,
             return;
         }
 
-        pdscp = get_priority(ofport, ctx->flow.priority);
+        pdscp = get_priority(ofport, ctx->flow.skb_priority);
         if (pdscp) {
             ctx->flow.nw_tos &= ~IP_DSCP_MASK;
             ctx->flow.nw_tos |= pdscp->dscp;
@@ -4201,7 +4244,7 @@ xlate_table_action(struct action_xlate_ctx *ctx,
         if (table_id > 0 && table_id < N_TABLES) {
             struct table_dpif *table = &ofproto->tables[table_id];
             if (table->other_table) {
-                ctx->tags |= (rule
+                ctx->tags |= (rule && rule->tag
                               ? rule->tag
                               : rule_calculate_tag(&ctx->flow,
                                                    &table->other_table->wc,
@@ -4218,8 +4261,12 @@ xlate_table_action(struct action_xlate_ctx *ctx,
         }
 
         if (rule) {
+            ovs_be64 old_cookie = ctx->cookie;
+
             ctx->recurse++;
+            ctx->cookie = rule->up.flow_cookie;
             do_xlate_actions(rule->up.actions, rule->up.n_actions, ctx);
+            ctx->cookie = old_cookie;
             ctx->recurse--;
         }
 
@@ -4270,16 +4317,62 @@ flood_packets(struct action_xlate_ctx *ctx, bool all)
 }
 
 static void
-compose_controller_action(struct action_xlate_ctx *ctx, int len)
+execute_controller_action(struct action_xlate_ctx *ctx, int len)
 {
-    struct user_action_cookie cookie;
+    struct ofputil_packet_in pin;
+    struct ofpbuf *packet;
 
-    commit_odp_actions(&ctx->flow, &ctx->base_flow, ctx->odp_actions);
-    cookie.type = USER_ACTION_COOKIE_CONTROLLER;
-    cookie.data = len;
-    cookie.n_output = 0;
-    cookie.vlan_tci = 0;
-    put_userspace_action(ctx->ofproto, ctx->odp_actions, &ctx->flow, &cookie);
+    ctx->may_set_up_flow = false;
+    if (!ctx->packet) {
+        return;
+    }
+
+    packet = ofpbuf_clone(ctx->packet);
+
+    if (packet->l2 && packet->l3) {
+        struct eth_header *eh;
+
+        eth_pop_vlan(packet);
+        eh = packet->l2;
+        assert(eh->eth_type == ctx->flow.dl_type);
+        memcpy(eh->eth_src, ctx->flow.dl_src, sizeof eh->eth_src);
+        memcpy(eh->eth_dst, ctx->flow.dl_dst, sizeof eh->eth_dst);
+
+        if (ctx->flow.vlan_tci & htons(VLAN_CFI)) {
+            eth_push_vlan(packet, ctx->flow.vlan_tci);
+        }
+
+        if (packet->l4) {
+            if (ctx->flow.dl_type == htons(ETH_TYPE_IP)) {
+                packet_set_ipv4(packet, ctx->flow.nw_src, ctx->flow.nw_dst,
+                                ctx->flow.nw_tos, ctx->flow.nw_ttl);
+            }
+
+            if (packet->l7) {
+                if (ctx->flow.nw_proto == IPPROTO_TCP) {
+                    packet_set_tcp_port(packet, ctx->flow.tp_src,
+                                        ctx->flow.tp_dst);
+                } else if (ctx->flow.nw_proto == IPPROTO_UDP) {
+                    packet_set_udp_port(packet, ctx->flow.tp_src,
+                                        ctx->flow.tp_dst);
+                }
+            }
+        }
+    }
+
+    pin.packet = packet->data;
+    pin.packet_len = packet->size;
+    pin.reason = OFPR_ACTION;
+    pin.table_id = ctx->table_id;
+    pin.cookie = ctx->cookie;
+
+    pin.buffer_id = 0;
+    pin.send_len = len;
+    pin.total_len = packet->size;
+    flow_get_metadata(&ctx->flow, &pin.fmd);
+
+    connmgr_send_packet_in(ctx->ofproto->up.connmgr, &pin, &ctx->flow);
+    ofpbuf_delete(packet);
 }
 
 static void
@@ -4307,13 +4400,11 @@ xlate_output_action__(struct action_xlate_ctx *ctx,
         flood_packets(ctx, true);
         break;
     case OFPP_CONTROLLER:
-        compose_controller_action(ctx, max_len);
-        break;
-    case OFPP_LOCAL:
-        compose_output_action(ctx, OFPP_LOCAL);
+        execute_controller_action(ctx, max_len);
         break;
     case OFPP_NONE:
         break;
+    case OFPP_LOCAL:
     default:
         if (port != ctx->flow.in_port) {
             compose_output_action(ctx, port);
@@ -4376,10 +4467,10 @@ xlate_enqueue_action(struct action_xlate_ctx *ctx,
     }
 
     /* Add datapath actions. */
-    flow_priority = ctx->flow.priority;
-    ctx->flow.priority = priority;
+    flow_priority = ctx->flow.skb_priority;
+    ctx->flow.skb_priority = priority;
     compose_output_action(ctx, ofp_port);
-    ctx->flow.priority = flow_priority;
+    ctx->flow.skb_priority = flow_priority;
 
     /* Update NetFlow output port. */
     if (ctx->nf_output_iface == NF_OUT_DROP) {
@@ -4404,7 +4495,7 @@ xlate_set_queue_action(struct action_xlate_ctx *ctx,
         return;
     }
 
-    ctx->flow.priority = priority;
+    ctx->flow.skb_priority = priority;
 }
 
 struct xlate_reg_state {
@@ -4565,8 +4656,11 @@ do_xlate_actions(const union ofp_action *in, size_t n_in,
             break;
 
         case OFPUTIL_OFPAT_SET_NW_TOS:
-            ctx->flow.nw_tos &= ~IP_DSCP_MASK;
-            ctx->flow.nw_tos |= ia->nw_tos.nw_tos & IP_DSCP_MASK;
+            /* OpenFlow 1.0 only supports IPv4. */
+            if (ctx->flow.dl_type == htons(ETH_TYPE_IP)) {
+                ctx->flow.nw_tos &= ~IP_DSCP_MASK;
+                ctx->flow.nw_tos |= ia->nw_tos.nw_tos & IP_DSCP_MASK;
+            }
             break;
 
         case OFPUTIL_OFPAT_SET_TP_SRC:
@@ -4602,7 +4696,7 @@ do_xlate_actions(const union ofp_action *in, size_t n_in,
             break;
 
         case OFPUTIL_NXAST_POP_QUEUE:
-            ctx->flow.priority = ctx->original_priority;
+            ctx->flow.skb_priority = ctx->orig_skb_priority;
             break;
 
         case OFPUTIL_NXAST_REG_MOVE:
@@ -4656,7 +4750,7 @@ do_xlate_actions(const union ofp_action *in, size_t n_in,
 
         case OFPUTIL_NXAST_LEARN:
             ctx->has_learn = true;
-            if (ctx->may_learn) {
+            if (ctx->may_flow_mod) {
                 xlate_learn_action(ctx, (const struct nx_action_learn *) ia);
             }
             break;
@@ -4678,15 +4772,18 @@ do_xlate_actions(const union ofp_action *in, size_t n_in,
 static void
 action_xlate_ctx_init(struct action_xlate_ctx *ctx,
                       struct ofproto_dpif *ofproto, const struct flow *flow,
-                      ovs_be16 initial_tci, const struct ofpbuf *packet)
+                      ovs_be16 initial_tci, ovs_be64 cookie,
+                      const struct ofpbuf *packet)
 {
     ctx->ofproto = ofproto;
     ctx->flow = *flow;
     ctx->base_flow = ctx->flow;
     ctx->base_flow.tun_id = 0;
     ctx->base_flow.vlan_tci = initial_tci;
+    ctx->cookie = cookie;
     ctx->packet = packet;
-    ctx->may_learn = packet != NULL;
+    ctx->may_learn_macs = packet != NULL;
+    ctx->may_flow_mod = packet != NULL;
     ctx->resubmit_hook = NULL;
 }
 
@@ -4707,7 +4804,7 @@ xlate_actions(struct action_xlate_ctx *ctx,
     ctx->nf_output_iface = NF_OUT_DROP;
     ctx->mirrors = 0;
     ctx->recurse = 0;
-    ctx->original_priority = ctx->flow.priority;
+    ctx->orig_skb_priority = ctx->flow.skb_priority;
     ctx->table_id = 0;
     ctx->exit = false;
 
@@ -4795,6 +4892,11 @@ input_vid_to_vlan(const struct ofbundle *in_bundle, uint16_t vid)
 static bool
 input_vid_is_valid(uint16_t vid, struct ofbundle *in_bundle, bool warn)
 {
+    /* Allow any VID on the OFPP_NONE port. */
+    if (in_bundle == &ofpp_none_bundle) {
+        return true;
+    }
+
     switch (in_bundle->vlan_mode) {
     case PORT_VLAN_ACCESS:
         if (vid) {
@@ -4977,22 +5079,17 @@ add_mirror_actions(struct action_xlate_ctx *ctx, const struct flow *orig_flow)
 {
     struct ofproto_dpif *ofproto = ctx->ofproto;
     mirror_mask_t mirrors;
-    struct ofport_dpif *in_port;
     struct ofbundle *in_bundle;
     uint16_t vlan;
     uint16_t vid;
     const struct nlattr *a;
     size_t left;
 
-    /* Obtain in_port from orig_flow.in_port.
-     *
-     * lookup_input_bundle() also ensures that in_port belongs to a bundle. */
-    in_port = lookup_input_bundle(ctx->ofproto, orig_flow->in_port,
-                                  ctx->packet != NULL);
-    if (!in_port) {
+    in_bundle = lookup_input_bundle(ctx->ofproto, orig_flow->in_port,
+                                    ctx->packet != NULL);
+    if (!in_bundle) {
         return;
     }
-    in_bundle = in_port->bundle;
     mirrors = in_bundle->src_mirrors;
 
     /* Drop frames on bundles reserved for mirroring. */
@@ -5113,6 +5210,11 @@ update_learning_table(struct ofproto_dpif *ofproto,
 {
     struct mac_entry *mac;
 
+    /* Don't learn the OFPP_NONE port. */
+    if (in_bundle == &ofpp_none_bundle) {
+        return;
+    }
+
     if (!mac_learning_may_learn(ofproto->ml, flow->dl_src, vlan)) {
         return;
     }
@@ -5143,15 +5245,21 @@ update_learning_table(struct ofproto_dpif *ofproto,
     }
 }
 
-static struct ofport_dpif *
+static struct ofbundle *
 lookup_input_bundle(struct ofproto_dpif *ofproto, uint16_t in_port, bool warn)
 {
     struct ofport_dpif *ofport;
 
+    /* Special-case OFPP_NONE, which a controller may use as the ingress
+     * port for traffic that it is sourcing. */
+    if (in_port == OFPP_NONE) {
+        return &ofpp_none_bundle;
+    }
+
     /* Find the port and bundle for the received packet. */
     ofport = get_ofp_port(ofproto, in_port);
     if (ofport && ofport->bundle) {
-        return ofport;
+        return ofport->bundle;
     }
 
     /* Odd.  A few possible reasons here:
@@ -5235,15 +5343,15 @@ xlate_normal(struct action_xlate_ctx *ctx)
 
     ctx->has_normal = true;
 
-    /* Obtain in_port from ctx->flow.in_port.
-     *
-     * lookup_input_bundle() also ensures that in_port belongs to a bundle. */
-    in_port = lookup_input_bundle(ctx->ofproto, ctx->flow.in_port,
+    in_bundle = lookup_input_bundle(ctx->ofproto, ctx->flow.in_port,
                                   ctx->packet != NULL);
-    if (!in_port) {
+    if (!in_bundle) {
         return;
     }
-    in_bundle = in_port->bundle;
+
+    /* We know 'in_port' exists unless it is "ofpp_none_bundle",
+     * since lookup_input_bundle() succeeded. */
+    in_port = get_ofp_port(ctx->ofproto, ctx->flow.in_port);
 
     /* Drop malformed frames. */
     if (ctx->flow.dl_type == htons(ETH_TYPE_VLAN) &&
@@ -5276,12 +5384,13 @@ xlate_normal(struct action_xlate_ctx *ctx)
     vlan = input_vid_to_vlan(in_bundle, vid);
 
     /* Check other admissibility requirements. */
-    if (!is_admissible(ctx->ofproto, &ctx->flow, in_port, vlan, &ctx->tags)) {
+    if (in_port &&
+         !is_admissible(ctx->ofproto, &ctx->flow, in_port, vlan, &ctx->tags)) {
         return;
     }
 
     /* Learn source MAC. */
-    if (ctx->may_learn) {
+    if (ctx->may_learn_macs) {
         update_learning_table(ctx->ofproto, &ctx->flow, vlan, in_bundle);
     }
 
@@ -5292,14 +5401,6 @@ xlate_normal(struct action_xlate_ctx *ctx)
         if (mac->port.p != in_bundle) {
             output_normal(ctx, mac->port.p, vlan);
         }
-    } else if (!ctx->packet && !eth_addr_is_multicast(ctx->flow.dl_dst)) {
-        /* If we are revalidating but don't have a learning entry then eject
-         * the flow.  Installing a flow that floods packets opens up a window
-         * of time where we could learn from a packet reflected on a bond and
-         * blackhole packets before the learning table is updated to reflect
-         * the correct port. */
-        ctx->may_set_up_flow = false;
-        return;
     } else {
         struct ofbundle *bundle;
 
@@ -5338,7 +5439,7 @@ xlate_normal(struct action_xlate_ctx *ctx)
 
 /* Calculates the tag to use for 'flow' and wildcards 'wc' when it is inserted
  * into an OpenFlow table with the given 'basis'. */
-static uint32_t
+static tag_type
 rule_calculate_tag(const struct flow *flow, const struct flow_wildcards *wc,
                    uint32_t secret)
 {
@@ -5459,15 +5560,24 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet,
                              ofproto->max_ports);
     if (!error) {
         struct odputil_keybuf keybuf;
-        struct action_xlate_ctx ctx;
         struct ofpbuf *odp_actions;
+        struct ofproto_push push;
         struct ofpbuf key;
 
         ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
         odp_flow_key_from_flow(&key, flow);
 
-        action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, packet);
-        odp_actions = xlate_actions(&ctx, ofp_actions, n_ofp_actions);
+        action_xlate_ctx_init(&push.ctx, ofproto, flow, flow->vlan_tci, 0,
+                              packet);
+
+        /* Ensure that resubmits in 'ofp_actions' get accounted to their
+         * matching rules. */
+        push.packets = 1;
+        push.bytes = packet->size;
+        push.used = time_msec();
+        push.ctx.resubmit_hook = push_resubmit;
+
+        odp_actions = xlate_actions(&push.ctx, ofp_actions, n_ofp_actions);
         dpif_execute(ofproto->dpif, key.data, key.size,
                      odp_actions->data, odp_actions->size, packet);
         ofpbuf_delete(odp_actions);
@@ -5555,30 +5665,38 @@ ofproto_dpif_lookup(const char *name)
 }
 
 static void
-ofproto_unixctl_fdb_flush(struct unixctl_conn *conn,
-                         const char *args, void *aux OVS_UNUSED)
+ofproto_unixctl_fdb_flush(struct unixctl_conn *conn, int argc,
+                          const char *argv[], void *aux OVS_UNUSED)
 {
-    const struct ofproto_dpif *ofproto;
+    struct ofproto_dpif *ofproto;
 
-    ofproto = ofproto_dpif_lookup(args);
-    if (!ofproto) {
-        unixctl_command_reply(conn, 501, "no such bridge");
-        return;
+    if (argc > 1) {
+        ofproto = ofproto_dpif_lookup(argv[1]);
+        if (!ofproto) {
+            unixctl_command_reply(conn, 501, "no such bridge");
+            return;
+        }
+        mac_learning_flush(ofproto->ml);
+        ofproto->need_revalidate = true;
+    } else {
+        HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+            mac_learning_flush(ofproto->ml);
+            ofproto->need_revalidate = true;
+        }
     }
-    mac_learning_flush(ofproto->ml);
 
     unixctl_command_reply(conn, 200, "table successfully flushed");
 }
 
 static void
-ofproto_unixctl_fdb_show(struct unixctl_conn *conn,
-                         const char *args, void *aux OVS_UNUSED)
+ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                         const char *argv[], void *aux OVS_UNUSED)
 {
     struct ds ds = DS_EMPTY_INITIALIZER;
     const struct ofproto_dpif *ofproto;
     const struct mac_entry *e;
 
-    ofproto = ofproto_dpif_lookup(args);
+    ofproto = ofproto_dpif_lookup(argv[1]);
     if (!ofproto) {
         unixctl_command_reply(conn, 501, "no such bridge");
         return;
@@ -5589,7 +5707,8 @@ ofproto_unixctl_fdb_show(struct unixctl_conn *conn,
         struct ofbundle *bundle = e->port.p;
         ds_put_format(&ds, "%5d  %4d  "ETH_ADDR_FMT"  %3d\n",
                       ofbundle_get_a_port(bundle)->odp_port,
-                      e->vlan, ETH_ADDR_ARGS(e->mac), mac_entry_age(e));
+                      e->vlan, ETH_ADDR_ARGS(e->mac),
+                      mac_entry_age(ofproto->ml, e));
     }
     unixctl_command_reply(conn, 200, ds_cstr(&ds));
     ds_destroy(&ds);
@@ -5664,12 +5783,10 @@ trace_resubmit(struct action_xlate_ctx *ctx, struct rule_dpif *rule)
 }
 
 static void
-ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_,
+ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
                       void *aux OVS_UNUSED)
 {
-    char *dpname, *arg1, *arg2, *arg3, *arg4;
-    char *args = xstrdup(args_);
-    char *save_ptr = NULL;
+    const char *dpname = argv[1];
     struct ofproto_dpif *ofproto;
     struct ofpbuf odp_key;
     struct ofpbuf *packet;
@@ -5683,29 +5800,21 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_,
     ofpbuf_init(&odp_key, 0);
     ds_init(&result);
 
-    dpname = strtok_r(args, " ", &save_ptr);
-    if (!dpname) {
-        unixctl_command_reply(conn, 501, "Bad command syntax");
-        goto exit;
-    }
-
     ofproto = ofproto_dpif_lookup(dpname);
     if (!ofproto) {
         unixctl_command_reply(conn, 501, "Unknown ofproto (use ofproto/list "
                               "for help)");
         goto exit;
     }
-    arg1 = strtok_r(NULL, " ", &save_ptr);
-    arg2 = strtok_r(NULL, " ", &save_ptr);
-    arg3 = strtok_r(NULL, " ", &save_ptr);
-    arg4 = strtok_r(NULL, "", &save_ptr); /* Get entire rest of line. */
-    if (dpname && arg1 && (!arg2 || !strcmp(arg2, "-generate")) && !arg3) {
+    if (argc == 3 || (argc == 4 && !strcmp(argv[3], "-generate"))) {
         /* ofproto/trace dpname flow [-generate] */
+        const char *flow_s = argv[2];
+        const char *generate_s = argv[3];
         int error;
 
         /* Convert string to datapath key. */
         ofpbuf_init(&odp_key, 0);
-        error = odp_flow_key_from_string(arg1, NULL, &odp_key);
+        error = odp_flow_key_from_string(flow_s, NULL, &odp_key);
         if (error) {
             unixctl_command_reply(conn, 501, "Bad flow syntax");
             goto exit;
@@ -5714,42 +5823,36 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_,
         /* Convert odp_key to flow. */
         error = ofproto_dpif_extract_flow_key(ofproto, odp_key.data,
                                               odp_key.size, &flow,
-                                              &initial_tci);
+                                              &initial_tci, NULL);
         if (error == ODP_FIT_ERROR) {
             unixctl_command_reply(conn, 501, "Invalid flow");
             goto exit;
         }
 
         /* Generate a packet, if requested. */
-        if (arg2) {
+        if (generate_s) {
             packet = ofpbuf_new(0);
             flow_compose(packet, &flow);
         }
-    } else if (dpname && arg1 && arg2 && arg3 && arg4) {
+    } else if (argc == 6) {
         /* ofproto/trace dpname priority tun_id in_port packet */
-        uint16_t in_port;
-        ovs_be64 tun_id;
-        uint32_t priority;
-
-        priority = atoi(arg1);
-        tun_id = htonll(strtoull(arg2, NULL, 0));
-        in_port = ofp_port_to_odp_port(atoi(arg3));
-
-        packet = ofpbuf_new(strlen(args) / 2);
-        arg4 = ofpbuf_put_hex(packet, arg4, NULL);
-        arg4 += strspn(arg4, " ");
-        if (*arg4 != '\0') {
-            unixctl_command_reply(conn, 501, "Trailing garbage in command");
-            goto exit;
-        }
-        if (packet->size < ETH_HEADER_LEN) {
-            unixctl_command_reply(conn, 501,
-                                  "Packet data too short for Ethernet");
+        const char *priority_s = argv[2];
+        const char *tun_id_s = argv[3];
+        const char *in_port_s = argv[4];
+        const char *packet_s = argv[5];
+        uint16_t in_port = ofp_port_to_odp_port(atoi(in_port_s));
+        ovs_be64 tun_id = htonll(strtoull(tun_id_s, NULL, 0));
+        uint32_t priority = atoi(priority_s);
+        const char *msg;
+
+        msg = eth_from_hex(packet_s, &packet);
+        if (msg) {
+            unixctl_command_reply(conn, 501, msg);
             goto exit;
         }
 
         ds_put_cstr(&result, "Packet: ");
-        s = ofp_packet_to_string(packet->data, packet->size, packet->size);
+        s = ofp_packet_to_string(packet->data, packet->size);
         ds_put_cstr(&result, s);
         free(s);
 
@@ -5772,7 +5875,8 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_,
 
         trace.result = &result;
         trace.flow = flow;
-        action_xlate_ctx_init(&trace.ctx, ofproto, &flow, initial_tci, packet);
+        action_xlate_ctx_init(&trace.ctx, ofproto, &flow, initial_tci,
+                              rule->up.flow_cookie, packet);
         trace.ctx.resubmit_hook = trace_resubmit;
         odp_actions = xlate_actions(&trace.ctx,
                                     rule->up.actions, rule->up.n_actions);
@@ -5799,20 +5903,19 @@ exit:
     ds_destroy(&result);
     ofpbuf_delete(packet);
     ofpbuf_uninit(&odp_key);
-    free(args);
 }
 
 static void
-ofproto_dpif_clog(struct unixctl_conn *conn OVS_UNUSED,
-                  const char *args_ OVS_UNUSED, void *aux OVS_UNUSED)
+ofproto_dpif_clog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
+                  const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
 {
     clogged = true;
     unixctl_command_reply(conn, 200, NULL);
 }
 
 static void
-ofproto_dpif_unclog(struct unixctl_conn *conn OVS_UNUSED,
-                    const char *args_ OVS_UNUSED, void *aux OVS_UNUSED)
+ofproto_dpif_unclog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
+                    const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
 {
     clogged = false;
     unixctl_command_reply(conn, 200, NULL);
@@ -5827,15 +5930,18 @@ ofproto_dpif_unixctl_init(void)
     }
     registered = true;
 
-    unixctl_command_register("ofproto/trace",
-                      "bridge {tun_id in_port packet | odp_flow [-generate]}",
-                      ofproto_unixctl_trace, NULL);
-    unixctl_command_register("fdb/flush", "bridge", ofproto_unixctl_fdb_flush,
-                             NULL);
-    unixctl_command_register("fdb/show", "bridge", ofproto_unixctl_fdb_show,
-                             NULL);
-    unixctl_command_register("ofproto/clog", "", ofproto_dpif_clog, NULL);
-    unixctl_command_register("ofproto/unclog", "", ofproto_dpif_unclog, NULL);
+    unixctl_command_register(
+        "ofproto/trace",
+        "bridge {tun_id in_port packet | odp_flow [-generate]}",
+        2, 5, ofproto_unixctl_trace, NULL);
+    unixctl_command_register("fdb/flush", "[bridge]", 0, 1,
+                             ofproto_unixctl_fdb_flush, NULL);
+    unixctl_command_register("fdb/show", "bridge", 1, 1,
+                             ofproto_unixctl_fdb_show, NULL);
+    unixctl_command_register("ofproto/clog", "", 0, 0,
+                             ofproto_dpif_clog, NULL);
+    unixctl_command_register("ofproto/unclog", "", 0, 0,
+                             ofproto_dpif_unclog, NULL);
 }
 \f
 /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
@@ -6038,5 +6144,6 @@ const struct ofproto_class ofproto_dpif_class = {
     set_flood_vlans,
     is_mirror_output_bundle,
     forward_bpdu_changed,
+    set_mac_idle_time,
     set_realdev,
 };