ofproto-dpif-trace: Make -generate send packets to controller again.

Prior to the OVS 2.9 development cycle, any flow that sent a packet to a
controller required that the flow be slow-pathed.  In some cases this led
to poor performance, so OVS 2.9 made controller actions fast-pathable.  As
a side effect of the change, "ovs-appctl ofproto/trace -generate" no longer
sent packets to the controller.  This usually didn't matter but it broke
the Faucet tutorial, which relied on this behavior.  This commit
reintroduces the original behavior and thus should fix the tutorial.

CC: Justin Pettit <jpettit@ovn.org>
Fixes: d39ec23de384 ("ofproto-dpif: Don't slow-path controller actions.")
Reported-by: macman31 <https://github.com/macman31>
Reported-at: https://github.com/openvswitch/ovs-issues/issues/145
Reported-by: Brad Cowie <brad@cowie.nz>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047234.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>

1 files changed, 79 insertions, 14 deletions

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index e8fe0c6c1..eca61cee2 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -574,6 +574,76 @@ exit:
 }
 
 static void
+explain_slow_path(enum slow_path_reason slow, struct ds *output)
+{
+    ds_put_cstr(output, "\nThis flow is handled by the userspace "
+                "slow path because it:");
+    for (; slow; slow = zero_rightmost_1bit(slow)) {
+        enum slow_path_reason bit = rightmost_1bit(slow);
+        ds_put_format(output, "\n  - %s.",
+                      slow_path_reason_to_explanation(bit));
+    }
+}
+
+/* Copies ODP actions from 'in' to 'out', dropping OVS_ACTION_ATTR_OUTPUT and
+ * OVS_ACTION_ATTR_RECIRC along the way. */
+static void
+prune_output_actions(const struct ofpbuf *in, struct ofpbuf *out)
+{
+    const struct nlattr *a;
+    unsigned int left;
+    NL_ATTR_FOR_EACH (a, left, in->data, in->size) {
+        if (a->nla_type == OVS_ACTION_ATTR_CLONE) {
+            struct ofpbuf in_nested;
+            nl_attr_get_nested(a, &in_nested);
+
+            size_t ofs = nl_msg_start_nested(out, OVS_ACTION_ATTR_CLONE);
+            prune_output_actions(&in_nested, out);
+            nl_msg_end_nested(out, ofs);
+        } else if (a->nla_type != OVS_ACTION_ATTR_OUTPUT &&
+                   a->nla_type != OVS_ACTION_ATTR_RECIRC) {
+            ofpbuf_put(out, a, NLA_ALIGN(a->nla_len));
+        }
+    }
+}
+
+/* Executes all of the datapath actions, except for any OVS_ACTION_ATTR_OUTPUT
+ * and OVS_ACTION_ATTR_RECIRC actions, in 'actions' on 'packet', which has the
+ * given 'flow', on 'dpif'.  The actions have slow path reason 'slow' (if any).
+ * Appends any error message to 'output'.
+ *
+ * With output and recirculation actions dropped, the only remaining side
+ * effects are from OVS_ACTION_ATTR_USERSPACE actions for executing actions to
+ * send a packet to an OpenFlow controller, IPFIX, NetFlow, and sFlow, etc. */
+static void
+execute_actions_except_outputs(struct dpif *dpif,
+                               const struct dp_packet *packet,
+                               const struct flow *flow,
+                               const struct ofpbuf *actions,
+                               enum slow_path_reason slow,
+                               struct ds *output)
+{
+    struct ofpbuf pruned_actions;
+    ofpbuf_init(&pruned_actions, 0);
+    prune_output_actions(actions, &pruned_actions);
+
+    struct dpif_execute execute = {
+        .actions = pruned_actions.data,
+        .actions_len = pruned_actions.size,
+        .needs_help = (slow & SLOW_ACTION) != 0,
+        .flow = flow,
+        .packet = dp_packet_clone_with_headroom(packet, 2),
+    };
+    int error = dpif_execute(dpif, &execute);
+    if (error) {
+        ds_put_format(output, "\nAction execution failed (%s)\n.",
+                      ovs_strerror(error));
+    }
+    dp_packet_delete(execute.packet);
+    ofpbuf_uninit(&pruned_actions);
+}
+
+static void
 ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow,
                 const struct dp_packet *packet, struct ovs_list *recirc_queue,
                 const struct ofpact ofpacts[], size_t ofpacts_len,
@@ -627,23 +697,18 @@ ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow,
         ds_put_format(output,
                       "\nTranslation failed (%s), packet is dropped.\n",
                       xlate_strerror(error));
-    } else if (xout.slow) {
-        enum slow_path_reason slow;
-
-        ds_put_cstr(output, "\nThis flow is handled by the userspace "
-                    "slow path because it:");
-
-        slow = xout.slow;
-        while (slow) {
-            enum slow_path_reason bit = rightmost_1bit(slow);
-
-            ds_put_format(output, "\n  - %s.",
-                          slow_path_reason_to_explanation(bit));
-
-            slow &= ~bit;
+    } else {
+        if (xout.slow) {
+            explain_slow_path(xout.slow, output);
+        }
+        if (packet) {
+            execute_actions_except_outputs(ofproto->backer->dpif, packet,
+                                           &initial_flow, &odp_actions,
+                                           xout.slow, output);
         }
     }
 
+
     xlate_out_uninit(&xout);
     ofpbuf_uninit(&odp_actions);
     oftrace_node_list_destroy(&trace);