From patchwork Wed Feb 10 15:34:03 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gaetan Rivet X-Patchwork-Id: 1439082 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=u256.net header.i=@u256.net header.a=rsa-sha256 header.s=fm1 header.b=ZvvGuDaf; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.a=rsa-sha256 header.s=fm2 header.b=TtZphAXM; dkim-atps=neutral Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DbP4W0xX8z9rx8 for ; Thu, 11 Feb 2021 02:36:27 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 9DB4B86CF4; Wed, 10 Feb 2021 15:36:25 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NfarffScgxeu; Wed, 10 Feb 2021 15:36:19 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 180AB8714A; Wed, 10 Feb 2021 15:34:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E7828C1E73; Wed, 10 Feb 2021 15:34:39 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5B3DCC013A for ; Wed, 10 Feb 2021 15:34:33 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 362E886C99 for ; Wed, 10 Feb 2021 15:34:33 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BDm4sXVcem1s for ; Wed, 10 Feb 2021 15:34:25 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from wnew4-smtp.messagingengine.com (wnew4-smtp.messagingengine.com [64.147.123.18]) by whitealder.osuosl.org (Postfix) with ESMTPS id 670C886CAB for ; Wed, 10 Feb 2021 15:34:19 +0000 (UTC) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailnew.west.internal (Postfix) with ESMTP id 0DA8C547; Wed, 10 Feb 2021 10:34:18 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 10 Feb 2021 10:34:19 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=u256.net; h=from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=fm1; bh=aeEgB+HbqGqYh qKB2avgbRIz+uXdWdyEx30Dj27A4WU=; b=ZvvGuDafOFnAq/M5Y9SOq3OJMV1R3 mVS1Jq6uZ3O7U3kuLzrYfx1Out7Z3kEEIHQCpam6owlokckF8WLExdNptTeYA2bv Sr2S87IIxEIY7ixwPIgwGkmTUVDdgEJFzfQCceAHQYBC0hnQvk33lxcLFIeuCuJw lWcmyRNi7gulLzy4zrAZ9MGsQfJKXCNMDx1EO1LMN0T9O1WjVo9A6MorjbUog9AH M+T6TEdD6Yyd/nMbLIYpZuHidL61vCjDAYgciVe8ScPxZ2f1lgsEkfBaYcikjlI0 SqvK9WeVg3h4qsu3Omiv+DruGZCjDv+Qq3pk5t2oNidazJcNrChnzLSFw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :in-reply-to:message-id:mime-version:references:subject:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; bh=aeEgB+HbqGqYhqKB2avgbRIz+uXdWdyEx30Dj27A4WU=; b=TtZphAXM n/P9bgiNspj9a/Aium3PWuyIsbNcVTWBqcRAoeUc70/5qZ+C7bOcvSniQrrwtmYk SvEIeLRBwBq65G+3UDuaSKe8Oy/9S6j7AG9Hjafks1bKJkyeS0aVvUP3UIsXbus5 M0OwAG96af3/1edKQJ+XDYVIlwg7XNHtuSqi5EIAA8xvqvvuopjhrFRzMSKFu7Q/ 7SRk+8WMuAcbe/o7xh4cpPt/80MUIYryQrnfhQkbIjMwhqVtzr8PyPfJgT1PCsuz 12Nf/Z7O72suRCmQckEprRHF4oKvrQCW15c2SqtEML1i5EiqsfIlms1YDO+rgqld eBNHMa2BJjqx2g== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrheejgdejkecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkofgjfhgggfestdekredtredttdenucfhrhhomhepifgrvghtrghn ucftihhvvghtuceoghhrihhvvgesuhdvheeirdhnvghtqeenucggtffrrghtthgvrhhnpe ehgfevffekteehteefieefvdehleefjeefheevudetjefhkeeutdekieeuvdetheenucfk phepkeeirddvheegrdduieegrddujeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrg hrrghmpehmrghilhhfrhhomhepghhrihhvvgesuhdvheeirdhnvght X-ME-Proxy: Received: from inocybe.home (lfbn-poi-1-842-174.w86-254.abo.wanadoo.fr [86.254.164.174]) by mail.messagingengine.com (Postfix) with ESMTPA id 1AC49240069; Wed, 10 Feb 2021 10:34:18 -0500 (EST) From: Gaetan Rivet To: dev@openvswitch.org Date: Wed, 10 Feb 2021 16:34:03 +0100 Message-Id: X-Mailer: git-send-email 2.30.0 In-Reply-To: References: MIME-Version: 1.0 Cc: Eli Britstein Subject: [ovs-dev] [PATCH v1 17/23] netdev-offload-dpdk: Lock rte_flow map access X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Add a lock to access the ufid to rte_flow map. This will protect it from concurrent write accesses when multiple threads attempt it. At this point, the reason for taking the lock is not to fullfill the needs of the DPDK offload implementation anymore. Rewrite the comments to reflect this change. The lock is still needed to protect against changes to netdev port mapping. Signed-off-by: Gaetan Rivet Reviewed-by: Eli Britstein --- lib/dpif-netdev.c | 8 ++--- lib/netdev-offload-dpdk.c | 61 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 9f9898caa..8b9115609 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -2566,7 +2566,7 @@ mark_to_flow_disassociate(struct dp_netdev_pmd_thread *pmd, port = netdev_ports_get(in_port, dpif_type_str); if (port) { /* Taking a global 'port_mutex' to fulfill thread safety - * restrictions for the netdev-offload-dpdk module. */ + * restrictions regarding netdev port mapping. */ ovs_mutex_lock(&pmd->dp->port_mutex); ret = netdev_flow_del(port, &flow->mega_ufid, NULL); ovs_mutex_unlock(&pmd->dp->port_mutex); @@ -2722,8 +2722,8 @@ dp_netdev_flow_offload_put(struct dp_offload_thread_item *offload) netdev_close(port); goto err_free; } - /* Taking a global 'port_mutex' to fulfill thread safety restrictions for - * the netdev-offload-dpdk module. */ + /* Taking a global 'port_mutex' to fulfill thread safety + * restrictions regarding the netdev port mapping. */ ovs_mutex_lock(&pmd->dp->port_mutex); ret = netdev_flow_put(port, &offload->match, CONST_CAST(struct nlattr *, offload->actions), @@ -3407,7 +3407,7 @@ dpif_netdev_get_flow_offload_status(const struct dp_netdev *dp, } ofpbuf_use_stack(&buf, &act_buf, sizeof act_buf); /* Taking a global 'port_mutex' to fulfill thread safety - * restrictions for the netdev-offload-dpdk module. + * restrictions regarding netdev port mapping. * * XXX: Main thread will try to pause/stop all revalidators during datapath * reconfiguration via datapath purge callback (dp_purge_cb) while diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index 338ca4dc6..617f87eca 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -38,9 +38,6 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(100, 5); * * Below API is NOT thread safe in following terms: * - * - The caller must be sure that none of these functions will be called - * simultaneously. Even for different 'netdev's. - * * - The caller must be sure that 'netdev' will not be destructed/deallocated. * * - The caller must be sure that 'netdev' configuration will not be changed. @@ -66,6 +63,7 @@ struct ufid_to_rte_flow_data { struct netdev_offload_dpdk_data { struct cmap ufid_to_rte_flow; uint64_t *rte_flow_counters; + struct ovs_mutex map_lock; }; static int @@ -74,6 +72,7 @@ offload_data_init(struct netdev *netdev) struct netdev_offload_dpdk_data *data; data = xzalloc(sizeof *data); + ovs_mutex_init(&data->map_lock); cmap_init(&data->ufid_to_rte_flow); data->rte_flow_counters = xcalloc(netdev_offload_thread_nb(), sizeof *data->rte_flow_counters); @@ -86,6 +85,7 @@ offload_data_init(struct netdev *netdev) static void offload_data_destroy__(struct netdev_offload_dpdk_data *data) { + ovs_mutex_destroy(&data->map_lock); free(data->rte_flow_counters); free(data); } @@ -117,6 +117,34 @@ offload_data_destroy(struct netdev *netdev) ovsrcu_set(&netdev->hw_info.offload_data, NULL); } +static void +offload_data_lock(struct netdev *netdev) + OVS_NO_THREAD_SAFETY_ANALYSIS +{ + struct netdev_offload_dpdk_data *data; + + data = (struct netdev_offload_dpdk_data *) + ovsrcu_get(void *, &netdev->hw_info.offload_data); + if (!data) { + return; + } + ovs_mutex_lock(&data->map_lock); +} + +static void +offload_data_unlock(struct netdev *netdev) + OVS_NO_THREAD_SAFETY_ANALYSIS +{ + struct netdev_offload_dpdk_data *data; + + data = (struct netdev_offload_dpdk_data *) + ovsrcu_get(void *, &netdev->hw_info.offload_data); + if (!data) { + return; + } + ovs_mutex_unlock(&data->map_lock); +} + static struct cmap * offload_data_map(struct netdev *netdev) { @@ -155,6 +183,24 @@ ufid_to_rte_flow_data_find(struct netdev *netdev, return NULL; } +/* Find rte_flow with @ufid, lock-protected. */ +static struct ufid_to_rte_flow_data * +ufid_to_rte_flow_data_find_protected(struct netdev *netdev, + const ovs_u128 *ufid) +{ + size_t hash = hash_bytes(ufid, sizeof *ufid, 0); + struct ufid_to_rte_flow_data *data; + struct cmap *map = offload_data_map(netdev); + + CMAP_FOR_EACH_WITH_HASH_PROTECTED (data, node, hash, map) { + if (ovs_u128_equals(*ufid, data->ufid)) { + return data; + } + } + + return NULL; +} + static inline struct ufid_to_rte_flow_data * ufid_to_rte_flow_associate(struct netdev *netdev, const ovs_u128 *ufid, struct rte_flow *rte_flow, bool actions_offloaded) @@ -170,13 +216,15 @@ ufid_to_rte_flow_associate(struct netdev *netdev, const ovs_u128 *ufid, data = xzalloc(sizeof *data); + offload_data_lock(netdev); + /* * We should not simply overwrite an existing rte flow. * We should have deleted it first before re-adding it. * Thus, if following assert triggers, something is wrong: * the rte_flow is not destroyed. */ - data_prev = ufid_to_rte_flow_data_find(netdev, ufid, false); + data_prev = ufid_to_rte_flow_data_find_protected(netdev, ufid); if (data_prev) { ovs_assert(data_prev->rte_flow == NULL); } @@ -187,6 +235,8 @@ ufid_to_rte_flow_associate(struct netdev *netdev, const ovs_u128 *ufid, data->actions_offloaded = actions_offloaded; cmap_insert(map, CONST_CAST(struct cmap_node *, &data->node), hash); + + offload_data_unlock(netdev); return data; } @@ -200,7 +250,10 @@ ufid_to_rte_flow_disassociate(struct ufid_to_rte_flow_data *data) return; } + offload_data_lock(data->netdev); cmap_remove(map, CONST_CAST(struct cmap_node *, &data->node), hash); + offload_data_unlock(data->netdev); + netdev_close(data->netdev); ovsrcu_postpone(free, data); }