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
Fixes: d39ec23de384 ("ofproto-dpif: Don't slow-path controller actions.")
Reported-by: macman31
Reported-at: https://github.com/openvswitch/ovs-issues/issues/145
Reported-by: Brad Cowie
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047234.html
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif-trace.c | 91 +---
ofproto/ofproto-unixctl.man | 6 +--
2 files changed, 80 insertions(+), 17 deletions(-)
diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index e8fe0c6c17e9..2110fc713f0d 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -573,6 +573,74 @@ exit:
free_ct_states(_ct_states);
}
+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' (with length 'size') to 'out', dropping
+ * OVS_ACTION_ATTR_OUTPUT 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, _nested);
+
+size_t ofs = nl_msg_start_nested(out, OVS_ACTION_ATTR_CLONE);
+prune_output_actions(_nested, out);
+nl_msg_end_nested(out, ofs);
+} else if (a->nla_type != OVS_ACTION_ATTR_OUTPUT) {
+ofpbuf_put(out, a, NLA_ALIGN(a->nla_len));
+}
+}
+}
+
+/* Executes all of the datapath actions, except for any OVS_ACTION_ATTR_OUTPUT
+ * 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'.
+ *
+ * This is mainly useful to execute actions to send a packet to an OpenFlow
+ * controller. */
+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(_actions, 0);
+prune_output_actions(actions, _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, );
+if (error) {
+ds_put_format(output, "\nAction execution failed (%s)\n.",
+ ovs_strerror(error));
+}
+dp_packet_delete(execute.packet);
+ofpbuf_uninit(_actions);
+}
+
static void
ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow,
const struct dp_packet *packet, struct ovs_list *recirc_queue,
@@ -627,23 +695,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,
+ _flow, _actions,
+ xout.slow, output);
