From patchwork Fri Aug 24 19:25:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 962002 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41xrq606X4z9s3C for ; Sat, 25 Aug 2018 05:25:52 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 011BCF83; Fri, 24 Aug 2018 19:25:50 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 598A2F7E for ; Fri, 24 Aug 2018 19:25:48 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 57E557DB for ; Fri, 24 Aug 2018 19:25:46 +0000 (UTC) Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id 59E1A240003; Fri, 24 Aug 2018 19:25:42 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Fri, 24 Aug 2018 12:25:39 -0700 Message-Id: <20180824192539.5286-1-blp@ovn.org> X-Mailer: git-send-email 2.16.1 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff , Brad Cowie Subject: [ovs-dev] [PATCH] ofproto-dpif-trace: Make -generate send packets to controller again. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org 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 Acked-by: Justin Pettit --- 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(&next_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, &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) { + 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(&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, @@ -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, + &initial_flow, &odp_actions, + xout.slow, output); } } + xlate_out_uninit(&xout); ofpbuf_uninit(&odp_actions); oftrace_node_list_destroy(&trace); diff --git a/ofproto/ofproto-unixctl.man b/ofproto/ofproto-unixctl.man index dede5f39da84..2db4fe6330f9 100644 --- a/ofproto/ofproto-unixctl.man +++ b/ofproto/ofproto-unixctl.man @@ -126,9 +126,9 @@ effects when a packet is specified. If you want side effects to take place, then you must supply a packet. . .IP -(Side effects when tracing do not have external consequences. Even if a -packet is specified, a trace will not output a packet or generate sFlow, -NetFlow or controller events.) +(Output actions are obviously side effects too, but +the trace commands never execute them, even when one specifies a +packet.) . .IP "Incomplete information." Most of the time, Open vSwitch can figure out everything about the