From patchwork Thu Dec 21 03:42:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Pettit X-Patchwork-Id: 851768 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 3z2HYx4QsNz9s0g for ; Thu, 21 Dec 2017 14:44:05 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 9E86BBE0; Thu, 21 Dec 2017 03:43:33 +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 68B61BCA for ; Thu, 21 Dec 2017 03:43:32 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id BCB35CA for ; Thu, 21 Dec 2017 03:43:31 +0000 (UTC) X-Originating-IP: 98.234.50.139 Received: from localhost.localdomain (unknown [98.234.50.139]) (Authenticated sender: jpettit@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 1CD2D41C07C for ; Thu, 21 Dec 2017 04:43:29 +0100 (CET) From: Justin Pettit To: dev@openvswitch.org Date: Wed, 20 Dec 2017 19:42:40 -0800 Message-Id: <1513827761-96181-2-git-send-email-jpettit@ovn.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1513827761-96181-1-git-send-email-jpettit@ovn.org> References: <1513827761-96181-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] [PATCH] ofproto-dpif: Remove variable length userdata probe. 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 Commit e995e3df57 ("Allow OVS_USERSPACE_ATTR_USERDATA to be variable length.") changed userspace action userdata from eight bytes to variable length. OVS only supports Linux kernels greater than or equal to 3.10, which include support for variable length userdata. Other datapaths are more modern and should have support, so it's no longer necessary to probe. Signed-off-by: Justin Pettit Acked-by: Ben Pfaff --- ofproto/ofproto-dpif-xlate.c | 7 ---- ofproto/ofproto-dpif.c | 85 -------------------------------------------- ofproto/ofproto-dpif.h | 6 ---- 3 files changed, 98 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index a2b4fdb3b6be..f0fc4bac98ed 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5211,13 +5211,6 @@ xlate_sample_action(struct xlate_ctx *ctx, * the same percentage. */ uint32_t probability = (os->probability << 16) | os->probability; - if (!ctx->xbridge->support.variable_length_userdata) { - xlate_report_error(ctx, "ignoring NXAST_SAMPLE action because " - "datapath lacks support (needs Linux 3.10+ or " - "kernel module from OVS 1.11+)"); - return; - } - /* If ofp_port in flow sample action is equel to ofp_port, * this sample action is a input port action. */ if (os->sampling_port != OFPP_NONE && diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 43b7b89f26e4..838a8de0c27f 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -676,7 +676,6 @@ struct odp_garbage { odp_port_t odp_port; }; -static bool check_variable_length_userdata(struct dpif_backer *backer); static void check_support(struct dpif_backer *backer); static int @@ -785,11 +784,6 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) udpif_set_threads(backer->udpif, n_handlers, n_revalidators); } - /* This check fails if performed before udpif threads have been set, - * as the kernel module checks that the 'pid' in userspace action - * is non-zero. */ - backer->rt_support.variable_length_userdata - = check_variable_length_userdata(backer); backer->dp_version_string = dpif_get_dp_version(backer->dpif); /* Manage Datapath meter IDs if supported. */ @@ -895,82 +889,6 @@ check_ufid(struct dpif_backer *backer) return enable_ufid; } -/* Tests whether 'backer''s datapath supports variable-length - * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions. We need - * to disable some features on older datapaths that don't support this - * feature. - * - * Returns false if 'backer' definitely does not support variable-length - * userdata, true if it seems to support them or if at least the error we get - * is ambiguous. */ -static bool -check_variable_length_userdata(struct dpif_backer *backer) -{ - struct eth_header *eth; - struct ofpbuf actions; - struct dpif_execute execute; - struct dp_packet packet; - struct flow flow; - size_t start; - int error; - - /* Compose a userspace action that will cause an ERANGE error on older - * datapaths that don't support variable-length userdata. - * - * We really test for using userdata longer than 8 bytes, but older - * datapaths accepted these, silently truncating the userdata to 8 bytes. - * The same older datapaths rejected userdata shorter than 8 bytes, so we - * test for that instead as a proxy for longer userdata support. */ - ofpbuf_init(&actions, 64); - start = nl_msg_start_nested(&actions, OVS_ACTION_ATTR_USERSPACE); - nl_msg_put_u32(&actions, OVS_USERSPACE_ATTR_PID, - dpif_port_get_pid(backer->dpif, ODPP_NONE, 0)); - nl_msg_put_unspec_zero(&actions, OVS_USERSPACE_ATTR_USERDATA, 4); - nl_msg_end_nested(&actions, start); - - /* Compose a dummy ethernet packet. */ - dp_packet_init(&packet, ETH_HEADER_LEN); - eth = dp_packet_put_zeros(&packet, ETH_HEADER_LEN); - eth->eth_type = htons(0x1234); - - flow_extract(&packet, &flow); - - /* Execute the actions. On older datapaths this fails with ERANGE, on - * newer datapaths it succeeds. */ - execute.actions = actions.data; - execute.actions_len = actions.size; - execute.packet = &packet; - execute.flow = &flow; - execute.needs_help = false; - execute.probe = true; - execute.mtu = 0; - - error = dpif_execute(backer->dpif, &execute); - - dp_packet_uninit(&packet); - ofpbuf_uninit(&actions); - - switch (error) { - case 0: - return true; - - case ERANGE: - /* Variable-length userdata is not supported. */ - VLOG_WARN("%s: datapath does not support variable-length userdata " - "feature (needs Linux 3.10+ or kernel module from OVS " - "1..11+). The NXAST_SAMPLE action will be ignored.", - dpif_name(backer->dpif)); - return false; - - default: - /* Something odd happened. We're not sure whether variable-length - * userdata is supported. Default to "yes". */ - VLOG_WARN("%s: variable-length userdata feature probe failed (%s)", - dpif_name(backer->dpif), ovs_strerror(error)); - return true; - } -} - /* Tests number of 802.1q VLAN headers supported by 'backer''s datapath. * * Returns the number of elements in a struct flow's vlan @@ -1374,9 +1292,6 @@ CHECK_FEATURE__(ct_orig_tuple6, ct_orig_tuple6, ct_nw_proto, 1, ETH_TYPE_IPV6) static void check_support(struct dpif_backer *backer) { - /* This feature needs to be tested after udpif threads are set. */ - backer->rt_support.variable_length_userdata = false; - /* Actions. */ backer->rt_support.odp.recirc = check_recirc(backer); backer->rt_support.odp.max_vlan_headers = check_max_vlan_headers(backer); diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index 0857c070c8ac..032b5d7d66c8 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -152,12 +152,6 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *, * They are defined as macros to keep 'dpif_show_support()' in sync * as new fields are added. */ #define DPIF_SUPPORT_FIELDS \ - /* True if the datapath supports variable-length \ - * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions. \ - * False if the datapath supports only 8-byte (or shorter) userdata. */ \ - DPIF_SUPPORT_FIELD(bool, variable_length_userdata, \ - "Variable length userdata") \ - \ /* True if the datapath supports masked data in OVS_ACTION_ATTR_SET \ * actions. */ \ DPIF_SUPPORT_FIELD(bool, masked_set_action, "Masked set action") \