Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/1525734/?format=api
{ "id": 1525734, "url": "http://patchwork.ozlabs.org/api/patches/1525734/?format=api", "web_url": "http://patchwork.ozlabs.org/project/openvswitch/patch/a911a2a8de415a9c1af9b03150b674f7221bc13d.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": "<a911a2a8de415a9c1af9b03150b674f7221bc13d.1631094144.git.grive@u256.net>", "list_archive_url": null, "date": "2021-09-08T09:47:45", "name": "[ovs-dev,v5,21/27] netdev-offload-dpdk: Lock rte_flow map access", "commit_ref": "54dcf60e6f7ee4a2a01581350b0b523aa470e1b7", "pull_url": null, "state": "accepted", "archived": false, "hash": "cb8e71160d86fe797cc7ec1f005160bd32ad27e4", "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/a911a2a8de415a9c1af9b03150b674f7221bc13d.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/1525734/comments/", "check": "success", "checks": "http://patchwork.ozlabs.org/api/patches/1525734/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=d+Z8T7DC;\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=epu/FODq;\n\tdkim-atps=neutral", "ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=140.211.166.136; helo=smtp3.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=<UNKNOWN>)", "smtp1.osuosl.org (amavisd-new);\n dkim=pass (2048-bit key) header.d=u256.net header.b=\"d+Z8T7DC\";\n dkim=pass (2048-bit key) header.d=messagingengine.com\n header.b=\"epu/FODq\"" ], "Received": [ "from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136])\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 4H4HSK73Hrz9sW8\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 8 Sep 2021 19:50:25 +1000 (AEST)", "from localhost (localhost [127.0.0.1])\n\tby smtp3.osuosl.org (Postfix) with ESMTP id CC5876153A;\n\tWed, 8 Sep 2021 09:50:23 +0000 (UTC)", "from smtp3.osuosl.org ([127.0.0.1])\n\tby localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id HnDlug0CUAO0; Wed, 8 Sep 2021 09:50:21 +0000 (UTC)", "from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])\n\tby smtp3.osuosl.org (Postfix) with ESMTPS id 6D729613B0;\n\tWed, 8 Sep 2021 09:50:19 +0000 (UTC)", "from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id 25F00C0025;\n\tWed, 8 Sep 2021 09:50:19 +0000 (UTC)", "from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138])\n by lists.linuxfoundation.org (Postfix) with ESMTP id 15AA4C0025\n for <ovs-dev@openvswitch.org>; Wed, 8 Sep 2021 09:50:18 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n by smtp1.osuosl.org (Postfix) with ESMTP id D8F4482FE4\n for <ovs-dev@openvswitch.org>; Wed, 8 Sep 2021 09:48:41 +0000 (UTC)", "from smtp1.osuosl.org ([127.0.0.1])\n by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n with ESMTP id fgJTNtlTfIoo for <ovs-dev@openvswitch.org>;\n Wed, 8 Sep 2021 09:48:38 +0000 (UTC)", "from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com\n [64.147.123.19])\n by smtp1.osuosl.org (Postfix) with ESMTPS id AF8498354B\n for <ovs-dev@openvswitch.org>; Wed, 8 Sep 2021 09:48:35 +0000 (UTC)", "from compute2.internal (compute2.nyi.internal [10.202.2.42])\n by mailout.west.internal (Postfix) with ESMTP id F3A7232009DA;\n Wed, 8 Sep 2021 05:48:34 -0400 (EDT)", "from mailfrontend2 ([10.202.2.163])\n by compute2.internal (MEProxy); Wed, 08 Sep 2021 05:48:35 -0400", "by mail.messagingengine.com (Postfix) with ESMTPA; Wed,\n 8 Sep 2021 05:48:33 -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=jyK0opkctG+pg\n N4ZKdmNINmjVaN2hzQZ9vP3v89wbks=; b=d+Z8T7DCNbC72pDCBlCFfBkcBQUTs\n 5YmBdcPX+nJ4MkdyVZnhy24+5cj1iEN91hFIWJM/lehjdx8MyF783Y6cEV0X/XwA\n fp5Xeb/FI5Vgn+UdQgHzIdpEfjRuiGMs79wk/nNH1rgucwGxiAo6jaE1KISeWCTC\n iBXviuHvqJZRRvEuhLydXepAB0ST07mZw/td/GVpTWqZSQ0gUsyuoJPmfbM4E9Vj\n 4D++TtFwYQ2VBOgGCMO6+v1eTxS+nsjOpasxh/IWV6lz3PpdOFCp8simH+fvbLLo\n B4svyutKGnEO3NMV8cugAxLY7Gu/ewYq0c/Dr+UPH3GwjKtJO6vorWUhg==", "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=jyK0opkctG+pgN4ZKdmNINmjVaN2hzQZ9vP3v89wbks=; b=epu/FODq\n H2L1wzvk1LHHO3loJUylbNyIBZ+OpK89b8BRbkPlSC08iputiuipL7TGXc7cGFhJ\n qW95+/aZ+Bol3u0WW2a6+5LZL3cP5ZGH9am+n9aTppOds/q7MBh0gmBeFp4rD/Dh\n 15Q6UyKlSGaUTTe1MQokD4SEUmnQII1mNib1j5xmt62yVjPPxZYu7Cp9lSrboyFN\n uV/u5Ldl9b5a3rB7fudciPkTS2WfFxRFJpEPgUUSSLRt6PfK3L3FA1gM+vyQmwPi\n 40nyBkEmi7wwcAgG0EjP78ueJ0QSdxmty/xiNz2q0xsFmS3Ne/HgrXJusTcfl7xF\n O5n8P2ayZRqdWQ==" ], "X-ME-Sender": "<xms:8oY4Yd6ZJNXIvjJC4RiHxMLeNyfLf7IUWB9rBySZUirpgZYBXBNdjg>\n <xme:8oY4Ya4TcUdcigpSkvePBT2ORzrwaEDMnz0faRzkv3nldGvKIF14rs75_pJzo-2P9\n n2stGrgeP1DHob8xEU>", "X-ME-Received": "\n <xmr:8oY4YUcNnAJyQnQieI7lXVHculavI1ke2wnzctZCobrNyIDmw0iEV6KXHNHsuaLGgPKQ-HOY2YIh7RofuDbGDeDP9Q>", "X-ME-Proxy-Cause": "\n gggruggvucftvghtrhhoucdtuddrgedvtddrudefjedgudekucetufdoteggodetrfdotf\n fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen\n uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne\n cujfgurhephffvufffkffojghfggfgsedtkeertdertddtnecuhfhrohhmpefirggvthgr\n nhcutfhivhgvthcuoehgrhhivhgvsehuvdehiedrnhgvtheqnecuggftrfgrthhtvghrnh\n ephefgveffkeetheetfeeifedvheelfeejfeehveduteejhfekuedtkeeiuedvteehnecu\n vehluhhsthgvrhfuihiivgepheenucfrrghrrghmpehmrghilhhfrhhomhepghhrihhvvg\n esuhdvheeirdhnvght", "X-ME-Proxy": "<xmx:8oY4YWLdtmTGnYFvktdQQbGUk-ZTt5gNlHl7b16fNkMvT1JsUiMzDg>\n <xmx:8oY4YRITXnumhQe69IWCCTQ2i6vxlP6TXAIuCrtwJ00jMtkNlkxQeg>\n <xmx:8oY4Yfzmrr2qXX2QXO_HM7AIYqv89EI07XmzOp_9wkHNozkOe2x6lA>\n <xmx:8oY4Yfhyk4eetdYJDfth1PqfZnJF6fejPvYLzYHalPeqnaQKjgheyw>", "From": "Gaetan Rivet <grive@u256.net>", "To": "ovs-dev@openvswitch.org", "Date": "Wed, 8 Sep 2021 11:47:45 +0200", "Message-Id": "\n <a911a2a8de415a9c1af9b03150b674f7221bc13d.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 21/27] netdev-offload-dpdk: Lock rte_flow map\n\taccess", "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": "Add a lock to access the ufid to rte_flow map. This will protect it\nfrom concurrent write accesses when multiple threads attempt it.\n\nAt this point, the reason for taking the lock is not to fullfill the\nneeds of the DPDK offload implementation anymore. Rewrite the comments\nto reflect this change. The lock is still needed to protect against\nchanges to netdev port mapping.\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/dpif-netdev.c | 8 ++---\n lib/netdev-offload-dpdk.c | 61 ++++++++++++++++++++++++++++++++++++---\n 2 files changed, 61 insertions(+), 8 deletions(-)", "diff": "diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c\nindex 381c959af..bf5785981 100644\n--- a/lib/dpif-netdev.c\n+++ b/lib/dpif-netdev.c\n@@ -2521,7 +2521,7 @@ mark_to_flow_disassociate(struct dp_netdev_pmd_thread *pmd,\n port = netdev_ports_get(in_port, dpif_type_str);\n if (port) {\n /* Taking a global 'port_mutex' to fulfill thread safety\n- * restrictions for the netdev-offload-dpdk module. */\n+ * restrictions regarding netdev port mapping. */\n ovs_mutex_lock(&pmd->dp->port_mutex);\n ret = netdev_flow_del(port, &flow->mega_ufid, NULL);\n ovs_mutex_unlock(&pmd->dp->port_mutex);\n@@ -2690,8 +2690,8 @@ dp_netdev_flow_offload_put(struct dp_offload_flow_item *offload)\n goto err_free;\n }\n \n- /* Taking a global 'port_mutex' to fulfill thread safety restrictions for\n- * the netdev-offload-dpdk module. */\n+ /* Taking a global 'port_mutex' to fulfill thread safety\n+ * restrictions regarding the netdev port mapping. */\n ovs_mutex_lock(&pmd->dp->port_mutex);\n ret = netdev_flow_put(port, &offload->match,\n CONST_CAST(struct nlattr *, offload->actions),\n@@ -3533,7 +3533,7 @@ dpif_netdev_get_flow_offload_status(const struct dp_netdev *dp,\n }\n ofpbuf_use_stack(&buf, &act_buf, sizeof act_buf);\n /* Taking a global 'port_mutex' to fulfill thread safety\n- * restrictions for the netdev-offload-dpdk module.\n+ * restrictions regarding netdev port mapping.\n *\n * XXX: Main thread will try to pause/stop all revalidators during datapath\n * reconfiguration via datapath purge callback (dp_purge_cb) while\ndiff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c\nindex 8ed869eb3..e76c50b72 100644\n--- a/lib/netdev-offload-dpdk.c\n+++ b/lib/netdev-offload-dpdk.c\n@@ -40,9 +40,6 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(600, 600);\n *\n * Below API is NOT thread safe in following terms:\n *\n- * - The caller must be sure that none of these functions will be called\n- * simultaneously. Even for different 'netdev's.\n- *\n * - The caller must be sure that 'netdev' will not be destructed/deallocated.\n *\n * - The caller must be sure that 'netdev' configuration will not be changed.\n@@ -69,6 +66,7 @@ struct ufid_to_rte_flow_data {\n struct netdev_offload_dpdk_data {\n struct cmap ufid_to_rte_flow;\n uint64_t *rte_flow_counters;\n+ struct ovs_mutex map_lock;\n };\n \n static int\n@@ -77,6 +75,7 @@ offload_data_init(struct netdev *netdev)\n struct netdev_offload_dpdk_data *data;\n \n data = xzalloc(sizeof *data);\n+ ovs_mutex_init(&data->map_lock);\n cmap_init(&data->ufid_to_rte_flow);\n data->rte_flow_counters = xcalloc(netdev_offload_thread_nb(),\n sizeof *data->rte_flow_counters);\n@@ -89,6 +88,7 @@ offload_data_init(struct netdev *netdev)\n static void\n offload_data_destroy__(struct netdev_offload_dpdk_data *data)\n {\n+ ovs_mutex_destroy(&data->map_lock);\n free(data->rte_flow_counters);\n free(data);\n }\n@@ -120,6 +120,34 @@ offload_data_destroy(struct netdev *netdev)\n ovsrcu_set(&netdev->hw_info.offload_data, NULL);\n }\n \n+static void\n+offload_data_lock(struct netdev *netdev)\n+ OVS_NO_THREAD_SAFETY_ANALYSIS\n+{\n+ struct netdev_offload_dpdk_data *data;\n+\n+ data = (struct netdev_offload_dpdk_data *)\n+ ovsrcu_get(void *, &netdev->hw_info.offload_data);\n+ if (!data) {\n+ return;\n+ }\n+ ovs_mutex_lock(&data->map_lock);\n+}\n+\n+static void\n+offload_data_unlock(struct netdev *netdev)\n+ OVS_NO_THREAD_SAFETY_ANALYSIS\n+{\n+ struct netdev_offload_dpdk_data *data;\n+\n+ data = (struct netdev_offload_dpdk_data *)\n+ ovsrcu_get(void *, &netdev->hw_info.offload_data);\n+ if (!data) {\n+ return;\n+ }\n+ ovs_mutex_unlock(&data->map_lock);\n+}\n+\n static struct cmap *\n offload_data_map(struct netdev *netdev)\n {\n@@ -158,6 +186,24 @@ ufid_to_rte_flow_data_find(struct netdev *netdev,\n return NULL;\n }\n \n+/* Find rte_flow with @ufid, lock-protected. */\n+static struct ufid_to_rte_flow_data *\n+ufid_to_rte_flow_data_find_protected(struct netdev *netdev,\n+ const ovs_u128 *ufid)\n+{\n+ size_t hash = hash_bytes(ufid, sizeof *ufid, 0);\n+ struct ufid_to_rte_flow_data *data;\n+ struct cmap *map = offload_data_map(netdev);\n+\n+ CMAP_FOR_EACH_WITH_HASH_PROTECTED (data, node, hash, map) {\n+ if (ovs_u128_equals(*ufid, data->ufid)) {\n+ return data;\n+ }\n+ }\n+\n+ return NULL;\n+}\n+\n static inline struct ufid_to_rte_flow_data *\n ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct netdev *netdev,\n struct netdev *physdev, struct rte_flow *rte_flow,\n@@ -174,13 +220,15 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct netdev *netdev,\n \n data = xzalloc(sizeof *data);\n \n+ offload_data_lock(netdev);\n+\n /*\n * We should not simply overwrite an existing rte flow.\n * We should have deleted it first before re-adding it.\n * Thus, if following assert triggers, something is wrong:\n * the rte_flow is not destroyed.\n */\n- data_prev = ufid_to_rte_flow_data_find(netdev, ufid, false);\n+ data_prev = ufid_to_rte_flow_data_find_protected(netdev, ufid);\n if (data_prev) {\n ovs_assert(data_prev->rte_flow == NULL);\n }\n@@ -192,6 +240,8 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct netdev *netdev,\n data->actions_offloaded = actions_offloaded;\n \n cmap_insert(map, CONST_CAST(struct cmap_node *, &data->node), hash);\n+\n+ offload_data_unlock(netdev);\n return data;\n }\n \n@@ -205,7 +255,10 @@ ufid_to_rte_flow_disassociate(struct ufid_to_rte_flow_data *data)\n return;\n }\n \n+ offload_data_lock(data->netdev);\n cmap_remove(map, CONST_CAST(struct cmap_node *, &data->node), hash);\n+ offload_data_unlock(data->netdev);\n+\n if (data->netdev != data->physdev) {\n netdev_close(data->netdev);\n }\n", "prefixes": [ "ovs-dev", "v5", "21/27" ] }