X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=blobdiff_plain;f=ofproto%2Fofproto-dpif.c;h=afdd08501c579adbe051425215cf22def04a6070;hb=fd19297bb3d61789aa2a8871e3316b2a4e3e34c3;hp=8565a8d86b33db85e2295324268296a800303db2;hpb=5da5ec37e45c0287ebe7e52278dfed3a485b6f2a;p=openvswitch

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8565a8d8..afdd0850 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -121,9 +121,10 @@ struct ofmirror {
     struct hmapx dsts;          /* Contains "struct ofbundle *"s. */
     unsigned long *vlans;       /* Bitmap of chosen VLANs, NULL selects all. */
 
-    /* Output (mutually exclusive). */
+    /* Output (exactly one of out == NULL and out_vlan == -1 is true). */
     struct ofbundle *out;       /* Output port or NULL. */
     int out_vlan;               /* Output VLAN or -1. */
+    mirror_mask_t dup_mirrors;  /* Bitmap of mirrors with the same output. */
 };
 
 static void mirror_destroy(struct ofmirror *);
@@ -144,6 +145,7 @@ struct ofbundle {
                                  * NULL if all VLANs are trunked. */
     struct lacp *lacp;          /* LACP if LACP is enabled, otherwise NULL. */
     struct bond *bond;          /* Nonnull iff more than one port. */
+    bool use_priority_tags;     /* Use 802.1p tag for frames in VLAN 0? */
 
     /* Status. */
     bool floodable;             /* True if no port has OFPPC_NO_FLOOD set. */
@@ -411,12 +413,9 @@ static struct ofport_dpif *get_odp_port(struct ofproto_dpif *,
 static void update_learning_table(struct ofproto_dpif *,
                                   const struct flow *, int vlan,
                                   struct ofbundle *);
-static bool is_admissible(struct ofproto_dpif *, const struct flow *,
-                          bool have_packet, tag_type *, int *vlanp,
-                          struct ofbundle **in_bundlep);
-
 /* Upcalls. */
 #define FLOW_MISS_MAX_BATCH 50
+
 static void handle_upcall(struct ofproto_dpif *, struct dpif_upcall *);
 static void handle_miss_upcalls(struct ofproto_dpif *,
                                 struct dpif_upcall *, size_t n);
@@ -1231,8 +1230,7 @@ bundle_update(struct ofbundle *bundle)
 
     bundle->floodable = true;
     LIST_FOR_EACH (port, bundle_node, &bundle->ports) {
-        if (port->up.opp.config & htonl(OFPPC_NO_FLOOD)
-                    || !stp_forward_in_state(port->stp_state)) {
+        if (port->up.opp.config & htonl(OFPPC_NO_FLOOD)) {
             bundle->floodable = false;
             break;
         }
@@ -1279,8 +1277,7 @@ bundle_add_port(struct ofbundle *bundle, uint32_t ofp_port,
 
         port->bundle = bundle;
         list_push_back(&bundle->ports, &port->bundle_node);
-        if (port->up.opp.config & htonl(OFPPC_NO_FLOOD)
-                    || !stp_forward_in_state(port->stp_state)) {
+        if (port->up.opp.config & htonl(OFPPC_NO_FLOOD)) {
             bundle->floodable = false;
         }
     }
@@ -1366,6 +1363,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
         bundle->vlan_mode = PORT_VLAN_TRUNK;
         bundle->vlan = -1;
         bundle->trunks = NULL;
+        bundle->use_priority_tags = s->use_priority_tags;
         bundle->lacp = NULL;
         bundle->bond = NULL;
 
@@ -1424,8 +1422,10 @@ bundle_set(struct ofproto *ofproto_, void *aux,
     }
 
     /* Set VLAN tagging mode */
-    if (s->vlan_mode != bundle->vlan_mode) {
+    if (s->vlan_mode != bundle->vlan_mode
+        || s->use_priority_tags != bundle->use_priority_tags) {
         bundle->vlan_mode = s->vlan_mode;
+        bundle->use_priority_tags = s->use_priority_tags;
         need_flush = true;
     }
 
@@ -1659,6 +1659,39 @@ mirror_lookup(struct ofproto_dpif *ofproto, void *aux)
     return NULL;
 }
 
+/* Update the 'dup_mirrors' member of each of the ofmirrors in 'ofproto'. */
+static void
+mirror_update_dups(struct ofproto_dpif *ofproto)
+{
+    int i;
+
+    for (i = 0; i < MAX_MIRRORS; i++) {
+        struct ofmirror *m = ofproto->mirrors[i];
+
+        if (m) {
+            m->dup_mirrors = MIRROR_MASK_C(1) << i;
+        }
+    }
+
+    for (i = 0; i < MAX_MIRRORS; i++) {
+        struct ofmirror *m1 = ofproto->mirrors[i];
+        int j;
+
+        if (!m1) {
+            continue;
+        }
+
+        for (j = i + 1; j < MAX_MIRRORS; j++) {
+            struct ofmirror *m2 = ofproto->mirrors[j];
+
+            if (m2 && m1->out == m2->out && m1->out_vlan == m2->out_vlan) {
+                m1->dup_mirrors |= MIRROR_MASK_C(1) << j;
+                m2->dup_mirrors |= m1->dup_mirrors;
+            }
+        }
+    }
+}
+
 static int
 mirror_set(struct ofproto *ofproto_, void *aux,
            const struct ofproto_mirror_settings *s)
@@ -1764,6 +1797,7 @@ mirror_set(struct ofproto *ofproto_, void *aux,
 
     ofproto->need_revalidate = true;
     mac_learning_flush(ofproto->ml);
+    mirror_update_dups(ofproto);
 
     return 0;
 }
@@ -1797,6 +1831,8 @@ mirror_destroy(struct ofmirror *mirror)
     ofproto->mirrors[mirror->idx] = NULL;
     free(mirror->name);
     free(mirror);
+
+    mirror_update_dups(ofproto);
 }
 
 static int
@@ -3479,19 +3515,12 @@ put_userspace_action(const struct ofproto_dpif *ofproto,
                      const struct flow *flow,
                      const struct user_action_cookie *cookie)
 {
-    size_t offset;
     uint32_t pid;
 
     pid = dpif_port_get_pid(ofproto->dpif,
                             ofp_port_to_odp_port(flow->in_port));
 
-    offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_USERSPACE);
-    nl_msg_put_u32(odp_actions, OVS_USERSPACE_ATTR_PID, pid);
-    nl_msg_put_unspec(odp_actions, OVS_USERSPACE_ATTR_USERDATA,
-                      cookie, sizeof *cookie);
-    nl_msg_end_nested(odp_actions, offset);
-
-    return odp_actions->size - NLA_ALIGN(sizeof *cookie);
+    return odp_put_userspace_action(pid, cookie, odp_actions);
 }
 
 /* Compose SAMPLE action for sFlow. */
@@ -3742,35 +3771,41 @@ commit_odp_actions(struct action_xlate_ctx *ctx)
 }
 
 static void
-compose_output_action(struct action_xlate_ctx *ctx, uint16_t odp_port)
+force_compose_output_action(struct action_xlate_ctx *ctx, uint16_t ofp_port)
 {
+    const struct ofport_dpif *ofport = get_ofp_port(ctx->ofproto, ofp_port);
+    uint16_t odp_port = ofp_port_to_odp_port(ofp_port);
+
+    if (ofport && ofport->up.opp.config & htonl(OFPPC_NO_FWD)) {
+        return;
+    }
+
     nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_OUTPUT, odp_port);
     ctx->sflow_odp_port = odp_port;
     ctx->sflow_n_outputs++;
 }
 
 static void
-add_output_action(struct action_xlate_ctx *ctx, uint16_t ofp_port)
+compose_output_action(struct action_xlate_ctx *ctx, uint16_t ofp_port)
 {
-    const struct ofport_dpif *ofport = get_ofp_port(ctx->ofproto, ofp_port);
-    uint16_t odp_port = ofp_port_to_odp_port(ofp_port);
+    struct ofport_dpif *ofport = get_ofp_port(ctx->ofproto, ofp_port);
 
-    if (ofport) {
-        if (ofport->up.opp.config & htonl(OFPPC_NO_FWD)
-                || !stp_forward_in_state(ofport->stp_state)) {
-            /* Forwarding disabled on port. */
-            return;
-        }
-    } else {
-        /*
-         * We don't have an ofport record for this port, but it doesn't hurt to
-         * allow forwarding to it anyhow.  Maybe such a port will appear later
-         * and we're pre-populating the flow table.
-         */
+    if (ofport && !stp_forward_in_state(ofport->stp_state)) {
+        /* Forwarding disabled on port. */
+        return;
     }
 
+    /* We may not have an ofport record for this port, but it doesn't hurt to
+     * allow forwarding to it anyhow.  Maybe such a port will appear later and
+     * we're pre-populating the flow table.  */
+    force_compose_output_action(ctx, ofp_port);
+}
+
+static void
+commit_output_action(struct action_xlate_ctx *ctx, uint16_t ofp_port)
+{
     commit_odp_actions(ctx);
-    compose_output_action(ctx, odp_port);
+    compose_output_action(ctx, ofp_port);
     ctx->nf_output_iface = ofp_port;
 }
 
@@ -3843,17 +3878,22 @@ xlate_resubmit_table(struct action_xlate_ctx *ctx,
 }
 
 static void
-flood_packets(struct action_xlate_ctx *ctx, ovs_be32 mask)
+flood_packets(struct action_xlate_ctx *ctx, bool all)
 {
     struct ofport_dpif *ofport;
 
     commit_odp_actions(ctx);
     HMAP_FOR_EACH (ofport, up.hmap_node, &ctx->ofproto->up.ports) {
         uint16_t ofp_port = ofport->up.ofp_port;
-        if (ofp_port != ctx->flow.in_port
-                && !(ofport->up.opp.config & mask)
-                && stp_forward_in_state(ofport->stp_state)) {
-            compose_output_action(ctx, ofport->odp_port);
+
+        if (ofp_port == ctx->flow.in_port) {
+            continue;
+        }
+
+        if (all) {
+            force_compose_output_action(ctx, ofp_port);
+        } else if (!(ofport->up.opp.config & htonl(OFPPC_NO_FLOOD))) {
+            compose_output_action(ctx, ofp_port);
         }
     }
 
@@ -3882,7 +3922,7 @@ xlate_output_action__(struct action_xlate_ctx *ctx,
 
     switch (port) {
     case OFPP_IN_PORT:
-        add_output_action(ctx, ctx->flow.in_port);
+        commit_output_action(ctx, ctx->flow.in_port);
         break;
     case OFPP_TABLE:
         xlate_table_action(ctx, ctx->flow.in_port, ctx->table_id);
@@ -3891,23 +3931,23 @@ xlate_output_action__(struct action_xlate_ctx *ctx,
         xlate_normal(ctx);
         break;
     case OFPP_FLOOD:
-        flood_packets(ctx,  htonl(OFPPC_NO_FLOOD));
+        flood_packets(ctx,  false);
         break;
     case OFPP_ALL:
-        flood_packets(ctx, htonl(0));
+        flood_packets(ctx, true);
         break;
     case OFPP_CONTROLLER:
         commit_odp_actions(ctx);
         compose_controller_action(ctx, max_len);
         break;
     case OFPP_LOCAL:
-        add_output_action(ctx, OFPP_LOCAL);
+        commit_output_action(ctx, OFPP_LOCAL);
         break;
     case OFPP_NONE:
         break;
     default:
         if (port != ctx->flow.in_port) {
-            add_output_action(ctx, port);
+            commit_output_action(ctx, port);
         }
         break;
     }
@@ -3946,7 +3986,7 @@ static void
 xlate_enqueue_action(struct action_xlate_ctx *ctx,
                      const struct ofp_action_enqueue *oae)
 {
-    uint16_t ofp_port, odp_port;
+    uint16_t ofp_port;
     uint32_t flow_priority, priority;
     int error;
 
@@ -3965,17 +4005,16 @@ xlate_enqueue_action(struct action_xlate_ctx *ctx,
     } else if (ofp_port == ctx->flow.in_port) {
         return;
     }
-    odp_port = ofp_port_to_odp_port(ofp_port);
 
     /* Add datapath actions. */
     flow_priority = ctx->flow.priority;
     ctx->flow.priority = priority;
-    add_output_action(ctx, odp_port);
+    commit_output_action(ctx, ofp_port);
     ctx->flow.priority = flow_priority;
 
     /* Update NetFlow output port. */
     if (ctx->nf_output_iface == NF_OUT_DROP) {
-        ctx->nf_output_iface = odp_port;
+        ctx->nf_output_iface = ofp_port;
     } else if (ctx->nf_output_iface != NF_OUT_FLOOD) {
         ctx->nf_output_iface = NF_OUT_MULTI;
     }
@@ -4333,7 +4372,7 @@ xlate_actions(struct action_xlate_ctx *ctx,
             if (ctx->packet
                 && connmgr_msg_in_hook(ctx->ofproto->up.connmgr, &ctx->flow,
                                        ctx->packet)) {
-                compose_output_action(ctx, OVSP_LOCAL);
+                compose_output_action(ctx, OFPP_LOCAL);
             }
         }
         fix_sflow_action(ctx);
@@ -4344,21 +4383,6 @@ xlate_actions(struct action_xlate_ctx *ctx,
 
 /* OFPP_NORMAL implementation. */
 
-struct dst {
-    struct ofport_dpif *port;
-    uint16_t vid;
-};
-
-struct dst_set {
-    struct dst builtin[32];
-    struct dst *dsts;
-    size_t n, allocated;
-};
-
-static void dst_set_init(struct dst_set *);
-static void dst_set_add(struct dst_set *, const struct dst *);
-static void dst_set_free(struct dst_set *);
-
 static struct ofport_dpif *ofbundle_get_a_port(const struct ofbundle *);
 
 /* Given 'vid', the VID obtained from the 802.1Q header that was received as
@@ -4465,20 +4489,37 @@ output_vlan_to_vid(const struct ofbundle *out_bundle, uint16_t vlan)
     }
 }
 
-static bool
-set_dst(struct action_xlate_ctx *ctx, struct dst *dst,
-        const struct ofbundle *in_bundle, const struct ofbundle *out_bundle)
+static void
+output_normal(struct action_xlate_ctx *ctx, const struct ofbundle *out_bundle,
+              uint16_t vlan)
 {
-    uint16_t vlan;
+    struct ofport_dpif *port;
+    uint16_t vid;
+    ovs_be16 tci;
+
+    vid = output_vlan_to_vid(out_bundle, vlan);
+    if (!out_bundle->bond) {
+        port = ofbundle_get_a_port(out_bundle);
+    } else {
+        port = bond_choose_output_slave(out_bundle->bond, &ctx->flow,
+                                        vid, &ctx->tags);
+        if (!port) {
+            /* No slaves enabled, so drop packet. */
+            return;
+        }
+    }
 
-    vlan = input_vid_to_vlan(in_bundle, vlan_tci_to_vid(ctx->flow.vlan_tci));
-    dst->vid = output_vlan_to_vid(out_bundle, vlan);
+    tci = htons(vid);
+    if (tci || out_bundle->use_priority_tags) {
+        tci |= ctx->flow.vlan_tci & htons(VLAN_PCP_MASK);
+        if (tci) {
+            tci |= htons(VLAN_CFI);
+        }
+    }
+    commit_vlan_action(ctx, tci);
 
-    dst->port = (!out_bundle->bond
-                 ? ofbundle_get_a_port(out_bundle)
-                 : bond_choose_output_slave(out_bundle->bond, &ctx->flow,
-                                            dst->vid, &ctx->tags));
-    return dst->port != NULL;
+    compose_output_action(ctx, port->up.ofp_port);
+    ctx->nf_output_iface = port->up.ofp_port;
 }
 
 static int
@@ -4488,54 +4529,6 @@ mirror_mask_ffs(mirror_mask_t mask)
     return ffs(mask);
 }
 
-static void
-dst_set_init(struct dst_set *set)
-{
-    set->dsts = set->builtin;
-    set->n = 0;
-    set->allocated = ARRAY_SIZE(set->builtin);
-}
-
-static void
-dst_set_add(struct dst_set *set, const struct dst *dst)
-{
-    if (set->n >= set->allocated) {
-        size_t new_allocated;
-        struct dst *new_dsts;
-
-        new_allocated = set->allocated * 2;
-        new_dsts = xmalloc(new_allocated * sizeof *new_dsts);
-        memcpy(new_dsts, set->dsts, set->n * sizeof *new_dsts);
-
-        dst_set_free(set);
-
-        set->dsts = new_dsts;
-        set->allocated = new_allocated;
-    }
-    set->dsts[set->n++] = *dst;
-}
-
-static void
-dst_set_free(struct dst_set *set)
-{
-    if (set->dsts != set->builtin) {
-        free(set->dsts);
-    }
-}
-
-static bool
-dst_is_duplicate(const struct dst_set *set, const struct dst *test)
-{
-    size_t i;
-    for (i = 0; i < set->n; i++) {
-        if (set->dsts[i].vid == test->vid
-            && set->dsts[i].port == test->port) {
-            return true;
-        }
-    }
-    return false;
-}
-
 static bool
 ofbundle_trunks_vlan(const struct ofbundle *bundle, uint16_t vlan)
 {
@@ -4557,12 +4550,12 @@ ofbundle_get_a_port(const struct ofbundle *bundle)
                         struct ofport_dpif, bundle_node);
 }
 
-static void
+static mirror_mask_t
 compose_dsts(struct action_xlate_ctx *ctx, uint16_t vlan,
              const struct ofbundle *in_bundle,
-             const struct ofbundle *out_bundle, struct dst_set *set)
+             const struct ofbundle *out_bundle)
 {
-    struct dst dst;
+    mirror_mask_t dst_mirrors = 0;
 
     if (out_bundle == OFBUNDLE_FLOOD) {
         struct ofbundle *bundle;
@@ -4571,16 +4564,18 @@ compose_dsts(struct action_xlate_ctx *ctx, uint16_t vlan,
             if (bundle != in_bundle
                 && ofbundle_includes_vlan(bundle, vlan)
                 && bundle->floodable
-                && !bundle->mirror_out
-                && set_dst(ctx, &dst, in_bundle, bundle)) {
-                dst_set_add(set, &dst);
+                && !bundle->mirror_out) {
+                output_normal(ctx, bundle, vlan);
+                dst_mirrors |= bundle->dst_mirrors;
             }
         }
         ctx->nf_output_iface = NF_OUT_FLOOD;
-    } else if (out_bundle && set_dst(ctx, &dst, in_bundle, out_bundle)) {
-        dst_set_add(set, &dst);
-        ctx->nf_output_iface = dst.port->odp_port;
+    } else if (out_bundle) {
+        output_normal(ctx, out_bundle, vlan);
+        dst_mirrors = out_bundle->dst_mirrors;
     }
+
+    return dst_mirrors;
 }
 
 static bool
@@ -4631,113 +4626,43 @@ eth_dst_may_rspan(const uint8_t dst[ETH_ADDR_LEN])
 }
 
 static void
-compose_mirror_dsts(struct action_xlate_ctx *ctx,
-                    uint16_t vlan, const struct ofbundle *in_bundle,
-                    struct dst_set *set)
+output_mirrors(struct action_xlate_ctx *ctx,
+               uint16_t vlan, const struct ofbundle *in_bundle,
+               mirror_mask_t dst_mirrors)
 {
     struct ofproto_dpif *ofproto = ctx->ofproto;
     mirror_mask_t mirrors;
-    uint16_t flow_vid;
-    size_t i;
-
-    mirrors = in_bundle->src_mirrors;
-    for (i = 0; i < set->n; i++) {
-        mirrors |= set->dsts[i].port->bundle->dst_mirrors;
-    }
 
+    mirrors = in_bundle->src_mirrors | dst_mirrors;
     if (!mirrors) {
         return;
     }
 
-    flow_vid = vlan_tci_to_vid(ctx->flow.vlan_tci);
     while (mirrors) {
-        struct ofmirror *m = ofproto->mirrors[mirror_mask_ffs(mirrors) - 1];
-        if (vlan_is_mirrored(m, vlan)) {
-            struct dst dst;
-
-            if (m->out) {
-                if (set_dst(ctx, &dst, in_bundle, m->out)
-                    && !dst_is_duplicate(set, &dst)) {
-                    dst_set_add(set, &dst);
-                }
-            } else if (eth_dst_may_rspan(ctx->flow.dl_dst)) {
-                struct ofbundle *bundle;
-
-                HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
-                    if (ofbundle_includes_vlan(bundle, m->out_vlan)
-                        && !bundle->mirror_out
-                        && set_dst(ctx, &dst, in_bundle, bundle))
-                    {
-                        /* set_dst() got dst->vid from the input packet's VLAN,
-                         * not from m->out_vlan, so recompute it. */
-                        dst.vid = output_vlan_to_vid(bundle, m->out_vlan);
-
-                        if (dst_is_duplicate(set, &dst)) {
-                            continue;
-                        }
-
-                        if (bundle == in_bundle && dst.vid == flow_vid) {
-                            /* Don't send out input port on same VLAN. */
-                            continue;
-                        }
-                        dst_set_add(set, &dst);
-                    }
-                }
-            }
-        }
-        mirrors &= mirrors - 1;
-    }
-}
+        struct ofmirror *m;
 
-static void
-compose_actions(struct action_xlate_ctx *ctx, uint16_t vlan,
-                const struct ofbundle *in_bundle,
-                const struct ofbundle *out_bundle)
-{
-    uint16_t initial_vid, cur_vid;
-    const struct dst *dst;
-    struct dst_set set;
-
-    dst_set_init(&set);
-    compose_dsts(ctx, vlan, in_bundle, out_bundle, &set);
-    compose_mirror_dsts(ctx, vlan, in_bundle, &set);
-    if (!set.n) {
-        dst_set_free(&set);
-        return;
-    }
-
-    /* Output all the packets we can without having to change the VLAN. */
-    commit_odp_actions(ctx);
-    initial_vid = vlan_tci_to_vid(ctx->flow.vlan_tci);
-    for (dst = set.dsts; dst < &set.dsts[set.n]; dst++) {
-        if (dst->vid != initial_vid) {
-            continue;
-        }
-        compose_output_action(ctx, dst->port->odp_port);
-    }
+        m = ofproto->mirrors[mirror_mask_ffs(mirrors) - 1];
 
-    /* Then output the rest. */
-    cur_vid = initial_vid;
-    for (dst = set.dsts; dst < &set.dsts[set.n]; dst++) {
-        if (dst->vid == initial_vid) {
+        if (!vlan_is_mirrored(m, vlan)) {
+            mirrors &= mirrors - 1;
             continue;
         }
-        if (dst->vid != cur_vid) {
-            ovs_be16 tci;
 
-            tci = htons(dst->vid);
-            tci |= ctx->flow.vlan_tci & htons(VLAN_PCP_MASK);
-            if (tci) {
-                tci |= htons(VLAN_CFI);
+        mirrors &= ~m->dup_mirrors;
+        if (m->out) {
+            output_normal(ctx, m->out, vlan);
+        } else if (eth_dst_may_rspan(ctx->flow.dl_dst)
+                   && vlan != m->out_vlan) {
+            struct ofbundle *bundle;
+
+            HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
+                if (ofbundle_includes_vlan(bundle, m->out_vlan)
+                    && !bundle->mirror_out) {
+                    output_normal(ctx, bundle, m->out_vlan);
+                }
             }
-            commit_vlan_action(ctx, tci);
-
-            cur_vid = dst->vid;
         }
-        compose_output_action(ctx, dst->port->odp_port);
     }
-
-    dst_set_free(&set);
 }
 
 /* A VM broadcasts a gratuitous ARP to indicate that it has resumed after
@@ -4790,96 +4715,62 @@ update_learning_table(struct ofproto_dpif *ofproto,
     }
 }
 
+static struct ofport_dpif *
+lookup_input_bundle(struct ofproto_dpif *ofproto, uint16_t in_port, bool warn)
+{
+    struct ofport_dpif *ofport;
+
+    /* Find the port and bundle for the received packet. */
+    ofport = get_ofp_port(ofproto, in_port);
+    if (ofport && ofport->bundle) {
+        return ofport;
+    }
+
+    /* Odd.  A few possible reasons here:
+     *
+     * - We deleted a port but there are still a few packets queued up
+     *   from it.
+     *
+     * - Someone externally added a port (e.g. "ovs-dpctl add-if") that
+     *   we don't know about.
+     *
+     * - The ofproto client didn't configure the port as part of a bundle.
+     */
+    if (warn) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+        VLOG_WARN_RL(&rl, "bridge %s: received packet on unknown "
+                     "port %"PRIu16, ofproto->up.name, in_port);
+    }
+    return NULL;
+}
+
 /* Determines whether packets in 'flow' within 'ofproto' should be forwarded or
  * dropped.  Returns true if they may be forwarded, false if they should be
  * dropped.
  *
- * If 'have_packet' is true, it indicates that the caller is processing a
- * received packet.  If 'have_packet' is false, then the caller is just
- * revalidating an existing flow because configuration has changed.  Either
- * way, 'have_packet' only affects logging (there is no point in logging errors
- * during revalidation).
- *
- * Sets '*in_bundlep' to the input bundle.  This will be a null pointer if
- * flow->in_port does not designate a known input port (in which case
- * is_admissible() returns false).
+ * 'in_port' must be the ofport_dpif that corresponds to flow->in_port.
+ * 'in_port' must be part of a bundle (e.g. in_port->bundle must be nonnull).
  *
- * When returning true, sets '*vlanp' to the effective VLAN of the input
- * packet, as returned by input_vid_to_vlan().
+ * 'vlan' must be the VLAN that corresponds to flow->vlan_tci on 'in_port', as
+ * returned by input_vid_to_vlan().  It must be a valid VLAN for 'in_port', as
+ * checked by input_vid_is_valid().
  *
  * May also add tags to '*tags', although the current implementation only does
  * so in one special case.
  */
 static bool
 is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
-              bool have_packet,
-              tag_type *tags, int *vlanp, struct ofbundle **in_bundlep)
+              struct ofport_dpif *in_port, uint16_t vlan, tag_type *tags)
 {
-    struct ofport_dpif *in_port;
-    struct ofbundle *in_bundle;
-    uint16_t vid;
-    int vlan;
-
-    *vlanp = -1;
-
-    /* Find the port and bundle for the received packet. */
-    in_port = get_ofp_port(ofproto, flow->in_port);
-    *in_bundlep = in_bundle = in_port ? in_port->bundle : NULL;
-    if (!in_port || !in_bundle) {
-        /* No interface?  Something fishy... */
-        if (have_packet) {
-            /* Odd.  A few possible reasons here:
-             *
-             * - We deleted a port but there are still a few packets queued up
-             *   from it.
-             *
-             * - Someone externally added a port (e.g. "ovs-dpctl add-if") that
-             *   we don't know about.
-             *
-             * - Packet arrived on the local port but the local port is not
-             *   part of a bundle.
-             */
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
-            VLOG_WARN_RL(&rl, "bridge %s: received packet on unknown "
-                         "port %"PRIu16,
-                         ofproto->up.name, flow->in_port);
-        }
-        return false;
-    }
-
-    if (flow->dl_type == htons(ETH_TYPE_VLAN) &&
-        !(flow->vlan_tci & htons(VLAN_CFI))) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-        VLOG_WARN_RL(&rl, "bridge %s: dropping packet with partial "
-                     "VLAN tag received on port %s",
-                     ofproto->up.name, in_bundle->name);
-        return -1;
-    }
-
-    vid = vlan_tci_to_vid(flow->vlan_tci);
-    if (!input_vid_is_valid(vid, in_bundle, have_packet)) {
-        return false;
-    }
-    *vlanp = vlan = input_vid_to_vlan(in_bundle, vid);
+    struct ofbundle *in_bundle = in_port->bundle;
 
-    /* Drop frames for reserved multicast addresses only if forward_bpdu
-     * option is absent. */
+    /* Drop frames for reserved multicast addresses
+     * only if forward_bpdu option is absent. */
     if (eth_addr_is_reserved(flow->dl_dst) && !ofproto->up.forward_bpdu) {
         return false;
     }
 
-    /* Drop frames on bundles reserved for mirroring. */
-    if (in_bundle->mirror_out) {
-        if (have_packet) {
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-            VLOG_WARN_RL(&rl, "bridge %s: dropping packet received on port "
-                         "%s, which is reserved exclusively for mirroring",
-                         ofproto->up.name, in_bundle->name);
-        }
-        return false;
-    }
-
     if (in_bundle->bond) {
         struct mac_entry *mac;
 
@@ -4908,18 +4799,60 @@ is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
 static void
 xlate_normal(struct action_xlate_ctx *ctx)
 {
+    mirror_mask_t dst_mirrors = 0;
+    struct ofport_dpif *in_port;
     struct ofbundle *in_bundle;
     struct ofbundle *out_bundle;
     struct mac_entry *mac;
-    int vlan;
+    uint16_t vlan;
+    uint16_t vid;
 
     ctx->has_normal = true;
 
-    /* Check whether we should drop packets in this flow. */
-    if (!is_admissible(ctx->ofproto, &ctx->flow, ctx->packet != NULL,
-                       &ctx->tags, &vlan, &in_bundle)) {
-        out_bundle = NULL;
-        goto done;
+    /* 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,
+                                  ctx->packet != NULL);
+    if (!in_port) {
+        return;
+    }
+    in_bundle = in_port->bundle;
+
+    /* Drop malformed frames. */
+    if (ctx->flow.dl_type == htons(ETH_TYPE_VLAN) &&
+        !(ctx->flow.vlan_tci & htons(VLAN_CFI))) {
+        if (ctx->packet != NULL) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_WARN_RL(&rl, "bridge %s: dropping packet with partial "
+                         "VLAN tag received on port %s",
+                         ctx->ofproto->up.name, in_bundle->name);
+        }
+        return;
+    }
+
+    /* Drop frames on bundles reserved for mirroring. */
+    if (in_bundle->mirror_out) {
+        if (ctx->packet != NULL) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_WARN_RL(&rl, "bridge %s: dropping packet received on port "
+                         "%s, which is reserved exclusively for mirroring",
+                         ctx->ofproto->up.name, in_bundle->name);
+        }
+        return;
+    }
+
+    /* Check VLAN. */
+    vid = vlan_tci_to_vid(ctx->flow.vlan_tci);
+    if (!input_vid_is_valid(vid, in_bundle, ctx->packet != NULL)) {
+        return;
+    }
+    vlan = input_vid_to_vlan(in_bundle, vid);
+
+    /* Check other admissibility requirements. */
+    if (!is_admissible(ctx->ofproto, &ctx->flow, in_port, vlan, &ctx->tags)) {
+        output_mirrors(ctx, vlan, in_bundle, 0);
+        return;
     }
 
     /* Learn source MAC. */
@@ -4945,14 +4878,10 @@ xlate_normal(struct action_xlate_ctx *ctx)
     }
 
     /* Don't send packets out their input bundles. */
-    if (in_bundle == out_bundle) {
-        out_bundle = NULL;
-    }
-
-done:
-    if (in_bundle) {
-        compose_actions(ctx, vlan, in_bundle, out_bundle);
+    if (in_bundle != out_bundle) {
+        dst_mirrors = compose_dsts(ctx, vlan, in_bundle, out_bundle);
     }
+    output_mirrors(ctx, vlan, in_bundle, dst_mirrors);
 }
 
 /* Optimized flow revalidation.
@@ -5272,7 +5201,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_,
 
         /* Convert string to datapath key. */
         ofpbuf_init(&odp_key, 0);
-        error = odp_flow_key_from_string(arg1, &odp_key);
+        error = odp_flow_key_from_string(arg1, NULL, &odp_key);
         if (error) {
             unixctl_command_reply(conn, 501, "Bad flow syntax");
             goto exit;
@@ -5404,7 +5333,7 @@ ofproto_dpif_unixctl_init(void)
     unixctl_command_register("fdb/flush", "bridge", ofproto_unixctl_fdb_flush,
                              NULL);
     unixctl_command_register("fdb/show", "bridge", ofproto_unixctl_fdb_show,
-                             NULL); 
+                             NULL);
     unixctl_command_register("ofproto/clog", "", ofproto_dpif_clog, NULL);
     unixctl_command_register("ofproto/unclog", "", ofproto_dpif_unclog, NULL);
 }