From patchwork Wed Jan 10 22:27:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 858590 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=) 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 3zH3ZQ5YkHz9s7g for ; Thu, 11 Jan 2018 09:28:46 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id DC5DEFFF; Wed, 10 Jan 2018 22:28:18 +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 62D14EED for ; Wed, 10 Jan 2018 22:28:17 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 8E2B7E3 for ; Wed, 10 Jan 2018 22:28:15 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 61122C5A51 for ; Wed, 10 Jan 2018 23:28:13 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 10 Jan 2018 14:27:19 -0800 Message-Id: <1515623246-3820-1-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 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 Subject: [ovs-dev] [no-slow v2 1/8] ofproto-dpif: Use a fixed size userspace cookie. 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 This simplifies the cookie handling a bit. Signed-off-by: Justin Pettit Acked-by: Ben Pfaff Tested-by: Greg Rose --- v1->v2: New to the series. --- lib/odp-util.c | 35 ++++++++------------------ lib/odp-util.h | 58 ++++++++++++++++++++++--------------------- ofproto/ofproto-dpif-ipfix.c | 2 +- ofproto/ofproto-dpif-ipfix.h | 3 ++- ofproto/ofproto-dpif-sflow.c | 14 +++++------ ofproto/ofproto-dpif-sflow.h | 8 +++--- ofproto/ofproto-dpif-upcall.c | 53 +++++++++++++++++++-------------------- ofproto/ofproto-dpif-xlate.c | 50 ++++++++++++++++--------------------- tests/odp.at | 2 -- 9 files changed, 103 insertions(+), 122 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index ff08821595fd..2910e1514985 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -437,31 +437,25 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, const uint8_t *userdata = nl_attr_get(userdata_attr); size_t userdata_len = nl_attr_get_size(userdata_attr); bool userdata_unspec = true; - union user_action_cookie cookie; + struct user_action_cookie cookie; - if (userdata_len >= sizeof cookie.type - && userdata_len <= sizeof cookie) { - - memset(&cookie, 0, sizeof cookie); - memcpy(&cookie, userdata, userdata_len); + if (userdata_len == sizeof cookie) { + memcpy(&cookie, userdata, sizeof cookie); userdata_unspec = false; - if (userdata_len == sizeof cookie.sflow - && cookie.type == USER_ACTION_COOKIE_SFLOW) { + if (cookie.type == USER_ACTION_COOKIE_SFLOW) { ds_put_format(ds, ",sFlow(" "vid=%"PRIu16",pcp=%d,output=%"PRIu32")", vlan_tci_to_vid(cookie.sflow.vlan_tci), vlan_tci_to_pcp(cookie.sflow.vlan_tci), cookie.sflow.output); - } else if (userdata_len == sizeof cookie.slow_path - && cookie.type == USER_ACTION_COOKIE_SLOW_PATH) { + } else if (cookie.type == USER_ACTION_COOKIE_SLOW_PATH) { ds_put_cstr(ds, ",slow_path("); format_flags(ds, slow_path_reason_to_string, cookie.slow_path.reason, ','); ds_put_format(ds, ")"); - } else if (userdata_len == sizeof cookie.flow_sample - && cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { + } else if (cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { ds_put_format(ds, ",flow_sample(probability=%"PRIu16 ",collector_set_id=%"PRIu32 ",obs_domain_id=%"PRIu32 @@ -479,8 +473,7 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, ds_put_cstr(ds, ",egress"); } ds_put_char(ds, ')'); - } else if (userdata_len >= sizeof cookie.ipfix - && cookie.type == USER_ACTION_COOKIE_IPFIX) { + } else if (cookie.type == USER_ACTION_COOKIE_IPFIX) { ds_put_format(ds, ",ipfix(output_port="); odp_portno_name_format(portno_names, cookie.ipfix.output_odp_port, ds); @@ -1111,7 +1104,7 @@ static int parse_odp_userspace_action(const char *s, struct ofpbuf *actions) { uint32_t pid; - union user_action_cookie cookie; + struct user_action_cookie cookie; struct ofpbuf buf; odp_port_t tunnel_out_port; int n = -1; @@ -1125,7 +1118,10 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) } ofpbuf_init(&buf, 16); + memset(&cookie, 0, sizeof cookie); + user_data = &cookie; + user_data_size = sizeof cookie; { uint32_t output; uint32_t probability; @@ -1148,8 +1144,6 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) cookie.type = USER_ACTION_COOKIE_SFLOW; cookie.sflow.vlan_tci = htons(tci); cookie.sflow.output = output; - user_data = &cookie; - user_data_size = sizeof cookie.sflow; } else if (ovs_scan(&s[n], ",slow_path(%n", &n1)) { n += n1; @@ -1164,9 +1158,6 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) goto out; } n += res + 1; - - user_data = &cookie; - user_data_size = sizeof cookie.slow_path; } else if (ovs_scan(&s[n], ",flow_sample(probability=%"SCNi32"," "collector_set_id=%"SCNi32"," "obs_domain_id=%"SCNi32"," @@ -1183,8 +1174,6 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) cookie.flow_sample.obs_domain_id = obs_domain_id; cookie.flow_sample.obs_point_id = obs_point_id; cookie.flow_sample.output_odp_port = u32_to_odp(output); - user_data = &cookie; - user_data_size = sizeof cookie.flow_sample; if (ovs_scan(&s[n], ",ingress%n", &n1)) { cookie.flow_sample.direction = NX_ACTION_SAMPLE_INGRESS; @@ -1205,8 +1194,6 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) n += n1; cookie.type = USER_ACTION_COOKIE_IPFIX; cookie.ipfix.output_odp_port = u32_to_odp(output); - user_data = &cookie; - user_data_size = sizeof cookie.ipfix; } else if (ovs_scan(&s[n], ",userdata(%n", &n1)) { char *end; diff --git a/lib/odp-util.h b/lib/odp-util.h index f7ce206510fb..b08ff7190168 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -300,37 +300,39 @@ enum user_action_cookie_type { }; /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ -union user_action_cookie { +struct user_action_cookie { uint16_t type; /* enum user_action_cookie_type. */ - struct { - uint16_t type; /* USER_ACTION_COOKIE_SFLOW. */ - ovs_be16 vlan_tci; /* Destination VLAN TCI. */ - uint32_t output; /* SFL_FLOW_SAMPLE_TYPE 'output' value. */ - } sflow; - - struct { - uint16_t type; /* USER_ACTION_COOKIE_SLOW_PATH. */ - uint16_t unused; - uint32_t reason; /* enum slow_path_reason. */ - } slow_path; - - struct { - uint16_t type; /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ - uint16_t probability; /* Sampling probability. */ - uint32_t collector_set_id; /* ID of IPFIX collector set. */ - uint32_t obs_domain_id; /* Observation Domain ID. */ - uint32_t obs_point_id; /* Observation Point ID. */ - odp_port_t output_odp_port; /* The output odp port. */ - enum nx_action_sample_direction direction; - } flow_sample; - - struct { - uint16_t type; /* USER_ACTION_COOKIE_IPFIX. */ - odp_port_t output_odp_port; /* The output odp port. */ - } ipfix; + union { + struct { + /* USER_ACTION_COOKIE_SFLOW. */ + ovs_be16 vlan_tci; /* Destination VLAN TCI. */ + uint32_t output; /* SFL_FLOW_SAMPLE_TYPE 'output' value. */ + } sflow; + + struct { + /* USER_ACTION_COOKIE_SLOW_PATH. */ + uint16_t unused; + uint32_t reason; /* enum slow_path_reason. */ + } slow_path; + + struct { + /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ + uint16_t probability; /* Sampling probability. */ + uint32_t collector_set_id; /* ID of IPFIX collector set. */ + uint32_t obs_domain_id; /* Observation Domain ID. */ + uint32_t obs_point_id; /* Observation Point ID. */ + odp_port_t output_odp_port; /* The output odp port. */ + enum nx_action_sample_direction direction; + } flow_sample; + + struct { + /* USER_ACTION_COOKIE_IPFIX. */ + odp_port_t output_odp_port; /* The output odp port. */ + } ipfix; + }; }; -BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 24); +BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) == 28); size_t odp_put_userspace_action(uint32_t pid, const void *userdata, size_t userdata_size, diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c index 2a4353720056..fb44fb98e4ed 100644 --- a/ofproto/ofproto-dpif-ipfix.c +++ b/ofproto/ofproto-dpif-ipfix.c @@ -2562,7 +2562,7 @@ dpif_ipfix_bridge_sample(struct dpif_ipfix *di, const struct dp_packet *packet, void dpif_ipfix_flow_sample(struct dpif_ipfix *di, const struct dp_packet *packet, const struct flow *flow, - const union user_action_cookie *cookie, + const struct user_action_cookie *cookie, odp_port_t input_odp_port, const struct flow_tnl *output_tunnel_key, const struct dpif_ipfix_actions *ipfix_actions) diff --git a/ofproto/ofproto-dpif-ipfix.h b/ofproto/ofproto-dpif-ipfix.h index f91d04181305..ad88ac5d5453 100644 --- a/ofproto/ofproto-dpif-ipfix.h +++ b/ofproto/ofproto-dpif-ipfix.h @@ -60,7 +60,8 @@ void dpif_ipfix_bridge_sample(struct dpif_ipfix *, const struct dp_packet *, odp_port_t, odp_port_t, const struct flow_tnl *, const struct dpif_ipfix_actions *); void dpif_ipfix_flow_sample(struct dpif_ipfix *, const struct dp_packet *, - const struct flow *, const union user_action_cookie *, + const struct flow *, + const struct user_action_cookie *, odp_port_t, const struct flow_tnl *, const struct dpif_ipfix_actions *); diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c index 2be586998cf0..e30a411f5a69 100644 --- a/ofproto/ofproto-dpif-sflow.c +++ b/ofproto/ofproto-dpif-sflow.c @@ -1233,7 +1233,7 @@ dpif_sflow_encode_mpls_stack(SFLLabelStack *stack, * See http://sflow.org/sflow_version_5.txt "Input/Output port information" */ static uint32_t -dpif_sflow_cookie_num_outputs(const union user_action_cookie *cookie) +dpif_sflow_cookie_num_outputs(const struct user_action_cookie *cookie) { uint32_t format = cookie->sflow.output & 0xC0000000; uint32_t port_n = cookie->sflow.output & 0x3FFFFFFF; @@ -1248,9 +1248,9 @@ dpif_sflow_cookie_num_outputs(const union user_action_cookie *cookie) void dpif_sflow_received(struct dpif_sflow *ds, const struct dp_packet *packet, - const struct flow *flow, odp_port_t odp_in_port, - const union user_action_cookie *cookie, - const struct dpif_sflow_actions *sflow_actions) + const struct flow *flow, odp_port_t odp_in_port, + const struct user_action_cookie *cookie, + const struct dpif_sflow_actions *sflow_actions) OVS_EXCLUDED(mutex) { SFL_FLOW_SAMPLE_TYPE fs; @@ -1283,9 +1283,9 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct dp_packet *packet, fs.input = SFL_DS_INDEX(in_dsp->dsi); } - /* Make the assumption that the random number generator in the datapath converges - * to the configured mean, and just increment the samplePool by the configured - * sampling rate every time. */ + /* Make the assumption that the random number generator in the + * datapath converges to the configured mean, and just increment the + * samplePool by the configured sampling rate every time. */ sampler->samplePool += sfl_sampler_get_sFlowFsPacketSamplingRate(sampler); /* Sampled header. */ diff --git a/ofproto/ofproto-dpif-sflow.h b/ofproto/ofproto-dpif-sflow.h index 014e6cce39c0..74fe58007af0 100644 --- a/ofproto/ofproto-dpif-sflow.h +++ b/ofproto/ofproto-dpif-sflow.h @@ -70,13 +70,13 @@ void dpif_sflow_run(struct dpif_sflow *); void dpif_sflow_wait(struct dpif_sflow *); void dpif_sflow_read_actions(const struct flow *, - const struct nlattr *actions, size_t actions_len, - struct dpif_sflow_actions *); + const struct nlattr *actions, size_t actions_len, + struct dpif_sflow_actions *); void dpif_sflow_received(struct dpif_sflow *, const struct dp_packet *, const struct flow *, odp_port_t odp_port, - const union user_action_cookie *, - const struct dpif_sflow_actions *); + const struct user_action_cookie *, + const struct dpif_sflow_actions *); int dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *, odp_port_t odp_port); diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 02cf5415bc3d..ddae02dabb3f 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -971,9 +971,6 @@ udpif_revalidator(void *arg) static enum upcall_type classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata) { - union user_action_cookie cookie; - size_t userdata_len; - /* First look at the upcall type. */ switch (type) { case DPIF_UC_ACTION: @@ -993,26 +990,22 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata) VLOG_WARN_RL(&rl, "action upcall missing cookie"); return BAD_UPCALL; } - userdata_len = nl_attr_get_size(userdata); - if (userdata_len < sizeof cookie.type - || userdata_len > sizeof cookie) { + + struct user_action_cookie cookie; + size_t userdata_len = nl_attr_get_size(userdata); + if (userdata_len != sizeof cookie) { VLOG_WARN_RL(&rl, "action upcall cookie has unexpected size %"PRIuSIZE, userdata_len); return BAD_UPCALL; } - memset(&cookie, 0, sizeof cookie); - memcpy(&cookie, nl_attr_get(userdata), userdata_len); - if (userdata_len == MAX(8, sizeof cookie.sflow) - && cookie.type == USER_ACTION_COOKIE_SFLOW) { + memcpy(&cookie, nl_attr_get(userdata), sizeof cookie); + if (cookie.type == USER_ACTION_COOKIE_SFLOW) { return SFLOW_UPCALL; - } else if (userdata_len == MAX(8, sizeof cookie.slow_path) - && cookie.type == USER_ACTION_COOKIE_SLOW_PATH) { + } else if (cookie.type == USER_ACTION_COOKIE_SLOW_PATH) { return MISS_UPCALL; - } else if (userdata_len == MAX(8, sizeof cookie.flow_sample) - && cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { + } else if (cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { return FLOW_SAMPLE_UPCALL; - } else if (userdata_len == MAX(8, sizeof cookie.ipfix) - && cookie.type == USER_ACTION_COOKIE_IPFIX) { + } else if (cookie.type == USER_ACTION_COOKIE_IPFIX) { return IPFIX_UPCALL; } else { VLOG_WARN_RL(&rl, "invalid user cookie of type %"PRIu16 @@ -1029,7 +1022,7 @@ compose_slow_path(struct udpif *udpif, struct xlate_out *xout, struct ofpbuf *buf, uint32_t slowpath_meter_id, uint32_t controller_meter_id) { - union user_action_cookie cookie; + struct user_action_cookie cookie; odp_port_t port; uint32_t pid; @@ -1056,7 +1049,7 @@ compose_slow_path(struct udpif *udpif, struct xlate_out *xout, nl_msg_put_u32(buf, OVS_ACTION_ATTR_METER, meter_id); } - odp_put_userspace_action(pid, &cookie, sizeof cookie.slow_path, + odp_put_userspace_action(pid, &cookie, sizeof cookie, ODPP_NONE, false, buf); if (meter_id != UINT32_MAX) { @@ -1349,12 +1342,14 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, case SFLOW_UPCALL: if (upcall->sflow) { - union user_action_cookie cookie; + struct user_action_cookie cookie; struct dpif_sflow_actions sflow_actions; + if (nl_attr_get_size(userdata) != sizeof cookie) { + return EINVAL; + } + memcpy(&cookie, nl_attr_get(userdata), sizeof cookie); memset(&sflow_actions, 0, sizeof sflow_actions); - memset(&cookie, 0, sizeof cookie); - memcpy(&cookie, nl_attr_get(userdata), sizeof cookie.sflow); actions_len = dpif_read_actions(udpif, upcall, flow, upcall_type, &sflow_actions); @@ -1366,12 +1361,14 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, case IPFIX_UPCALL: if (upcall->ipfix) { - union user_action_cookie cookie; + struct user_action_cookie cookie; struct flow_tnl output_tunnel_key; struct dpif_ipfix_actions ipfix_actions; - memset(&cookie, 0, sizeof cookie); - memcpy(&cookie, nl_attr_get(userdata), sizeof cookie.ipfix); + if (nl_attr_get_size(userdata) != sizeof cookie) { + return EINVAL; + } + memcpy(&cookie, nl_attr_get(userdata), sizeof cookie); memset(&ipfix_actions, 0, sizeof ipfix_actions); if (upcall->out_tun_key) { @@ -1391,12 +1388,14 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, case FLOW_SAMPLE_UPCALL: if (upcall->ipfix) { - union user_action_cookie cookie; + struct user_action_cookie cookie; struct flow_tnl output_tunnel_key; struct dpif_ipfix_actions ipfix_actions; - memset(&cookie, 0, sizeof cookie); - memcpy(&cookie, nl_attr_get(userdata), sizeof cookie.flow_sample); + if (nl_attr_get_size(userdata) != sizeof cookie) { + return EINVAL; + } + memcpy(&cookie, nl_attr_get(userdata), sizeof cookie); memset(&ipfix_actions, 0, sizeof ipfix_actions); if (upcall->out_tun_key) { diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 9e05529d14a4..804b8b88681a 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2853,19 +2853,18 @@ xlate_normal(struct xlate_ctx *ctx) /* Appends a "sample" action for sFlow or IPFIX to 'ctx->odp_actions'. The * 'probability' is the number of packets out of UINT32_MAX to sample. The - * 'cookie' (of length 'cookie_size' bytes) is passed back in the callback for - * each sampled packet. 'tunnel_out_port', if not ODPP_NONE, is added as the - * OVS_USERSPACE_ATTR_EGRESS_TUN_PORT attribute. If 'include_actions', an - * OVS_USERSPACE_ATTR_ACTIONS attribute is added. If 'emit_set_tunnel', - * sample(sampling_port=1) would translate into datapath sample action - * set(tunnel(...)), sample(...) and it is used for sampling egress tunnel - * information. + * 'cookie' is passed back in the callback for each sampled packet. + * 'tunnel_out_port', if not ODPP_NONE, is added as the + * OVS_USERSPACE_ATTR_EGRESS_TUN_PORT attribute. If 'include_actions', + * an OVS_USERSPACE_ATTR_ACTIONS attribute is added. If + * 'emit_set_tunnel', sample(sampling_port=1) would translate into + * datapath sample action set(tunnel(...)), sample(...) and it is used + * for sampling egress tunnel information. */ static size_t compose_sample_action(struct xlate_ctx *ctx, const uint32_t probability, - const union user_action_cookie *cookie, - const size_t cookie_size, + const struct user_action_cookie *cookie, const odp_port_t tunnel_out_port, bool include_actions) { @@ -2900,7 +2899,7 @@ compose_sample_action(struct xlate_ctx *ctx, ctx->xbridge, ctx->xin->flow.in_port.ofp_port); uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port, flow_hash_5tuple(&ctx->xin->flow, 0)); - int cookie_offset = odp_put_userspace_action(pid, cookie, cookie_size, + int cookie_offset = odp_put_userspace_action(pid, cookie, sizeof *cookie, tunnel_out_port, include_actions, ctx->odp_actions); @@ -2928,10 +2927,9 @@ compose_sflow_action(struct xlate_ctx *ctx) return 0; } - union user_action_cookie cookie = { .type = USER_ACTION_COOKIE_SFLOW }; + struct user_action_cookie cookie = { .type = USER_ACTION_COOKIE_SFLOW }; return compose_sample_action(ctx, dpif_sflow_get_probability(sflow), - &cookie, sizeof cookie.sflow, ODPP_NONE, - true); + &cookie, ODPP_NONE, true); } /* If flow IPFIX is enabled, make sure IPFIX flow sample action @@ -2969,30 +2967,27 @@ compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t output_odp_port) } } - union user_action_cookie cookie = { - .ipfix = { - .type = USER_ACTION_COOKIE_IPFIX, - .output_odp_port = output_odp_port, - } + struct user_action_cookie cookie = { + .type = USER_ACTION_COOKIE_IPFIX, + .ipfix.output_odp_port = output_odp_port, }; compose_sample_action(ctx, dpif_ipfix_get_bridge_exporter_probability(ipfix), - &cookie, sizeof cookie.ipfix, tunnel_out_port, - false); + &cookie, tunnel_out_port, false); } /* Fix "sample" action according to data collected while composing ODP actions, * as described in compose_sflow_action(). * - * 'user_cookie_offset' must be the offset returned by add_sflow_action(). */ + * 'user_cookie_offset' must be the offset returned by + * compose_sflow_action(). */ static void fix_sflow_action(struct xlate_ctx *ctx, unsigned int user_cookie_offset) { const struct flow *base = &ctx->base_flow; - union user_action_cookie *cookie; + struct user_action_cookie *cookie; - cookie = ofpbuf_at(ctx->odp_actions, user_cookie_offset, - sizeof cookie->sflow); + cookie = ofpbuf_at(ctx->odp_actions, user_cookie_offset, sizeof *cookie); ovs_assert(cookie->type == USER_ACTION_COOKIE_SFLOW); cookie->type = USER_ACTION_COOKIE_SFLOW; @@ -5273,9 +5268,9 @@ xlate_sample_action(struct xlate_ctx *ctx, } } - union user_action_cookie cookie = { + struct user_action_cookie cookie = { + .type = USER_ACTION_COOKIE_FLOW_SAMPLE, .flow_sample = { - .type = USER_ACTION_COOKIE_FLOW_SAMPLE, .probability = os->probability, .collector_set_id = os->collector_set_id, .obs_domain_id = os->obs_domain_id, @@ -5284,8 +5279,7 @@ xlate_sample_action(struct xlate_ctx *ctx, .direction = os->direction, } }; - compose_sample_action(ctx, probability, &cookie, sizeof cookie.flow_sample, - tunnel_out_port, false); + compose_sample_action(ctx, probability, &cookie, tunnel_out_port, false); } /* Determine if an datapath action translated from the openflow action diff --git a/tests/odp.at b/tests/odp.at index 1a80322890eb..4891653eb81a 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -246,8 +246,6 @@ AT_CLEANUP AT_SETUP([OVS datapath actions parsing and formatting - valid forms]) AT_DATA([actions.txt], [dnl 1,2,3 -userspace(pid=555666777) -userspace(pid=555666777,tunnel_out_port=10) userspace(pid=6633,sFlow(vid=9,pcp=7,output=10),actions) userspace(pid=6633,sFlow(vid=9,pcp=7,output=10),actions,tunnel_out_port=10) userspace(pid=9765,slow_path(0)) From patchwork Wed Jan 10 22:27:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 858591 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=) 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 3zH3b51X3fz9s4s for ; Thu, 11 Jan 2018 09:29:21 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id E60FFFEE; Wed, 10 Jan 2018 22:28:19 +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 8E82FEED for ; Wed, 10 Jan 2018 22:28:17 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 8432C12D for ; Wed, 10 Jan 2018 22:28:16 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id D3700C5A53 for ; Wed, 10 Jan 2018 23:28:14 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 10 Jan 2018 14:27:20 -0800 Message-Id: <1515623246-3820-2-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1515623246-3820-1-git-send-email-jpettit@ovn.org> References: <1515623246-3820-1-git-send-email-jpettit@ovn.org> 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 Subject: [ovs-dev] [no-slow v2 2/8] ofproto-dpif: Add ability to look up an ofproto by UUID. 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 This will have callers in the future. Signed-off-by: Justin Pettit Acked-by: Ben Pfaff --- v1->v2: Changes suggested by Ben. --- ofproto/ofproto-dpif-trace.c | 2 +- ofproto/ofproto-dpif.c | 92 +++++++++++++++++++++++++++++++------------- ofproto/ofproto-dpif.h | 14 ++++--- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c index d5da48e326bb..4999d1d6f326 100644 --- a/ofproto/ofproto-dpif-trace.c +++ b/ofproto/ofproto-dpif-trace.c @@ -354,7 +354,7 @@ parse_flow_and_packet(int argc, const char *argv[], goto exit; } - *ofprotop = ofproto_dpif_lookup(argv[1]); + *ofprotop = ofproto_dpif_lookup_by_name(argv[1]); if (!*ofprotop) { error = "Unknown bridge name"; goto exit; diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 838a8de0c27f..5edf1a9ebf25 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -58,6 +58,7 @@ #include "openvswitch/ofp-print.h" #include "openvswitch/ofp-util.h" #include "openvswitch/ofpbuf.h" +#include "openvswitch/uuid.h" #include "openvswitch/vlog.h" #include "ovs-lldp.h" #include "ovs-rcu.h" @@ -71,6 +72,7 @@ #include "unaligned.h" #include "unixctl.h" #include "util.h" +#include "uuid.h" #include "vlan-bitmap.h" VLOG_DEFINE_THIS_MODULE(ofproto_dpif); @@ -187,7 +189,12 @@ COVERAGE_DEFINE(rev_mcast_snooping); struct shash all_dpif_backers = SHASH_INITIALIZER(&all_dpif_backers); /* All existing ofproto_dpif instances, indexed by ->up.name. */ -struct hmap all_ofproto_dpifs = HMAP_INITIALIZER(&all_ofproto_dpifs); +static struct hmap all_ofproto_dpifs_by_name = + HMAP_INITIALIZER(&all_ofproto_dpifs_by_name); + +/* All existing ofproto_dpif instances, indexed by ->uuid. */ +static struct hmap all_ofproto_dpifs_by_uuid = + HMAP_INITIALIZER(&all_ofproto_dpifs_by_uuid); static bool ofproto_use_tnl_push_pop = true; static void ofproto_unixctl_init(void); @@ -268,7 +275,8 @@ enumerate_names(const char *type, struct sset *names) struct ofproto_dpif *ofproto; sset_clear(names); - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { if (strcmp(type, ofproto->up.type)) { continue; } @@ -311,7 +319,8 @@ lookup_ofproto_dpif_by_port_name(const char *name) { struct ofproto_dpif *ofproto; - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { if (sset_contains(&ofproto->ports, name)) { return ofproto; } @@ -368,7 +377,8 @@ type_run(const char *type) simap_init(&tmp_backers); simap_swap(&backer->tnl_backers, &tmp_backers); - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { struct ofport_dpif *iter; if (backer != ofproto->backer) { @@ -433,7 +443,8 @@ type_run(const char *type) backer->need_revalidate = 0; xlate_txn_start(); - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { struct ofport_dpif *ofport; struct ofbundle *bundle; @@ -522,7 +533,8 @@ process_dpif_all_ports_changed(struct dpif_backer *backer) const char *devname; sset_init(&devnames); - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { if (ofproto->backer == backer) { struct ofport *ofport; @@ -552,8 +564,8 @@ process_dpif_port_change(struct dpif_backer *backer, const char *devname) return; } - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, - &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { if (simap_contains(&ofproto->backer->tnl_backers, devname)) { return; } @@ -604,7 +616,8 @@ process_dpif_port_error(struct dpif_backer *backer, int error) { struct ofproto_dpif *ofproto; - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { if (ofproto->backer == backer) { sset_clear(&ofproto->port_poll_set); ofproto->port_poll_errno = error; @@ -1373,8 +1386,12 @@ construct(struct ofproto *ofproto_) } } - hmap_insert(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node, + hmap_insert(&all_ofproto_dpifs_by_name, + &ofproto->all_ofproto_dpifs_by_name_node, hash_string(ofproto->up.name, 0)); + hmap_insert(&all_ofproto_dpifs_by_uuid, + &ofproto->all_ofproto_dpifs_by_uuid_node, + uuid_hash(&ofproto->uuid)); memset(&ofproto->stats, 0, sizeof ofproto->stats); ofproto_init_tables(ofproto_, N_TABLES); @@ -1473,7 +1490,10 @@ destruct(struct ofproto *ofproto_, bool del) * to the ofproto or anything in it. */ udpif_synchronize(ofproto->backer->udpif); - hmap_remove(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node); + hmap_remove(&all_ofproto_dpifs_by_name, + &ofproto->all_ofproto_dpifs_by_name_node); + hmap_remove(&all_ofproto_dpifs_by_uuid, + &ofproto->all_ofproto_dpifs_by_uuid_node); OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) { CLS_FOR_EACH (rule, up.cr, &table->cls) { @@ -2764,7 +2784,8 @@ bundle_flush_macs(struct ofbundle *bundle, bool all_ofprotos) if (all_ofprotos) { struct ofproto_dpif *o; - HMAP_FOR_EACH (o, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (o, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { if (o != ofproto) { struct mac_entry *e; @@ -3443,7 +3464,8 @@ ofport_update_peer(struct ofport_dpif *ofport) return; } - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { struct ofport *peer_ofport; struct ofport_dpif *peer; char *peer_peer; @@ -4858,12 +4880,13 @@ get_netflow_ids(const struct ofproto *ofproto_, } struct ofproto_dpif * -ofproto_dpif_lookup(const char *name) +ofproto_dpif_lookup_by_name(const char *name) { struct ofproto_dpif *ofproto; - HMAP_FOR_EACH_WITH_HASH (ofproto, all_ofproto_dpifs_node, - hash_string(name, 0), &all_ofproto_dpifs) { + HMAP_FOR_EACH_WITH_HASH (ofproto, all_ofproto_dpifs_by_name_node, + hash_string(name, 0), + &all_ofproto_dpifs_by_name) { if (!strcmp(ofproto->up.name, name)) { return ofproto; } @@ -4871,6 +4894,20 @@ ofproto_dpif_lookup(const char *name) return NULL; } +struct ofproto_dpif * +ofproto_dpif_lookup_by_uuid(const struct uuid *uuid) +{ + struct ofproto_dpif *ofproto; + + HMAP_FOR_EACH_WITH_HASH (ofproto, all_ofproto_dpifs_by_uuid_node, + uuid_hash(uuid), &all_ofproto_dpifs_by_uuid) { + if (uuid_equals(&ofproto->uuid, uuid)) { + return ofproto; + } + } + return NULL; +} + static void ofproto_unixctl_fdb_flush(struct unixctl_conn *conn, int argc, const char *argv[], void *aux OVS_UNUSED) @@ -4878,7 +4915,7 @@ ofproto_unixctl_fdb_flush(struct unixctl_conn *conn, int argc, struct ofproto_dpif *ofproto; if (argc > 1) { - ofproto = ofproto_dpif_lookup(argv[1]); + ofproto = ofproto_dpif_lookup_by_name(argv[1]); if (!ofproto) { unixctl_command_reply_error(conn, "no such bridge"); return; @@ -4887,7 +4924,8 @@ ofproto_unixctl_fdb_flush(struct unixctl_conn *conn, int argc, mac_learning_flush(ofproto->ml); ovs_rwlock_unlock(&ofproto->ml->rwlock); } else { - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { ovs_rwlock_wrlock(&ofproto->ml->rwlock); mac_learning_flush(ofproto->ml); ovs_rwlock_unlock(&ofproto->ml->rwlock); @@ -4904,7 +4942,7 @@ ofproto_unixctl_mcast_snooping_flush(struct unixctl_conn *conn, int argc, struct ofproto_dpif *ofproto; if (argc > 1) { - ofproto = ofproto_dpif_lookup(argv[1]); + ofproto = ofproto_dpif_lookup_by_name(argv[1]); if (!ofproto) { unixctl_command_reply_error(conn, "no such bridge"); return; @@ -4916,7 +4954,8 @@ ofproto_unixctl_mcast_snooping_flush(struct unixctl_conn *conn, int argc, } mcast_snooping_mdb_flush(ofproto->ms); } else { - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { if (!mcast_snooping_enabled(ofproto->ms)) { continue; } @@ -4942,7 +4981,7 @@ ofproto_unixctl_fdb_show(struct unixctl_conn *conn, int argc OVS_UNUSED, const struct ofproto_dpif *ofproto; const struct mac_entry *e; - ofproto = ofproto_dpif_lookup(argv[1]); + ofproto = ofproto_dpif_lookup_by_name(argv[1]); if (!ofproto) { unixctl_command_reply_error(conn, "no such bridge"); return; @@ -4978,7 +5017,7 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn *conn, struct mcast_group_bundle *b; struct mcast_mrouter_bundle *mrouter; - ofproto = ofproto_dpif_lookup(argv[1]); + ofproto = ofproto_dpif_lookup_by_name(argv[1]); if (!ofproto) { unixctl_command_reply_error(conn, "no such bridge"); return; @@ -5029,7 +5068,8 @@ get_ofprotos(struct shash *ofproto_shash) { const struct ofproto_dpif *ofproto; - HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) { + HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_by_name_node, + &all_ofproto_dpifs_by_name) { char *name = xasprintf("%s@%s", ofproto->up.type, ofproto->up.name); shash_add_nocopy(ofproto_shash, name, ofproto); } @@ -5318,7 +5358,7 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn, struct dpif_flow f; int error; - ofproto = ofproto_dpif_lookup(argv[argc - 1]); + ofproto = ofproto_dpif_lookup_by_name(argv[argc - 1]); if (!ofproto) { unixctl_command_reply_error(conn, "no such bridge"); return; @@ -5403,7 +5443,7 @@ ofproto_unixctl_dpif_show_dp_features(struct unixctl_conn *conn, { struct ds ds = DS_EMPTY_INITIALIZER; const char *br = argv[argc -1]; - struct ofproto_dpif *ofproto = ofproto_dpif_lookup(br); + struct ofproto_dpif *ofproto = ofproto_dpif_lookup_by_name(br); if (!ofproto) { unixctl_command_reply_error(conn, "no such bridge"); @@ -5422,7 +5462,7 @@ ofproto_unixctl_dpif_set_dp_features(struct unixctl_conn *conn, struct ds ds = DS_EMPTY_INITIALIZER; const char *br = argv[1]; const char *name, *value; - struct ofproto_dpif *ofproto = ofproto_dpif_lookup(br); + struct ofproto_dpif *ofproto = ofproto_dpif_lookup_by_name(br); bool changed; if (!ofproto) { diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index 032b5d7d66c8..c96e00e6a2ab 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -60,6 +60,7 @@ struct dpif_flow_stats; struct ofproto_async_msg; struct ofproto_dpif; +struct uuid; struct xlate_cache; /* Number of implemented OpenFlow tables. */ @@ -245,7 +246,12 @@ struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t); /* A bridge based on a "dpif" datapath. */ struct ofproto_dpif { - struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'. */ + /* In 'all_ofproto_dpifs_by_name'. */ + struct hmap_node all_ofproto_dpifs_by_name_node; + + /* In 'all_ofproto_dpifs_by_uuid'. */ + struct hmap_node all_ofproto_dpifs_by_uuid_node; + struct ofproto up; struct dpif_backer *backer; @@ -298,10 +304,8 @@ struct ofproto_dpif { uint64_t ams_seqno; }; -/* All existing ofproto_dpif instances, indexed by ->up.name. */ -extern struct hmap all_ofproto_dpifs; - -struct ofproto_dpif *ofproto_dpif_lookup(const char *name); +struct ofproto_dpif *ofproto_dpif_lookup_by_name(const char *name); +struct ofproto_dpif *ofproto_dpif_lookup_by_uuid(const struct uuid *uuid); ovs_version_t ofproto_dpif_get_tables_version(struct ofproto_dpif *); From patchwork Wed Jan 10 22:27:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 858592 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=) 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 3zH3bq01N8z9s4s for ; Thu, 11 Jan 2018 09:29:58 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 2B66B1030; Wed, 10 Jan 2018 22:28:21 +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 7A6DCFFF for ; Wed, 10 Jan 2018 22:28:18 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 906B9E3 for ; Wed, 10 Jan 2018 22:28:17 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id D761AC5A50 for ; Wed, 10 Jan 2018 23:28:15 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 10 Jan 2018 14:27:21 -0800 Message-Id: <1515623246-3820-3-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1515623246-3820-1-git-send-email-jpettit@ovn.org> References: <1515623246-3820-1-git-send-email-jpettit@ovn.org> 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 Subject: [ovs-dev] [no-slow v2 3/8] ofproto-dpif: Reorganize upcall handling. 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 - This reduces the number of times upcall cookies are processed. - It separate true miss calls from slow-path actions. The reorganization will also be useful for a future commit. Signed-off-by: Justin Pettit --- v1->v2: Changes suggested by Ben. --- ofproto/ofproto-dpif-upcall.c | 81 +++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 45 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index ddae02dabb3f..83007d00b46c 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -183,6 +183,7 @@ struct udpif { enum upcall_type { BAD_UPCALL, /* Some kind of bug somewhere. */ MISS_UPCALL, /* A flow miss. */ + SLOW_PATH_UPCALL, /* Slow path upcall. */ SFLOW_UPCALL, /* sFlow sample. */ FLOW_SAMPLE_UPCALL, /* Per-flow sampling. */ IPFIX_UPCALL /* Per-bridge sampling. */ @@ -210,8 +211,7 @@ struct upcall { uint16_t mru; /* If !0, Maximum receive unit of fragmented IP packet */ - enum dpif_upcall_type type; /* Datapath type of the upcall. */ - const struct nlattr *userdata; /* Userdata for DPIF_UC_ACTION Upcalls. */ + enum upcall_type type; /* Type of the upcall. */ const struct nlattr *actions; /* Flow actions in DPIF_UC_ACTION Upcalls. */ bool xout_initialized; /* True if 'xout' must be uninitialized. */ @@ -235,6 +235,8 @@ struct upcall { size_t key_len; /* Datapath flow key length. */ const struct nlattr *out_tun_key; /* Datapath output tunnel key. */ + struct user_action_cookie cookie; + uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */ }; @@ -367,7 +369,8 @@ static int ukey_acquire(struct udpif *, const struct dpif_flow *, static void ukey_delete__(struct udpif_key *); static void ukey_delete(struct umap *, struct udpif_key *); static enum upcall_type classify_upcall(enum dpif_upcall_type type, - const struct nlattr *userdata); + const struct nlattr *userdata, + struct user_action_cookie *cookie); static void put_op_init(struct ukey_op *op, struct udpif_key *ukey, enum dpif_flow_put_flags flags); @@ -969,7 +972,8 @@ udpif_revalidator(void *arg) } static enum upcall_type -classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata) +classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, + struct user_action_cookie *cookie) { /* First look at the upcall type. */ switch (type) { @@ -991,25 +995,24 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata) return BAD_UPCALL; } - struct user_action_cookie cookie; size_t userdata_len = nl_attr_get_size(userdata); - if (userdata_len != sizeof cookie) { + if (userdata_len != sizeof *cookie) { VLOG_WARN_RL(&rl, "action upcall cookie has unexpected size %"PRIuSIZE, userdata_len); return BAD_UPCALL; } - memcpy(&cookie, nl_attr_get(userdata), sizeof cookie); - if (cookie.type == USER_ACTION_COOKIE_SFLOW) { + memcpy(cookie, nl_attr_get(userdata), sizeof *cookie); + if (cookie->type == USER_ACTION_COOKIE_SFLOW) { return SFLOW_UPCALL; - } else if (cookie.type == USER_ACTION_COOKIE_SLOW_PATH) { - return MISS_UPCALL; - } else if (cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { + } else if (cookie->type == USER_ACTION_COOKIE_SLOW_PATH) { + return SLOW_PATH_UPCALL; + } else if (cookie->type == USER_ACTION_COOKIE_FLOW_SAMPLE) { return FLOW_SAMPLE_UPCALL; - } else if (cookie.type == USER_ACTION_COOKIE_IPFIX) { + } else if (cookie->type == USER_ACTION_COOKIE_IPFIX) { return IPFIX_UPCALL; } else { VLOG_WARN_RL(&rl, "invalid user cookie of type %"PRIu16 - " and size %"PRIuSIZE, cookie.type, userdata_len); + " and size %"PRIuSIZE, cookie->type, userdata_len); return BAD_UPCALL; } } @@ -1071,6 +1074,11 @@ upcall_receive(struct upcall *upcall, const struct dpif_backer *backer, { int error; + upcall->type = classify_upcall(type, userdata, &upcall->cookie); + if (upcall->type == BAD_UPCALL) { + return EAGAIN; + } + error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix, &upcall->sflow, NULL, &upcall->in_port); if (error) { @@ -1083,8 +1091,6 @@ upcall_receive(struct upcall *upcall, const struct dpif_backer *backer, upcall->packet = packet; upcall->ufid = ufid; upcall->pmd_id = pmd_id; - upcall->type = type; - upcall->userdata = userdata; ofpbuf_use_stub(&upcall->odp_actions, upcall->odp_actions_stub, sizeof upcall->odp_actions_stub); ofpbuf_init(&upcall->put_actions, 0); @@ -1120,7 +1126,7 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, upcall->flow, upcall->in_port, NULL, stats.tcp_flags, upcall->packet, wc, odp_actions); - if (upcall->type == DPIF_UC_MISS) { + if (upcall->type == MISS_UPCALL) { xin.resubmit_stats = &stats; if (xin.frozen_state) { @@ -1169,7 +1175,7 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, /* This function is also called for slow-pathed flows. As we are only * going to create new datapath flows for actual datapath misses, there is * no point in creating a ukey otherwise. */ - if (upcall->type == DPIF_UC_MISS) { + if (upcall->type == MISS_UPCALL) { upcall->ukey = ukey_create_from_upcall(upcall, wc); } } @@ -1205,7 +1211,7 @@ should_install_flow(struct udpif *udpif, struct upcall *upcall) { unsigned int flow_limit; - if (upcall->type != DPIF_UC_MISS) { + if (upcall->type != MISS_UPCALL) { return false; } else if (upcall->recirc && !upcall->have_recirc_ref) { VLOG_DBG_RL(&rl, "upcall: no reference for recirc flow"); @@ -1318,6 +1324,7 @@ dpif_read_actions(struct udpif *udpif, struct upcall *upcall, break; case BAD_UPCALL: case MISS_UPCALL: + case SLOW_PATH_UPCALL: default: break; } @@ -1329,57 +1336,46 @@ static int process_upcall(struct udpif *udpif, struct upcall *upcall, struct ofpbuf *odp_actions, struct flow_wildcards *wc) { - const struct nlattr *userdata = upcall->userdata; const struct dp_packet *packet = upcall->packet; const struct flow *flow = upcall->flow; size_t actions_len = 0; - enum upcall_type upcall_type = classify_upcall(upcall->type, userdata); - switch (upcall_type) { + switch (upcall->type) { case MISS_UPCALL: + case SLOW_PATH_UPCALL: upcall_xlate(udpif, upcall, odp_actions, wc); return 0; case SFLOW_UPCALL: if (upcall->sflow) { - struct user_action_cookie cookie; struct dpif_sflow_actions sflow_actions; - if (nl_attr_get_size(userdata) != sizeof cookie) { - return EINVAL; - } - memcpy(&cookie, nl_attr_get(userdata), sizeof cookie); memset(&sflow_actions, 0, sizeof sflow_actions); - actions_len = dpif_read_actions(udpif, upcall, flow, upcall_type, - &sflow_actions); + actions_len = dpif_read_actions(udpif, upcall, flow, + upcall->type, &sflow_actions); dpif_sflow_received(upcall->sflow, packet, flow, - flow->in_port.odp_port, &cookie, + flow->in_port.odp_port, &upcall->cookie, actions_len > 0 ? &sflow_actions : NULL); } break; case IPFIX_UPCALL: if (upcall->ipfix) { - struct user_action_cookie cookie; struct flow_tnl output_tunnel_key; struct dpif_ipfix_actions ipfix_actions; - if (nl_attr_get_size(userdata) != sizeof cookie) { - return EINVAL; - } - memcpy(&cookie, nl_attr_get(userdata), sizeof cookie); memset(&ipfix_actions, 0, sizeof ipfix_actions); if (upcall->out_tun_key) { odp_tun_key_from_attr(upcall->out_tun_key, &output_tunnel_key); } - actions_len = dpif_read_actions(udpif, upcall, flow, upcall_type, - &ipfix_actions); + actions_len = dpif_read_actions(udpif, upcall, flow, + upcall->type, &ipfix_actions); dpif_ipfix_bridge_sample(upcall->ipfix, packet, flow, flow->in_port.odp_port, - cookie.ipfix.output_odp_port, + upcall->cookie.ipfix.output_odp_port, upcall->out_tun_key ? &output_tunnel_key : NULL, actions_len > 0 ? &ipfix_actions: NULL); @@ -1388,26 +1384,21 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, case FLOW_SAMPLE_UPCALL: if (upcall->ipfix) { - struct user_action_cookie cookie; struct flow_tnl output_tunnel_key; struct dpif_ipfix_actions ipfix_actions; - if (nl_attr_get_size(userdata) != sizeof cookie) { - return EINVAL; - } - memcpy(&cookie, nl_attr_get(userdata), sizeof cookie); memset(&ipfix_actions, 0, sizeof ipfix_actions); if (upcall->out_tun_key) { odp_tun_key_from_attr(upcall->out_tun_key, &output_tunnel_key); } - actions_len = dpif_read_actions(udpif, upcall, flow, upcall_type, - &ipfix_actions); + actions_len = dpif_read_actions(udpif, upcall, flow, + upcall->type, &ipfix_actions); /* The flow reflects exactly the contents of the packet. * Sample the packet using it. */ dpif_ipfix_flow_sample(upcall->ipfix, packet, flow, - &cookie, flow->in_port.odp_port, + &upcall->cookie, flow->in_port.odp_port, upcall->out_tun_key ? &output_tunnel_key : NULL, actions_len > 0 ? &ipfix_actions: NULL); From patchwork Wed Jan 10 22:27:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 858593 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=) 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 3zH3cN04BHz9s4s for ; Thu, 11 Jan 2018 09:30:28 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 732DD1020; Wed, 10 Jan 2018 22:28:22 +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 10B6B1009 for ; Wed, 10 Jan 2018 22:28:19 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 98AEAE3 for ; Wed, 10 Jan 2018 22:28:18 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id E3F7FC5A44 for ; Wed, 10 Jan 2018 23:28:16 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 10 Jan 2018 14:27:22 -0800 Message-Id: <1515623246-3820-4-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1515623246-3820-1-git-send-email-jpettit@ovn.org> References: <1515623246-3820-1-git-send-email-jpettit@ovn.org> 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 Subject: [ovs-dev] [no-slow v2 4/8] ofproto-dpif: Modify process_upcall() to remove some redundant code. 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 Signed-off-by: Justin Pettit Acked-by: Ben Pfaff --- v1->v2: New to the series. --- ofproto/ofproto-dpif-upcall.c | 45 ++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 83007d00b46c..5ba1006893dc 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1361,27 +1361,6 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, break; case IPFIX_UPCALL: - if (upcall->ipfix) { - struct flow_tnl output_tunnel_key; - struct dpif_ipfix_actions ipfix_actions; - - memset(&ipfix_actions, 0, sizeof ipfix_actions); - - if (upcall->out_tun_key) { - odp_tun_key_from_attr(upcall->out_tun_key, &output_tunnel_key); - } - - actions_len = dpif_read_actions(udpif, upcall, flow, - upcall->type, &ipfix_actions); - dpif_ipfix_bridge_sample(upcall->ipfix, packet, flow, - flow->in_port.odp_port, - upcall->cookie.ipfix.output_odp_port, - upcall->out_tun_key ? - &output_tunnel_key : NULL, - actions_len > 0 ? &ipfix_actions: NULL); - } - break; - case FLOW_SAMPLE_UPCALL: if (upcall->ipfix) { struct flow_tnl output_tunnel_key; @@ -1395,13 +1374,23 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, actions_len = dpif_read_actions(udpif, upcall, flow, upcall->type, &ipfix_actions); - /* The flow reflects exactly the contents of the packet. - * Sample the packet using it. */ - dpif_ipfix_flow_sample(upcall->ipfix, packet, flow, - &upcall->cookie, flow->in_port.odp_port, - upcall->out_tun_key ? - &output_tunnel_key : NULL, - actions_len > 0 ? &ipfix_actions: NULL); + if (upcall->type == IPFIX_UPCALL) { + dpif_ipfix_bridge_sample(upcall->ipfix, packet, flow, + flow->in_port.odp_port, + upcall->cookie.ipfix.output_odp_port, + upcall->out_tun_key ? + &output_tunnel_key : NULL, + actions_len > 0 ? + &ipfix_actions: NULL); + } else { + /* The flow reflects exactly the contents of the packet. + * Sample the packet using it. */ + dpif_ipfix_flow_sample(upcall->ipfix, packet, flow, + &upcall->cookie, flow->in_port.odp_port, + upcall->out_tun_key ? + &output_tunnel_key : NULL, + actions_len > 0 ? &ipfix_actions: NULL); + } } break; From patchwork Wed Jan 10 22:27:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 858595 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=) 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 3zH3d23rPbz9sNw for ; Thu, 11 Jan 2018 09:31:02 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id DC2DC103F; Wed, 10 Jan 2018 22:28:23 +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 2CCC1100D for ; Wed, 10 Jan 2018 22:28:20 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7C90AE3 for ; Wed, 10 Jan 2018 22:28:19 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id EB446C5A51 for ; Wed, 10 Jan 2018 23:28:17 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 10 Jan 2018 14:27:23 -0800 Message-Id: <1515623246-3820-5-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1515623246-3820-1-git-send-email-jpettit@ovn.org> References: <1515623246-3820-1-git-send-email-jpettit@ovn.org> 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 Subject: [ovs-dev] [no-slow v2 5/8] ofp-actions: Add action "debug_slow" for testing slow-path. 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 It isn't otherwise useful and in fact hurts performance so it's disabled without --enable-dummy. An upcoming commit will make use of this. Signed-off-by: Justin Pettit Acked-by: Ben Pfaff --- v1->v2: Unchanged. --- include/openvswitch/ofp-actions.h | 1 + lib/ofp-actions.c | 48 ++++++++++++++++++++++++++++++++++++++- ofproto/ofproto-dpif-xlate.c | 7 ++++++ tests/ofproto-dpif.at | 19 ++++++++++++++++ 4 files changed, 74 insertions(+), 1 deletion(-) diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h index 3d9775582f6d..a7808eb4fa60 100644 --- a/include/openvswitch/ofp-actions.h +++ b/include/openvswitch/ofp-actions.h @@ -128,6 +128,7 @@ struct vl_mff_map; * These are intentionally undocumented, subject to change, and \ * only accepted if ovs-vswitchd is started with --enable-dummy. */ \ OFPACT(DEBUG_RECIRC, ofpact_null, ofpact, "debug_recirc") \ + OFPACT(DEBUG_SLOW, ofpact_null, ofpact, "debug_slow") \ \ /* Instructions. */ \ OFPACT(METER, ofpact_meter, ofpact, "meter") \ diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 4918498efb30..1d364f98bbd1 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -357,6 +357,9 @@ enum ofp_raw_action_type { /* These are intentionally undocumented, subject to change, and ovs-vswitchd */ /* accepts them only if started with --enable-dummy. */ + /* NX1.0+(254): void. */ + NXAST_RAW_DEBUG_SLOW, + /* NX1.0+(255): void. */ NXAST_RAW_DEBUG_RECIRC, }; @@ -475,6 +478,7 @@ ofpact_next_flattened(const struct ofpact *ofpact) case OFPACT_UNROLL_XLATE: case OFPACT_CT_CLEAR: case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: case OFPACT_METER: case OFPACT_CLEAR_ACTIONS: case OFPACT_WRITE_METADATA: @@ -5802,7 +5806,7 @@ format_SAMPLE(const struct ofpact_sample *a, ds_put_format(s, "%s)%s", colors.paren, colors.end); } -/* debug_recirc instruction. */ +/* debug instructions. */ static bool enable_debug; @@ -5849,6 +5853,43 @@ format_DEBUG_RECIRC(const struct ofpact_null *a OVS_UNUSED, ds_put_format(s, "%sdebug_recirc%s", colors.value, colors.end); } +static enum ofperr +decode_NXAST_RAW_DEBUG_SLOW(struct ofpbuf *out) +{ + if (!enable_debug) { + return OFPERR_OFPBAC_BAD_VENDOR_TYPE; + } + + ofpact_put_DEBUG_SLOW(out); + return 0; +} + +static void +encode_DEBUG_SLOW(const struct ofpact_null *n OVS_UNUSED, + enum ofp_version ofp_version OVS_UNUSED, + struct ofpbuf *out) +{ + put_NXAST_DEBUG_SLOW(out); +} + +static char * OVS_WARN_UNUSED_RESULT +parse_DEBUG_SLOW(char *arg OVS_UNUSED, + const struct ofputil_port_map *port_map OVS_UNUSED, + struct ofpbuf *ofpacts, + enum ofputil_protocol *usable_protocols OVS_UNUSED) +{ + ofpact_put_DEBUG_SLOW(ofpacts); + return NULL; +} + +static void +format_DEBUG_SLOW(const struct ofpact_null *a OVS_UNUSED, + const struct ofputil_port_map *port_map OVS_UNUSED, + struct ds *s) +{ + ds_put_format(s, "%sdebug_slow%s", colors.value, colors.end); +} + /* Action structure for NXAST_CT. * * Pass traffic to the connection tracker. @@ -7151,6 +7192,7 @@ ofpact_is_set_or_move_action(const struct ofpact *a) case OFPACT_WRITE_ACTIONS: case OFPACT_WRITE_METADATA: case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: return false; default: OVS_NOT_REACHED(); @@ -7219,6 +7261,7 @@ ofpact_is_allowed_in_actions_set(const struct ofpact *a) case OFPACT_STACK_POP: case OFPACT_STACK_PUSH: case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: /* The action set may only include actions and thus * may not include any instructions */ @@ -7441,6 +7484,7 @@ ovs_instruction_type_from_ofpact_type(enum ofpact_type type) case OFPACT_UNROLL_XLATE: case OFPACT_SAMPLE: case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: case OFPACT_CT: case OFPACT_CT_CLEAR: case OFPACT_NAT: @@ -8107,6 +8151,7 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a, return OFPERR_OFPBAC_BAD_TYPE; case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: return 0; case OFPACT_ENCAP: @@ -8622,6 +8667,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, ofp_port_t port) case OFPACT_METER: case OFPACT_GROUP: case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: case OFPACT_CT: case OFPACT_CT_CLEAR: case OFPACT_NAT: diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 804b8b88681a..63d15e94331f 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5302,6 +5302,7 @@ reversible_actions(const struct ofpact *ofpacts, size_t ofpacts_len) case OFPACT_CONTROLLER: case OFPACT_CT_CLEAR: case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: case OFPACT_DEC_MPLS_TTL: case OFPACT_DEC_TTL: case OFPACT_ENQUEUE: @@ -5635,6 +5636,7 @@ freeze_unroll_actions(const struct ofpact *a, const struct ofpact *end, case OFPACT_ENCAP: case OFPACT_DECAP: case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: case OFPACT_CT: case OFPACT_CT_CLEAR: case OFPACT_NAT: @@ -6130,6 +6132,7 @@ recirc_for_mpls(const struct ofpact *a, struct xlate_ctx *ctx) case OFPACT_CT_CLEAR: case OFPACT_NAT: case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: case OFPACT_METER: case OFPACT_CLEAR_ACTIONS: case OFPACT_WRITE_ACTIONS: @@ -6578,6 +6581,10 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, ctx_trigger_freeze(ctx); a = ofpact_next(a); break; + + case OFPACT_DEBUG_SLOW: + ctx->xout->slow |= SLOW_ACTION; + break; } /* Check if need to store this and the remaining actions for later diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 9a51a1364157..d9dab9ba159a 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -4816,6 +4816,25 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath actions: 2 OVS_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([ofproto-dpif - debug_slow action]) +OVS_VSWITCHD_START +add_of_ports br0 1 2 3 + +AT_CHECK([ovs-ofctl add-flow br0 "in_port=1,actions=debug_slow,2"]) + +flow="in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)" + +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow" -generate], [0], [stdout]) +AT_CHECK_UNQUOTED([tail -3 stdout], [0], [Datapath actions: 2 +This flow is handled by the userspace slow path because it: + - Uses action(s) not supported by datapath. +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + + dnl CHECK_CONTINUATION(TITLE, N_PORTS0, N_PORTS1, ACTIONS0, ACTIONS1, [EXTRA_SETUP]) dnl dnl Checks the implementation of the continuation mechanism that allows the From patchwork Wed Jan 10 22:27:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 858596 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=) 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 3zH3dW6pFkz9s4s for ; Thu, 11 Jan 2018 09:31:27 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id F2E7A1001; Wed, 10 Jan 2018 22:28:24 +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 4D8B61033 for ; Wed, 10 Jan 2018 22:28:21 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 81833E3 for ; Wed, 10 Jan 2018 22:28:20 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id C6DF7C5A50 for ; Wed, 10 Jan 2018 23:28:18 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 10 Jan 2018 14:27:24 -0800 Message-Id: <1515623246-3820-6-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1515623246-3820-1-git-send-email-jpettit@ovn.org> References: <1515623246-3820-1-git-send-email-jpettit@ovn.org> 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 Subject: [ovs-dev] [no-slow v2 6/8] ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user action cookie. 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 Previously, the ofproto instance and OpenFlow port have been derived based on the datapath port number. This change explicitly declares them both, which will be helpful in future commits that no longer can depend on having a unique datapath port (e.g., a source port that represents the controller). Signed-off-by: Justin Pettit Acked-by: Ben Pfaff --- v1->v2: Changes suggested by Ben. --- lib/odp-util.c | 9 +++++++- lib/odp-util.h | 5 ++++- ofproto/ofproto-dpif-upcall.c | 49 ++++++++++++++++++++++++++++++------------- ofproto/ofproto-dpif-xlate.c | 13 +++++++++--- 4 files changed, 56 insertions(+), 20 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index 2910e1514985..a2a8d615f708 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -1142,13 +1142,16 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) } cookie.type = USER_ACTION_COOKIE_SFLOW; + cookie.ofp_in_port = OFPP_NONE; + cookie.ofproto_uuid = UUID_ZERO; cookie.sflow.vlan_tci = htons(tci); cookie.sflow.output = output; } else if (ovs_scan(&s[n], ",slow_path(%n", &n1)) { n += n1; cookie.type = USER_ACTION_COOKIE_SLOW_PATH; - cookie.slow_path.unused = 0; + cookie.ofp_in_port = OFPP_NONE; + cookie.ofproto_uuid = UUID_ZERO; cookie.slow_path.reason = 0; res = parse_odp_flags(&s[n], slow_path_reason_to_string, @@ -1169,6 +1172,8 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) n += n1; cookie.type = USER_ACTION_COOKIE_FLOW_SAMPLE; + cookie.ofp_in_port = OFPP_NONE; + cookie.ofproto_uuid = UUID_ZERO; cookie.flow_sample.probability = probability; cookie.flow_sample.collector_set_id = collector_set_id; cookie.flow_sample.obs_domain_id = obs_domain_id; @@ -1193,6 +1198,8 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) &output, &n1) ) { n += n1; cookie.type = USER_ACTION_COOKIE_IPFIX; + cookie.ofp_in_port = OFPP_NONE; + cookie.ofproto_uuid = UUID_ZERO; cookie.ipfix.output_odp_port = u32_to_odp(output); } else if (ovs_scan(&s[n], ",userdata(%n", &n1)) { diff --git a/lib/odp-util.h b/lib/odp-util.h index b08ff7190168..2a4b3d13812b 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -25,6 +25,7 @@ #include "hash.h" #include "openvswitch/hmap.h" #include "openvswitch/ofp-actions.h" +#include "openvswitch/uuid.h" #include "odp-netlink.h" #include "openflow/openflow.h" #include "util.h" @@ -302,6 +303,8 @@ enum user_action_cookie_type { /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ struct user_action_cookie { uint16_t type; /* enum user_action_cookie_type. */ + ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ + struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ union { struct { @@ -332,7 +335,7 @@ struct user_action_cookie { } ipfix; }; }; -BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) == 28); +BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) == 48); size_t odp_put_userspace_action(uint32_t pid, const void *userdata, size_t userdata_size, diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 5ba1006893dc..bd7dc9e7b718 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -38,6 +38,7 @@ #include "packets.h" #include "openvswitch/poll-loop.h" #include "seq.h" +#include "tunnel.h" #include "unixctl.h" #include "openvswitch/vlog.h" @@ -207,7 +208,7 @@ struct upcall { const ovs_u128 *ufid; /* Unique identifier for 'flow'. */ unsigned pmd_id; /* Datapath poll mode driver id. */ const struct dp_packet *packet; /* Packet associated with this upcall. */ - ofp_port_t in_port; /* OpenFlow in port, or OFPP_NONE. */ + ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ uint16_t mru; /* If !0, Maximum receive unit of fragmented IP packet */ @@ -1021,16 +1022,18 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, * initialized with at least 128 bytes of space. */ static void compose_slow_path(struct udpif *udpif, struct xlate_out *xout, - const struct flow *flow, odp_port_t odp_in_port, + const struct flow *flow, + odp_port_t odp_in_port, ofp_port_t ofp_in_port, struct ofpbuf *buf, uint32_t slowpath_meter_id, - uint32_t controller_meter_id) + uint32_t controller_meter_id, struct uuid *ofproto_uuid) { struct user_action_cookie cookie; odp_port_t port; uint32_t pid; cookie.type = USER_ACTION_COOKIE_SLOW_PATH; - cookie.slow_path.unused = 0; + cookie.ofp_in_port = ofp_in_port; + cookie.ofproto_uuid = *ofproto_uuid; cookie.slow_path.reason = xout->slow; port = xout->slow & (SLOW_CFM | SLOW_BFD | SLOW_LACP | SLOW_STP) @@ -1077,12 +1080,23 @@ upcall_receive(struct upcall *upcall, const struct dpif_backer *backer, upcall->type = classify_upcall(type, userdata, &upcall->cookie); if (upcall->type == BAD_UPCALL) { return EAGAIN; - } - - error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix, - &upcall->sflow, NULL, &upcall->in_port); - if (error) { - return error; + } else if (upcall->type == MISS_UPCALL) { + error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix, + &upcall->sflow, NULL, &upcall->ofp_in_port); + if (error) { + return error; + } + } else { + struct ofproto_dpif *ofproto + = ofproto_dpif_lookup_by_uuid(&upcall->cookie.ofproto_uuid); + if (!ofproto) { + VLOG_INFO_RL(&rl, "upcall could not find ofproto"); + return ENODEV; + } + upcall->ofproto = ofproto; + upcall->ipfix = ofproto->ipfix; + upcall->sflow = ofproto->sflow; + upcall->ofp_in_port = upcall->cookie.ofp_in_port; } upcall->recirc = NULL; @@ -1123,7 +1137,7 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, xlate_in_init(&xin, upcall->ofproto, ofproto_dpif_get_tables_version(upcall->ofproto), - upcall->flow, upcall->in_port, NULL, + upcall->flow, upcall->ofp_in_port, NULL, stats.tcp_flags, upcall->packet, wc, odp_actions); if (upcall->type == MISS_UPCALL) { @@ -1168,8 +1182,9 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, uint32_t cmid = upcall->ofproto->up.controller_meter_id; /* upcall->put_actions already initialized by upcall_receive(). */ compose_slow_path(udpif, &upcall->xout, upcall->flow, - upcall->flow->in_port.odp_port, - &upcall->put_actions, smid, cmid); + upcall->flow->in_port.odp_port, upcall->ofp_in_port, + &upcall->put_actions, smid, cmid, + &upcall->ofproto->uuid); } /* This function is also called for slow-pathed flows. As we are only @@ -2025,13 +2040,17 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey, if (xoutp->slow) { struct ofproto_dpif *ofproto; - ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, NULL); + ofp_port_t ofp_in_port; + + ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, + &ofp_in_port); uint32_t smid = ofproto ? ofproto->up.slowpath_meter_id : UINT32_MAX; uint32_t cmid = ofproto ? ofproto->up.controller_meter_id : UINT32_MAX; ofpbuf_clear(odp_actions); compose_slow_path(udpif, xoutp, &ctx.flow, ctx.flow.in_port.odp_port, - odp_actions, smid, cmid); + ofp_in_port, odp_actions, smid, cmid, + &ofproto->uuid); } if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, &dp_mask, &ctx.flow) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 63d15e94331f..2b1cbddae4e2 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2927,7 +2927,11 @@ compose_sflow_action(struct xlate_ctx *ctx) return 0; } - struct user_action_cookie cookie = { .type = USER_ACTION_COOKIE_SFLOW }; + struct user_action_cookie cookie = { + .type = USER_ACTION_COOKIE_SFLOW, + .ofp_in_port = ctx->xin->flow.in_port.ofp_port, + .ofproto_uuid = ctx->xbridge->ofproto->uuid + }; return compose_sample_action(ctx, dpif_sflow_get_probability(sflow), &cookie, ODPP_NONE, true); } @@ -2969,7 +2973,9 @@ compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t output_odp_port) struct user_action_cookie cookie = { .type = USER_ACTION_COOKIE_IPFIX, - .ipfix.output_odp_port = output_odp_port, + .ofp_in_port = ctx->xin->flow.in_port.ofp_port, + .ofproto_uuid = ctx->xbridge->ofproto->uuid, + .ipfix.output_odp_port = output_odp_port }; compose_sample_action(ctx, dpif_ipfix_get_bridge_exporter_probability(ipfix), @@ -2990,7 +2996,6 @@ fix_sflow_action(struct xlate_ctx *ctx, unsigned int user_cookie_offset) cookie = ofpbuf_at(ctx->odp_actions, user_cookie_offset, sizeof *cookie); ovs_assert(cookie->type == USER_ACTION_COOKIE_SFLOW); - cookie->type = USER_ACTION_COOKIE_SFLOW; cookie->sflow.vlan_tci = base->vlans[0].tci; /* See http://www.sflow.org/sflow_version_5.txt (search for "Input/output @@ -5270,6 +5275,8 @@ xlate_sample_action(struct xlate_ctx *ctx, struct user_action_cookie cookie = { .type = USER_ACTION_COOKIE_FLOW_SAMPLE, + .ofp_in_port = ctx->xin->flow.in_port.ofp_port, + .ofproto_uuid = ctx->xbridge->ofproto->uuid, .flow_sample = { .probability = os->probability, .collector_set_id = os->collector_set_id, From patchwork Wed Jan 10 22:27:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 858600 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=) 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 3zH3fc707vz9s4s for ; Thu, 11 Jan 2018 09:32:24 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 237611034; Wed, 10 Jan 2018 22:28:28 +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 501801021 for ; Wed, 10 Jan 2018 22:28:25 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7D14214D for ; Wed, 10 Jan 2018 22:28:21 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id C58CAC5A44 for ; Wed, 10 Jan 2018 23:28:19 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 10 Jan 2018 14:27:25 -0800 Message-Id: <1515623246-3820-7-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1515623246-3820-1-git-send-email-jpettit@ovn.org> References: <1515623246-3820-1-git-send-email-jpettit@ovn.org> 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 Subject: [ovs-dev] [no-slow v2 7/8] ofproto-dpif: Don't slow-path controller actions. 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 Controller actions have become more commonly used for purposes other than just making forwarding decisions (e.g., packet logging). A packet that needs to be copied to the controller and forwarded would always be sent to ovs-vswitchd to be handled, which could negatively affect performance and cause heavier CPU utilization in ovs-vswitchd. This commit changes the behavior so that OpenFlow controller actions become userspace datapath actions while continuing to let packet forwarding and manipulation continue to be handled by the datapath directly. This patch still slow-paths controller actions with the "pause" flag set. A future patch will stop slow-pathing these pause actions as well. Signed-off-by: Justin Pettit Acked-by: Ben Pfaff --- v1->v2: Changes suggested by Ben. --- Documentation/tutorials/faucet.rst | 46 ++---- NEWS | 3 + lib/odp-util.c | 54 ++++++- lib/odp-util.h | 16 +- ofproto/ofproto-dpif-rid.c | 10 +- ofproto/ofproto-dpif-rid.h | 4 + ofproto/ofproto-dpif-upcall.c | 95 +++++++++--- ofproto/ofproto-dpif-xlate.c | 297 +++++++++++-------------------------- ofproto/ofproto-unixctl.man | 6 +- tests/odp.at | 2 + tests/ofproto-dpif.at | 65 +++++--- tests/pmd.at | 4 +- tests/tunnel-push-pop-ipv6.at | 2 +- tests/tunnel-push-pop.at | 2 +- 14 files changed, 307 insertions(+), 299 deletions(-) diff --git a/Documentation/tutorials/faucet.rst b/Documentation/tutorials/faucet.rst index 2f7ac62343ba..2672791908e6 100644 --- a/Documentation/tutorials/faucet.rst +++ b/Documentation/tutorials/faucet.rst @@ -620,9 +620,7 @@ trivial example:: Final flow: unchanged Megaflow: recirc_id=0,eth,in_port=1,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,dl_type=0x0000 - Datapath actions: push_vlan(vid=100,pcp=0),pop_vlan,2,3 - This flow is handled by the userspace slow path because it: - - Sends "packet-in" messages to the OpenFlow controller. + Datapath actions: push_vlan(vid=100,pcp=0),userspace(pid=0,controller(reason=1,flags=1,recirc_id=1,rule_cookie=0x5adc15c0,controller_id=0,max_len=96)),pop_vlan,2,3 The first line of output, beginning with ``Flow:``, just repeats our request in a more verbose form, including the L2 fields that were @@ -640,18 +638,7 @@ Summary information follows the numbered tables. The packet hasn't been changed (overall, even though a VLAN was pushed and then popped back off) since ingress, hence ``Final flow: unchanged``. We'll look at the ``Megaflow`` information later. The ``Datapath actions`` -summarize what would actually happen to such a packet. Finally, the -note at the end gives a hint that this flow would not perform well for -large volumes of traffic, because it has to be handled in the switch's -slow path since it sends OpenFlow messages to the controller. - -.. note:: - - This performance limitation is probably not problematic in this case - because it is only used for MAC learning, so that most packets won't - encounter it. However, the Open vSwitch 2.9 release (which is - upcoming as of this writing) will likely remove this performance - limitation anyway. +summarize what would actually happen to such a packet. Triggering MAC Learning ~~~~~~~~~~~~~~~~~~~~~~~ @@ -753,18 +740,15 @@ without any further MAC learning, e.g.:: Performance ~~~~~~~~~~~ -We've already seen one factor that can be important for performance: -Open vSwitch forces any flow that sends a packet to an OpenFlow -controller into its "slow path", which means that processing packets -in the flow will be orders of magnitude slower than otherwise. This -distinction between "slow path" and "fast path" is the key to making -sure that Open vSwitch performs as fast as possible. - -In addition to sending packets to a controller, some other factors can -force a flow or a packet to take the slow path. As one example, all -CFM, BFD, LACP, STP, and LLDP processing takes place in the slow path, -in the cases where Open vSwitch processes these protocols itself -instead of delegating to controller-written flows. As a second +Open vSwitch has a concept of a "fast path" and a "slow path"; ideally +all packets stay in the fast path. This distinction between slow path +and fast path is the key to making sure that Open vSwitch performs as +fast as possible. + +Some factors can force a flow or a packet to take the slow path. As one +example, all CFM, BFD, LACP, STP, and LLDP processing takes place in the +slow path, in the cases where Open vSwitch processes these protocols +itself instead of delegating to controller-written flows. As a second example, any flow that modifies ARP fields is processed in the slow path. These are corner cases that are unlikely to cause performance problems in practice because these protocols send packets at a @@ -1142,9 +1126,7 @@ this:: Final flow: udp,in_port=1,dl_vlan=100,dl_vlan_pcp=0,vlan_tci1=0x0000,dl_src=00:01:02:03:04:05,dl_dst=0e:00:00:00:00:01,nw_src=10.100.0.1,nw_dst=10.200.0.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=0 Megaflow: recirc_id=0,eth,ip,in_port=1,vlan_tci=0x0000/0x1fff,dl_src=00:01:02:03:04:05,dl_dst=0e:00:00:00:00:01,nw_dst=10.200.0.0/25,nw_frag=no - Datapath actions: push_vlan(vid=100,pcp=0) - This flow is handled by the userspace slow path because it: - - Sends "packet-in" messages to the OpenFlow controller. + Datapath actions: push_vlan(vid=100,pcp=0),userspace(pid=0,controller(reason=1,flags=0,recirc_id=6,rule_cookie=0x5adc15c0,controller_id=0,max_len=128)) Observe that the packet gets recognized as destined to the router, in table 3, and then as properly destined to the 10.200.0.0/24 network, @@ -1201,9 +1183,7 @@ reply:: Final flow: arp,in_port=4,dl_vlan=200,dl_vlan_pcp=0,vlan_tci1=0x0000,dl_src=00:10:20:30:40:50,dl_dst=0e:00:00:00:00:01,arp_spa=10.200.0.1,arp_tpa=10.200.0.254,arp_op=2,arp_sha=00:10:20:30:40:50,arp_tha=0e:00:00:00:00:01 Megaflow: recirc_id=0,eth,arp,in_port=4,vlan_tci=0x0000/0x1fff,dl_dst=0e:00:00:00:00:01,arp_tpa=10.200.0.254 - Datapath actions: push_vlan(vid=200,pcp=0) - This flow is handled by the userspace slow path because it: - - Sends "packet-in" messages to the OpenFlow controller. + Datapath actions: push_vlan(vid=200,pcp=0),userspace(pid=0,controller(reason=1,flags=0,recirc_id=7,rule_cookie=0x5adc15c0,controller_id=0,max_len=128)) It shows up in ``inst/faucet.log``:: diff --git a/NEWS b/NEWS index fd0e2d84e507..11c086518dfc 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,9 @@ Post-v2.8.0 * ovsdb-tool: New "db-name" and "schema-name" commands. - ovs-vsctl and other commands that display data in tables now support a --max-column-width option to limit column width. + - No longer slow-path traffic that sends to a controller. Applications, + such as OVN ACL logging, want to send a copy of a packet to a + controller while leaving the actual packet forwarding in the datapath. - OVN: * The "requested-chassis" option for a logical switch port now accepts a chassis "hostname" in addition to a chassis "name". diff --git a/lib/odp-util.c b/lib/odp-util.c index a2a8d615f708..1b4b847e1285 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -478,6 +478,21 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, odp_portno_name_format(portno_names, cookie.ipfix.output_odp_port, ds); ds_put_char(ds, ')'); + } else if (cookie.type == USER_ACTION_COOKIE_CONTROLLER) { + ds_put_format(ds, ",controller(reason=%"PRIu16 + ",dont_send=%"PRIu8 + ",recirc_id=%"PRIu32 + ",rule_cookie=%#"PRIx64 + ",controller_id=%"PRIu16 + ",max_len=%"PRIu16, + cookie.controller.reason, + cookie.controller.dont_send ? 1 : 0, + cookie.controller.recirc_id, + ntohll(get_32aligned_be64( + &cookie.controller.rule_cookie)), + cookie.controller.controller_id, + cookie.controller.max_len); + ds_put_char(ds, ')'); } else { userdata_unspec = true; } @@ -1128,6 +1143,15 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) uint32_t collector_set_id; uint32_t obs_domain_id; uint32_t obs_point_id; + + /* USER_ACTION_COOKIE_CONTROLLER. */ + uint8_t dont_send; + uint16_t reason; + uint32_t recirc_id; + ovs_be64 rule_cookie; + uint16_t controller_id; + uint16_t max_len; + int vid, pcp; int n1 = -1; if (ovs_scan(&s[n], ",sFlow(vid=%i," @@ -1201,8 +1225,26 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) cookie.ofp_in_port = OFPP_NONE; cookie.ofproto_uuid = UUID_ZERO; cookie.ipfix.output_odp_port = u32_to_odp(output); - } else if (ovs_scan(&s[n], ",userdata(%n", - &n1)) { + } else if (ovs_scan(&s[n], ",controller(reason=%"SCNu16 + ",dont_send=%"SCNu8 + ",recirc_id=%"SCNu32 + ",rule_cookie=%"SCNx64 + ",controller_id=%"SCNu16 + ",max_len=%"SCNu16")%n", + &reason, &dont_send, &recirc_id, &rule_cookie, + &controller_id, &max_len, &n1)) { + n += n1; + cookie.type = USER_ACTION_COOKIE_CONTROLLER; + cookie.ofp_in_port = OFPP_NONE; + cookie.ofproto_uuid = UUID_ZERO; + cookie.controller.dont_send = dont_send ? true : false; + cookie.controller.reason = reason; + cookie.controller.recirc_id = recirc_id; + put_32aligned_be64(&cookie.controller.rule_cookie, + htonll(rule_cookie)); + cookie.controller.controller_id = controller_id; + cookie.controller.max_len = max_len; + } else if (ovs_scan(&s[n], ",userdata(%n", &n1)) { char *end; n += n1; @@ -3266,7 +3308,7 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr, case OVS_TUNNEL_KEY_ATTR_ID: format_be64(ds, "tun_id", nl_attr_get_be64(a), ma ? nl_attr_get(ma) : NULL, verbose); - flags |= FLOW_TNL_F_KEY; + flags |= FLOW_TNL_F_KEY; if (ma) { mask_flags |= FLOW_TNL_F_KEY; } @@ -3302,10 +3344,10 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr, ma ? nl_attr_get(ma) : NULL, verbose); break; case OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT: - flags |= FLOW_TNL_F_DONT_FRAGMENT; + flags |= FLOW_TNL_F_DONT_FRAGMENT; break; case OVS_TUNNEL_KEY_ATTR_CSUM: - flags |= FLOW_TNL_F_CSUM; + flags |= FLOW_TNL_F_CSUM; break; case OVS_TUNNEL_KEY_ATTR_TP_SRC: format_be16(ds, "tp_src", nl_attr_get_be16(a), @@ -3316,7 +3358,7 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr, ma ? nl_attr_get(ma) : NULL, verbose); break; case OVS_TUNNEL_KEY_ATTR_OAM: - flags |= FLOW_TNL_F_OAM; + flags |= FLOW_TNL_F_OAM; break; case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS: ds_put_cstr(ds, "vxlan("); diff --git a/lib/odp-util.h b/lib/odp-util.h index 2a4b3d13812b..981dcd8f2a23 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -42,9 +42,8 @@ struct pkt_metadata; SPR(SLOW_BFD, "bfd", "Consists of BFD packets") \ SPR(SLOW_LACP, "lacp", "Consists of LACP packets") \ SPR(SLOW_STP, "stp", "Consists of STP packets") \ - SPR(SLOW_LLDP, "lldp", "Consists of LLDP packets") \ - SPR(SLOW_CONTROLLER, "controller", \ - "Sends \"packet-in\" messages to the OpenFlow controller") \ + SPR(SLOW_LLDP, "lldp", "Consists of LLDP packets") \ + SPR(SLOW_PAUSE, "pause", "Controller action with pause") \ SPR(SLOW_ACTION, "action", \ "Uses action(s) not supported by datapath") @@ -298,6 +297,7 @@ enum user_action_cookie_type { USER_ACTION_COOKIE_SLOW_PATH, /* Userspace must process this flow. */ USER_ACTION_COOKIE_FLOW_SAMPLE, /* Packet for per-flow sampling. */ USER_ACTION_COOKIE_IPFIX, /* Packet for per-bridge IPFIX sampling. */ + USER_ACTION_COOKIE_CONTROLLER, /* Forward packet to controller. */ }; /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ @@ -333,6 +333,16 @@ struct user_action_cookie { /* USER_ACTION_COOKIE_IPFIX. */ odp_port_t output_odp_port; /* The output odp port. */ } ipfix; + + struct { + /* USER_ACTION_COOKIE_CONTROLLER. */ + bool dont_send; /* Don't send the packet to controller. */ + uint16_t reason; + uint32_t recirc_id; + ovs_32aligned_be64 rule_cookie; + uint16_t controller_id; + uint16_t max_len; + } controller; }; }; BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) == 48); diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index fc570048954c..83278d82bbef 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -142,6 +142,9 @@ frozen_state_hash(const struct frozen_state *state) hash = hash_bytes64(ALIGNED_CAST(const uint64_t *, state->ofpacts), state->ofpacts_len, hash); } + if (state->userdata && state->userdata_len) { + hash = hash_bytes(state->userdata, state->userdata_len, hash); + } return hash; } @@ -158,7 +161,8 @@ frozen_state_equal(const struct frozen_state *a, const struct frozen_state *b) && ofpacts_equal(a->ofpacts, a->ofpacts_len, b->ofpacts, b->ofpacts_len) && ofpacts_equal(a->action_set, a->action_set_len, - b->action_set, b->action_set_len)); + b->action_set, b->action_set_len) + && !memcmp(a->userdata, b->userdata, a->userdata_len)); } /* Lockless RCU protected lookup. If node is needed accross RCU quiescent @@ -203,6 +207,9 @@ frozen_state_clone(struct frozen_state *new, const struct frozen_state *old) new->action_set = (new->action_set_len ? xmemdup(new->action_set, new->action_set_len) : NULL); + new->userdata = (new->userdata_len + ? xmemdup(new->userdata, new->userdata_len) + : NULL); } static void @@ -211,6 +218,7 @@ frozen_state_free(struct frozen_state *state) free(state->stack); free(state->ofpacts); free(state->action_set); + free(state->userdata); } /* Allocate a unique recirculation id for the given set of flow metadata. diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index 19fc27c7ca0c..858b02e39302 100644 --- a/ofproto/ofproto-dpif-rid.h +++ b/ofproto/ofproto-dpif-rid.h @@ -149,6 +149,10 @@ struct frozen_state { size_t ofpacts_len; /* Size of 'ofpacts', in bytes. */ struct ofpact *action_set; size_t action_set_len; /* Size of 'action_set', in bytes. */ + + /* User data for controller userspace cookie. */ + uint8_t *userdata; + size_t userdata_len; }; /* This maps a recirculation ID to saved state that flow translation can diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index bd7dc9e7b718..c7bfa472f24a 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -187,7 +187,8 @@ enum upcall_type { SLOW_PATH_UPCALL, /* Slow path upcall. */ SFLOW_UPCALL, /* sFlow sample. */ FLOW_SAMPLE_UPCALL, /* Per-flow sampling. */ - IPFIX_UPCALL /* Per-bridge sampling. */ + IPFIX_UPCALL, /* Per-bridge sampling. */ + CONTROLLER_UPCALL /* Destined for the controller. */ }; enum reval_result { @@ -1011,6 +1012,8 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, return FLOW_SAMPLE_UPCALL; } else if (cookie->type == USER_ACTION_COOKIE_IPFIX) { return IPFIX_UPCALL; + } else if (cookie->type == USER_ACTION_COOKIE_CONTROLLER) { + return CONTROLLER_UPCALL; } else { VLOG_WARN_RL(&rl, "invalid user cookie of type %"PRIu16 " and size %"PRIuSIZE, cookie->type, userdata_len); @@ -1024,8 +1027,8 @@ static void compose_slow_path(struct udpif *udpif, struct xlate_out *xout, const struct flow *flow, odp_port_t odp_in_port, ofp_port_t ofp_in_port, - struct ofpbuf *buf, uint32_t slowpath_meter_id, - uint32_t controller_meter_id, struct uuid *ofproto_uuid) + struct ofpbuf *buf, uint32_t meter_id, + struct uuid *ofproto_uuid) { struct user_action_cookie cookie; odp_port_t port; @@ -1043,9 +1046,6 @@ compose_slow_path(struct udpif *udpif, struct xlate_out *xout, size_t offset; size_t ac_offset; - uint32_t meter_id = xout->slow & SLOW_CONTROLLER ? controller_meter_id - : slowpath_meter_id; - if (meter_id != UINT32_MAX) { /* If slowpath meter is configured, generate clone(meter, userspace) * action. */ @@ -1178,12 +1178,11 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, ofpbuf_use_const(&upcall->put_actions, odp_actions->data, odp_actions->size); } else { - uint32_t smid = upcall->ofproto->up.slowpath_meter_id; - uint32_t cmid = upcall->ofproto->up.controller_meter_id; /* upcall->put_actions already initialized by upcall_receive(). */ compose_slow_path(udpif, &upcall->xout, upcall->flow, upcall->flow->in_port.odp_port, upcall->ofp_in_port, - &upcall->put_actions, smid, cmid, + &upcall->put_actions, + upcall->ofproto->up.slowpath_meter_id, &upcall->ofproto->uuid); } @@ -1340,6 +1339,7 @@ dpif_read_actions(struct udpif *udpif, struct upcall *upcall, case BAD_UPCALL: case MISS_UPCALL: case SLOW_PATH_UPCALL: + case CONTROLLER_UPCALL: default: break; } @@ -1409,6 +1409,68 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, } break; + case CONTROLLER_UPCALL: + { + struct user_action_cookie *cookie = &upcall->cookie; + + if (cookie->controller.dont_send) { + return 0; + } + + uint32_t recirc_id = cookie->controller.recirc_id; + if (!recirc_id) { + break; + } + + const struct recirc_id_node *recirc_node + = recirc_id_node_find(recirc_id); + if (!recirc_node) { + break; + } + + struct ofproto_async_msg *am = xmalloc(sizeof *am); + *am = (struct ofproto_async_msg) { + .controller_id = cookie->controller.controller_id, + .oam = OAM_PACKET_IN, + .pin = { + .up = { + .base = { + .packet = xmemdup(dp_packet_data(packet), + dp_packet_size(packet)), + .packet_len = dp_packet_size(packet), + .reason = cookie->controller.reason, + .table_id = recirc_node->state.table_id, + .cookie = get_32aligned_be64( + &cookie->controller.rule_cookie), + .userdata = (recirc_node->state.userdata_len + ? xmemdup(recirc_node->state.userdata, + recirc_node->state.userdata_len) + : NULL), + .userdata_len = recirc_node->state.userdata_len, + }, + }, + .max_len = cookie->controller.max_len, + }, + }; + + /* We don't want to use the upcall 'flow', since it may be + * more specific than the point at which the "controller" + * action was specified. */ + struct flow frozen_flow; + + frozen_flow = *flow; + if (!recirc_node->state.conntracked) { + flow_clear_conntrack(&frozen_flow); + } + + frozen_metadata_to_flow(&recirc_node->state.metadata, + &frozen_flow); + flow_get_metadata(&frozen_flow, &am->pin.up.base.flow_metadata); + + ofproto_dpif_send_async_msg(upcall->ofproto, am); + } + break; + case BAD_UPCALL: break; } @@ -1430,11 +1492,11 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls, * translation is what processes received packets for these * protocols. * - * - For SLOW_CONTROLLER, translation sends the packet to the OpenFlow - * controller. - * * - For SLOW_ACTION, translation executes the actions directly. * + * - For SLOW_PAUSE, translation needs to handle a pause request + * from the controller. + * * The loop fills 'ops' with an array of operations to execute in the * datapath. */ n_ops = 0; @@ -2042,15 +2104,12 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey, struct ofproto_dpif *ofproto; ofp_port_t ofp_in_port; - ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, - &ofp_in_port); - uint32_t smid = ofproto ? ofproto->up.slowpath_meter_id : UINT32_MAX; - uint32_t cmid = ofproto ? ofproto->up.controller_meter_id : UINT32_MAX; + ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, &ofp_in_port); ofpbuf_clear(odp_actions); compose_slow_path(udpif, xoutp, &ctx.flow, ctx.flow.in_port.odp_port, - ofp_in_port, odp_actions, smid, cmid, - &ofproto->uuid); + ofp_in_port, odp_actions, + ofproto->up.slowpath_meter_id, &ofproto->uuid); } if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, &dp_mask, &ctx.flow) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 2b1cbddae4e2..a2520cf487a3 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -4355,174 +4355,13 @@ flood_packets(struct xlate_ctx *ctx, bool all, bool is_last_action) ctx->nf_output_iface = NF_OUT_FLOOD; } -/* Copy and reformat a partially xlated odp actions to a new - * odp actions list in 'b', so that the new actions list - * can be executed by odp_execute_actions. - * - * When xlate using nested odp actions, such as sample and clone, - * the nested action created by nl_msg_start_nested() may not - * have been properly closed yet, thus can not be executed - * directly. - * - * Since unclosed nested action has to be last action, it can be - * fixed by skipping the outer header, and treating the actions within - * as if they are outside the nested attribute since the effect - * of executing them on packet is the same. - * - * As an optimization, a fully closed 'sample' or 'clone' action - * is skipped since their execution has no effect to the packet. - * - * Returns true if success. 'b' contains the new actions list. - * The caller is responsible for disposing 'b'. - * - * Returns false if error, 'b' has been freed already. */ -static bool -xlate_fixup_actions(struct ofpbuf *b, const struct nlattr *actions, - size_t actions_len) -{ - const struct nlattr *a; - unsigned int left; - - NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) { - int type = nl_attr_type(a); - - switch ((enum ovs_action_attr) type) { - case OVS_ACTION_ATTR_HASH: - case OVS_ACTION_ATTR_PUSH_VLAN: - case OVS_ACTION_ATTR_POP_VLAN: - case OVS_ACTION_ATTR_PUSH_MPLS: - case OVS_ACTION_ATTR_POP_MPLS: - case OVS_ACTION_ATTR_SET: - case OVS_ACTION_ATTR_SET_MASKED: - case OVS_ACTION_ATTR_TRUNC: - case OVS_ACTION_ATTR_OUTPUT: - case OVS_ACTION_ATTR_TUNNEL_PUSH: - case OVS_ACTION_ATTR_TUNNEL_POP: - case OVS_ACTION_ATTR_USERSPACE: - case OVS_ACTION_ATTR_RECIRC: - case OVS_ACTION_ATTR_CT: - case OVS_ACTION_ATTR_PUSH_ETH: - case OVS_ACTION_ATTR_POP_ETH: - case OVS_ACTION_ATTR_PUSH_NSH: - case OVS_ACTION_ATTR_POP_NSH: - case OVS_ACTION_ATTR_METER: - ofpbuf_put(b, a, nl_attr_len_pad(a, left)); - break; - - case OVS_ACTION_ATTR_CLONE: - /* If the clone action has been fully xlated, it can - * be skipped, since any actions executed within clone - * do not affect the current packet. - * - * When xlating actions within clone, the clone action, - * because it is an nested netlink attribute, do not have - * a valid 'nla_len'; it will be zero instead. Skip - * the clone header to find the start of the actions - * enclosed. Treat those actions as if they are written - * outside of clone. */ - if (!a->nla_len) { - bool ok; - if (left < NLA_HDRLEN) { - goto error; - } - - ok = xlate_fixup_actions(b, nl_attr_get_unspec(a, 0), - left - NLA_HDRLEN); - if (!ok) { - goto error; - } - } - break; - - case OVS_ACTION_ATTR_SAMPLE: - if (!a->nla_len) { - bool ok; - if (left < NLA_HDRLEN) { - goto error; - } - const struct nlattr *attr = nl_attr_get_unspec(a, 0); - left -= NLA_HDRLEN; - - while (left > 0 && - nl_attr_type(attr) != OVS_SAMPLE_ATTR_ACTIONS) { - /* Only OVS_SAMPLE_ATTR_ACTIONS can have unclosed - * nested netlink attribute. */ - if (!attr->nla_len) { - goto error; - } - - left -= NLA_ALIGN(attr->nla_len); - attr = nl_attr_next(attr); - } - - if (left < NLA_HDRLEN) { - goto error; - } - - ok = xlate_fixup_actions(b, nl_attr_get_unspec(attr, 0), - left - NLA_HDRLEN); - if (!ok) { - goto error; - } - } - break; - - case OVS_ACTION_ATTR_UNSPEC: - case __OVS_ACTION_ATTR_MAX: - OVS_NOT_REACHED(); - } - } - - return true; - -error: - ofpbuf_delete(b); - return false; -} - -static bool -xlate_execute_odp_actions(struct dp_packet *packet, - const struct nlattr *actions, int actions_len) -{ - struct dp_packet_batch batch; - struct ofpbuf *b = ofpbuf_new(actions_len); - - if (!xlate_fixup_actions(b, actions, actions_len)) { - return false; - } - - dp_packet_batch_init_packet(&batch, packet); - odp_execute_actions(NULL, &batch, false, b->data, b->size, NULL); - ofpbuf_delete(b); - - return true; -} - static void -execute_controller_action(struct xlate_ctx *ctx, int len, - enum ofp_packet_in_reason reason, - uint16_t controller_id, - const uint8_t *userdata, size_t userdata_len) +xlate_controller_action(struct xlate_ctx *ctx, int len, + enum ofp_packet_in_reason reason, + uint16_t controller_id, + const uint8_t *userdata, size_t userdata_len) { - struct dp_packet *packet; - - ctx->xout->slow |= SLOW_CONTROLLER; xlate_commit_actions(ctx); - if (!ctx->xin->packet) { - return; - } - - if (!ctx->xin->allow_side_effects && !ctx->xin->xcache) { - return; - } - - packet = dp_packet_clone(ctx->xin->packet); - if (!xlate_execute_odp_actions(packet, ctx->odp_actions->data, - ctx->odp_actions->size)) { - xlate_report_error(ctx, "Failed to execute controller action"); - dp_packet_delete(packet); - return; - } /* A packet sent by an action in a table-miss rule is considered an * explicit table miss. OpenFlow before 1.3 doesn't have that concept so @@ -4532,44 +4371,74 @@ execute_controller_action(struct xlate_ctx *ctx, int len, reason = OFPR_EXPLICIT_MISS; } - size_t packet_len = dp_packet_size(packet); - - struct ofproto_async_msg *am = xmalloc(sizeof *am); - *am = (struct ofproto_async_msg) { - .controller_id = controller_id, - .oam = OAM_PACKET_IN, - .pin = { - .up = { - .base = { - .packet = dp_packet_steal_data(packet), - .packet_len = packet_len, - .reason = reason, - .table_id = ctx->table_id, - .cookie = ctx->rule_cookie, - .userdata = (userdata_len - ? xmemdup(userdata, userdata_len) - : NULL), - .userdata_len = userdata_len, - } - }, - .max_len = len, - }, + struct frozen_state state = { + .table_id = ctx->table_id, + .ofproto_uuid = ctx->xbridge->ofproto->uuid, + .stack = ctx->stack.data, + .stack_size = ctx->stack.size, + .mirrors = ctx->mirrors, + .conntracked = ctx->conntracked, + .ofpacts = NULL, + .ofpacts_len = 0, + .action_set = NULL, + .action_set_len = 0, + .userdata = CONST_CAST(uint8_t *, userdata), + .userdata_len = userdata_len, }; - flow_get_metadata(&ctx->xin->flow, &am->pin.up.base.flow_metadata); + frozen_metadata_from_flow(&state.metadata, &ctx->xin->flow); - /* Async messages are only sent once, so if we send one now, no - * xlate cache entry is created. */ - if (ctx->xin->allow_side_effects) { - ofproto_dpif_send_async_msg(ctx->xbridge->ofproto, am); - } else /* xcache */ { - struct xc_entry *entry; + uint32_t recirc_id = recirc_alloc_id_ctx(&state); + if (!recirc_id) { + xlate_report_error(ctx, "Failed to allocate recirculation id"); + ctx->error = XLATE_NO_RECIRCULATION_CONTEXT; + return; + } + recirc_refs_add(&ctx->xout->recircs, recirc_id); - entry = xlate_cache_add_entry(ctx->xin->xcache, XC_CONTROLLER); - entry->controller.ofproto = ctx->xbridge->ofproto; - entry->controller.am = am; + size_t offset; + size_t ac_offset; + uint32_t meter_id = ctx->xbridge->ofproto->up.controller_meter_id; + if (meter_id != UINT32_MAX) { + /* If controller meter is configured, generate clone(meter, userspace) + * action. */ + offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_SAMPLE); + nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, + UINT32_MAX); + ac_offset = nl_msg_start_nested(ctx->odp_actions, + OVS_SAMPLE_ATTR_ACTIONS); + nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER, meter_id); } - dp_packet_delete(packet); + struct user_action_cookie cookie; + + memset(&cookie, 0, sizeof cookie); + cookie.type = USER_ACTION_COOKIE_CONTROLLER; + cookie.ofp_in_port = OFPP_NONE; + cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid; + cookie.controller.dont_send = false; + cookie.controller.reason = reason; + cookie.controller.recirc_id = recirc_id; + put_32aligned_be64(&cookie.controller.rule_cookie, ctx->rule_cookie); + cookie.controller.max_len = len; + cookie.controller.controller_id = controller_id; + + /* Generate the datapath flows even if we don't send the packet-in + * so that debugging more closely represents normal state. */ + if (!ctx->xin->allow_side_effects && !ctx->xin->xcache) { + cookie.controller.dont_send = true; + } + + odp_port_t odp_port = ofp_port_to_odp_port( + ctx->xbridge, ctx->xin->flow.in_port.ofp_port); + uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port, + flow_hash_5tuple(&ctx->xin->flow, 0)); + odp_put_userspace_action(pid, &cookie, sizeof cookie, ODPP_NONE, + false, ctx->odp_actions); + + if (meter_id != UINT32_MAX) { + nl_msg_end_nested(ctx->odp_actions, ac_offset); + nl_msg_end_nested(ctx->odp_actions, offset); + } } static void @@ -4782,8 +4651,8 @@ compose_dec_ttl(struct xlate_ctx *ctx, struct ofpact_cnt_ids *ids) size_t i; for (i = 0; i < ids->n_controllers; i++) { - execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, - ids->cnt_ids[i], NULL, 0); + xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, + ids->cnt_ids[i], NULL, 0); } /* Stop processing for current table. */ @@ -4834,8 +4703,8 @@ compose_dec_mpls_ttl_action(struct xlate_ctx *ctx) set_mpls_lse_ttl(&flow->mpls_lse[0], ttl); return false; } else { - execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0, - NULL, 0); + xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0, + NULL, 0); } } @@ -4888,12 +4757,12 @@ xlate_output_action(struct xlate_ctx *ctx, ofp_port_t port, flood_packets(ctx, true, is_last_action); break; case OFPP_CONTROLLER: - execute_controller_action(ctx, controller_len, - (ctx->in_packet_out ? OFPR_PACKET_OUT - : ctx->in_group ? OFPR_GROUP - : ctx->in_action_set ? OFPR_ACTION_SET - : OFPR_ACTION), - 0, NULL, 0); + xlate_controller_action(ctx, controller_len, + (ctx->in_packet_out ? OFPR_PACKET_OUT + : ctx->in_group ? OFPR_GROUP + : ctx->in_action_set ? OFPR_ACTION_SET + : OFPR_ACTION), + 0, NULL, 0); break; case OFPP_NONE: break; @@ -6251,16 +6120,16 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, controller = ofpact_get_CONTROLLER(a); if (controller->pause) { ctx->pause = controller; - ctx->xout->slow |= SLOW_CONTROLLER; + ctx->xout->slow |= SLOW_PAUSE; *ctx->paused_flow = ctx->xin->flow; ctx_trigger_freeze(ctx); a = ofpact_next(a); } else { - execute_controller_action(ctx, controller->max_len, - controller->reason, - controller->controller_id, - controller->userdata, - controller->userdata_len); + xlate_controller_action(ctx, controller->max_len, + controller->reason, + controller->controller_id, + controller->userdata, + controller->userdata_len); } break; diff --git a/ofproto/ofproto-unixctl.man b/ofproto/ofproto-unixctl.man index f511c392b548..ee1f81fceaec 100644 --- a/ofproto/ofproto-unixctl.man +++ b/ofproto/ofproto-unixctl.man @@ -107,9 +107,9 @@ effects when a packet is specified. If you want side effects to take place, then you must supply a packet. . .IP -(Output actions are obviously side effects too, but -the trace commands never execute them, even when one specifies a -packet.) +(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.) . .IP "Incomplete information." Most of the time, Open vSwitch can figure out everything about the diff --git a/tests/odp.at b/tests/odp.at index 4891653eb81a..1f8080aed1ed 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -260,6 +260,8 @@ userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_ userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10,egress),tunnel_out_port=10) userspace(pid=6633,ipfix(output_port=10)) userspace(pid=6633,ipfix(output_port=10),tunnel_out_port=10) +userspace(pid=6633,controller(reason=1,dont_send=0,recirc_id=4444,rule_cookie=0x5555,controller_id=0,max_len=65535)) +userspace(pid=6633,controller(reason=1,dont_send=1,recirc_id=4444,rule_cookie=0x5555,controller_id=0,max_len=65535)) set(in_port(2)) set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15)) set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15/ff:ff:ff:00:00:00)) diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index d9dab9ba159a..4c092bc2e3d5 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -781,11 +781,10 @@ table=1 in_port=1 action=dec_ttl,output:3 ]) AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=111,tos=0,ttl=2,frag=no)' -generate], [0], [stdout]) -AT_CHECK([tail -4 stdout], [0], - [Megaflow: recirc_id=0,eth,ip,in_port=1,nw_ttl=2,nw_frag=no -Datapath actions: set(ipv4(ttl=1)),2,4 -This flow is handled by the userspace slow path because it: - - Sends "packet-in" messages to the OpenFlow controller. +AT_CHECK([tail -4 stdout], [0], [ +Final flow: ip,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=111,nw_tos=0,nw_ecn=0,nw_ttl=1 +Megaflow: recirc_id=0,eth,ip,in_port=1,nw_ttl=2,nw_frag=no +Datapath actions: set(ipv4(ttl=1)),2,userspace(pid=0,controller(reason=2,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)),4 ]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=111,tos=0,ttl=3,frag=no)'], [0], [stdout]) AT_CHECK([tail -2 stdout], [0], @@ -800,7 +799,9 @@ Datapath actions: set(ipv6(hlimit=127)),2,set(ipv6(hlimit=126)),3,4 AT_CAPTURE_FILE([ofctl_monitor.log]) AT_CHECK([ovs-ofctl -P nxt_packet_in monitor br0 65534 invalid_ttl --detach --no-chdir --pidfile 2> ofctl_monitor.log]) -AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=111,tos=0,ttl=2,frag=no)' -generate], [0], [stdout]) + +ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=111,tos=0,ttl=2,frag=no)' + OVS_APP_EXIT_AND_WAIT([ovs-ofctl]) AT_CHECK([cat ofctl_monitor.log], [0], [dnl NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=34 in_port=1 (via invalid_ttl) data_len=34 (unbuffered) @@ -1628,6 +1629,37 @@ NXST_FLOW reply: OVS_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([ofproto-dpif - controller with slow-path action]) +OVS_VSWITCHD_START +add_of_ports br0 1 2 + +AT_CHECK([ovs-ofctl add-flow br0 "in_port=1,actions=debug_slow,controller"]) + +AT_CHECK([ovs-ofctl monitor -P standard br0 65534 --detach --no-chdir --pidfile 2> ofctl_monitor.log]) + +for i in 1 2 3 ; do + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=9),tcp_flags(0x010)' +done + +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 3]) +OVS_APP_EXIT_AND_WAIT(ovs-ofctl) + +AT_CHECK([cat ofctl_monitor.log], [0], [dnl +OFPT_PACKET_IN (xid=0x0): total_len=54 in_port=1 (via action) data_len=54 (unbuffered) +tcp,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=9,tcp_flags=ack tcp_csum:2e70 +dnl +OFPT_PACKET_IN (xid=0x0): total_len=54 in_port=1 (via action) data_len=54 (unbuffered) +tcp,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=9,tcp_flags=ack tcp_csum:2e70 +dnl +OFPT_PACKET_IN (xid=0x0): total_len=54 in_port=1 (via action) data_len=54 (unbuffered) +tcp,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=9,tcp_flags=ack tcp_csum:2e70 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + + AT_SETUP([ofproto-dpif - controller action without megaflows]) OVS_VSWITCHD_START add_of_ports br0 1 @@ -1649,7 +1681,7 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/.*\(packets:\)/\1/' | sed 's/used:[[0-9]].[[0-9]]*s/used:0.001s/'], [0], [dnl flow-dump from non-dpdk interfaces: -packets:1, bytes:14, used:0.001s, actions:userspace(pid=0,slow_path(controller)) +packets:1, bytes:14, used:0.001s, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) ]) AT_CHECK([cat ofctl_monitor.log], [0], [dnl @@ -1663,7 +1695,7 @@ AT_CHECK([ovs-appctl revalidator/purge]) AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl -P nxt_packet_in --detach --no-chdir --pidfile 2> ofctl_monitor.log]) dnl Add a controller meter. -AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=controller pktps stats bands=type=drop rate=1']) +AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=controller pktps stats bands=type=drop rate=2']) dnl Advance time by 1 second. AT_CHECK([ovs-appctl time/warp 1000], [0], [ignore]) @@ -1674,15 +1706,14 @@ done AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/.*\(packets:\)/\1/' | sed 's/used:[[0-9]].[[0-9]]*s/used:0.001s/'], [0], [dnl flow-dump from non-dpdk interfaces: -packets:7, bytes:98, used:0.001s, actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,slow_path(controller)))) +packets:7, bytes:98, used:0.001s, actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535)))) ]) AT_CHECK([ovs-appctl time/warp 1], [0], [ignore]) OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) -dnl Out of 8 packets we sent, one executes the controller action via -dnl miss upcall. Another one got passed the rate limiter. -dnl The rest of packets are blocked by the rate limiter. +dnl Out of 8 packets we sent, two were passed by the rate limiter, and +dnl the rest of packets were blocked. AT_CHECK([cat ofctl_monitor.log], [0], [dnl NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=14 in_port=1 (via action) data_len=14 (unbuffered) vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x4321 @@ -1693,7 +1724,7 @@ dnl Check meter stats to make it gives the same picture; dnl 7 packets hit the meter, but 6 packets are dropped by band0. AT_CHECK([ovs-ofctl -O OpenFlow13 meter-stats br0 | strip_timers], [0], [dnl OFPST_METER reply (OF1.3) (xid=0x2): -meter:controller flow_count:0 packet_in_count:7 byte_in_count:98 duration:0.0s bands: +meter:controller flow_count:0 packet_in_count:8 byte_in_count:112 duration:0.0s bands: 0: packet_count:6 byte_count:84 ]) @@ -7398,8 +7429,8 @@ for dl_src in 00 01; do done sleep 1 # wait for the datapath flow installed AT_CHECK_UNQUOTED([strip_ufid < ovs-vswitchd.log | filter_flow_install | strip_used], [0], [dnl -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:00),eth_type(0x8847),mpls(label=20,tc=0,ttl=32,bos=0,label=20,tc=0,ttl=32,bos=1), actions:userspace(pid=0,slow_path(controller)) -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:01),eth_type(0x8847),mpls(label=20/0x0,tc=0/0,ttl=32/0x0,bos=0/1,label=20/0xfffff,tc=0/7,ttl=32/0xff,bos=1/1), actions:userspace(pid=0,slow_path(controller)) +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:00),eth_type(0x8847),mpls(label=20,tc=0,ttl=32,bos=0,label=20,tc=0,ttl=32,bos=1), actions:push_mpls(label=20,tc=0,ttl=32,bos=0,eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:01),eth_type(0x8847),mpls(label=20/0x0,tc=0/0,ttl=32/0x0,bos=0/1,label=20/0xfffff,tc=0/7,ttl=32/0xff,bos=1/1), actions:pop_mpls(eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) ]) OVS_VSWITCHD_STOP @@ -7437,8 +7468,8 @@ for dl_src in 00 01; do done sleep 1 # wait for the datapath flow installed AT_CHECK_UNQUOTED([strip_ufid < ovs-vswitchd.log | filter_flow_install | strip_used], [0], [dnl -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:00),eth_type(0x8847),mpls(label=20,tc=0,ttl=32,bos=0,label=20,tc=0,ttl=32,bos=1), actions:userspace(pid=0,slow_path(controller)) -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:01),eth_type(0x8847),mpls(label=20/0x0,tc=0/0,ttl=32/0x0,bos=0/1,label=20/0xfffff,tc=0/7,ttl=32/0xff,bos=1/1), actions:userspace(pid=0,slow_path(controller)) +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:00),eth_type(0x8847),mpls(label=20,tc=0,ttl=32,bos=0,label=20,tc=0,ttl=32,bos=1), actions:push_mpls(label=20,tc=0,ttl=32,bos=0,eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:01),eth_type(0x8847),mpls(label=20/0x0,tc=0/0,ttl=32/0x0,bos=0/1,label=20/0xfffff,tc=0/7,ttl=32/0xff,bos=1/1), actions:pop_mpls(eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) ]) OVS_VSWITCHD_STOP diff --git a/tests/pmd.at b/tests/pmd.at index e39a23a43386..adfdb4c01e78 100644 --- a/tests/pmd.at +++ b/tests/pmd.at @@ -303,8 +303,8 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) dnl Make sure that both flows have been installed AT_CHECK([ovs-appctl dpctl/dump-flows | flow_dump_prepend_pmd], [0], [dnl -0 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) -1 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) +0 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +1 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) ]) AT_CHECK([cat ofctl_monitor.log], [0], [dnl diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at index 8f51c067ebd4..4d609b2f1944 100644 --- a/tests/tunnel-push-pop-ipv6.at +++ b/tests/tunnel-push-pop-ipv6.at @@ -164,7 +164,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 5'], [0], [dnl port 5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=? ]) AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl -tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) +tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535)) ]) ovs-appctl time/warp 10000 diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 5f2f172a2f7e..cdfbb36a47d9 100644 --- a/tests/tunnel-push-pop.at +++ b/tests/tunnel-push-pop.at @@ -215,7 +215,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 5'], [0], [dnl port 5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=? ]) AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl -tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) +tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535)) ]) ovs-appctl time/warp 10000 From patchwork Wed Jan 10 22:27:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 858598 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=) 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 3zH3f42Nx2z9s4s for ; Thu, 11 Jan 2018 09:31:56 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 17D2D104D; Wed, 10 Jan 2018 22:28:27 +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 8A3381001 for ; Wed, 10 Jan 2018 22:28:24 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 81D2112D for ; Wed, 10 Jan 2018 22:28:22 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id C09C6C5A51 for ; Wed, 10 Jan 2018 23:28:20 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 10 Jan 2018 14:27:26 -0800 Message-Id: <1515623246-3820-8-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1515623246-3820-1-git-send-email-jpettit@ovn.org> References: <1515623246-3820-1-git-send-email-jpettit@ovn.org> 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 Subject: [ovs-dev] [no-slow v2 8/8] ofproto-dpif: Don't slow-path controller actions with pause. 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 A previous patch removed slow-pathing for controller actions with the exception of ones that specified "pause". This commit removes that restriction so that no controller actions are slow-pathed. Signed-off-by: Justin Pettit --- v1->v2: Changes suggested by Ben. --- lib/odp-util.c | 9 ++- lib/odp-util.h | 2 +- ofproto/ofproto-dpif-upcall.c | 31 ++++++-- ofproto/ofproto-dpif-xlate-cache.c | 13 ---- ofproto/ofproto-dpif-xlate-cache.h | 1 - ofproto/ofproto-dpif-xlate.c | 151 ++++++++++++++----------------------- ofproto/ofproto-dpif.c | 1 - tests/odp.at | 4 +- tests/ofproto-dpif.at | 14 ++-- tests/pmd.at | 4 +- tests/tunnel-push-pop-ipv6.at | 2 +- tests/tunnel-push-pop.at | 2 +- 12 files changed, 102 insertions(+), 132 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index 1b4b847e1285..f8c84e17330f 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -481,12 +481,14 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, } else if (cookie.type == USER_ACTION_COOKIE_CONTROLLER) { ds_put_format(ds, ",controller(reason=%"PRIu16 ",dont_send=%"PRIu8 + ",continuation=%"PRIu8 ",recirc_id=%"PRIu32 ",rule_cookie=%#"PRIx64 ",controller_id=%"PRIu16 ",max_len=%"PRIu16, cookie.controller.reason, cookie.controller.dont_send ? 1 : 0, + cookie.controller.continuation ? 1 : 0, cookie.controller.recirc_id, ntohll(get_32aligned_be64( &cookie.controller.rule_cookie)), @@ -1146,6 +1148,7 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) /* USER_ACTION_COOKIE_CONTROLLER. */ uint8_t dont_send; + uint8_t continuation; uint16_t reason; uint32_t recirc_id; ovs_be64 rule_cookie; @@ -1227,17 +1230,19 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) cookie.ipfix.output_odp_port = u32_to_odp(output); } else if (ovs_scan(&s[n], ",controller(reason=%"SCNu16 ",dont_send=%"SCNu8 + ",continuation=%"SCNu8 ",recirc_id=%"SCNu32 ",rule_cookie=%"SCNx64 ",controller_id=%"SCNu16 ",max_len=%"SCNu16")%n", - &reason, &dont_send, &recirc_id, &rule_cookie, - &controller_id, &max_len, &n1)) { + &reason, &dont_send, &continuation, &recirc_id, + &rule_cookie, &controller_id, &max_len, &n1)) { n += n1; cookie.type = USER_ACTION_COOKIE_CONTROLLER; cookie.ofp_in_port = OFPP_NONE; cookie.ofproto_uuid = UUID_ZERO; cookie.controller.dont_send = dont_send ? true : false; + cookie.controller.continuation = continuation ? true : false; cookie.controller.reason = reason; cookie.controller.recirc_id = recirc_id; put_32aligned_be64(&cookie.controller.rule_cookie, diff --git a/lib/odp-util.h b/lib/odp-util.h index 981dcd8f2a23..028c9e37ce63 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -43,7 +43,6 @@ struct pkt_metadata; SPR(SLOW_LACP, "lacp", "Consists of LACP packets") \ SPR(SLOW_STP, "stp", "Consists of STP packets") \ SPR(SLOW_LLDP, "lldp", "Consists of LLDP packets") \ - SPR(SLOW_PAUSE, "pause", "Controller action with pause") \ SPR(SLOW_ACTION, "action", \ "Uses action(s) not supported by datapath") @@ -337,6 +336,7 @@ struct user_action_cookie { struct { /* USER_ACTION_COOKIE_CONTROLLER. */ bool dont_send; /* Don't send the packet to controller. */ + bool continuation; /* Send packet-in as a continuation. */ uint16_t reason; uint32_t recirc_id; ovs_32aligned_be64 rule_cookie; diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index c7bfa472f24a..cb5018d9952c 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1428,6 +1428,8 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, break; } + const struct frozen_state *state = &recirc_node->state; + struct ofproto_async_msg *am = xmalloc(sizeof *am); *am = (struct ofproto_async_msg) { .controller_id = cookie->controller.controller_id, @@ -1439,7 +1441,7 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, dp_packet_size(packet)), .packet_len = dp_packet_size(packet), .reason = cookie->controller.reason, - .table_id = recirc_node->state.table_id, + .table_id = state->table_id, .cookie = get_32aligned_be64( &cookie->controller.rule_cookie), .userdata = (recirc_node->state.userdata_len @@ -1453,18 +1455,36 @@ process_upcall(struct udpif *udpif, struct upcall *upcall, }, }; + if (cookie->controller.continuation) { + am->pin.up.stack = (state->stack_size + ? xmemdup(state->stack, state->stack_size) + : NULL), + am->pin.up.stack_size = state->stack_size, + am->pin.up.mirrors = state->mirrors, + am->pin.up.conntracked = state->conntracked, + am->pin.up.actions = (state->ofpacts_len + ? xmemdup(state->ofpacts, + state->ofpacts_len) : NULL), + am->pin.up.actions_len = state->ofpacts_len, + am->pin.up.action_set = (state->action_set_len + ? xmemdup(state->action_set, + state->action_set_len) + : NULL), + am->pin.up.action_set_len = state->action_set_len, + am->pin.up.bridge = upcall->ofproto->uuid; + } + /* We don't want to use the upcall 'flow', since it may be * more specific than the point at which the "controller" * action was specified. */ struct flow frozen_flow; frozen_flow = *flow; - if (!recirc_node->state.conntracked) { + if (!state->conntracked) { flow_clear_conntrack(&frozen_flow); } - frozen_metadata_to_flow(&recirc_node->state.metadata, - &frozen_flow); + frozen_metadata_to_flow(&state->metadata, &frozen_flow); flow_get_metadata(&frozen_flow, &am->pin.up.base.flow_metadata); ofproto_dpif_send_async_msg(upcall->ofproto, am); @@ -1494,9 +1514,6 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls, * * - For SLOW_ACTION, translation executes the actions directly. * - * - For SLOW_PAUSE, translation needs to handle a pause request - * from the controller. - * * The loop fills 'ops' with an array of operations to execute in the * datapath. */ n_ops = 0; diff --git a/ofproto/ofproto-dpif-xlate-cache.c b/ofproto/ofproto-dpif-xlate-cache.c index 5f53a525a6ae..2a2e77a42f8b 100644 --- a/ofproto/ofproto-dpif-xlate-cache.c +++ b/ofproto/ofproto-dpif-xlate-cache.c @@ -154,13 +154,6 @@ xlate_push_stats_entry(struct xc_entry *entry, tnl_neigh_lookup(entry->tnl_neigh_cache.br_name, &entry->tnl_neigh_cache.d_ipv6, &dmac); break; - case XC_CONTROLLER: - if (entry->controller.am) { - ofproto_dpif_send_async_msg(entry->controller.ofproto, - entry->controller.am); - entry->controller.am = NULL; /* One time only. */ - } - break; case XC_TUNNEL_HEADER: if (entry->tunnel_hdr.operation == ADD) { stats->n_bytes += stats->n_packets * entry->tunnel_hdr.hdr_size; @@ -248,12 +241,6 @@ xlate_cache_clear_entry(struct xc_entry *entry) break; case XC_TNL_NEIGH: break; - case XC_CONTROLLER: - if (entry->controller.am) { - ofproto_async_msg_free(entry->controller.am); - entry->controller.am = NULL; - } - break; case XC_TUNNEL_HEADER: break; default: diff --git a/ofproto/ofproto-dpif-xlate-cache.h b/ofproto/ofproto-dpif-xlate-cache.h index 19c1ef7b95ad..e5dae6dc660e 100644 --- a/ofproto/ofproto-dpif-xlate-cache.h +++ b/ofproto/ofproto-dpif-xlate-cache.h @@ -52,7 +52,6 @@ enum xc_type { XC_FIN_TIMEOUT, /* Calls back to ofproto. */ XC_GROUP, XC_TNL_NEIGH, - XC_CONTROLLER, XC_TUNNEL_HEADER, }; diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index a2520cf487a3..e1fc7d58a622 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -365,7 +365,6 @@ struct xlate_ctx { uint32_t dp_hash_basis; struct ofpbuf frozen_actions; const struct ofpact_controller *pause; - struct flow *paused_flow; /* True if a packet was but is no longer MPLS (due to an MPLS pop action). * This is a trigger for recirculation in cases where translating an action @@ -4356,6 +4355,35 @@ flood_packets(struct xlate_ctx *ctx, bool all, bool is_last_action) } static void +put_controller_user_action(struct xlate_ctx *ctx, + bool dont_send, bool continuation, + uint32_t recirc_id, int len, + enum ofp_packet_in_reason reason, + uint16_t controller_id) +{ + struct user_action_cookie cookie; + + memset(&cookie, 0, sizeof cookie); + cookie.type = USER_ACTION_COOKIE_CONTROLLER; + cookie.ofp_in_port = OFPP_NONE, + cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid; + cookie.controller.dont_send = dont_send; + cookie.controller.continuation = continuation; + cookie.controller.reason = reason; + cookie.controller.recirc_id = recirc_id; + put_32aligned_be64(&cookie.controller.rule_cookie, ctx->rule_cookie); + cookie.controller.controller_id = controller_id; + cookie.controller.max_len = len; + + odp_port_t odp_port = ofp_port_to_odp_port(ctx->xbridge, + ctx->xin->flow.in_port.ofp_port); + uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port, + flow_hash_5tuple(&ctx->xin->flow, 0)); + odp_put_userspace_action(pid, &cookie, sizeof cookie, ODPP_NONE, + false, ctx->odp_actions); +} + +static void xlate_controller_action(struct xlate_ctx *ctx, int len, enum ofp_packet_in_reason reason, uint16_t controller_id, @@ -4409,31 +4437,14 @@ xlate_controller_action(struct xlate_ctx *ctx, int len, nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER, meter_id); } - struct user_action_cookie cookie; - - memset(&cookie, 0, sizeof cookie); - cookie.type = USER_ACTION_COOKIE_CONTROLLER; - cookie.ofp_in_port = OFPP_NONE; - cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid; - cookie.controller.dont_send = false; - cookie.controller.reason = reason; - cookie.controller.recirc_id = recirc_id; - put_32aligned_be64(&cookie.controller.rule_cookie, ctx->rule_cookie); - cookie.controller.max_len = len; - cookie.controller.controller_id = controller_id; - /* Generate the datapath flows even if we don't send the packet-in * so that debugging more closely represents normal state. */ + bool dont_send = false; if (!ctx->xin->allow_side_effects && !ctx->xin->xcache) { - cookie.controller.dont_send = true; + dont_send = true; } - - odp_port_t odp_port = ofp_port_to_odp_port( - ctx->xbridge, ctx->xin->flow.in_port.ofp_port); - uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port, - flow_hash_5tuple(&ctx->xin->flow, 0)); - odp_put_userspace_action(pid, &cookie, sizeof cookie, ODPP_NONE, - false, ctx->odp_actions); + put_controller_user_action(ctx, dont_send, false, recirc_id, len, + reason, controller_id); if (meter_id != UINT32_MAX) { nl_msg_end_nested(ctx->odp_actions, ac_offset); @@ -4441,57 +4452,6 @@ xlate_controller_action(struct xlate_ctx *ctx, int len, } } -static void -emit_continuation(struct xlate_ctx *ctx, const struct frozen_state *state) -{ - if (!ctx->xin->allow_side_effects && !ctx->xin->xcache) { - return; - } - - struct ofproto_async_msg *am = xmalloc(sizeof *am); - *am = (struct ofproto_async_msg) { - .controller_id = ctx->pause->controller_id, - .oam = OAM_PACKET_IN, - .pin = { - .up = { - .base = { - .userdata = xmemdup(ctx->pause->userdata, - ctx->pause->userdata_len), - .userdata_len = ctx->pause->userdata_len, - .packet = xmemdup(dp_packet_data(ctx->xin->packet), - dp_packet_size(ctx->xin->packet)), - .packet_len = dp_packet_size(ctx->xin->packet), - .reason = ctx->pause->reason, - }, - .bridge = ctx->xbridge->ofproto->uuid, - .stack = xmemdup(state->stack, state->stack_size), - .stack_size = state->stack_size, - .mirrors = state->mirrors, - .conntracked = state->conntracked, - .actions = xmemdup(state->ofpacts, state->ofpacts_len), - .actions_len = state->ofpacts_len, - .action_set = xmemdup(state->action_set, - state->action_set_len), - .action_set_len = state->action_set_len, - }, - .max_len = UINT16_MAX, - }, - }; - flow_get_metadata(ctx->paused_flow, &am->pin.up.base.flow_metadata); - - /* Async messages are only sent once, so if we send one now, no - * xlate cache entry is created. */ - if (ctx->xin->allow_side_effects) { - ofproto_dpif_send_async_msg(ctx->xbridge->ofproto, am); - } else /* xcache */ { - struct xc_entry *entry; - - entry = xlate_cache_add_entry(ctx->xin->xcache, XC_CONTROLLER); - entry->controller.ofproto = ctx->xbridge->ofproto; - entry->controller.am = am; - } -} - /* Creates a frozen state, and allocates a unique recirc id for the given * state. Returns a non-zero recirc id if it is allocated successfully. * Returns 0 otherwise. @@ -4499,7 +4459,6 @@ emit_continuation(struct xlate_ctx *ctx, const struct frozen_state *state) static uint32_t finish_freezing__(struct xlate_ctx *ctx, uint8_t table) { - uint32_t id = 0; ovs_assert(ctx->freezing); struct frozen_state state = { @@ -4513,27 +4472,35 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table) .ofpacts_len = ctx->frozen_actions.size, .action_set = ctx->action_set.data, .action_set_len = ctx->action_set.size, + .userdata = ctx->pause ? CONST_CAST(uint8_t *,ctx->pause->userdata) + : NULL, + .userdata_len = ctx->pause ? ctx->pause->userdata_len : 0, }; frozen_metadata_from_flow(&state.metadata, &ctx->xin->flow); + /* Allocate a unique recirc id for the given metadata state in the + * flow. An existing id, with a new reference to the corresponding + * recirculation context, will be returned if possible. + * The life-cycle of this recirc id is managed by associating it + * with the udpif key ('ukey') created for each new datapath flow. */ + uint32_t recirc_id = recirc_alloc_id_ctx(&state); + if (!recirc_id) { + xlate_report_error(ctx, "Failed to allocate recirculation id"); + ctx->error = XLATE_NO_RECIRCULATION_CONTEXT; + return 0; + } + recirc_refs_add(&ctx->xout->recircs, recirc_id); + if (ctx->pause) { - if (ctx->xin->packet) { - emit_continuation(ctx, &state); - } - } else { - /* Allocate a unique recirc id for the given metadata state in the - * flow. An existing id, with a new reference to the corresponding - * recirculation context, will be returned if possible. - * The life-cycle of this recirc id is managed by associating it - * with the udpif key ('ukey') created for each new datapath flow. */ - id = recirc_alloc_id_ctx(&state); - if (!id) { - xlate_report_error(ctx, "Failed to allocate recirculation id"); - ctx->error = XLATE_NO_RECIRCULATION_CONTEXT; + if (!ctx->xin->allow_side_effects && !ctx->xin->xcache) { return 0; } - recirc_refs_add(&ctx->xout->recircs, id); + put_controller_user_action(ctx, false, true, recirc_id, + ctx->pause->max_len, + ctx->pause->reason, + ctx->pause->controller_id); + } else { if (ctx->recirc_update_dp_hash) { struct ovs_action_hash *act_hash; @@ -4544,12 +4511,12 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table) act_hash->hash_alg = OVS_HASH_ALG_L4; /* Make configurable. */ act_hash->hash_basis = 0; /* Make configurable. */ } - nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_RECIRC, id); + nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_RECIRC, recirc_id); } /* Undo changes done by freezing. */ ctx_cancel_freeze(ctx); - return id; + return recirc_id; } /* Called only when we're freezing. */ @@ -6120,8 +6087,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, controller = ofpact_get_CONTROLLER(a); if (controller->pause) { ctx->pause = controller; - ctx->xout->slow |= SLOW_PAUSE; - *ctx->paused_flow = ctx->xin->flow; ctx_trigger_freeze(ctx); a = ofpact_next(a); } else { @@ -6756,7 +6721,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) uint64_t frozen_actions_stub[1024 / 8]; uint64_t actions_stub[256 / 8]; struct ofpbuf scratch_actions = OFPBUF_STUB_INITIALIZER(actions_stub); - struct flow paused_flow; struct xlate_ctx ctx = { .xin = xin, .xout = xout, @@ -6792,7 +6756,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) .recirc_update_dp_hash = false, .frozen_actions = OFPBUF_STUB_INITIALIZER(frozen_actions_stub), .pause = NULL, - .paused_flow = &paused_flow, .was_mpls = false, .conntracked = false, diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 5edf1a9ebf25..274d4ceb18dd 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4586,7 +4586,6 @@ ofproto_dpif_xcache_execute(struct ofproto_dpif *ofproto, case XC_NORMAL: case XC_GROUP: case XC_TNL_NEIGH: - case XC_CONTROLLER: case XC_TUNNEL_HEADER: xlate_push_stats_entry(entry, stats); break; diff --git a/tests/odp.at b/tests/odp.at index 1f8080aed1ed..270b9ff7a82e 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -260,8 +260,8 @@ userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_ userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10,egress),tunnel_out_port=10) userspace(pid=6633,ipfix(output_port=10)) userspace(pid=6633,ipfix(output_port=10),tunnel_out_port=10) -userspace(pid=6633,controller(reason=1,dont_send=0,recirc_id=4444,rule_cookie=0x5555,controller_id=0,max_len=65535)) -userspace(pid=6633,controller(reason=1,dont_send=1,recirc_id=4444,rule_cookie=0x5555,controller_id=0,max_len=65535)) +userspace(pid=6633,controller(reason=1,dont_send=0,continuation=1,recirc_id=4444,rule_cookie=0x5555,controller_id=0,max_len=65535)) +userspace(pid=6633,controller(reason=1,dont_send=1,continuation=0,recirc_id=4444,rule_cookie=0x5555,controller_id=0,max_len=65535)) set(in_port(2)) set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15)) set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15/ff:ff:ff:00:00:00)) diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 4c092bc2e3d5..a582aaf391b1 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -784,7 +784,7 @@ AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00: AT_CHECK([tail -4 stdout], [0], [ Final flow: ip,in_port=1,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=111,nw_tos=0,nw_ecn=0,nw_ttl=1 Megaflow: recirc_id=0,eth,ip,in_port=1,nw_ttl=2,nw_frag=no -Datapath actions: set(ipv4(ttl=1)),2,userspace(pid=0,controller(reason=2,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)),4 +Datapath actions: set(ipv4(ttl=1)),2,userspace(pid=0,controller(reason=2,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)),4 ]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=111,tos=0,ttl=3,frag=no)'], [0], [stdout]) AT_CHECK([tail -2 stdout], [0], @@ -1681,7 +1681,7 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/.*\(packets:\)/\1/' | sed 's/used:[[0-9]].[[0-9]]*s/used:0.001s/'], [0], [dnl flow-dump from non-dpdk interfaces: -packets:1, bytes:14, used:0.001s, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +packets:1, bytes:14, used:0.001s, actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) ]) AT_CHECK([cat ofctl_monitor.log], [0], [dnl @@ -1706,7 +1706,7 @@ done AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/.*\(packets:\)/\1/' | sed 's/used:[[0-9]].[[0-9]]*s/used:0.001s/'], [0], [dnl flow-dump from non-dpdk interfaces: -packets:7, bytes:98, used:0.001s, actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535)))) +packets:7, bytes:98, used:0.001s, actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535)))) ]) AT_CHECK([ovs-appctl time/warp 1], [0], [ignore]) @@ -7429,8 +7429,8 @@ for dl_src in 00 01; do done sleep 1 # wait for the datapath flow installed AT_CHECK_UNQUOTED([strip_ufid < ovs-vswitchd.log | filter_flow_install | strip_used], [0], [dnl -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:00),eth_type(0x8847),mpls(label=20,tc=0,ttl=32,bos=0,label=20,tc=0,ttl=32,bos=1), actions:push_mpls(label=20,tc=0,ttl=32,bos=0,eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:01),eth_type(0x8847),mpls(label=20/0x0,tc=0/0,ttl=32/0x0,bos=0/1,label=20/0xfffff,tc=0/7,ttl=32/0xff,bos=1/1), actions:pop_mpls(eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:00),eth_type(0x8847),mpls(label=20,tc=0,ttl=32,bos=0,label=20,tc=0,ttl=32,bos=1), actions:push_mpls(label=20,tc=0,ttl=32,bos=0,eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:01),eth_type(0x8847),mpls(label=20/0x0,tc=0/0,ttl=32/0x0,bos=0/1,label=20/0xfffff,tc=0/7,ttl=32/0xff,bos=1/1), actions:pop_mpls(eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) ]) OVS_VSWITCHD_STOP @@ -7468,8 +7468,8 @@ for dl_src in 00 01; do done sleep 1 # wait for the datapath flow installed AT_CHECK_UNQUOTED([strip_ufid < ovs-vswitchd.log | filter_flow_install | strip_used], [0], [dnl -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:00),eth_type(0x8847),mpls(label=20,tc=0,ttl=32,bos=0,label=20,tc=0,ttl=32,bos=1), actions:push_mpls(label=20,tc=0,ttl=32,bos=0,eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:01),eth_type(0x8847),mpls(label=20/0x0,tc=0/0,ttl=32/0x0,bos=0/1,label=20/0xfffff,tc=0/7,ttl=32/0xff,bos=1/1), actions:pop_mpls(eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:00),eth_type(0x8847),mpls(label=20,tc=0,ttl=32,bos=0,label=20,tc=0,ttl=32,bos=1), actions:push_mpls(label=20,tc=0,ttl=32,bos=0,eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=60:66:66:66:66:01),eth_type(0x8847),mpls(label=20/0x0,tc=0/0,ttl=32/0x0,bos=0/1,label=20/0xfffff,tc=0/7,ttl=32/0xff,bos=1/1), actions:pop_mpls(eth_type=0x8847),userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) ]) OVS_VSWITCHD_STOP diff --git a/tests/pmd.at b/tests/pmd.at index adfdb4c01e78..fcb007ce04f4 100644 --- a/tests/pmd.at +++ b/tests/pmd.at @@ -303,8 +303,8 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) dnl Make sure that both flows have been installed AT_CHECK([ovs-appctl dpctl/dump-flows | flow_dump_prepend_pmd], [0], [dnl -0 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) -1 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +0 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) +1 recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535)) ]) AT_CHECK([cat ofctl_monitor.log], [0], [dnl diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at index 4d609b2f1944..7ca522ae2a0f 100644 --- a/tests/tunnel-push-pop-ipv6.at +++ b/tests/tunnel-push-pop-ipv6.at @@ -164,7 +164,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 5'], [0], [dnl port 5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=? ]) AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl -tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535)) +tunnel(tun_id=0x7b,ipv6_src=2001:cafe::92,ipv6_dst=2001:cafe::88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535)) ]) ovs-appctl time/warp 10000 diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index cdfbb36a47d9..cc8b1b522741 100644 --- a/tests/tunnel-push-pop.at +++ b/tests/tunnel-push-pop.at @@ -215,7 +215,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 5'], [0], [dnl port 5: rx pkts=1, bytes=98, drop=?, errs=?, frame=?, over=?, crc=? ]) AT_CHECK([ovs-appctl dpif/dump-flows int-br | grep 'in_port(6081)'], [0], [dnl -tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535)) +tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),recirc_id(0),in_port(6081),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=3,rule_cookie=0,controller_id=0,max_len=65535)) ]) ovs-appctl time/warp 10000