[ovs-dev] [DPDK Upcalls v3 2/3] ofproto: Reorganize in preparation for direct dpdk upcalls.

Ethan Jackson ethan at nicira.com
Thu Aug 14 01:46:17 UTC 2014


This patch reorganizes ofproto-dpif in preparation for future patches
which allow direct upcall processing from dpif-netdev.  The main goals
are to share as much code as possible between the dpif-linux and
dpif-netdev upcall paths.  Additionally, to avoid confusing the
dpif-netdev fast path, the packet processing path should treat packets
and struct flow's as const.

Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
 ofproto/ofproto-dpif-ipfix.c  |   8 +-
 ofproto/ofproto-dpif-ipfix.h  |   4 +-
 ofproto/ofproto-dpif-sflow.c  |   2 +-
 ofproto/ofproto-dpif-sflow.h  |   6 +-
 ofproto/ofproto-dpif-upcall.c | 555 ++++++++++++++++++++++++------------------
 ofproto/ofproto-dpif-upcall.h |   3 -
 ofproto/ofproto-dpif-xlate.c  | 132 +++++-----
 ofproto/ofproto-dpif-xlate.h  |  17 +-
 ofproto/ofproto-dpif.c        |  60 +++--
 ofproto/ofproto-dpif.h        |   3 +-
 10 files changed, 437 insertions(+), 353 deletions(-)

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 1584c25..ff498d0 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -1019,7 +1019,7 @@ ipfix_cache_update(struct dpif_ipfix_exporter *exporter,
 
 static void
 ipfix_cache_entry_init(struct ipfix_flow_cache_entry *entry,
-                       struct ofpbuf *packet, const struct flow *flow,
+                       const struct ofpbuf *packet, const struct flow *flow,
                        uint64_t packet_delta_count, uint32_t obs_domain_id,
                        uint32_t obs_point_id)
 {
@@ -1284,7 +1284,7 @@ ipfix_send_data_msg(struct dpif_ipfix_exporter *exporter,
 
 static void
 dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
-                  struct ofpbuf *packet, const struct flow *flow,
+                  const struct ofpbuf *packet, const struct flow *flow,
                   uint64_t packet_delta_count, uint32_t obs_domain_id,
                   uint32_t obs_point_id)
 {
@@ -1298,7 +1298,7 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
 }
 
 void
-dpif_ipfix_bridge_sample(struct dpif_ipfix *di, struct ofpbuf *packet,
+dpif_ipfix_bridge_sample(struct dpif_ipfix *di, const struct ofpbuf *packet,
                          const struct flow *flow) OVS_EXCLUDED(mutex)
 {
     uint64_t packet_delta_count;
@@ -1315,7 +1315,7 @@ dpif_ipfix_bridge_sample(struct dpif_ipfix *di, struct ofpbuf *packet,
 }
 
 void
-dpif_ipfix_flow_sample(struct dpif_ipfix *di, struct ofpbuf *packet,
+dpif_ipfix_flow_sample(struct dpif_ipfix *di, const struct ofpbuf *packet,
                        const struct flow *flow, uint32_t collector_set_id,
                        uint16_t probability, uint32_t obs_domain_id,
                        uint32_t obs_point_id) OVS_EXCLUDED(mutex)
diff --git a/ofproto/ofproto-dpif-ipfix.h b/ofproto/ofproto-dpif-ipfix.h
index 6ebf8b0..9de17ab 100644
--- a/ofproto/ofproto-dpif-ipfix.h
+++ b/ofproto/ofproto-dpif-ipfix.h
@@ -35,9 +35,9 @@ void dpif_ipfix_set_options(
     const struct ofproto_ipfix_bridge_exporter_options *,
     const struct ofproto_ipfix_flow_exporter_options *, size_t);
 
-void dpif_ipfix_bridge_sample(struct dpif_ipfix *, struct ofpbuf *,
+void dpif_ipfix_bridge_sample(struct dpif_ipfix *, const struct ofpbuf *,
                               const struct flow *);
-void dpif_ipfix_flow_sample(struct dpif_ipfix *, struct ofpbuf *,
+void dpif_ipfix_flow_sample(struct dpif_ipfix *, const struct ofpbuf *,
                             const struct flow *, uint32_t, uint16_t, uint32_t,
                             uint32_t);
 
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 69dfc03..32cf51b 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -563,7 +563,7 @@ dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *ds,
 }
 
 void
-dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet,
+dpif_sflow_received(struct dpif_sflow *ds, const struct ofpbuf *packet,
                     const struct flow *flow, odp_port_t odp_in_port,
                     const union user_action_cookie *cookie)
     OVS_EXCLUDED(mutex)
diff --git a/ofproto/ofproto-dpif-sflow.h b/ofproto/ofproto-dpif-sflow.h
index d53c95c..130568a 100644
--- a/ofproto/ofproto-dpif-sflow.h
+++ b/ofproto/ofproto-dpif-sflow.h
@@ -46,10 +46,8 @@ void dpif_sflow_del_port(struct dpif_sflow *, odp_port_t odp_port);
 void dpif_sflow_run(struct dpif_sflow *);
 void dpif_sflow_wait(struct dpif_sflow *);
 
-void dpif_sflow_received(struct dpif_sflow *,
-                         struct ofpbuf *,
-                         const struct flow *,
-                         odp_port_t odp_port,
+void dpif_sflow_received(struct dpif_sflow *, const struct ofpbuf *,
+                         const struct flow *, odp_port_t odp_port,
                          const union user_action_cookie *);
 
 int dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *,
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 66728e6..dd5262a 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -146,24 +146,32 @@ enum upcall_type {
 };
 
 struct upcall {
-    struct ofproto_dpif *ofproto;
+    struct ofproto_dpif *ofproto;  /* Parent ofproto. */
 
-    struct flow flow;
-    const struct nlattr *key;
-    size_t key_len;
-    enum dpif_upcall_type upcall_type;
-    struct dpif_flow_stats stats;
-    odp_port_t odp_in_port;
+    /* The flow and packet are only required to be constant when using
+     * dpif-netdev.  If a modification is absolutely necessary, a const cast
+     * may be used with other datapaths. */
+    const struct flow *flow;       /* Parsed representation of the packet. */
+    const struct ofpbuf *packet;   /* Packet associated with this upcall. */
+    ofp_port_t in_port;            /* OpenFlow in port, or OFPP_NONE. */
 
-    uint64_t slow_path_buf[128 / 8];
-    struct odputil_keybuf mask_buf;
+    enum dpif_upcall_type type;    /* Datapath type of the upcall. */
+    const struct nlattr *userdata; /* Userdata for DPIF_UC_ACTION Upcalls. */
+
+    bool xout_initialized;         /* True if 'xout' must be uninitialized. */
+    struct xlate_out xout;         /* Result of xlate_actions(). */
+    struct ofpbuf put_actions;     /* Actions 'put' in the fastapath. */
+
+    struct dpif_ipfix *ipfix;      /* IPFIX reference or NULL. */
+    struct dpif_sflow *sflow;      /* SFlow reference or NULL. */
+    struct netflow *netflow;       /* Netlow reference or NULL. */
 
-    struct xlate_out xout;
+    bool vsp_adjusted;             /* 'packet' and 'flow' were adjusted for
+                                      VLAN splinters if true. */
 
-    /* Raw upcall plus data for keeping track of the memory backing it. */
-    struct dpif_upcall dpif_upcall; /* As returned by dpif_recv() */
-    struct ofpbuf upcall_buf;       /* Owns some data in 'dpif_upcall'. */
-    uint64_t upcall_stub[512 / 8];  /* Buffer to reduce need for malloc(). */
+    /* Not used by the upcall callback interface. */
+    const struct nlattr *key;      /* Datapath flow key. */
+    size_t key_len;                /* Datapath flow key length. */
 };
 
 /* 'udpif_key's are responsible for tracking the little bit of state udpif
@@ -202,10 +210,9 @@ struct udpif_key {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 static struct list all_udpifs = LIST_INITIALIZER(&all_udpifs);
 
-static size_t read_upcalls(struct handler *,
-                           struct upcall upcalls[UPCALL_MAX_BATCH]);
-static void free_upcall(struct upcall *);
-static int convert_upcall(struct udpif *, struct upcall *);
+static size_t recv_upcalls(struct handler *);
+static int process_upcall(struct udpif *, struct upcall *,
+                          struct ofpbuf *odp_actions);
 static void handle_upcalls(struct udpif *, struct upcall *, size_t n_upcalls);
 static void udpif_stop_threads(struct udpif *);
 static void udpif_start_threads(struct udpif *, size_t n_handlers,
@@ -236,6 +243,16 @@ static bool ukey_acquire(struct udpif *udpif, const struct nlattr *key,
                          size_t key_len, long long int used,
                          struct udpif_key **result);
 static void ukey_delete(struct revalidator *, struct udpif_key *);
+static enum upcall_type classify_upcall(enum dpif_upcall_type type,
+                                        const struct nlattr *userdata);
+
+static void exec_upcalls(struct dpif *, struct dpif_upcall *, struct ofpbuf *,
+                         int cnt);
+
+static int upcall_receive(struct upcall *, const struct dpif_backer *,
+                          const struct ofpbuf *packet, enum dpif_upcall_type,
+                          const struct nlattr *userdata, const struct flow *);
+static void upcall_uninit(struct upcall *);
 
 static atomic_bool enable_megaflows = ATOMIC_VAR_INIT(true);
 
@@ -541,20 +558,10 @@ udpif_upcall_handler(void *arg)
     struct udpif *udpif = handler->udpif;
 
     while (!latch_is_set(&handler->udpif->exit_latch)) {
-        struct upcall upcalls[UPCALL_MAX_BATCH];
-        size_t n_upcalls, i;
-
-        n_upcalls = read_upcalls(handler, upcalls);
-        if (!n_upcalls) {
+        if (!recv_upcalls(handler)) {
             dpif_recv_wait(udpif->dpif, handler->handler_id);
             latch_wait(&udpif->exit_latch);
             poll_block();
-        } else {
-            handle_upcalls(handler->udpif, upcalls, n_upcalls);
-
-            for (i = 0; i < n_upcalls; i++) {
-                free_upcall(&upcalls[i]);
-            }
         }
         coverage_clear();
     }
@@ -562,6 +569,89 @@ udpif_upcall_handler(void *arg)
     return NULL;
 }
 
+static size_t
+recv_upcalls(struct handler *handler)
+{
+    struct udpif *udpif = handler->udpif;
+    uint64_t recv_stubs[UPCALL_MAX_BATCH][512 / 8];
+    struct ofpbuf recv_bufs[UPCALL_MAX_BATCH];
+    struct upcall upcalls[UPCALL_MAX_BATCH];
+    size_t n_upcalls, i;
+
+    n_upcalls = 0;
+    while (n_upcalls < UPCALL_MAX_BATCH) {
+        struct ofpbuf *recv_buf = &recv_bufs[n_upcalls];
+        struct upcall *upcall = &upcalls[n_upcalls];
+        struct dpif_upcall dupcall;
+        struct pkt_metadata md;
+        struct flow flow;
+        int error;
+
+        ofpbuf_use_stub(&recv_buf[n_upcalls], recv_stubs[n_upcalls],
+                        sizeof recv_stubs[n_upcalls]);
+        if (dpif_recv(udpif->dpif, handler->handler_id, &dupcall, recv_buf)) {
+            ofpbuf_uninit(recv_buf);
+            break;
+        }
+
+        if (odp_flow_key_to_flow(dupcall.key, dupcall.key_len, &flow)
+            == ODP_FIT_ERROR) {
+            goto free_dupcall;
+        }
+
+        error = upcall_receive(upcall, udpif->backer, &dupcall.packet,
+                               dupcall.type, dupcall.userdata, &flow);
+        if (error) {
+            if (error == ENODEV) {
+                /* Received packet on datapath port for which we couldn't
+                 * associate an ofproto.  This can happen if a port is removed
+                 * while traffic is being received.  Print a rate-limited
+                 * message in case it happens frequently. */
+                dpif_flow_put(udpif->dpif, DPIF_FP_CREATE, dupcall.key,
+                              dupcall.key_len, NULL, 0, NULL, 0, NULL);
+                VLOG_INFO_RL(&rl, "received packet on unassociated datapath "
+                             "port %"PRIu32, flow.in_port.odp_port);
+            }
+            goto cleanup;
+        }
+
+        upcall->key = dupcall.key;
+        upcall->key_len = dupcall.key_len;
+
+        if (vsp_adjust_flow(upcall->ofproto, &flow, &dupcall.packet)) {
+            upcall->vsp_adjusted = true;
+        }
+
+        md = pkt_metadata_from_flow(&flow);
+        flow_extract(&dupcall.packet, &md, &flow);
+
+        error = process_upcall(udpif, upcall, NULL);
+        if (error) {
+            goto cleanup;
+        }
+
+        n_upcalls++;
+        continue;
+
+cleanup:
+        upcall_uninit(upcall);
+free_dupcall:
+        ofpbuf_uninit(&dupcall.packet);
+        ofpbuf_uninit(recv_buf);
+    }
+
+    if (n_upcalls) {
+        handle_upcalls(handler->udpif, upcalls, n_upcalls);
+        for (i = 0; i < n_upcalls; i++) {
+            ofpbuf_uninit(CONST_CAST(struct ofpbuf *, upcalls[i].packet));
+            ofpbuf_uninit(&recv_bufs[i]);
+            upcall_uninit(&upcalls[i]);
+        }
+    }
+
+    return n_upcalls;
+}
+
 static void *
 udpif_revalidator(void *arg)
 {
@@ -650,14 +740,13 @@ udpif_revalidator(void *arg)
 }
 
 static enum upcall_type
-classify_upcall(const struct upcall *upcall)
+classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata)
 {
-    const struct dpif_upcall *dpif_upcall = &upcall->dpif_upcall;
     union user_action_cookie cookie;
     size_t userdata_len;
 
     /* First look at the upcall type. */
-    switch (dpif_upcall->type) {
+    switch (type) {
     case DPIF_UC_ACTION:
         break;
 
@@ -666,17 +755,16 @@ classify_upcall(const struct upcall *upcall)
 
     case DPIF_N_UC_TYPES:
     default:
-        VLOG_WARN_RL(&rl, "upcall has unexpected type %"PRIu32,
-                     dpif_upcall->type);
+        VLOG_WARN_RL(&rl, "upcall has unexpected type %"PRIu32, type);
         return BAD_UPCALL;
     }
 
     /* "action" upcalls need a closer look. */
-    if (!dpif_upcall->userdata) {
+    if (!userdata) {
         VLOG_WARN_RL(&rl, "action upcall missing cookie");
         return BAD_UPCALL;
     }
-    userdata_len = nl_attr_get_size(dpif_upcall->userdata);
+    userdata_len = nl_attr_get_size(userdata);
     if (userdata_len < sizeof cookie.type
         || userdata_len > sizeof cookie) {
         VLOG_WARN_RL(&rl, "action upcall cookie has unexpected size %"PRIuSIZE,
@@ -684,7 +772,7 @@ classify_upcall(const struct upcall *upcall)
         return BAD_UPCALL;
     }
     memset(&cookie, 0, sizeof cookie);
-    memcpy(&cookie, nl_attr_get(dpif_upcall->userdata), userdata_len);
+    memcpy(&cookie, nl_attr_get(userdata), userdata_len);
     if (userdata_len == MAX(8, sizeof cookie.sflow)
         && cookie.type == USER_ACTION_COOKIE_SFLOW) {
         return SFLOW_UPCALL;
@@ -708,7 +796,7 @@ classify_upcall(const struct upcall *upcall)
  * initialized with at least 128 bytes of space. */
 static void
 compose_slow_path(struct udpif *udpif, struct xlate_out *xout,
-                  struct flow *flow, odp_port_t odp_in_port,
+                  const struct flow *flow, odp_port_t odp_in_port,
                   struct ofpbuf *buf)
 {
     union user_action_cookie cookie;
@@ -726,31 +814,53 @@ compose_slow_path(struct udpif *udpif, struct xlate_out *xout,
     odp_put_userspace_action(pid, &cookie, sizeof cookie.slow_path, buf);
 }
 
+static int
+upcall_receive(struct upcall *upcall, const struct dpif_backer *backer,
+               const struct ofpbuf *packet, enum dpif_upcall_type type,
+               const struct nlattr *userdata, const struct flow *flow)
+{
+    int error;
+
+    error = xlate_receive(backer, flow, &upcall->ofproto, &upcall->ipfix,
+                          &upcall->sflow, &upcall->netflow,
+                          &upcall->in_port);
+    if (error) {
+        return error;
+    }
+
+    upcall->flow = flow;
+    upcall->packet = packet;
+    upcall->type = type;
+    upcall->userdata = userdata;
+    ofpbuf_init(&upcall->put_actions, 0);
+
+    upcall->xout_initialized = false;
+    upcall->vsp_adjusted = false;
+
+    upcall->key = NULL;
+    upcall->key_len = 0;
+
+    return 0;
+}
+
 static void
-upcall_init(struct upcall *upcall, struct flow *flow, struct ofpbuf *packet,
-            struct ofproto_dpif *ofproto, struct dpif_upcall *dupcall,
-            odp_port_t odp_in_port)
+upcall_xlate(struct udpif *udpif, struct upcall *upcall,
+             struct ofpbuf *odp_actions)
 {
-    struct pkt_metadata md = pkt_metadata_from_flow(flow);
+    struct dpif_flow_stats stats;
     struct xlate_in xin;
 
-    flow_extract(packet, &md, &upcall->flow);
+    stats.n_packets = 1;
+    stats.n_bytes = ofpbuf_size(upcall->packet);
+    stats.used = time_msec();
+    stats.tcp_flags = ntohs(upcall->flow->tcp_flags);
 
-    upcall->ofproto = ofproto;
-    upcall->key = dupcall->key;
-    upcall->key_len = dupcall->key_len;
-    upcall->upcall_type = dupcall->type;
-    upcall->stats.n_packets = 1;
-    upcall->stats.n_bytes = ofpbuf_size(packet);
-    upcall->stats.used = time_msec();
-    upcall->stats.tcp_flags = ntohs(upcall->flow.tcp_flags);
-    upcall->odp_in_port = odp_in_port;
+    xlate_in_init(&xin, upcall->ofproto, upcall->flow, upcall->in_port, NULL,
+                  stats.tcp_flags, upcall->packet);
+    xin.odp_actions = odp_actions;
 
-    xlate_in_init(&xin, upcall->ofproto, &upcall->flow, NULL,
-                  upcall->stats.tcp_flags, packet);
-
-    if (upcall->upcall_type == DPIF_UC_MISS) {
-        xin.resubmit_stats = &upcall->stats;
+    if (upcall->type == DPIF_UC_MISS) {
+        xin.resubmit_stats = &stats;
     } else {
         /* For non-miss upcalls, there's a flow in the datapath which this
          * packet was accounted to.  Presumably the revalidators will deal
@@ -758,14 +868,57 @@ upcall_init(struct upcall *upcall, struct flow *flow, struct ofpbuf *packet,
     }
 
     xlate_actions(&xin, &upcall->xout);
+    upcall->xout_initialized = true;
+
+    /* Special case for fail-open mode.
+     *
+     * If we are in fail-open mode, but we are connected to a controller too,
+     * then we should send the packet up to the controller in the hope that it
+     * will try to set up a flow and thereby allow us to exit fail-open.
+     *
+     * See the top-level comment in fail-open.c for more information.
+     *
+     * Copy packets before they are modified by execution. */
+    if (upcall->xout.fail_open) {
+        const struct ofpbuf *packet = upcall->packet;
+        struct ofproto_packet_in *pin;
+
+        pin = xmalloc(sizeof *pin);
+        pin->up.packet = xmemdup(ofpbuf_data(packet), ofpbuf_size(packet));
+        pin->up.packet_len = ofpbuf_size(packet);
+        pin->up.reason = OFPR_NO_MATCH;
+        pin->up.table_id = 0;
+        pin->up.cookie = OVS_BE64_MAX;
+        flow_get_metadata(upcall->flow, &pin->up.fmd);
+        pin->send_len = 0; /* Not used for flow table misses. */
+        pin->miss_type = OFPROTO_PACKET_IN_NO_MISS;
+        ofproto_dpif_send_packet_in(upcall->ofproto, pin);
+    }
+
+    if (!upcall->xout.slow) {
+        ofpbuf_use_const(&upcall->put_actions,
+                         ofpbuf_data(upcall->xout.odp_actions),
+                         ofpbuf_size(upcall->xout.odp_actions));
+    } else {
+        ofpbuf_init(&upcall->put_actions, 0);
+        compose_slow_path(udpif, &upcall->xout, upcall->flow,
+                          upcall->flow->in_port.odp_port,
+                          &upcall->put_actions);
+    }
 }
 
 static void
-free_upcall(struct upcall *upcall)
+upcall_uninit(struct upcall *upcall)
 {
-    xlate_out_uninit(&upcall->xout);
-    ofpbuf_uninit(&upcall->dpif_upcall.packet);
-    ofpbuf_uninit(&upcall->upcall_buf);
+    if (upcall) {
+        if (upcall->xout_initialized) {
+            xlate_out_uninit(&upcall->xout);
+        }
+        ofpbuf_uninit(&upcall->put_actions);
+        dpif_ipfix_unref(upcall->ipfix);
+        dpif_sflow_unref(upcall->sflow);
+        netflow_unref(upcall->netflow);
+    }
 }
 
 static struct udpif *
@@ -781,9 +934,9 @@ find_udpif(struct dpif *dpif)
     return NULL;
 }
 
-void
+static void
 exec_upcalls(struct dpif *dpif, struct dpif_upcall *dupcalls,
-             struct ofpbuf *bufs, int cnt)
+             struct ofpbuf *bufs OVS_UNUSED, int cnt)
 {
     struct upcall upcalls[UPCALL_MAX_BATCH];
     struct udpif *udpif;
@@ -797,156 +950,114 @@ exec_upcalls(struct dpif *dpif, struct dpif_upcall *dupcalls,
         for (j = i; j < MIN(i + UPCALL_MAX_BATCH, cnt); j++) {
             struct upcall *upcall = &upcalls[n_upcalls];
             struct dpif_upcall *dupcall = &dupcalls[j];
-            struct ofpbuf *buf = &bufs[j];
-
-            upcall->dpif_upcall = *dupcall;
-            upcall->upcall_buf = *buf;
+            struct pkt_metadata md;
+            struct flow flow;
+            int error;
 
             dpif_print_packet(dpif, dupcall);
-            if (!convert_upcall(udpif, upcall)) {
-                n_upcalls += 1;
+
+            if (odp_flow_key_to_flow(dupcall->key, dupcall->key_len, &flow)
+                == ODP_FIT_ERROR) {
+                continue;
             }
-        }
 
-        if (n_upcalls) {
-            handle_upcalls(udpif, upcalls, n_upcalls);
-            for (j = 0; j < n_upcalls; j++) {
-                free_upcall(&upcalls[j]);
+            error = upcall_receive(upcall, udpif->backer, &dupcall->packet,
+                                   dupcall->type, dupcall->userdata, &flow);
+            if (error) {
+                goto cleanup;
             }
-        }
-    }
-}
 
-/* Reads and classifies upcalls.  Returns the number of upcalls successfully
- * read. */
-static size_t
-read_upcalls(struct handler *handler,
-             struct upcall upcalls[UPCALL_MAX_BATCH])
-{
-    struct udpif *udpif = handler->udpif;
-    size_t i;
-    size_t n_upcalls = 0;
+            upcall->key = dupcall->key;
+            upcall->key_len = dupcall->key_len;
 
-    /* Try reading UPCALL_MAX_BATCH upcalls from dpif. */
-    for (i = 0; i < UPCALL_MAX_BATCH; i++) {
-        struct upcall *upcall = &upcalls[n_upcalls];
-        int error;
+            md = pkt_metadata_from_flow(&flow);
+            flow_extract(&dupcall->packet, &md, &flow);
 
-        ofpbuf_use_stub(&upcall->upcall_buf, upcall->upcall_stub,
-                        sizeof upcall->upcall_stub);
-        error = dpif_recv(udpif->dpif, handler->handler_id,
-                          &upcall->dpif_upcall, &upcall->upcall_buf);
-        if (error) {
-            ofpbuf_uninit(&upcall->upcall_buf);
-            break;
+            error = process_upcall(udpif, upcall, NULL);
+            if (error) {
+                goto cleanup;
+            }
+
+            n_upcalls++;
+            continue;
+
+cleanup:
+            upcall_uninit(upcall);
         }
 
-        if (!convert_upcall(udpif, upcall)) {
-            n_upcalls += 1;
+        if (n_upcalls) {
+            handle_upcalls(udpif, upcalls, n_upcalls);
+            for (j = 0; j < n_upcalls; j++) {
+                upcall_uninit(&upcalls[j]);
+            }
         }
     }
-    return n_upcalls;
 }
 
 static int
-convert_upcall(struct udpif *udpif, struct upcall *upcall)
+process_upcall(struct udpif *udpif, struct upcall *upcall,
+               struct ofpbuf *odp_actions)
 {
-    struct dpif_upcall *dupcall = &upcall->dpif_upcall;
-    struct ofpbuf *packet = &dupcall->packet;
-    struct ofproto_dpif *ofproto;
-    struct dpif_sflow *sflow;
-    struct dpif_ipfix *ipfix;
-    struct flow flow;
-    enum upcall_type type;
-    odp_port_t odp_in_port;
-    int error;
-
-    error = xlate_receive(udpif->backer, packet, dupcall->key,
-                          dupcall->key_len, &flow,
-                          &ofproto, &ipfix, &sflow, NULL, &odp_in_port);
-
-    if (error) {
-        if (error == ENODEV) {
-            /* Received packet on datapath port for which we couldn't
-             * associate an ofproto.  This can happen if a port is removed
-             * while traffic is being received.  Print a rate-limited
-             * message in case it happens frequently.  Install a drop flow
-             * so that future packets of the flow are inexpensively dropped
-             * in the kernel. */
-            VLOG_INFO_RL(&rl, "received packet on unassociated datapath "
-                         "port %"PRIu32, odp_in_port);
-            dpif_flow_put(udpif->dpif, DPIF_FP_CREATE,
-                          dupcall->key, dupcall->key_len, NULL, 0, NULL, 0,
-                          NULL);
-        }
-        goto destroy_upcall;
-    }
+    const struct nlattr *userdata = upcall->userdata;
+    const struct ofpbuf *packet = upcall->packet;
+    const struct flow *flow = upcall->flow;
 
-    type = classify_upcall(upcall);
-    if (type == MISS_UPCALL) {
-        upcall_init(upcall, &flow, packet, ofproto, dupcall, odp_in_port);
-        return error;
-    }
+    switch (classify_upcall(upcall->type, userdata)) {
+    case MISS_UPCALL:
+        upcall_xlate(udpif, upcall, odp_actions);
+        return 0;
 
-    switch (type) {
     case SFLOW_UPCALL:
-        if (sflow) {
+        if (upcall->sflow) {
             union user_action_cookie cookie;
 
             memset(&cookie, 0, sizeof cookie);
-            memcpy(&cookie, nl_attr_get(dupcall->userdata),
-                   sizeof cookie.sflow);
-            dpif_sflow_received(sflow, packet, &flow, odp_in_port,
-                                &cookie);
+            memcpy(&cookie, nl_attr_get(userdata), sizeof cookie.sflow);
+            dpif_sflow_received(upcall->sflow, packet, flow,
+                                flow->in_port.odp_port, &cookie);
         }
         break;
+
     case IPFIX_UPCALL:
-        if (ipfix) {
-            dpif_ipfix_bridge_sample(ipfix, packet, &flow);
+        if (upcall->ipfix) {
+            dpif_ipfix_bridge_sample(upcall->ipfix, packet, flow);
         }
         break;
+
     case FLOW_SAMPLE_UPCALL:
-        if (ipfix) {
+        if (upcall->ipfix) {
             union user_action_cookie cookie;
 
             memset(&cookie, 0, sizeof cookie);
-            memcpy(&cookie, nl_attr_get(dupcall->userdata),
-                   sizeof cookie.flow_sample);
+            memcpy(&cookie, nl_attr_get(userdata), sizeof cookie.flow_sample);
 
             /* The flow reflects exactly the contents of the packet.
              * Sample the packet using it. */
-            dpif_ipfix_flow_sample(ipfix, packet, &flow,
+            dpif_ipfix_flow_sample(upcall->ipfix, packet, flow,
                                    cookie.flow_sample.collector_set_id,
                                    cookie.flow_sample.probability,
                                    cookie.flow_sample.obs_domain_id,
                                    cookie.flow_sample.obs_point_id);
         }
         break;
+
     case BAD_UPCALL:
         break;
-    case MISS_UPCALL:
-        OVS_NOT_REACHED();
     }
 
-    dpif_ipfix_unref(ipfix);
-    dpif_sflow_unref(sflow);
-    error = EAGAIN;
-
-destroy_upcall:
-    ofpbuf_uninit(&upcall->dpif_upcall.packet);
-    ofpbuf_uninit(&upcall->upcall_buf);
-    return error;
+    return EAGAIN;
 }
 
 static void
 handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
                size_t n_upcalls)
 {
+    struct odputil_keybuf mask_bufs[UPCALL_MAX_BATCH];
     struct dpif_op *opsp[UPCALL_MAX_BATCH * 2];
     struct dpif_op ops[UPCALL_MAX_BATCH * 2];
-    size_t n_ops, i;
     unsigned int flow_limit;
-    bool fail_open, may_put;
+    size_t n_ops, i;
+    bool may_put;
 
     atomic_read(&udpif->flow_limit, &flow_limit);
     may_put = udpif_get_n_flows(udpif) < flow_limit;
@@ -961,32 +1072,25 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
      *
      * The loop fills 'ops' with an array of operations to execute in the
      * datapath. */
-    fail_open = false;
     n_ops = 0;
     for (i = 0; i < n_upcalls; i++) {
         struct upcall *upcall = &upcalls[i];
-        struct ofpbuf *packet = &upcall->dpif_upcall.packet;
+        const struct ofpbuf *packet = upcall->packet;
         struct dpif_op *op;
 
-        fail_open = fail_open || upcall->xout.fail_open;
-
-        if (upcall->flow.in_port.ofp_port
-            != vsp_realdev_to_vlandev(upcall->ofproto,
-                                      upcall->flow.in_port.ofp_port,
-                                      upcall->flow.vlan_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. */
-            if (ofpbuf_size(&upcall->xout.odp_actions)) {
-                eth_pop_vlan(packet);
+        if (upcall->vsp_adjusted) {
+            /* 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. */
+            if (ofpbuf_size(upcall->xout.odp_actions)) {
+                eth_pop_vlan(CONST_CAST(struct ofpbuf *, upcall->packet));
             }
 
             /* Remove the flow vlan tags inserted by vlan splinter logic
              * to ensure megaflow masks generated match the data path flow. */
-            upcall->flow.vlan_tci = 0;
+            CONST_CAST(struct flow *, upcall->flow)->vlan_tci = 0;
         }
 
         /* Do not install a flow into the datapath if:
@@ -995,13 +1099,12 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
          *
          *    - We received this packet via some flow installed in the kernel
          *      already. */
-        if (may_put
-            && upcall->dpif_upcall.type == DPIF_UC_MISS) {
+        if (may_put && upcall->type == DPIF_UC_MISS) {
             struct ofpbuf mask;
             bool megaflow;
 
             atomic_read(&enable_megaflows, &megaflow);
-            ofpbuf_use_stack(&mask, &upcall->mask_buf, sizeof upcall->mask_buf);
+            ofpbuf_use_stack(&mask, &mask_bufs[i], sizeof mask_bufs[i]);
             if (megaflow) {
                 size_t max_mpls;
                 bool recirc;
@@ -1009,7 +1112,7 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
                 recirc = ofproto_dpif_get_enable_recirc(upcall->ofproto);
                 max_mpls = ofproto_dpif_get_max_mpls_depth(upcall->ofproto);
                 odp_flow_key_from_mask(&mask, &upcall->xout.wc.masks,
-                                       &upcall->flow, UINT32_MAX, max_mpls,
+                                       upcall->flow, UINT32_MAX, max_mpls,
                                        recirc);
             }
 
@@ -1021,63 +1124,22 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
             op->u.flow_put.mask = ofpbuf_data(&mask);
             op->u.flow_put.mask_len = ofpbuf_size(&mask);
             op->u.flow_put.stats = NULL;
-
-            if (!upcall->xout.slow) {
-                op->u.flow_put.actions = ofpbuf_data(&upcall->xout.odp_actions);
-                op->u.flow_put.actions_len = ofpbuf_size(&upcall->xout.odp_actions);
-            } else {
-                struct ofpbuf buf;
-
-                ofpbuf_use_stack(&buf, upcall->slow_path_buf,
-                                 sizeof upcall->slow_path_buf);
-                compose_slow_path(udpif, &upcall->xout, &upcall->flow,
-                                  upcall->odp_in_port, &buf);
-                op->u.flow_put.actions = ofpbuf_data(&buf);
-                op->u.flow_put.actions_len = ofpbuf_size(&buf);
-            }
+            op->u.flow_put.actions = ofpbuf_data(&upcall->put_actions);
+            op->u.flow_put.actions_len = ofpbuf_size(&upcall->put_actions);
         }
 
-        if (ofpbuf_size(&upcall->xout.odp_actions)) {
-
+        if (ofpbuf_size(upcall->xout.odp_actions)) {
             op = &ops[n_ops++];
             op->type = DPIF_OP_EXECUTE;
-            op->u.execute.packet = packet;
+            op->u.execute.packet = CONST_CAST(struct ofpbuf *, packet);
             odp_key_to_pkt_metadata(upcall->key, upcall->key_len,
                                     &op->u.execute.md);
-            op->u.execute.actions = ofpbuf_data(&upcall->xout.odp_actions);
-            op->u.execute.actions_len = ofpbuf_size(&upcall->xout.odp_actions);
+            op->u.execute.actions = ofpbuf_data(upcall->xout.odp_actions);
+            op->u.execute.actions_len = ofpbuf_size(upcall->xout.odp_actions);
             op->u.execute.needs_help = (upcall->xout.slow & SLOW_ACTION) != 0;
         }
     }
 
-    /* Special case for fail-open mode.
-     *
-     * If we are in fail-open mode, but we are connected to a controller too,
-     * then we should send the packet up to the controller in the hope that it
-     * will try to set up a flow and thereby allow us to exit fail-open.
-     *
-     * See the top-level comment in fail-open.c for more information.
-     *
-     * Copy packets before they are modified by execution. */
-    if (fail_open) {
-        for (i = 0; i < n_upcalls; i++) {
-            struct upcall *upcall = &upcalls[i];
-            struct ofpbuf *packet = &upcall->dpif_upcall.packet;
-            struct ofproto_packet_in *pin;
-
-            pin = xmalloc(sizeof *pin);
-            pin->up.packet = xmemdup(ofpbuf_data(packet), ofpbuf_size(packet));
-            pin->up.packet_len = ofpbuf_size(packet);
-            pin->up.reason = OFPR_NO_MATCH;
-            pin->up.table_id = 0;
-            pin->up.cookie = OVS_BE64_MAX;
-            flow_get_metadata(&upcall->flow, &pin->up.fmd);
-            pin->send_len = 0; /* Not used for flow table misses. */
-            pin->miss_type = OFPROTO_PACKET_IN_NO_MISS;
-            ofproto_dpif_send_packet_in(upcall->ofproto, pin);
-        }
-    }
-
     /* Execute batch. */
     for (i = 0; i < n_ops; i++) {
         opsp[i] = &ops[i];
@@ -1219,7 +1281,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
     struct ofpbuf xout_actions;
     struct flow flow, dp_mask;
     uint32_t *dp32, *xout32;
-    odp_port_t odp_in_port;
+    ofp_port_t ofp_in_port;
     struct xlate_in xin;
     long long int last_used;
     int error;
@@ -1260,8 +1322,13 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
         goto exit;
     }
 
-    error = xlate_receive(udpif->backer, NULL, ukey->key, ukey->key_len, &flow,
-                          &ofproto, NULL, NULL, &netflow, &odp_in_port);
+    if (odp_flow_key_to_flow(ukey->key, ukey->key_len, &flow)
+        == ODP_FIT_ERROR) {
+        goto exit;
+    }
+
+    error = xlate_receive(udpif->backer, &flow, &ofproto, NULL, NULL, &netflow,
+                          &ofp_in_port);
     if (error) {
         goto exit;
     }
@@ -1273,7 +1340,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
         ukey->xcache = xlate_cache_new();
     }
 
-    xlate_in_init(&xin, ofproto, &flow, NULL, push.tcp_flags, NULL);
+    xlate_in_init(&xin, ofproto, &flow, ofp_in_port, NULL, push.tcp_flags,
+                  NULL);
     xin.resubmit_stats = push.n_packets ? &push : NULL;
     xin.xcache = ukey->xcache;
     xin.may_learn = may_learn;
@@ -1287,11 +1355,12 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
     }
 
     if (!xout.slow) {
-        ofpbuf_use_const(&xout_actions, ofpbuf_data(&xout.odp_actions),
-                         ofpbuf_size(&xout.odp_actions));
+        ofpbuf_use_const(&xout_actions, ofpbuf_data(xout.odp_actions),
+                         ofpbuf_size(xout.odp_actions));
     } else {
         ofpbuf_use_stack(&xout_actions, slow_path_buf, sizeof slow_path_buf);
-        compose_slow_path(udpif, &xout, &flow, odp_in_port, &xout_actions);
+        compose_slow_path(udpif, &xout, &flow, flow.in_port.odp_port,
+                          &xout_actions);
     }
 
     if (f->actions_len != ofpbuf_size(&xout_actions)
@@ -1375,6 +1444,7 @@ push_dump_ops__(struct udpif *udpif, struct dump_op *ops, size_t n_ops)
         if (push->n_packets || netflow_exists()) {
             struct ofproto_dpif *ofproto;
             struct netflow *netflow;
+            ofp_port_t ofp_in_port;
             struct flow flow;
             bool may_learn;
             int error;
@@ -1388,14 +1458,19 @@ push_dump_ops__(struct udpif *udpif, struct dump_op *ops, size_t n_ops)
             }
             ovs_mutex_unlock(&op->ukey->mutex);
 
-            error = xlate_receive(udpif->backer, NULL, op->op.u.flow_del.key,
-                                  op->op.u.flow_del.key_len, &flow, &ofproto,
-                                  NULL, NULL, &netflow, NULL);
+            if (odp_flow_key_to_flow(op->op.u.flow_del.key,
+                                     op->op.u.flow_del.key_len, &flow)
+                == ODP_FIT_ERROR) {
+                continue;
+            }
+
+            error = xlate_receive(udpif->backer, &flow, &ofproto,
+                                  NULL, NULL, &netflow, &ofp_in_port);
             if (!error) {
                 struct xlate_in xin;
 
-                xlate_in_init(&xin, ofproto, &flow, NULL, push->tcp_flags,
-                              NULL);
+                xlate_in_init(&xin, ofproto, &flow, ofp_in_port, NULL,
+                              push->tcp_flags, NULL);
                 xin.resubmit_stats = push->n_packets ? push : NULL;
                 xin.may_learn = may_learn;
                 xin.skip_wildcards = true;
diff --git a/ofproto/ofproto-dpif-upcall.h b/ofproto/ofproto-dpif-upcall.h
index 2b197ad..ec19bd0 100644
--- a/ofproto/ofproto-dpif-upcall.h
+++ b/ofproto/ofproto-dpif-upcall.h
@@ -28,9 +28,6 @@ struct simap;
  * them.  Additionally, it's responsible for maintaining the datapath flow
  * table. */
 
-void exec_upcalls(struct dpif *, struct dpif_upcall *, struct ofpbuf *,
-                  int cnt);
-
 struct udpif *udpif_create(struct dpif_backer *, struct dpif *);
 void udpif_run(struct udpif *udpif);
 void udpif_set_threads(struct udpif *, size_t n_handlers,
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4aedb59..b4ad649 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -934,23 +934,15 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
     xlate_xport_remove(new_xcfg, xport);
 }
 
-/* Given a datpath, packet, and flow metadata ('backer', 'packet', and 'key'
- * respectively), populates 'flow' with the result of odp_flow_key_to_flow().
- * Optionally populates 'ofproto' with the ofproto_dpif, 'odp_in_port' with
- * the datapath in_port, that 'packet' ingressed, and 'ipfix', 'sflow', and
- * 'netflow' with the appropriate handles for those protocols if they're
- * enabled.  Caller is responsible for unrefing them.
+/* Given a datapath and flow metadata ('backer', and 'flow' respectively),
+ * Optionally populates 'ofproto' with the ofproto_dpif, 'ofp_in_port' with the
+ * openflow in_port, and 'ipfix', 'sflow', and 'netflow' with the appropriate
+ * handles for those protocols if they're enabled.  Caller is responsible for
+ * unrefing them.
  *
  * If 'ofproto' is nonnull, requires 'flow''s in_port to exist.  Otherwise sets
  * 'flow''s in_port to OFPP_NONE.
  *
- * This function does post-processing on data returned from
- * odp_flow_key_to_flow() 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).
- *
  * Similarly, this function also includes some logic to help with tunnels.  It
  * may modify 'flow' as necessary to make the tunneling implementation
  * transparent to the upcall processing logic.
@@ -958,44 +950,25 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
  * Returns 0 if successful, ENODEV if the parsed flow has no associated ofport,
  * or some other positive errno if there are other problems. */
 int
-xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
-              const struct nlattr *key, size_t key_len, struct flow *flow,
+xlate_receive(const struct dpif_backer *backer, const struct flow *flow,
               struct ofproto_dpif **ofproto, struct dpif_ipfix **ipfix,
               struct dpif_sflow **sflow, struct netflow **netflow,
-              odp_port_t *odp_in_port)
+              ofp_port_t *ofp_in_port)
 {
     struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
-    int error = ENODEV;
     const struct xport *xport;
 
-    if (odp_flow_key_to_flow(key, key_len, flow) == ODP_FIT_ERROR) {
-        error = EINVAL;
-        return error;
-    }
-
-    if (odp_in_port) {
-        *odp_in_port = flow->in_port.odp_port;
-    }
-
     xport = xport_lookup(xcfg, tnl_port_should_receive(flow)
                          ? tnl_port_receive(flow)
                          : odp_port_to_ofport(backer, flow->in_port.odp_port));
 
-    flow->in_port.ofp_port = xport ? xport->ofp_port : OFPP_NONE;
-    if (!xport) {
-        return error;
+    if (ofp_in_port) {
+        *ofp_in_port = xport ? xport->ofp_port : OFPP_NONE;
     }
 
-    if (vsp_adjust_flow(xport->xbridge->ofproto, flow)) {
-        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'. */
-            eth_push_vlan(packet, htons(ETH_TYPE_VLAN), flow->vlan_tci);
-        }
+    if (!xport) {
+        return ENODEV;
     }
-    error = 0;
 
     if (ofproto) {
         *ofproto = xport->xbridge->ofproto;
@@ -1012,7 +985,7 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
     if (netflow) {
         *netflow = netflow_ref(xport->xbridge->netflow);
     }
-    return error;
+    return 0;
 }
 
 static struct xbridge *
@@ -1368,7 +1341,7 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct flow *orig_flow)
                          "%s, which is reserved exclusively for mirroring",
                          ctx->xbridge->name, in_xbundle->name);
         }
-        ofpbuf_clear(&ctx->xout->odp_actions);
+        ofpbuf_clear(ctx->xout->odp_actions);
         return;
     }
 
@@ -2276,7 +2249,7 @@ static void
 add_sflow_action(struct xlate_ctx *ctx)
 {
     ctx->user_cookie_offset = compose_sflow_action(ctx->xbridge,
-                                                   &ctx->xout->odp_actions,
+                                                   ctx->xout->odp_actions,
                                                    &ctx->xin->flow, ODPP_NONE);
     ctx->sflow_odp_port = 0;
     ctx->sflow_n_outputs = 0;
@@ -2287,7 +2260,7 @@ add_sflow_action(struct xlate_ctx *ctx)
 static void
 add_ipfix_action(struct xlate_ctx *ctx)
 {
-    compose_ipfix_action(ctx->xbridge, &ctx->xout->odp_actions,
+    compose_ipfix_action(ctx->xbridge, ctx->xout->odp_actions,
                          &ctx->xin->flow);
 }
 
@@ -2304,7 +2277,7 @@ fix_sflow_action(struct xlate_ctx *ctx)
         return;
     }
 
-    cookie = ofpbuf_at(&ctx->xout->odp_actions, ctx->user_cookie_offset,
+    cookie = ofpbuf_at(ctx->xout->odp_actions, ctx->user_cookie_offset,
                        sizeof cookie->sflow);
     ovs_assert(cookie->type == USER_ACTION_COOKIE_SFLOW);
 
@@ -2415,12 +2388,12 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
                 /* Forwarding is disabled by STP.  Let OFPP_NORMAL and the
                  * learning action look at the packet, then drop it. */
                 struct flow old_base_flow = ctx->base_flow;
-                size_t old_size = ofpbuf_size(&ctx->xout->odp_actions);
+                size_t old_size = ofpbuf_size(ctx->xout->odp_actions);
                 mirror_mask_t old_mirrors = ctx->xout->mirrors;
                 xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true);
                 ctx->xout->mirrors = old_mirrors;
                 ctx->base_flow = old_base_flow;
-                ofpbuf_set_size(&ctx->xout->odp_actions, old_size);
+                ofpbuf_set_size(ctx->xout->odp_actions, old_size);
             }
         }
 
@@ -2481,7 +2454,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         }
         out_port = odp_port;
         commit_odp_tunnel_action(flow, &ctx->base_flow,
-                                 &ctx->xout->odp_actions);
+                                 ctx->xout->odp_actions);
         flow->tunnel = flow_tnl; /* Restore tunnel metadata */
     } else {
         odp_port = xport->odp_port;
@@ -2501,7 +2474,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
 
     if (out_port != ODPP_NONE) {
         ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
-                                              &ctx->xout->odp_actions,
+                                              ctx->xout->odp_actions,
                                               &ctx->xout->wc);
 
         if (ctx->use_recirc) {
@@ -2509,17 +2482,17 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
             struct xlate_recirc *xr = &ctx->recirc;
 
             /* Hash action. */
-            act_hash = nl_msg_put_unspec_uninit(&ctx->xout->odp_actions,
+            act_hash = nl_msg_put_unspec_uninit(ctx->xout->odp_actions,
                                                 OVS_ACTION_ATTR_HASH,
                                                 sizeof *act_hash);
             act_hash->hash_alg = xr->hash_alg;
             act_hash->hash_basis = xr->hash_basis;
 
             /* Recirc action. */
-            nl_msg_put_u32(&ctx->xout->odp_actions, OVS_ACTION_ATTR_RECIRC,
+            nl_msg_put_u32(ctx->xout->odp_actions, OVS_ACTION_ATTR_RECIRC,
                            xr->recirc_id);
         } else {
-            nl_msg_put_odp_port(&ctx->xout->odp_actions, OVS_ACTION_ATTR_OUTPUT,
+            nl_msg_put_odp_port(ctx->xout->odp_actions, OVS_ACTION_ATTR_OUTPUT,
                                 out_port);
         }
 
@@ -2570,7 +2543,7 @@ xlate_resubmit_resource_check(struct xlate_ctx *ctx)
                     MAX_RESUBMIT_RECURSION);
     } else if (ctx->resubmits >= MAX_RESUBMITS + MAX_INTERNAL_RESUBMITS) {
         VLOG_ERR_RL(&rl, "over %d resubmit actions", MAX_RESUBMITS);
-    } else if (ofpbuf_size(&ctx->xout->odp_actions) > UINT16_MAX) {
+    } else if (ofpbuf_size(ctx->xout->odp_actions) > UINT16_MAX) {
         VLOG_ERR_RL(&rl, "resubmits yielded over 64 kB of actions");
     } else if (ofpbuf_size(&ctx->stack) >= 65536) {
         VLOG_ERR_RL(&rl, "resubmits yielded over 64 kB of stack");
@@ -2881,12 +2854,12 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
     packet = dpif_packet_clone_from_ofpbuf(ctx->xin->packet);
 
     ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
-                                          &ctx->xout->odp_actions,
+                                          ctx->xout->odp_actions,
                                           &ctx->xout->wc);
 
     odp_execute_actions(NULL, &packet, 1, false, &md,
-                        ofpbuf_data(&ctx->xout->odp_actions),
-                        ofpbuf_size(&ctx->xout->odp_actions), NULL);
+                        ofpbuf_data(ctx->xout->odp_actions),
+                        ofpbuf_size(ctx->xout->odp_actions), NULL);
 
     pin = xmalloc(sizeof *pin);
     pin->up.packet_len = ofpbuf_size(&packet->ofpbuf);
@@ -2977,9 +2950,9 @@ compose_recirculate_action(struct xlate_ctx *ctx,
     }
 
     ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
-                                          &ctx->xout->odp_actions,
+                                          ctx->xout->odp_actions,
                                           &ctx->xout->wc);
-    nl_msg_put_u32(&ctx->xout->odp_actions, OVS_ACTION_ATTR_RECIRC, id);
+    nl_msg_put_u32(ctx->xout->odp_actions, OVS_ACTION_ATTR_RECIRC, id);
 }
 
 static void
@@ -2994,7 +2967,7 @@ compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls)
     n = flow_count_mpls_labels(flow, wc);
     if (!n) {
         ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
-                                              &ctx->xout->odp_actions,
+                                              ctx->xout->odp_actions,
                                               &ctx->xout->wc);
     } else if (n >= FLOW_MAX_MPLS_LABELS) {
         if (ctx->xin->packet != NULL) {
@@ -3034,7 +3007,7 @@ compose_mpls_pop_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
                          ctx->xbridge->name, FLOW_MAX_MPLS_LABELS);
         }
         ctx->exit = true;
-        ofpbuf_clear(&ctx->xout->odp_actions);
+        ofpbuf_clear(ctx->xout->odp_actions);
     }
 }
 
@@ -3354,12 +3327,12 @@ xlate_sample_action(struct xlate_ctx *ctx,
   }
 
   ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
-                                        &ctx->xout->odp_actions,
+                                        ctx->xout->odp_actions,
                                         &ctx->xout->wc);
 
   compose_flow_sample_cookie(os->probability, os->collector_set_id,
                              os->obs_domain_id, os->obs_point_id, &cookie);
-  compose_sample_action(ctx->xbridge, &ctx->xout->odp_actions, &ctx->xin->flow,
+  compose_sample_action(ctx->xbridge, ctx->xout->odp_actions, &ctx->xin->flow,
                         probability, &cookie, sizeof cookie.flow_sample);
 }
 
@@ -3782,11 +3755,13 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
 
 void
 xlate_in_init(struct xlate_in *xin, struct ofproto_dpif *ofproto,
-              const struct flow *flow, struct rule_dpif *rule,
-              uint16_t tcp_flags, const struct ofpbuf *packet)
+              const struct flow *flow, ofp_port_t in_port,
+              struct rule_dpif *rule, uint16_t tcp_flags,
+              const struct ofpbuf *packet)
 {
     xin->ofproto = ofproto;
     xin->flow = *flow;
+    xin->flow.in_port.ofp_port = in_port;
     xin->packet = packet;
     xin->may_learn = packet != NULL;
     xin->rule = rule;
@@ -3798,13 +3773,14 @@ xlate_in_init(struct xlate_in *xin, struct ofproto_dpif *ofproto,
     xin->report_hook = NULL;
     xin->resubmit_stats = NULL;
     xin->skip_wildcards = false;
+    xin->odp_actions = NULL;
 }
 
 void
 xlate_out_uninit(struct xlate_out *xout)
 {
-    if (xout) {
-        ofpbuf_uninit(&xout->odp_actions);
+    if (xout && xout->odp_actions == &xout->odp_actions_buf) {
+        ofpbuf_uninit(xout->odp_actions);
     }
 }
 
@@ -3838,10 +3814,11 @@ xlate_out_copy(struct xlate_out *dst, const struct xlate_out *src)
     dst->nf_output_iface = src->nf_output_iface;
     dst->mirrors = src->mirrors;
 
-    ofpbuf_use_stub(&dst->odp_actions, dst->odp_actions_stub,
+    dst->odp_actions = &dst->odp_actions_buf;
+    ofpbuf_use_stub(dst->odp_actions, dst->odp_actions_stub,
                     sizeof dst->odp_actions_stub);
-    ofpbuf_put(&dst->odp_actions, ofpbuf_data(&src->odp_actions),
-               ofpbuf_size(&src->odp_actions));
+    ofpbuf_put(dst->odp_actions, ofpbuf_data(src->odp_actions),
+               ofpbuf_size(src->odp_actions));
 }
 
 static struct skb_priority_to_dscp *
@@ -3886,8 +3863,8 @@ actions_output_to_local_port(const struct xlate_ctx *ctx)
     const struct nlattr *a;
     unsigned int left;
 
-    NL_ATTR_FOR_EACH_UNSAFE (a, left, ofpbuf_data(&ctx->xout->odp_actions),
-                             ofpbuf_size(&ctx->xout->odp_actions)) {
+    NL_ATTR_FOR_EACH_UNSAFE (a, left, ofpbuf_data(ctx->xout->odp_actions),
+                             ofpbuf_size(ctx->xout->odp_actions)) {
         if (nl_attr_type(a) == OVS_ACTION_ATTR_OUTPUT
             && nl_attr_get_odp_port(a) == local_odp_port) {
             return true;
@@ -3950,9 +3927,14 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
     ctx.xout->has_fin_timeout = false;
     ctx.xout->nf_output_iface = NF_OUT_DROP;
     ctx.xout->mirrors = 0;
-    ofpbuf_use_stub(&ctx.xout->odp_actions, ctx.xout->odp_actions_stub,
-                    sizeof ctx.xout->odp_actions_stub);
-    ofpbuf_reserve(&ctx.xout->odp_actions, NL_A_U32_SIZE);
+
+    xout->odp_actions = xin->odp_actions;
+    if (!xout->odp_actions) {
+        xout->odp_actions = &xout->odp_actions_buf;
+        ofpbuf_use_stub(xout->odp_actions, xout->odp_actions_stub,
+                        sizeof xout->odp_actions_stub);
+    }
+    ofpbuf_reserve(xout->odp_actions, NL_A_U32_SIZE);
 
     ctx.xbridge = xbridge_lookup(xcfg, xin->ofproto);
     if (!ctx.xbridge) {
@@ -4082,7 +4064,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
         add_sflow_action(&ctx);
         add_ipfix_action(&ctx);
-        sample_actions_len = ofpbuf_size(&ctx.xout->odp_actions);
+        sample_actions_len = ofpbuf_size(ctx.xout->odp_actions);
 
         if (tnl_may_send && (!in_port || may_receive(in_port, &ctx))) {
             do_xlate_actions(ofpacts, ofpacts_len, &ctx);
@@ -4090,7 +4072,7 @@ 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. */
             if (in_port && !xport_stp_forward_state(in_port)) {
-                ofpbuf_set_size(&ctx.xout->odp_actions, sample_actions_len);
+                ofpbuf_set_size(ctx.xout->odp_actions, sample_actions_len);
             }
         }
 
@@ -4111,7 +4093,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         }
     }
 
-    if (nl_attr_oversized(ofpbuf_size(&ctx.xout->odp_actions))) {
+    if (nl_attr_oversized(ofpbuf_size(ctx.xout->odp_actions))) {
         /* These datapath actions are too big for a Netlink attribute, so we
          * can't hand them to the kernel directly.  dpif_execute() can execute
          * them one by one with help, so just mark the result as SLOW_ACTION to
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 4bdf2d3..7394249 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -59,7 +59,8 @@ struct xlate_out {
     mirror_mask_t mirrors;      /* Bitmap of associated mirrors. */
 
     uint64_t odp_actions_stub[256 / 8];
-    struct ofpbuf odp_actions;
+    struct ofpbuf odp_actions_buf;
+    struct ofpbuf *odp_actions;
 };
 
 struct xlate_in {
@@ -135,6 +136,11 @@ struct xlate_in {
      * This is normally null so the client has to set it manually after
      * calling xlate_in_init(). */
     struct xlate_cache *xcache;
+
+    /* Allows callers to optionally supply their own buffer for the resulting
+     * odp_actions stored in xlate_out.  If NULL, the default buffer will be
+     * used. */
+    struct ofpbuf *odp_actions;
 };
 
 void xlate_ofproto_set(struct ofproto_dpif *, const char *name,
@@ -167,15 +173,14 @@ void xlate_ofport_set(struct ofproto_dpif *, struct ofbundle *,
                       bool may_enable);
 void xlate_ofport_remove(struct ofport_dpif *);
 
-int xlate_receive(const struct dpif_backer *, struct ofpbuf *packet,
-                  const struct nlattr *key, size_t key_len,
-                  struct flow *, struct ofproto_dpif **, struct dpif_ipfix **,
+int xlate_receive(const struct dpif_backer *, const struct flow *,
+                  struct ofproto_dpif **, struct dpif_ipfix **,
                   struct dpif_sflow **, struct netflow **,
-                  odp_port_t *odp_in_port);
+                  ofp_port_t *ofp_in_port);
 
 void xlate_actions(struct xlate_in *, struct xlate_out *);
 void xlate_in_init(struct xlate_in *, struct ofproto_dpif *,
-                   const struct flow *, struct rule_dpif *,
+                   const struct flow *, ofp_port_t in_port, struct rule_dpif *,
                    uint16_t tcp_flags, const struct ofpbuf *packet);
 void xlate_out_uninit(struct xlate_out *);
 void xlate_actions_for_side_effects(struct xlate_in *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index e17377f..7cf85f7 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3154,14 +3154,15 @@ ofproto_dpif_execute_actions(struct ofproto_dpif *ofproto,
         rule_dpif_credit_stats(rule, &stats);
     }
 
-    xlate_in_init(&xin, ofproto, flow, rule, stats.tcp_flags, packet);
+    xlate_in_init(&xin, ofproto, flow, flow->in_port.ofp_port, rule,
+                  stats.tcp_flags, packet);
     xin.ofpacts = ofpacts;
     xin.ofpacts_len = ofpacts_len;
     xin.resubmit_stats = &stats;
     xlate_actions(&xin, &xout);
 
-    execute.actions = ofpbuf_data(&xout.odp_actions);
-    execute.actions_len = ofpbuf_size(&xout.odp_actions);
+    execute.actions = ofpbuf_data(xout.odp_actions);
+    execute.actions_len = ofpbuf_size(xout.odp_actions);
     execute.packet = packet;
     execute.md = pkt_metadata_from_flow(flow);
     execute.needs_help = (xout.slow & SLOW_ACTION) != 0;
@@ -4056,7 +4057,7 @@ static void
 trace_format_odp(struct ds *result, int level, const char *title,
                  struct trace_ctx *trace)
 {
-    struct ofpbuf *odp_actions = &trace->xout.odp_actions;
+    struct ofpbuf *odp_actions = trace->xout.odp_actions;
 
     ds_put_char_multiple(result, '\t', level);
     ds_put_format(result, "%s: ", title);
@@ -4185,12 +4186,20 @@ parse_flow_and_packet(int argc, const char *argv[],
             goto exit;
         }
 
-        if (xlate_receive(backer, NULL, ofpbuf_data(&odp_key),
-                          ofpbuf_size(&odp_key), flow,
-                          ofprotop, NULL, NULL, NULL, NULL)) {
+        if (odp_flow_key_to_flow(ofpbuf_data(&odp_key), ofpbuf_size(&odp_key),
+                                 flow) == ODP_FIT_ERROR) {
+            error = "Failed to parse flow key";
+            goto exit;
+        }
+
+        if (xlate_receive(backer, flow, ofprotop, NULL, NULL, NULL,
+                          &flow->in_port.ofp_port)) {
             error = "Invalid datapath flow";
             goto exit;
         }
+
+        vsp_adjust_flow(*ofprotop, flow, NULL);
+
     } else {
         char *err = parse_ofp_exact_flow(flow, NULL, argv[argc - 1], NULL);
 
@@ -4398,8 +4407,8 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
         trace.result = ds;
         trace.key = flow; /* Original flow key, used for megaflow. */
         trace.flow = *flow; /* May be modified by actions. */
-        xlate_in_init(&trace.xin, ofproto, flow, rule, ntohs(flow->tcp_flags),
-                      packet);
+        xlate_in_init(&trace.xin, ofproto, flow, flow->in_port.ofp_port, rule,
+                      ntohs(flow->tcp_flags), packet);
         if (ofpacts) {
             trace.xin.ofpacts = ofpacts;
             trace.xin.ofpacts_len = ofpacts_len;
@@ -4414,8 +4423,8 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
         trace_format_megaflow(ds, 0, "Megaflow", &trace);
 
         ds_put_cstr(ds, "Datapath actions: ");
-        format_odp_actions(ds, ofpbuf_data(&trace.xout.odp_actions),
-                           ofpbuf_size(&trace.xout.odp_actions));
+        format_odp_actions(ds, ofpbuf_data(trace.xout.odp_actions),
+                           ofpbuf_size(trace.xout.odp_actions));
 
         if (trace.xout.slow) {
             enum slow_path_reason slow;
@@ -4572,8 +4581,14 @@ ofproto_dpif_contains_flow(const struct ofproto_dpif *ofproto,
     struct ofproto_dpif *ofp;
     struct flow flow;
 
-    xlate_receive(ofproto->backer, NULL, key, key_len, &flow, &ofp,
-                  NULL, NULL, NULL, NULL);
+    if (odp_flow_key_to_flow(key, key_len, &flow) == ODP_FIT_ERROR) {
+        return false;
+    }
+
+    if (xlate_receive(ofproto->backer, &flow, &ofp, NULL, NULL, NULL, NULL)) {
+        return false;
+    }
+
     return ofp == ofproto;
 }
 
@@ -4829,11 +4844,13 @@ vsp_vlandev_to_realdev(const struct ofproto_dpif *ofproto,
 /* Given 'flow', a flow representing a packet received on 'ofproto', checks
  * whether 'flow->in_port' represents a Linux VLAN device.  If so, changes
  * 'flow->in_port' to the "real" device backing the VLAN device, sets
- * 'flow->vlan_tci' to the VLAN VID, and returns true.  Otherwise (which is
- * always the case unless VLAN splinters are enabled), returns false without
- * making any changes. */
+ * 'flow->vlan_tci' to the VLAN VID, and returns true.  Optionally pushes the
+ * appropriate VLAN on 'packet' if provided.  Otherwise (which is always the
+ * case unless VLAN splinters are enabled), returns false without making any
+ * changes. */
 bool
-vsp_adjust_flow(const struct ofproto_dpif *ofproto, struct flow *flow)
+vsp_adjust_flow(const struct ofproto_dpif *ofproto, struct flow *flow,
+                struct ofpbuf *packet)
     OVS_EXCLUDED(ofproto->vsp_mutex)
 {
     ofp_port_t realdev;
@@ -4855,6 +4872,15 @@ vsp_adjust_flow(const struct ofproto_dpif *ofproto, struct flow *flow)
      * the VLAN device's VLAN ID. */
     flow->in_port.ofp_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'. */
+        eth_push_vlan(packet, htons(ETH_TYPE_VLAN), flow->vlan_tci);
+    }
+
     return true;
 }
 
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 2f150b5..a8c5d48 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -143,7 +143,8 @@ bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
 ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
                                   ofp_port_t realdev_ofp_port,
                                   ovs_be16 vlan_tci);
-bool vsp_adjust_flow(const struct ofproto_dpif *, struct flow *);
+bool vsp_adjust_flow(const struct ofproto_dpif *, struct flow *,
+                     struct ofpbuf *packet);
 
 int ofproto_dpif_execute_actions(struct ofproto_dpif *, const struct flow *,
                                  struct rule_dpif *, const struct ofpact *,
-- 
1.8.1.2




More information about the dev mailing list