From patchwork Wed Sep 8 09:47:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gaetan Rivet X-Patchwork-Id: 1525734 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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=fm2 header.b=d+Z8T7DC; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.a=rsa-sha256 header.s=fm3 header.b=epu/FODq; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4H4HSK73Hrz9sW8 for ; Wed, 8 Sep 2021 19:50:25 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id CC5876153A; Wed, 8 Sep 2021 09:50:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id HnDlug0CUAO0; Wed, 8 Sep 2021 09:50:21 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 6D729613B0; Wed, 8 Sep 2021 09:50:19 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 25F00C0025; Wed, 8 Sep 2021 09:50:19 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 15AA4C0025 for ; Wed, 8 Sep 2021 09:50:18 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id D8F4482FE4 for ; Wed, 8 Sep 2021 09:48:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp1.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=u256.net header.b="d+Z8T7DC"; dkim=pass (2048-bit key) header.d=messagingengine.com header.b="epu/FODq" Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fgJTNtlTfIoo for ; Wed, 8 Sep 2021 09:48:38 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by smtp1.osuosl.org (Postfix) with ESMTPS id AF8498354B for ; Wed, 8 Sep 2021 09:48:35 +0000 (UTC) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.west.internal (Postfix) with ESMTP id F3A7232009DA; Wed, 8 Sep 2021 05:48:34 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Wed, 08 Sep 2021 05:48:35 -0400 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=fm2; bh=jyK0opkctG+pg N4ZKdmNINmjVaN2hzQZ9vP3v89wbks=; b=d+Z8T7DCNbC72pDCBlCFfBkcBQUTs 5YmBdcPX+nJ4MkdyVZnhy24+5cj1iEN91hFIWJM/lehjdx8MyF783Y6cEV0X/XwA fp5Xeb/FI5Vgn+UdQgHzIdpEfjRuiGMs79wk/nNH1rgucwGxiAo6jaE1KISeWCTC iBXviuHvqJZRRvEuhLydXepAB0ST07mZw/td/GVpTWqZSQ0gUsyuoJPmfbM4E9Vj 4D++TtFwYQ2VBOgGCMO6+v1eTxS+nsjOpasxh/IWV6lz3PpdOFCp8simH+fvbLLo B4svyutKGnEO3NMV8cugAxLY7Gu/ewYq0c/Dr+UPH3GwjKtJO6vorWUhg== 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= fm3; bh=jyK0opkctG+pgN4ZKdmNINmjVaN2hzQZ9vP3v89wbks=; b=epu/FODq H2L1wzvk1LHHO3loJUylbNyIBZ+OpK89b8BRbkPlSC08iputiuipL7TGXc7cGFhJ qW95+/aZ+Bol3u0WW2a6+5LZL3cP5ZGH9am+n9aTppOds/q7MBh0gmBeFp4rD/Dh 15Q6UyKlSGaUTTe1MQokD4SEUmnQII1mNib1j5xmt62yVjPPxZYu7Cp9lSrboyFN uV/u5Ldl9b5a3rB7fudciPkTS2WfFxRFJpEPgUUSSLRt6PfK3L3FA1gM+vyQmwPi 40nyBkEmi7wwcAgG0EjP78ueJ0QSdxmty/xiNz2q0xsFmS3Ne/HgrXJusTcfl7xF O5n8P2ayZRqdWQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudefjedgudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkffojghfggfgsedtkeertdertddtnecuhfhrohhmpefirggvthgr nhcutfhivhgvthcuoehgrhhivhgvsehuvdehiedrnhgvtheqnecuggftrfgrthhtvghrnh ephefgveffkeetheetfeeifedvheelfeejfeehveduteejhfekuedtkeeiuedvteehnecu vehluhhsthgvrhfuihiivgepheenucfrrghrrghmpehmrghilhhfrhhomhepghhrihhvvg esuhdvheeirdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 8 Sep 2021 05:48:33 -0400 (EDT) From: Gaetan Rivet To: ovs-dev@openvswitch.org Date: Wed, 8 Sep 2021 11:47:45 +0200 Message-Id: X-Mailer: git-send-email 2.31.1 In-Reply-To: References: MIME-Version: 1.0 Cc: Eli Britstein , Maxime Coquelin Subject: [ovs-dev] [PATCH v5 21/27] 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 Reviewed-by: Maxime Coquelin --- 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 381c959af..bf5785981 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -2521,7 +2521,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); @@ -2690,8 +2690,8 @@ dp_netdev_flow_offload_put(struct dp_offload_flow_item *offload) 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), @@ -3533,7 +3533,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 8ed869eb3..e76c50b72 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -40,9 +40,6 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(600, 600); * * 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. @@ -69,6 +66,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 @@ -77,6 +75,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); @@ -89,6 +88,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); } @@ -120,6 +120,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) { @@ -158,6 +186,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(const ovs_u128 *ufid, struct netdev *netdev, struct netdev *physdev, struct rte_flow *rte_flow, @@ -174,13 +220,15 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct netdev *netdev, 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); } @@ -192,6 +240,8 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct netdev *netdev, data->actions_offloaded = actions_offloaded; cmap_insert(map, CONST_CAST(struct cmap_node *, &data->node), hash); + + offload_data_unlock(netdev); return data; } @@ -205,7 +255,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); + if (data->netdev != data->physdev) { netdev_close(data->netdev); }