Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/79590/?format=api
{ "id": 79590, "url": "http://patches.dpdk.org/api/patches/79590/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/1601855286-81511-3-git-send-email-suanmingm@nvidia.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<1601855286-81511-3-git-send-email-suanmingm@nvidia.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1601855286-81511-3-git-send-email-suanmingm@nvidia.com", "date": "2020-10-04T23:48:06", "name": "[v2,2/2] ethdev: make rte_flow API thread safe", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "c48b6580b951eaccb91f12294d87f6397882ffa5", "submitter": { "id": 1887, "url": "http://patches.dpdk.org/api/people/1887/?format=api", "name": "Suanming Mou", "email": "suanmingm@nvidia.com" }, "delegate": { "id": 1, "url": "http://patches.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/1601855286-81511-3-git-send-email-suanmingm@nvidia.com/mbox/", "series": [ { "id": 12681, "url": "http://patches.dpdk.org/api/series/12681/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=12681", "date": "2020-10-04T23:48:04", "name": "ethdev: make rte_flow API thread safe", "version": 2, "mbox": "http://patches.dpdk.org/series/12681/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/79590/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/79590/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from dpdk.org (dpdk.org [92.243.14.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 57920A04BA;\n\tMon, 5 Oct 2020 01:48:57 +0200 (CEST)", "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id ABB731B6F3;\n\tMon, 5 Oct 2020 01:48:31 +0200 (CEST)", "from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129])\n by dpdk.org (Postfix) with ESMTP id 5AF231B6E7\n for <dev@dpdk.org>; Mon, 5 Oct 2020 01:48:30 +0200 (CEST)", "from Internal Mail-Server by MTLPINE1 (envelope-from\n suanmingm@nvidia.com) with SMTP; 5 Oct 2020 02:48:27 +0300", "from nvidia.com (mtbc-r640-04.mtbc.labs.mlnx [10.75.70.9])\n by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 094NmFKo031192;\n Mon, 5 Oct 2020 02:48:25 +0300" ], "From": "Suanming Mou <suanmingm@nvidia.com>", "To": "Ori Kam <orika@mellanox.com>, John McNamara <john.mcnamara@intel.com>,\n Marko Kovacevic <marko.kovacevic@intel.com>,\n Thomas Monjalon <thomas@monjalon.net>,\n Ferruh Yigit <ferruh.yigit@intel.com>,\n Andrew Rybchenko <arybchenko@solarflare.com>", "Cc": "dev@dpdk.org", "Date": "Mon, 5 Oct 2020 07:48:06 +0800", "Message-Id": "<1601855286-81511-3-git-send-email-suanmingm@nvidia.com>", "X-Mailer": "git-send-email 1.8.3.1", "In-Reply-To": "<1601855286-81511-1-git-send-email-suanmingm@nvidia.com>", "References": "<1601194817-208834-1-git-send-email-suanmingm@nvidia.com>\n <1601855286-81511-1-git-send-email-suanmingm@nvidia.com>", "Subject": "[dpdk-dev] [PATCH v2 2/2] ethdev: make rte_flow API thread safe", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Currently, the rte_flow functions are not defined as thread safe.\nDPDK applications either call the functions in single thread or add\nlocks around the functions for the critical section.\n\nFor PMDs support the flow operations thread safe natively, the\nredundant protection in application hurts the performance of the\nrte_flow operation functions.\n\nAnd the restriction of thread safe not guaranteed for the rte_flow\nfunctions also limits the applications' expectation.\n\nThis feature is going to change the rte_flow functions to be thread\nsafe. As different PMDs have different flow operations, some may\nsupport thread safe already and others may not. For PMDs don't\nsupport flow thread safe operation, a new lock is defined in ethdev\nin order to protects thread unsafe PMDs from rte_flow level.\n\nA new RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE device flag is added to\ndetermine whether the PMD supports thread safe flow operation or not.\nFor PMDs support thread safe flow operations, set the\nRTE_ETH_DEV_FLOW_OPS_THREAD_SAFE flag, rte_flow level functions will\nskip the thread safe helper lock for these PMDs. Again the rte_flow\nlevel thread safe lock only works when PMD operation functions are\nnot thread safe.\n\nFor the PMDs which don't want the default mutex lock, just set the\nflag in the PMD, and add the prefer type of lock in the PMD. Then\nthe default mutex lock is easily replaced by the PMD level lock.\n\nThe change has no effect on the current DPDK applications. No change\nis required for the current DPDK applications. For the standard posix\npthread_mutex, if no lock contention with the added rte_flow level\nmutex, the mutex only does the atomic increasing in\npthread_mutex_lock() and decreasing in\npthread_mutex_unlock(). No futex() syscall will be involved.\n\nSigned-off-by: Suanming Mou <suanmingm@nvidia.com>\n---\n\nv2:\n - Update commit info and description doc.\n - Add inline for the flow lock and unlock functions.\n - Remove the PMD sample part flag configuration.\n\n---\n doc/guides/prog_guide/rte_flow.rst | 9 ++--\n lib/librte_ethdev/rte_ethdev.c | 2 +\n lib/librte_ethdev/rte_ethdev.h | 2 +\n lib/librte_ethdev/rte_ethdev_core.h | 4 ++\n lib/librte_ethdev/rte_flow.c | 84 ++++++++++++++++++++++++++++---------\n 5 files changed, 78 insertions(+), 23 deletions(-)", "diff": "diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst\nindex 119b128..ae2ddb3 100644\n--- a/doc/guides/prog_guide/rte_flow.rst\n+++ b/doc/guides/prog_guide/rte_flow.rst\n@@ -3046,10 +3046,6 @@ Caveats\n - API operations are synchronous and blocking (``EAGAIN`` cannot be\n returned).\n \n-- There is no provision for re-entrancy/multi-thread safety, although nothing\n- should prevent different devices from being configured at the same\n- time. PMDs may protect their control path functions accordingly.\n-\n - Stopping the data path (TX/RX) should not be necessary when managing flow\n rules. If this cannot be achieved naturally or with workarounds (such as\n temporarily replacing the burst function pointers), an appropriate error\n@@ -3101,6 +3097,11 @@ This interface additionally defines the following helper function:\n - ``rte_flow_ops_get()``: get generic flow operations structure from a\n port.\n \n+If PMD interfaces do not support re-entrancy/multi-thread safety, rte_flow\n+level functions will do it by mutex. The application can test the dev_flags\n+with RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE in struct rte_eth_dev_data to know\n+if the rte_flow thread-safe works under rte_flow level or PMD level.\n+\n More will be added over time.\n \n Device compatibility\ndiff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c\nindex dfe5c1b..d6fd17e 100644\n--- a/lib/librte_ethdev/rte_ethdev.c\n+++ b/lib/librte_ethdev/rte_ethdev.c\n@@ -500,6 +500,7 @@ struct rte_eth_dev *\n \tstrlcpy(eth_dev->data->name, name, sizeof(eth_dev->data->name));\n \teth_dev->data->port_id = port_id;\n \teth_dev->data->mtu = RTE_ETHER_MTU;\n+\tpthread_mutex_init(ð_dev->data->fts_mutex, NULL);\n \n unlock:\n \trte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);\n@@ -562,6 +563,7 @@ struct rte_eth_dev *\n \t\trte_free(eth_dev->data->mac_addrs);\n \t\trte_free(eth_dev->data->hash_mac_addrs);\n \t\trte_free(eth_dev->data->dev_private);\n+\t\tpthread_mutex_destroy(ð_dev->data->fts_mutex);\n \t\tmemset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data));\n \t}\n \ndiff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h\nindex 645a186..61bd09f 100644\n--- a/lib/librte_ethdev/rte_ethdev.h\n+++ b/lib/librte_ethdev/rte_ethdev.h\n@@ -1669,6 +1669,8 @@ struct rte_eth_dev_owner {\n #define RTE_ETH_DEV_REPRESENTOR 0x0010\n /** Device does not support MAC change after started */\n #define RTE_ETH_DEV_NOLIVE_MAC_ADDR 0x0020\n+/** Device PMD supports thread safety flow operation */\n+#define RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE 0x0040\n \n /**\n * Iterates over valid ethdev ports owned by a specific owner.\ndiff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h\nindex fd3bf92..89df65a 100644\n--- a/lib/librte_ethdev/rte_ethdev_core.h\n+++ b/lib/librte_ethdev/rte_ethdev_core.h\n@@ -5,6 +5,9 @@\n #ifndef _RTE_ETHDEV_CORE_H_\n #define _RTE_ETHDEV_CORE_H_\n \n+#include <pthread.h>\n+#include <sys/types.h>\n+\n /**\n * @file\n *\n@@ -180,6 +183,7 @@ struct rte_eth_dev_data {\n \t\t\t * Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.\n \t\t\t */\n \n+\tpthread_mutex_t fts_mutex; /**< rte flow ops thread safety mutex. */\n \tuint64_t reserved_64s[4]; /**< Reserved for future fields */\n \tvoid *reserved_ptrs[4]; /**< Reserved for future fields */\n } __rte_cache_aligned;\ndiff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c\nindex f8fdd68..2ac966d 100644\n--- a/lib/librte_ethdev/rte_flow.c\n+++ b/lib/librte_ethdev/rte_flow.c\n@@ -207,6 +207,20 @@ struct rte_flow_desc_data {\n \treturn -rte_errno;\n }\n \n+static inline void\n+flow_lock(struct rte_eth_dev *dev)\n+{\n+\tif (!(dev->data->dev_flags & RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE))\n+\t\tpthread_mutex_lock(&dev->data->fts_mutex);\n+}\n+\n+static inline void\n+flow_unlock(struct rte_eth_dev *dev)\n+{\n+\tif (!(dev->data->dev_flags & RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE))\n+\t\tpthread_mutex_unlock(&dev->data->fts_mutex);\n+}\n+\n static int\n flow_err(uint16_t port_id, int ret, struct rte_flow_error *error)\n {\n@@ -346,12 +360,16 @@ struct rte_flow_desc_data {\n {\n \tconst struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);\n \tstruct rte_eth_dev *dev = &rte_eth_devices[port_id];\n+\tint ret;\n \n \tif (unlikely(!ops))\n \t\treturn -rte_errno;\n-\tif (likely(!!ops->validate))\n-\t\treturn flow_err(port_id, ops->validate(dev, attr, pattern,\n-\t\t\t\t\t\t actions, error), error);\n+\tif (likely(!!ops->validate)) {\n+\t\tflow_lock(dev);\n+\t\tret = ops->validate(dev, attr, pattern, actions, error);\n+\t\tflow_unlock(dev);\n+\t\treturn flow_err(port_id, ret, error);\n+\t}\n \treturn rte_flow_error_set(error, ENOSYS,\n \t\t\t\t RTE_FLOW_ERROR_TYPE_UNSPECIFIED,\n \t\t\t\t NULL, rte_strerror(ENOSYS));\n@@ -372,7 +390,9 @@ struct rte_flow *\n \tif (unlikely(!ops))\n \t\treturn NULL;\n \tif (likely(!!ops->create)) {\n+\t\tflow_lock(dev);\n \t\tflow = ops->create(dev, attr, pattern, actions, error);\n+\t\tflow_unlock(dev);\n \t\tif (flow == NULL)\n \t\t\tflow_err(port_id, -rte_errno, error);\n \t\treturn flow;\n@@ -390,12 +410,16 @@ struct rte_flow *\n {\n \tstruct rte_eth_dev *dev = &rte_eth_devices[port_id];\n \tconst struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);\n+\tint ret;\n \n \tif (unlikely(!ops))\n \t\treturn -rte_errno;\n-\tif (likely(!!ops->destroy))\n-\t\treturn flow_err(port_id, ops->destroy(dev, flow, error),\n-\t\t\t\terror);\n+\tif (likely(!!ops->destroy)) {\n+\t\tflow_lock(dev);\n+\t\tret = ops->destroy(dev, flow, error);\n+\t\tflow_unlock(dev);\n+\t\treturn flow_err(port_id, ret, error);\n+\t}\n \treturn rte_flow_error_set(error, ENOSYS,\n \t\t\t\t RTE_FLOW_ERROR_TYPE_UNSPECIFIED,\n \t\t\t\t NULL, rte_strerror(ENOSYS));\n@@ -408,11 +432,16 @@ struct rte_flow *\n {\n \tstruct rte_eth_dev *dev = &rte_eth_devices[port_id];\n \tconst struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);\n+\tint ret;\n \n \tif (unlikely(!ops))\n \t\treturn -rte_errno;\n-\tif (likely(!!ops->flush))\n-\t\treturn flow_err(port_id, ops->flush(dev, error), error);\n+\tif (likely(!!ops->flush)) {\n+\t\tflow_lock(dev);\n+\t\tret = ops->flush(dev, error);\n+\t\tflow_unlock(dev);\n+\t\treturn flow_err(port_id, ret, error);\n+\t}\n \treturn rte_flow_error_set(error, ENOSYS,\n \t\t\t\t RTE_FLOW_ERROR_TYPE_UNSPECIFIED,\n \t\t\t\t NULL, rte_strerror(ENOSYS));\n@@ -428,12 +457,16 @@ struct rte_flow *\n {\n \tstruct rte_eth_dev *dev = &rte_eth_devices[port_id];\n \tconst struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);\n+\tint ret;\n \n \tif (!ops)\n \t\treturn -rte_errno;\n-\tif (likely(!!ops->query))\n-\t\treturn flow_err(port_id, ops->query(dev, flow, action, data,\n-\t\t\t\t\t\t error), error);\n+\tif (likely(!!ops->query)) {\n+\t\tflow_lock(dev);\n+\t\tret = ops->query(dev, flow, action, data, error);\n+\t\tflow_unlock(dev);\n+\t\treturn flow_err(port_id, ret, error);\n+\t}\n \treturn rte_flow_error_set(error, ENOSYS,\n \t\t\t\t RTE_FLOW_ERROR_TYPE_UNSPECIFIED,\n \t\t\t\t NULL, rte_strerror(ENOSYS));\n@@ -447,11 +480,16 @@ struct rte_flow *\n {\n \tstruct rte_eth_dev *dev = &rte_eth_devices[port_id];\n \tconst struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);\n+\tint ret;\n \n \tif (!ops)\n \t\treturn -rte_errno;\n-\tif (likely(!!ops->isolate))\n-\t\treturn flow_err(port_id, ops->isolate(dev, set, error), error);\n+\tif (likely(!!ops->isolate)) {\n+\t\tflow_lock(dev);\n+\t\tret = ops->isolate(dev, set, error);\n+\t\tflow_unlock(dev);\n+\t\treturn flow_err(port_id, ret, error);\n+\t}\n \treturn rte_flow_error_set(error, ENOSYS,\n \t\t\t\t RTE_FLOW_ERROR_TYPE_UNSPECIFIED,\n \t\t\t\t NULL, rte_strerror(ENOSYS));\n@@ -1224,12 +1262,16 @@ enum rte_flow_conv_item_spec_type {\n {\n \tstruct rte_eth_dev *dev = &rte_eth_devices[port_id];\n \tconst struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);\n+\tint ret;\n \n \tif (unlikely(!ops))\n \t\treturn -rte_errno;\n-\tif (likely(!!ops->dev_dump))\n-\t\treturn flow_err(port_id, ops->dev_dump(dev, file, error),\n-\t\t\t\terror);\n+\tif (likely(!!ops->dev_dump)) {\n+\t\tflow_lock(dev);\n+\t\tret = ops->dev_dump(dev, file, error);\n+\t\tflow_unlock(dev);\n+\t\treturn flow_err(port_id, ret, error);\n+\t}\n \treturn rte_flow_error_set(error, ENOSYS,\n \t\t\t\t RTE_FLOW_ERROR_TYPE_UNSPECIFIED,\n \t\t\t\t NULL, rte_strerror(ENOSYS));\n@@ -1241,12 +1283,16 @@ enum rte_flow_conv_item_spec_type {\n {\n \tstruct rte_eth_dev *dev = &rte_eth_devices[port_id];\n \tconst struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);\n+\tint ret;\n \n \tif (unlikely(!ops))\n \t\treturn -rte_errno;\n-\tif (likely(!!ops->get_aged_flows))\n-\t\treturn flow_err(port_id, ops->get_aged_flows(dev, contexts,\n-\t\t\t\tnb_contexts, error), error);\n+\tif (likely(!!ops->get_aged_flows)) {\n+\t\tflow_lock(dev);\n+\t\tret = ops->get_aged_flows(dev, contexts, nb_contexts, error);\n+\t\tflow_unlock(dev);\n+\t\treturn flow_err(port_id, ret, error);\n+\t}\n \treturn rte_flow_error_set(error, ENOTSUP,\n \t\t\t\t RTE_FLOW_ERROR_TYPE_UNSPECIFIED,\n \t\t\t\t NULL, rte_strerror(ENOTSUP));\n", "prefixes": [ "v2", "2/2" ] }