diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst index 119b128739..07f44bb3c2 100644 --- a/doc/guides/prog_guide/rte_flow.rst +++ b/doc/guides/prog_guide/rte_flow.rst @@ -3046,10 +3046,6 @@ Caveats - API operations are synchronous and blocking (``EAGAIN`` cannot be returned). -- There is no provision for re-entrancy/multi-thread safety, although nothing - should prevent different devices from being configured at the same - time. PMDs may protect their control path functions accordingly. - - Stopping the data path (TX/RX) should not be necessary when managing flow rules. If this cannot be achieved naturally or with workarounds (such as temporarily replacing the burst function pointers), an appropriate error @@ -3101,6 +3097,14 @@ This interface additionally defines the following helper function: - ``rte_flow_ops_get()``: get generic flow operations structure from a port. +If PMD interfaces don't support re-entrancy/multi-thread safety, +the rte_flow API functions will protect threads by mutex per port. +The application can check whether ``RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE`` +is set in ``dev_flags``, meaning the PMD is thread-safe regarding rte_flow, +so the API level protection is disabled. +Please note that this API-level mutex protects only rte_flow functions, +other control path functions are not in scope. + More will be added over time. Device compatibility diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst index 708ebb01c8..1ddbdce08e 100644 --- a/doc/guides/rel_notes/release_20_11.rst +++ b/doc/guides/rel_notes/release_20_11.rst @@ -79,6 +79,12 @@ New Features Added the FEC API which provides functions for query FEC capabilities and current FEC mode from device. Also, API for configuring FEC mode is also provided. +* **Added thread safety to rte_flow functions.** + + Added ``RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE`` device flag to indicate + whether PMD supports thread safe operations. If PMD doesn't set the flag, + rte_flow API level functions will protect the flow operations with mutex. + * **Updated Broadcom bnxt driver.** Updated the Broadcom bnxt driver with new features and improvements, including: diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 59beb8aec2..c8ed890686 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -500,6 +500,7 @@ rte_eth_dev_allocate(const char *name) strlcpy(eth_dev->data->name, name, sizeof(eth_dev->data->name)); eth_dev->data->port_id = port_id; eth_dev->data->mtu = RTE_ETHER_MTU; + pthread_mutex_init(ð_dev->data->flow_ops_mutex, NULL); unlock: rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock); @@ -564,6 +565,7 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev) rte_free(eth_dev->data->mac_addrs); rte_free(eth_dev->data->hash_mac_addrs); rte_free(eth_dev->data->dev_private); + pthread_mutex_destroy(ð_dev->data->flow_ops_mutex); memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); } diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h index 3a31f94367..500b2ff766 100644 --- a/lib/librte_ethdev/rte_ethdev.h +++ b/lib/librte_ethdev/rte_ethdev.h @@ -1677,6 +1677,8 @@ struct rte_eth_dev_owner { char name[RTE_ETH_MAX_OWNER_NAME_LEN]; /**< The owner name. */ }; +/** PMD supports thread-safe flow operations */ +#define RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE 0x0001 /** Device supports link state interrupt */ #define RTE_ETH_DEV_INTR_LSC 0x0002 /** Device is a bonded slave */ diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h index fd3bf92496..918a34ed1f 100644 --- a/lib/librte_ethdev/rte_ethdev_core.h +++ b/lib/librte_ethdev/rte_ethdev_core.h @@ -5,6 +5,9 @@ #ifndef _RTE_ETHDEV_CORE_H_ #define _RTE_ETHDEV_CORE_H_ +#include +#include + /** * @file * @@ -180,6 +183,7 @@ struct rte_eth_dev_data { * Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags. */ + pthread_mutex_t flow_ops_mutex; /**< rte_flow ops mutex. */ uint64_t reserved_64s[4]; /**< Reserved for future fields */ void *reserved_ptrs[4]; /**< Reserved for future fields */ } __rte_cache_aligned; diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c index 8d1b279bcb..4101b271bb 100644 --- a/lib/librte_ethdev/rte_flow.c +++ b/lib/librte_ethdev/rte_flow.c @@ -207,6 +207,20 @@ error: return -rte_errno; } +static inline void +fts_enter(struct rte_eth_dev *dev) +{ + if (!(dev->data->dev_flags & RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE)) + pthread_mutex_lock(&dev->data->flow_ops_mutex); +} + +static inline void +fts_exit(struct rte_eth_dev *dev) +{ + if (!(dev->data->dev_flags & RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE)) + pthread_mutex_unlock(&dev->data->flow_ops_mutex); +} + static int flow_err(uint16_t port_id, int ret, struct rte_flow_error *error) { @@ -253,12 +267,16 @@ rte_flow_validate(uint16_t port_id, { const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); struct rte_eth_dev *dev = &rte_eth_devices[port_id]; + int ret; if (unlikely(!ops)) return -rte_errno; - if (likely(!!ops->validate)) - return flow_err(port_id, ops->validate(dev, attr, pattern, - actions, error), error); + if (likely(!!ops->validate)) { + fts_enter(dev); + ret = ops->validate(dev, attr, pattern, actions, error); + fts_exit(dev); + return flow_err(port_id, ret, error); + } return rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, rte_strerror(ENOSYS)); @@ -279,7 +297,9 @@ rte_flow_create(uint16_t port_id, if (unlikely(!ops)) return NULL; if (likely(!!ops->create)) { + fts_enter(dev); flow = ops->create(dev, attr, pattern, actions, error); + fts_exit(dev); if (flow == NULL) flow_err(port_id, -rte_errno, error); return flow; @@ -297,12 +317,16 @@ rte_flow_destroy(uint16_t port_id, { struct rte_eth_dev *dev = &rte_eth_devices[port_id]; const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); + int ret; if (unlikely(!ops)) return -rte_errno; - if (likely(!!ops->destroy)) - return flow_err(port_id, ops->destroy(dev, flow, error), - error); + if (likely(!!ops->destroy)) { + fts_enter(dev); + ret = ops->destroy(dev, flow, error); + fts_exit(dev); + return flow_err(port_id, ret, error); + } return rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, rte_strerror(ENOSYS)); @@ -315,11 +339,16 @@ rte_flow_flush(uint16_t port_id, { struct rte_eth_dev *dev = &rte_eth_devices[port_id]; const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); + int ret; if (unlikely(!ops)) return -rte_errno; - if (likely(!!ops->flush)) - return flow_err(port_id, ops->flush(dev, error), error); + if (likely(!!ops->flush)) { + fts_enter(dev); + ret = ops->flush(dev, error); + fts_exit(dev); + return flow_err(port_id, ret, error); + } return rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, rte_strerror(ENOSYS)); @@ -335,12 +364,16 @@ rte_flow_query(uint16_t port_id, { struct rte_eth_dev *dev = &rte_eth_devices[port_id]; const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); + int ret; if (!ops) return -rte_errno; - if (likely(!!ops->query)) - return flow_err(port_id, ops->query(dev, flow, action, data, - error), error); + if (likely(!!ops->query)) { + fts_enter(dev); + ret = ops->query(dev, flow, action, data, error); + fts_exit(dev); + return flow_err(port_id, ret, error); + } return rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, rte_strerror(ENOSYS)); @@ -354,11 +387,16 @@ rte_flow_isolate(uint16_t port_id, { struct rte_eth_dev *dev = &rte_eth_devices[port_id]; const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); + int ret; if (!ops) return -rte_errno; - if (likely(!!ops->isolate)) - return flow_err(port_id, ops->isolate(dev, set, error), error); + if (likely(!!ops->isolate)) { + fts_enter(dev); + ret = ops->isolate(dev, set, error); + fts_exit(dev); + return flow_err(port_id, ret, error); + } return rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, rte_strerror(ENOSYS)); @@ -962,12 +1000,16 @@ rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error) { struct rte_eth_dev *dev = &rte_eth_devices[port_id]; const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); + int ret; if (unlikely(!ops)) return -rte_errno; - if (likely(!!ops->dev_dump)) - return flow_err(port_id, ops->dev_dump(dev, file, error), - error); + if (likely(!!ops->dev_dump)) { + fts_enter(dev); + ret = ops->dev_dump(dev, file, error); + fts_exit(dev); + return flow_err(port_id, ret, error); + } return rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, rte_strerror(ENOSYS)); @@ -979,12 +1021,16 @@ rte_flow_get_aged_flows(uint16_t port_id, void **contexts, { struct rte_eth_dev *dev = &rte_eth_devices[port_id]; const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); + int ret; if (unlikely(!ops)) return -rte_errno; - if (likely(!!ops->get_aged_flows)) - return flow_err(port_id, ops->get_aged_flows(dev, contexts, - nb_contexts, error), error); + if (likely(!!ops->get_aged_flows)) { + fts_enter(dev); + ret = ops->get_aged_flows(dev, contexts, nb_contexts, error); + fts_exit(dev); + return flow_err(port_id, ret, error); + } return rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, rte_strerror(ENOTSUP));