From patchwork Wed Jun 9 13:09:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gaetan Rivet X-Patchwork-Id: 1489879 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.133; helo=smtp2.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=fm2 header.b=BXvseIqG; 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=A5Bd9a9l; dkim-atps=neutral Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (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 4G0SDy5xccz9sXL for ; Wed, 9 Jun 2021 23:12:02 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id F15C641DBB; Wed, 9 Jun 2021 13:12:00 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FkpQXbLYv4xS; Wed, 9 Jun 2021 13:11:57 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id 5300F41D6E; Wed, 9 Jun 2021 13:11:56 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2DBFCC0024; Wed, 9 Jun 2021 13:11:56 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 235FFC0024 for ; Wed, 9 Jun 2021 13:11:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 5761D405FE for ; Wed, 9 Jun 2021 13:10:34 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id H5qp0s0J1AJN for ; Wed, 9 Jun 2021 13:10:30 +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 smtp2.osuosl.org (Postfix) with ESMTPS id BFFEF419BA for ; Wed, 9 Jun 2021 13:10:20 +0000 (UTC) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 33A3E260C; Wed, 9 Jun 2021 09:10:20 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Wed, 09 Jun 2021 09:10:20 -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=/HaOcMxuQaon1 FZdqqHULzAU5+YeR5UeqM+D3ZN1DnM=; b=BXvseIqGctypyl/bOZUDvWy9zpuz3 QPyI2l01rf9iSOp+fYfFzNYpkvIJZfD5/SnEaRuOKYcUPhGlg7ElDkTgGIVffucY e2KA1VaKgfkHr6sHUtZLR/xWFU2d7s1hhurKA1buNIXk2Cw4S3xf8BF9prAym6Vp ZGPju2PSzJdQYBp5sovekAMAzOM8ce3U8zW6+KywFG/9X8u4XoogbaO/ll4a/qvp WAS9HsS3TsXxjH0QT+lwAErzDjdCe9c8DqkNcrCAOQ0iN7G0yUhqjnYp7585BK1/ oNQ8+BnQGnT1mDyVNllWbmtuQfIjuVO4nuJKIQMDwVBCV02Bgvd4Pdopg== 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=/HaOcMxuQaon1FZdqqHULzAU5+YeR5UeqM+D3ZN1DnM=; b=A5Bd9a9l MiMeIgeyCqf0WlMa6bVeVe/kfszF+O7yW+xaMyhXUI0fIyIfICjg9bm3S88R3HOx yxjzwy6RqYI0hXM42VBTFzDJa7Yqjc2Q46R2lD1N1R4qbCNz1PZ68LaSPP+D7dWE hBcF9n/2ZIIHxmbn/Hs2unaysF/ibGnd1WdgP4ViPNqj/JVBMogsVJncrLZIV1Ko levpmaXDU6MEio1MpaHwXAcpsTZonojMxwC18foQ5Wxb4oX65ZdglvS8tls95OP/ hRA9NCOpb3ire81b//77eBrfk5R91PNie2Arm9o1CtXf9MpSsKsZuVJqL+Y4PzYv CmtdhhpvQhRH2Q== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrfeduuddgieefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkffojghfggfgsedtkeertdertddtnecuhfhrohhmpefirggvthgr nhcutfhivhgvthcuoehgrhhivhgvsehuvdehiedrnhgvtheqnecuggftrfgrthhtvghrnh ephefgveffkeetheetfeeifedvheelfeejfeehveduteejhfekuedtkeeiuedvteehnecu vehluhhsthgvrhfuihiivgepgeenucfrrghrrghmpehmrghilhhfrhhomhepghhrihhvvg esuhdvheeirdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 9 Jun 2021 09:10:19 -0400 (EDT) From: Gaetan Rivet To: ovs-dev@openvswitch.org Date: Wed, 9 Jun 2021 15:09:30 +0200 Message-Id: <7c5893d998ab29f8eeec3f0f9e481894a4925034.1623234822.git.grive@u256.net> 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 v4 22/27] netdev-offload-dpdk: Protect concurrent offload destroy/query 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" The rte_flow API in DPDK is now thread safe for insertion and deletion. It is not however safe for concurrent query while the offload is being inserted or deleted. Insertion is not an issue as the rte_flow handle will be published to other threads only once it has been inserted in the hardware, so the query will only be able to proceed once it is already available. For the deletion path however, offload status queries can be made while an offload is being destroyed. This would create race conditions and use-after-free if not properly protected. As a pre-step before removing the OVS-level locks on the rte_flow API, mutually exclude offload query and deletion from concurrent execution. Signed-off-by: Gaetan Rivet Reviewed-by: Eli Britstein Reviewed-by: Maxime Coquelin --- lib/netdev-offload-dpdk.c | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index 4459a0aa1..13e017ef8 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -58,6 +58,8 @@ struct ufid_to_rte_flow_data { struct rte_flow *rte_flow; bool actions_offloaded; struct dpif_flow_stats stats; + struct ovs_mutex lock; + bool dead; }; struct netdev_offload_dpdk_data { @@ -233,6 +235,7 @@ ufid_to_rte_flow_associate(struct netdev *netdev, const ovs_u128 *ufid, data->netdev = netdev_ref(netdev); data->rte_flow = rte_flow; data->actions_offloaded = actions_offloaded; + ovs_mutex_init(&data->lock); cmap_insert(map, CONST_CAST(struct cmap_node *, &data->node), hash); @@ -240,8 +243,16 @@ ufid_to_rte_flow_associate(struct netdev *netdev, const ovs_u128 *ufid, return data; } +static void +rte_flow_data_unref(struct ufid_to_rte_flow_data *data) +{ + ovs_mutex_destroy(&data->lock); + free(data); +} + static inline void ufid_to_rte_flow_disassociate(struct ufid_to_rte_flow_data *data) + OVS_REQUIRES(data->lock) { size_t hash = hash_bytes(&data->ufid, sizeof data->ufid, 0); struct cmap *map = offload_data_map(data->netdev); @@ -255,7 +266,7 @@ ufid_to_rte_flow_disassociate(struct ufid_to_rte_flow_data *data) offload_data_unlock(data->netdev); netdev_close(data->netdev); - ovsrcu_postpone(free, data); + ovsrcu_postpone(rte_flow_data_unref, data); } /* @@ -1581,6 +1592,15 @@ netdev_offload_dpdk_flow_destroy(struct ufid_to_rte_flow_data *rte_flow_data) ovs_u128 *ufid; int ret; + ovs_mutex_lock(&rte_flow_data->lock); + + if (rte_flow_data->dead) { + ovs_mutex_unlock(&rte_flow_data->lock); + return 0; + } + + rte_flow_data->dead = true; + rte_flow = rte_flow_data->rte_flow; netdev = rte_flow_data->netdev; ufid = &rte_flow_data->ufid; @@ -1607,6 +1627,8 @@ netdev_offload_dpdk_flow_destroy(struct ufid_to_rte_flow_data *rte_flow_data) UUID_ARGS((struct uuid *) ufid)); } + ovs_mutex_unlock(&rte_flow_data->lock); + return ret; } @@ -1702,8 +1724,19 @@ netdev_offload_dpdk_flow_get(struct netdev *netdev, struct rte_flow_error error; int ret = 0; + attrs->dp_extra_info = NULL; + rte_flow_data = ufid_to_rte_flow_data_find(netdev, ufid, false); - if (!rte_flow_data || !rte_flow_data->rte_flow) { + if (!rte_flow_data || !rte_flow_data->rte_flow || + rte_flow_data->dead || ovs_mutex_trylock(&rte_flow_data->lock)) { + return -1; + } + + /* Check again whether the data is dead, as it could have been + * updated while the lock was not yet taken. The first check above + * was only to avoid unnecessary locking if possible. + */ + if (rte_flow_data->dead) { ret = -1; goto out; } @@ -1730,7 +1763,7 @@ netdev_offload_dpdk_flow_get(struct netdev *netdev, } memcpy(stats, &rte_flow_data->stats, sizeof *stats); out: - attrs->dp_extra_info = NULL; + ovs_mutex_unlock(&rte_flow_data->lock); return ret; }