Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/1525729/?format=api
{ "id": 1525729, "url": "http://patchwork.ozlabs.org/api/patches/1525729/?format=api", "web_url": "http://patchwork.ozlabs.org/project/openvswitch/patch/19d3b47ef290a31be471b5817db6ed3f361b12c9.1631094144.git.grive@u256.net/", "project": { "id": 47, "url": "http://patchwork.ozlabs.org/api/projects/47/?format=api", "name": "Open vSwitch", "link_name": "openvswitch", "list_id": "ovs-dev.openvswitch.org", "list_email": "ovs-dev@openvswitch.org", "web_url": "http://openvswitch.org/", "scm_url": "git@github.com:openvswitch/ovs.git", "webscm_url": "https://github.com/openvswitch/ovs", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<19d3b47ef290a31be471b5817db6ed3f361b12c9.1631094144.git.grive@u256.net>", "list_archive_url": null, "date": "2021-09-08T09:47:46", "name": "[ovs-dev,v5,22/27] netdev-offload-dpdk: Protect concurrent offload destroy/query", "commit_ref": "b3e029f7c16560bba7b8d0f651b554c5e3ed9f54", "pull_url": null, "state": "accepted", "archived": false, "hash": "cdfa5c56c332d0d7790e8a4703eb3c884839f341", "submitter": { "id": 78795, "url": "http://patchwork.ozlabs.org/api/people/78795/?format=api", "name": "Gaetan Rivet", "email": "grive@u256.net" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/openvswitch/patch/19d3b47ef290a31be471b5817db6ed3f361b12c9.1631094144.git.grive@u256.net/mbox/", "series": [ { "id": 261424, "url": "http://patchwork.ozlabs.org/api/series/261424/?format=api", "web_url": "http://patchwork.ozlabs.org/project/openvswitch/list/?series=261424", "date": "2021-09-08T09:47:24", "name": "dpif-netdev: Parallel offload processing", "version": 5, "mbox": "http://patchwork.ozlabs.org/series/261424/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/1525729/comments/", "check": "success", "checks": "http://patchwork.ozlabs.org/api/patches/1525729/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<ovs-dev-bounces@openvswitch.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "ovs-dev@openvswitch.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "ovs-dev@lists.linuxfoundation.org" ], "Authentication-Results": [ "ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=u256.net header.i=@u256.net header.a=rsa-sha256\n header.s=fm2 header.b=Zf42mK+O;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=messagingengine.com header.i=@messagingengine.com\n header.a=rsa-sha256 header.s=fm3 header.b=ierIjOuq;\n\tdkim-atps=neutral", "ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=<UNKNOWN>)" ], "Received": [ "from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 4H4HRr36jgz9sW8\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 8 Sep 2021 19:50:00 +1000 (AEST)", "from localhost (localhost [127.0.0.1])\n\tby smtp2.osuosl.org (Postfix) with ESMTP id DA9234071D;\n\tWed, 8 Sep 2021 09:49:57 +0000 (UTC)", "from smtp2.osuosl.org ([127.0.0.1])\n\tby localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id Enb5OFzgKOe0; Wed, 8 Sep 2021 09:49:56 +0000 (UTC)", "from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])\n\tby smtp2.osuosl.org (Postfix) with ESMTPS id D48EF406C5;\n\tWed, 8 Sep 2021 09:49:54 +0000 (UTC)", "from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id F09A8C0020;\n\tWed, 8 Sep 2021 09:49:52 +0000 (UTC)", "from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133])\n by lists.linuxfoundation.org (Postfix) with ESMTP id 51317C001D\n for <ovs-dev@openvswitch.org>; Wed, 8 Sep 2021 09:49:52 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n by smtp2.osuosl.org (Postfix) with ESMTP id 62C93406C5\n for <ovs-dev@openvswitch.org>; Wed, 8 Sep 2021 09:48:38 +0000 (UTC)", "from smtp2.osuosl.org ([127.0.0.1])\n by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n with ESMTP id e1XxqA973j-k for <ovs-dev@openvswitch.org>;\n Wed, 8 Sep 2021 09:48:37 +0000 (UTC)", "from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com\n [64.147.123.19])\n by smtp2.osuosl.org (Postfix) with ESMTPS id 15825406EA\n for <ovs-dev@openvswitch.org>; Wed, 8 Sep 2021 09:48:37 +0000 (UTC)", "from compute2.internal (compute2.nyi.internal [10.202.2.42])\n by mailout.west.internal (Postfix) with ESMTP id 80EB932009E8;\n Wed, 8 Sep 2021 05:48:36 -0400 (EDT)", "from mailfrontend2 ([10.202.2.163])\n by compute2.internal (MEProxy); Wed, 08 Sep 2021 05:48:36 -0400", "by mail.messagingengine.com (Postfix) with ESMTPA; Wed,\n 8 Sep 2021 05:48:35 -0400 (EDT)" ], "X-Virus-Scanned": [ "amavisd-new at osuosl.org", "amavisd-new at osuosl.org" ], "X-Greylist": "from auto-whitelisted by SQLgrey-1.8.0", "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/relaxed; d=u256.net; h=from\n :to:cc:subject:date:message-id:in-reply-to:references\n :mime-version:content-transfer-encoding; s=fm2; bh=04HG9NUrjuDw2\n gE1V/Q8ddYlOIiRhHATYHcpgPUqsSw=; b=Zf42mK+ODjPQNyj5hkU+JnJQaE18P\n EaoH5qgdhIGNID1gvaJD+wKGZzqRVQ6CCCYF7D3+bzNNpMQ/FjgA7ODaX5+TMVyo\n A5S6do/2alNWLp5gLihzfyNuSmZBfovOS2cO/PSqnJHuMBxuCb7XS2bSDBcXmKvA\n gFaVcXFurWWrm9+pbGizLco6M4AvT9URwTaXgKDz3yvEDQgJUJLiV1W21PmeRVas\n leK+c3BktCI0QtvDUl9uuOsxfK89eDW/H32p8mRA0gBL7rCx42KNLKam17e9xwyy\n PV6LDK+XV2lsozGWOuJtVZn1QlG2Q+ZeGUfab2uKRUKu1o35RjSwQNgBA==", "v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n messagingengine.com; h=cc:content-transfer-encoding:date:from\n :in-reply-to:message-id:mime-version:references:subject:to\n :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=\n fm3; bh=04HG9NUrjuDw2gE1V/Q8ddYlOIiRhHATYHcpgPUqsSw=; b=ierIjOuq\n 9XH1y5/7faJXEkFLaZ4uQa5r+uxBTdwcGUeitGE/8CXWhFh9+s/yRRH5L57iggrz\n OQ9Pz0HQv/sA1oS3mqn++V6n9KDMiFDSaLL/JH9I7t4Jn8exGYQxwgwCgXj3acT/\n DVapZh5l4l5ogSwBZeOfyZPgAGjF12jVUJQ2G2DPFmF25JqTa5s4kDuyzYA1vz/Z\n xSxsIZlLnC6ZcQpca43DtkCN7WOlFEpYDUOhDe+w9NIMvN/qQjGz3g47ejDf05n5\n SYETKJ31rdH9fYkYAwP9UhtkLuVjObrhE85sn0FRghk2m7lpMkXBnX6K/oz1JWBj\n HGlbnJjHuNc5UQ==" ], "X-ME-Sender": "<xms:9IY4YZ-Tw1dfUm-M44FU1mVT_J2U0oA4bOT85QUZBL3Dvw8P_xblNQ>\n <xme:9IY4YdvQcFVce45SIaccmaTR6sSkPzP1r144Fh9iEVEM4iDZWV_PDSexMEih_6IAP\n QV3Nainik7JE8cPKEM>", "X-ME-Received": "\n <xmr:9IY4YXAH8icgVN3G0w5osiLwLoCbCyFcTAFEigoJNqmSUbsPE_wv2ji3c-wCRlIoDuBWjftuENnGf-cDsxPP2bLJYQ>", "X-ME-Proxy-Cause": "\n gggruggvucftvghtrhhoucdtuddrgedvtddrudefjedgudekucetufdoteggodetrfdotf\n fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen\n uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne\n cujfgurhephffvufffkffojghfggfgsedtkeertdertddtnecuhfhrohhmpefirggvthgr\n nhcutfhivhgvthcuoehgrhhivhgvsehuvdehiedrnhgvtheqnecuggftrfgrthhtvghrnh\n ephefgveffkeetheetfeeifedvheelfeejfeehveduteejhfekuedtkeeiuedvteehnecu\n vehluhhsthgvrhfuihiivgepheenucfrrghrrghmpehmrghilhhfrhhomhepghhrihhvvg\n esuhdvheeirdhnvght", "X-ME-Proxy": "<xmx:9IY4Ydc237Qn6mdbLvqIAQfQMMJ1-loxF6rEvBJjI2fVEnHRQX4TBA>\n <xmx:9IY4YeNHx8qMDlM5rb4rhE4q8FR6gs03pODgN355nqTwK4PylyOgQQ>\n <xmx:9IY4YfkpmgEueZ9xo-A_m9yvdpHXMzQAJJwLOZJXvQ0CjELAs84evg>\n <xmx:9IY4YQ2mP4X2uf56nr0UT1AaiOqZQ4o5ouuOpTbCoddQ_WjLsiWnxA>", "From": "Gaetan Rivet <grive@u256.net>", "To": "ovs-dev@openvswitch.org", "Date": "Wed, 8 Sep 2021 11:47:46 +0200", "Message-Id": "\n <19d3b47ef290a31be471b5817db6ed3f361b12c9.1631094144.git.grive@u256.net>", "X-Mailer": "git-send-email 2.31.1", "In-Reply-To": "<cover.1631094144.git.grive@u256.net>", "References": "<cover.1631094144.git.grive@u256.net>", "MIME-Version": "1.0", "Cc": "Eli Britstein <elibr@nvidia.com>,\n Maxime Coquelin <maxime.coquelin@redhat.com>", "Subject": "[ovs-dev] [PATCH v5 22/27] netdev-offload-dpdk: Protect concurrent\n\toffload destroy/query", "X-BeenThere": "ovs-dev@openvswitch.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "<ovs-dev.openvswitch.org>", "List-Unsubscribe": "<https://mail.openvswitch.org/mailman/options/ovs-dev>,\n <mailto:ovs-dev-request@openvswitch.org?subject=unsubscribe>", "List-Archive": "<http://mail.openvswitch.org/pipermail/ovs-dev/>", "List-Post": "<mailto:ovs-dev@openvswitch.org>", "List-Help": "<mailto:ovs-dev-request@openvswitch.org?subject=help>", "List-Subscribe": "<https://mail.openvswitch.org/mailman/listinfo/ovs-dev>,\n <mailto:ovs-dev-request@openvswitch.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "ovs-dev-bounces@openvswitch.org", "Sender": "\"dev\" <ovs-dev-bounces@openvswitch.org>" }, "content": "The rte_flow API in DPDK is now thread safe for insertion and deletion.\nIt is not however safe for concurrent query while the offload is being\ninserted or deleted.\n\nInsertion is not an issue as the rte_flow handle will be published to\nother threads only once it has been inserted in the hardware, so the\nquery will only be able to proceed once it is already available.\n\nFor the deletion path however, offload status queries can be made while\nan offload is being destroyed. This would create race conditions and\nuse-after-free if not properly protected.\n\nAs a pre-step before removing the OVS-level locks on the rte_flow API,\nmutually exclude offload query and deletion from concurrent execution.\n\nSigned-off-by: Gaetan Rivet <grive@u256.net>\nReviewed-by: Eli Britstein <elibr@nvidia.com>\nReviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>\n---\n lib/netdev-offload-dpdk.c | 39 ++++++++++++++++++++++++++++++++++++---\n 1 file changed, 36 insertions(+), 3 deletions(-)", "diff": "diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c\nindex e76c50b72..28cb2f96b 100644\n--- a/lib/netdev-offload-dpdk.c\n+++ b/lib/netdev-offload-dpdk.c\n@@ -61,6 +61,8 @@ struct ufid_to_rte_flow_data {\n bool actions_offloaded;\n struct dpif_flow_stats stats;\n struct netdev *physdev;\n+ struct ovs_mutex lock;\n+ bool dead;\n };\n \n struct netdev_offload_dpdk_data {\n@@ -238,6 +240,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct netdev *netdev,\n data->physdev = netdev != physdev ? netdev_ref(physdev) : physdev;\n data->rte_flow = rte_flow;\n data->actions_offloaded = actions_offloaded;\n+ ovs_mutex_init(&data->lock);\n \n cmap_insert(map, CONST_CAST(struct cmap_node *, &data->node), hash);\n \n@@ -245,8 +248,16 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct netdev *netdev,\n return data;\n }\n \n+static void\n+rte_flow_data_unref(struct ufid_to_rte_flow_data *data)\n+{\n+ ovs_mutex_destroy(&data->lock);\n+ free(data);\n+}\n+\n static inline void\n ufid_to_rte_flow_disassociate(struct ufid_to_rte_flow_data *data)\n+ OVS_REQUIRES(data->lock)\n {\n size_t hash = hash_bytes(&data->ufid, sizeof data->ufid, 0);\n struct cmap *map = offload_data_map(data->netdev);\n@@ -263,7 +274,7 @@ ufid_to_rte_flow_disassociate(struct ufid_to_rte_flow_data *data)\n netdev_close(data->netdev);\n }\n netdev_close(data->physdev);\n- ovsrcu_postpone(free, data);\n+ ovsrcu_postpone(rte_flow_data_unref, data);\n }\n \n /*\n@@ -2033,6 +2044,15 @@ netdev_offload_dpdk_flow_destroy(struct ufid_to_rte_flow_data *rte_flow_data)\n ovs_u128 *ufid;\n int ret;\n \n+ ovs_mutex_lock(&rte_flow_data->lock);\n+\n+ if (rte_flow_data->dead) {\n+ ovs_mutex_unlock(&rte_flow_data->lock);\n+ return 0;\n+ }\n+\n+ rte_flow_data->dead = true;\n+\n rte_flow = rte_flow_data->rte_flow;\n physdev = rte_flow_data->physdev;\n netdev = rte_flow_data->netdev;\n@@ -2062,6 +2082,8 @@ netdev_offload_dpdk_flow_destroy(struct ufid_to_rte_flow_data *rte_flow_data)\n UUID_ARGS((struct uuid *) ufid));\n }\n \n+ ovs_mutex_unlock(&rte_flow_data->lock);\n+\n return ret;\n }\n \n@@ -2194,8 +2216,19 @@ netdev_offload_dpdk_flow_get(struct netdev *netdev,\n struct rte_flow_error error;\n int ret = 0;\n \n+ attrs->dp_extra_info = NULL;\n+\n rte_flow_data = ufid_to_rte_flow_data_find(netdev, ufid, false);\n- if (!rte_flow_data || !rte_flow_data->rte_flow) {\n+ if (!rte_flow_data || !rte_flow_data->rte_flow ||\n+ rte_flow_data->dead || ovs_mutex_trylock(&rte_flow_data->lock)) {\n+ return -1;\n+ }\n+\n+ /* Check again whether the data is dead, as it could have been\n+ * updated while the lock was not yet taken. The first check above\n+ * was only to avoid unnecessary locking if possible.\n+ */\n+ if (rte_flow_data->dead) {\n ret = -1;\n goto out;\n }\n@@ -2223,7 +2256,7 @@ netdev_offload_dpdk_flow_get(struct netdev *netdev,\n }\n memcpy(stats, &rte_flow_data->stats, sizeof *stats);\n out:\n- attrs->dp_extra_info = NULL;\n+ ovs_mutex_unlock(&rte_flow_data->lock);\n return ret;\n }\n \n", "prefixes": [ "ovs-dev", "v5", "22/27" ] }