net/mlx5: reject flow template API reconfiguration
Checks
Commit Message
Flow Template API allows rte_flow_configure() to be called
more than once, to allow applications to dynamically reconfigure
the amount of resources available for flow table and flow rule creation.
PMDs can reject the change in configuration and
keep the old configuration.
Before this patch, during Flow Template API reconfiguration in mlx5 PMD,
all the allocated resources (HW/FW object pools, flow rules, pattern and
actions templates) were released and reallocated according to
the new configuration. This however leads to undefined behavior,
since all references to templates, flows or indirect actions,
which are held by the user, are now invalidated.
This patch changes the reconfiguration behavior.
Configuration provided to rte_flow_configure() is stored in port's
private data structure. On any subsequent call to rte_flow_configure(),
the provided configuration is compared against the stored configuration.
If both are equal, rte_flow_configure() reports a success and
configuration is unaffected. Otherwise, (-ENOTSUP) is returned.
Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
doc/guides/nics/mlx5.rst | 3 +
drivers/net/mlx5/mlx5.h | 8 +++
drivers/net/mlx5/mlx5_flow_hw.c | 104 +++++++++++++++++++++++++++++++-
3 files changed, 112 insertions(+), 3 deletions(-)
Comments
Hi,
> -----Original Message-----
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Sent: Saturday, 25 February 2023 21:58
> Subject: [PATCH] net/mlx5: reject flow template API reconfiguration
>
> Flow Template API allows rte_flow_configure() to be called
> more than once, to allow applications to dynamically reconfigure
> the amount of resources available for flow table and flow rule creation.
> PMDs can reject the change in configuration and
> keep the old configuration.
>
> Before this patch, during Flow Template API reconfiguration in mlx5 PMD,
> all the allocated resources (HW/FW object pools, flow rules, pattern and
> actions templates) were released and reallocated according to
> the new configuration. This however leads to undefined behavior,
> since all references to templates, flows or indirect actions,
> which are held by the user, are now invalidated.
>
> This patch changes the reconfiguration behavior.
> Configuration provided to rte_flow_configure() is stored in port's
> private data structure. On any subsequent call to rte_flow_configure(),
> the provided configuration is compared against the stored configuration.
> If both are equal, rte_flow_configure() reports a success and
> configuration is unaffected. Otherwise, (-ENOTSUP) is returned.
>
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---
Acked-by: Ori Kam <orika@nvidia.com>
> -----Original Message-----
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Sent: Sunday, February 26, 2023 3:58 AM
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [PATCH] net/mlx5: reject flow template API reconfiguration
>
> Flow Template API allows rte_flow_configure() to be called more than once, to
> allow applications to dynamically reconfigure the amount of resources available
> for flow table and flow rule creation.
> PMDs can reject the change in configuration and keep the old configuration.
>
> Before this patch, during Flow Template API reconfiguration in mlx5 PMD, all the
> allocated resources (HW/FW object pools, flow rules, pattern and actions
> templates) were released and reallocated according to the new configuration.
> This however leads to undefined behavior, since all references to templates,
> flows or indirect actions, which are held by the user, are now invalidated.
>
> This patch changes the reconfiguration behavior.
> Configuration provided to rte_flow_configure() is stored in port's private data
> structure. On any subsequent call to rte_flow_configure(), the provided
> configuration is compared against the stored configuration.
> If both are equal, rte_flow_configure() reports a success and configuration is
> unaffected. Otherwise, (-ENOTSUP) is returned.
>
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Suanming Mou <suanmingm@nvidia.com>
From: Dariusz Sosnowski
> Flow Template API allows rte_flow_configure() to be called more than once,
> to allow applications to dynamically reconfigure the amount of resources
> available for flow table and flow rule creation.
> PMDs can reject the change in configuration and keep the old configuration.
>
> Before this patch, during Flow Template API reconfiguration in mlx5 PMD, all
> the allocated resources (HW/FW object pools, flow rules, pattern and actions
> templates) were released and reallocated according to the new
> configuration. This however leads to undefined behavior, since all references
> to templates, flows or indirect actions, which are held by the user, are now
> invalidated.
>
> This patch changes the reconfiguration behavior.
> Configuration provided to rte_flow_configure() is stored in port's private
> data structure. On any subsequent call to rte_flow_configure(), the provided
> configuration is compared against the stored configuration.
> If both are equal, rte_flow_configure() reports a success and configuration is
> unaffected. Otherwise, (-ENOTSUP) is returned.
>
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
Hi,
> -----Original Message-----
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Sent: Saturday, February 25, 2023 9:58 PM
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [PATCH] net/mlx5: reject flow template API reconfiguration
>
> Flow Template API allows rte_flow_configure() to be called more than once, to
> allow applications to dynamically reconfigure the amount of resources
> available for flow table and flow rule creation.
> PMDs can reject the change in configuration and keep the old configuration.
>
> Before this patch, during Flow Template API reconfiguration in mlx5 PMD, all
> the allocated resources (HW/FW object pools, flow rules, pattern and actions
> templates) were released and reallocated according to the new configuration.
> This however leads to undefined behavior, since all references to templates,
> flows or indirect actions, which are held by the user, are now invalidated.
>
> This patch changes the reconfiguration behavior.
> Configuration provided to rte_flow_configure() is stored in port's private data
> structure. On any subsequent call to rte_flow_configure(), the provided
> configuration is compared against the stored configuration.
> If both are equal, rte_flow_configure() reports a success and configuration is
> unaffected. Otherwise, (-ENOTSUP) is returned.
>
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Patch applied to net-net-mlx,
Kindest regards
Raslan Darawsheh
@@ -160,6 +160,9 @@ Limitations
- Set ``dv_flow_en`` to 2 in order to enable HW steering.
- Async queue-based ``rte_flow_async`` APIs supported only.
- NIC ConnectX-5 and before are not supported.
+ - Reconfiguring flow API engine is not supported.
+ Any subsequent call to ``rte_flow_configure()`` with different configuration
+ than initially provided will be rejected with ``-ENOTSUP`` error code.
- Partial match with item template is not supported.
- IPv6 5-tuple matching is not supported.
- With E-Switch enabled, ports which share the E-Switch domain
@@ -1656,6 +1656,13 @@ struct mlx5_hw_ctrl_flow {
struct rte_flow *flow;
};
+/* HW Steering port configuration passed to rte_flow_configure(). */
+struct mlx5_flow_hw_attr {
+ struct rte_flow_port_attr port_attr;
+ uint16_t nb_queue;
+ struct rte_flow_queue_attr *queue_attr;
+};
+
struct mlx5_flow_hw_ctrl_rx;
struct mlx5_priv {
@@ -1763,6 +1770,7 @@ struct mlx5_priv {
uint32_t nb_queue; /* HW steering queue number. */
struct mlx5_hws_cnt_pool *hws_cpool; /* HW steering's counter pool. */
uint32_t hws_mark_refcnt; /* HWS mark action reference counter. */
+ struct mlx5_flow_hw_attr *hw_attr; /* HW Steering port configuration. */
#if defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_INFINIBAND_VERBS_H)
/* Item template list. */
LIST_HEAD(flow_hw_itt, rte_flow_pattern_template) flow_hw_itt;
@@ -6792,6 +6792,86 @@ mlx5_flow_hw_cleanup_ctrl_rx_templates(struct rte_eth_dev *dev)
}
}
+/**
+ * Copy the provided HWS configuration to a newly allocated buffer.
+ *
+ * @param[in] port_attr
+ * Port configuration attributes.
+ * @param[in] nb_queue
+ * Number of queue.
+ * @param[in] queue_attr
+ * Array that holds attributes for each flow queue.
+ *
+ * @return
+ * Pointer to copied HWS configuration is returned on success.
+ * Otherwise, NULL is returned and rte_errno is set.
+ */
+static struct mlx5_flow_hw_attr *
+flow_hw_alloc_copy_config(const struct rte_flow_port_attr *port_attr,
+ const uint16_t nb_queue,
+ const struct rte_flow_queue_attr *queue_attr[],
+ struct rte_flow_error *error)
+{
+ struct mlx5_flow_hw_attr *hw_attr;
+ size_t hw_attr_size;
+ unsigned int i;
+
+ hw_attr_size = sizeof(*hw_attr) + nb_queue * sizeof(*hw_attr->queue_attr);
+ hw_attr = mlx5_malloc(MLX5_MEM_ZERO, hw_attr_size, 0, SOCKET_ID_ANY);
+ if (!hw_attr) {
+ rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+ "Not enough memory to store configuration");
+ return NULL;
+ }
+ memcpy(&hw_attr->port_attr, port_attr, sizeof(*port_attr));
+ hw_attr->nb_queue = nb_queue;
+ /* Queue attributes are placed after the mlx5_flow_hw_attr. */
+ hw_attr->queue_attr = (struct rte_flow_queue_attr *)(hw_attr + 1);
+ for (i = 0; i < nb_queue; ++i)
+ memcpy(&hw_attr->queue_attr[i], queue_attr[i], sizeof(hw_attr->queue_attr[i]));
+ return hw_attr;
+}
+
+/**
+ * Compares the preserved HWS configuration with the provided one.
+ *
+ * @param[in] hw_attr
+ * Pointer to preserved HWS configuration.
+ * @param[in] new_pa
+ * Port configuration attributes to compare.
+ * @param[in] new_nbq
+ * Number of queues to compare.
+ * @param[in] new_qa
+ * Array that holds attributes for each flow queue.
+ *
+ * @return
+ * True if configurations are the same, false otherwise.
+ */
+static bool
+flow_hw_compare_config(const struct mlx5_flow_hw_attr *hw_attr,
+ const struct rte_flow_port_attr *new_pa,
+ const uint16_t new_nbq,
+ const struct rte_flow_queue_attr *new_qa[])
+{
+ const struct rte_flow_port_attr *old_pa = &hw_attr->port_attr;
+ const uint16_t old_nbq = hw_attr->nb_queue;
+ const struct rte_flow_queue_attr *old_qa = hw_attr->queue_attr;
+ unsigned int i;
+
+ if (old_pa->nb_counters != new_pa->nb_counters ||
+ old_pa->nb_aging_objects != new_pa->nb_aging_objects ||
+ old_pa->nb_meters != new_pa->nb_meters ||
+ old_pa->nb_conn_tracks != new_pa->nb_conn_tracks ||
+ old_pa->flags != new_pa->flags)
+ return false;
+ if (old_nbq != new_nbq)
+ return false;
+ for (i = 0; i < old_nbq; ++i)
+ if (old_qa[i].size != new_qa[i]->size)
+ return false;
+ return true;
+}
+
/**
* Configure port HWS resources.
*
@@ -6846,9 +6926,12 @@ flow_hw_configure(struct rte_eth_dev *dev,
rte_errno = EINVAL;
goto err;
}
- /* In case re-configuring, release existing context at first. */
+ /*
+ * Calling rte_flow_configure() again is allowed if and only if
+ * provided configuration matches the initially provided one.
+ */
if (priv->dr_ctx) {
- /* */
+ MLX5_ASSERT(priv->hw_attr != NULL);
for (i = 0; i < priv->nb_queue; i++) {
hw_q = &priv->hw_q[i];
/* Make sure all queues are empty. */
@@ -6857,7 +6940,18 @@ flow_hw_configure(struct rte_eth_dev *dev,
goto err;
}
}
- flow_hw_resource_release(dev);
+ if (flow_hw_compare_config(priv->hw_attr, port_attr, nb_queue, queue_attr))
+ return 0;
+ else
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+ "Changing HWS configuration attributes "
+ "is not supported");
+ }
+ priv->hw_attr = flow_hw_alloc_copy_config(port_attr, nb_queue, queue_attr, error);
+ if (!priv->hw_attr) {
+ ret = -rte_errno;
+ goto err;
}
ctrl_queue_attr.size = queue_attr[0]->size;
nb_q_updated = nb_queue + 1;
@@ -7142,6 +7236,8 @@ flow_hw_configure(struct rte_eth_dev *dev,
__atomic_fetch_sub(&host_priv->shared_refcnt, 1, __ATOMIC_RELAXED);
priv->shared_host = NULL;
}
+ mlx5_free(priv->hw_attr);
+ priv->hw_attr = NULL;
/* Do not overwrite the internal errno information. */
if (ret)
return ret;
@@ -7226,6 +7322,8 @@ flow_hw_resource_release(struct rte_eth_dev *dev)
priv->shared_host = NULL;
}
priv->dr_ctx = NULL;
+ mlx5_free(priv->hw_attr);
+ priv->hw_attr = NULL;
priv->nb_queue = 0;
}