[ovs-dev] [DPDK Upcalls 06/11] xlate: Allow callers to supply their own odp_actions buffer.

Ethan Jackson ethan at nicira.com
Sat Aug 2 01:39:17 UTC 2014


In future patches, this will allow dpif-netdev to avoid a copy.

Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
 ofproto/ofproto-dpif-upcall.c | 16 ++++-----
 ofproto/ofproto-dpif-xlate.c  | 75 +++++++++++++++++++++++--------------------
 ofproto/ofproto-dpif-xlate.h  |  8 ++++-
 ofproto/ofproto-dpif.c        | 10 +++---
 4 files changed, 61 insertions(+), 48 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 654fbd3..48393b0 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -977,7 +977,7 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
              * 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)) {
+            if (ofpbuf_size(upcall->xout.odp_actions)) {
                 eth_pop_vlan(packet);
             }
 
@@ -1020,8 +1020,8 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
             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);
+                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;
 
@@ -1034,15 +1034,15 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
             }
         }
 
-        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;
             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;
         }
     }
@@ -1284,8 +1284,8 @@ 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);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4aedb59..c816135 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1368,7 +1368,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 +2276,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 +2287,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 +2304,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 +2415,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 +2481,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 +2501,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 +2509,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 +2570,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 +2881,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 +2977,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 +2994,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 +3034,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 +3354,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);
 }
 
@@ -3798,13 +3798,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 +3839,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 +3888,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 +3952,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 +4089,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 +4097,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 +4118,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..3f6807d 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,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 980b04f..161b26a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3180,8 +3180,8 @@ ofproto_dpif_execute_actions(struct ofproto_dpif *ofproto,
     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;
@@ -4076,7 +4076,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);
@@ -4434,8 +4434,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;
-- 
1.8.1.2




More information about the dev mailing list