ofproto-dpif: Avoid extra flow copy in xlate_actions() for unneeded warnings.

The copy of the extra flow copy here was showing up in profiles.  We only
need this copy if we end up doing a "trace" to warn the user.  Most runs
won't ever do that, so don't start making copies until we actually hit
such a case.

This has a small behavioral change in that we'll only get a warning on the
*second* time we hit the resubmit recursion limit, not on the first.  I
doubt that's really a problem.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 84bd66765c9d4f8b7f7bb8d83661f420bc1e49a0..1c5d331259198a4e39eaebf11afb5478fe6a7a84 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5310,6 +5310,11 @@ xlate_actions(struct action_xlate_ctx *ctx,
               const union ofp_action *in, size_t n_in,
               struct ofpbuf *odp_actions)
 {
+    /* Normally false.  Set to true if we ever hit MAX_RESUBMIT_RECURSION, so
+     * that in the future we always keep a copy of the original flow for
+     * tracing purposes. */
+    static bool hit_resubmit_limit;
+
     COVERAGE_INC(ofproto_dpif_xlate);
 
     ofpbuf_clear(odp_actions);
@@ -5329,7 +5334,7 @@ xlate_actions(struct action_xlate_ctx *ctx,
     ctx->table_id = 0;
     ctx->exit = false;
 
-    if (ctx->ofproto->has_mirrors) {
+    if (ctx->ofproto->has_mirrors || hit_resubmit_limit) {
         /* Do this conditionally because the copy is expensive enough that it
          * shows up in profiles.
          *
@@ -5366,21 +5371,25 @@ xlate_actions(struct action_xlate_ctx *ctx,
         ctx->may_set_up_flow = false;
     } else {
         static struct vlog_rate_limit trace_rl = VLOG_RATE_LIMIT_INIT(1, 1);
-        struct flow original_flow = ctx->flow;
         ovs_be16 initial_tci = ctx->base_flow.vlan_tci;
 
         add_sflow_action(ctx);
         do_xlate_actions(in, n_in, ctx);
 
-        if (ctx->max_resubmit_trigger && !ctx->resubmit_hook
-            && !VLOG_DROP_ERR(&trace_rl)) {
-            struct ds ds = DS_EMPTY_INITIALIZER;
-
-            ofproto_trace(ctx->ofproto, &original_flow, ctx->packet,
-                          initial_tci, &ds);
-            VLOG_ERR("Trace triggered by excessive resubmit recursion:\n%s",
-                     ds_cstr(&ds));
-            ds_destroy(&ds);
+        if (ctx->max_resubmit_trigger && !ctx->resubmit_hook) {
+            if (!hit_resubmit_limit) {
+                /* We didn't record the original flow.  Make sure we do from
+                 * now on. */
+                hit_resubmit_limit = true;
+            } else if (!VLOG_DROP_ERR(&trace_rl)) {
+                struct ds ds = DS_EMPTY_INITIALIZER;
+
+                ofproto_trace(ctx->ofproto, &ctx->orig_flow, ctx->packet,
+                              initial_tci, &ds);
+                VLOG_ERR("Trace triggered by excessive resubmit "
+                         "recursion:\n%s", ds_cstr(&ds));
+                ds_destroy(&ds);
+            }
         }
 
         if (!connmgr_may_set_up_flow(ctx->ofproto->up.connmgr, &ctx->flow,