[dpdk-dev] [PATCH 1/1] net/mlx5: add stub functions to null drv ops

[dpdk-dev] [PATCH 1/1] net/mlx5: add stub functions to null drv ops

[Asaf Penso]

There are several functions implementation that queries the drv type
to understand which fops function to use.
In case the type is DV, the fucntion gets the concrete DV function and
calls it.
In case it’s not, the function returns an error.

The current implementation is not flexible enough and will not support
future concrete functions in a good way.

This patch adds more stub functions that include error handling and
are assigned to the null drv ops struct.
This allows the functions to always call the virtual function without
basing the decision on a specific drv type.

Signed-off-by: Asaf Penso <asafp@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c       | 87 +++++++++++++++++-------------
 drivers/net/mlx5/mlx5_flow.h       | 12 +++++
 drivers/net/mlx5/mlx5_flow_verbs.c |  4 ++
 3 files changed, 66 insertions(+), 37 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index e63a297e2a..894c16ba20 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3231,6 +3231,45 @@ flow_null_sync_domain(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+int
+flow_null_get_aged_flows(struct rte_eth_dev *dev,
+		    void **context __rte_unused,
+		    uint32_t nb_contexts __rte_unused,
+		    struct rte_flow_error *error __rte_unused)
+{
+	DRV_LOG(ERR, "port %u get aged flows is not supported.",
+		dev->data->port_id);
+	return -ENOTSUP;
+}
+
+uint32_t
+flow_null_counter_allocate(struct rte_eth_dev *dev)
+{
+	DRV_LOG(ERR, "port %u counter allocate is not supported.",
+		dev->data->port_id);
+	return 0;
+}
+
+void
+flow_null_counter_free(struct rte_eth_dev *dev,
+			uint32_t counter __rte_unused)
+{
+	DRV_LOG(ERR, "port %u counter free is not supported.",
+		 dev->data->port_id);
+}
+
+int
+flow_null_counter_query(struct rte_eth_dev *dev,
+			uint32_t counter __rte_unused,
+			bool clear __rte_unused,
+			uint64_t *pkts __rte_unused,
+			uint64_t *bytes __rte_unused)
+{
+	DRV_LOG(ERR, "port %u counter query is not supported.",
+		 dev->data->port_id);
+	return -ENOTSUP;
+}
+
 /* Void driver to protect from null pointer reference. */
 const struct mlx5_flow_driver_ops mlx5_flow_null_drv_ops = {
 	.validate = flow_null_validate,
@@ -3241,6 +3280,10 @@ const struct mlx5_flow_driver_ops mlx5_flow_null_drv_ops = {
 	.destroy = flow_null_destroy,
 	.query = flow_null_query,
 	.sync_domain = flow_null_sync_domain,
+	.get_aged_flows = flow_null_get_aged_flows,
+	.counter_alloc = flow_null_counter_allocate,
+	.counter_free = flow_null_counter_free,
+	.counter_query = flow_null_counter_query,
 };
 
 /**
@@ -7497,17 +7540,10 @@ mlx5_flow_mtr_free(struct rte_eth_dev *dev, uint32_t mtr_idx)
 uint32_t
 mlx5_counter_alloc(struct rte_eth_dev *dev)
 {
-	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
 
-	if (flow_get_drv_type(dev, &attr) == MLX5_FLOW_TYPE_DV) {
-		fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV);
-		return fops->counter_alloc(dev);
-	}
-	DRV_LOG(ERR,
-		"port %u counter allocate is not supported.",
-		 dev->data->port_id);
-	return 0;
+	return flow_get_drv_ops(flow_get_drv_type(dev, &attr))->counter_alloc
+		(dev);
 }
 
 /**
@@ -7521,17 +7557,9 @@ mlx5_counter_alloc(struct rte_eth_dev *dev)
 void
 mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt)
 {
-	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
 
-	if (flow_get_drv_type(dev, &attr) == MLX5_FLOW_TYPE_DV) {
-		fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV);
-		fops->counter_free(dev, cnt);
-		return;
-	}
-	DRV_LOG(ERR,
-		"port %u counter free is not supported.",
-		 dev->data->port_id);
+	flow_get_drv_ops(flow_get_drv_type(dev, &attr))->counter_free(dev, cnt);
 }
 
 /**
@@ -7555,17 +7583,10 @@ int
 mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		   bool clear, uint64_t *pkts, uint64_t *bytes)
 {
-	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
 
-	if (flow_get_drv_type(dev, &attr) == MLX5_FLOW_TYPE_DV) {
-		fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV);
-		return fops->counter_query(dev, cnt, clear, pkts, bytes);
-	}
-	DRV_LOG(ERR,
-		"port %u counter query is not supported.",
-		 dev->data->port_id);
-	return -ENOTSUP;
+	return flow_get_drv_ops(flow_get_drv_type(dev, &attr))->counter_query
+		(dev, cnt, clear, pkts, bytes);
 }
 
 /**
@@ -8279,18 +8300,10 @@ int
 mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error)
 {
-	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
 
-	if (flow_get_drv_type(dev, &attr) == MLX5_FLOW_TYPE_DV) {
-		fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV);
-		return fops->get_aged_flows(dev, contexts, nb_contexts,
-						    error);
-	}
-	DRV_LOG(ERR,
-		"port %u get aged flows is not supported.",
-		 dev->data->port_id);
-	return -ENOTSUP;
+	return flow_get_drv_ops(flow_get_drv_type(dev, &attr))->get_aged_flows
+		(dev, contexts, nb_contexts, error);
 }
 
 /* Wrapper for driver action_validate op callback */
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 76ad53f2a1..bb23a5efce 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1713,5 +1713,17 @@ mlx5_get_tof(const struct rte_flow_item *items,
 	     const struct rte_flow_action *actions,
 	     enum mlx5_tof_rule_type *rule_type);
 
+int flow_null_get_aged_flows(struct rte_eth_dev *dev,
+		    void **context,
+		    uint32_t nb_contexts,
+		    struct rte_flow_error *error);
+uint32_t flow_null_counter_allocate(struct rte_eth_dev *dev);
+void flow_null_counter_free(struct rte_eth_dev *dev,
+			uint32_t counter);
+int flow_null_counter_query(struct rte_eth_dev *dev,
+			uint32_t counter,
+			bool clear,
+		    uint64_t *pkts,
+			uint64_t *bytes);
 
 #endif /* RTE_PMD_MLX5_FLOW_H_ */
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index b93fd4d2c9..b90d683916 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -2105,4 +2105,8 @@ const struct mlx5_flow_driver_ops mlx5_flow_verbs_drv_ops = {
 	.destroy = flow_verbs_destroy,
 	.query = flow_verbs_query,
 	.sync_domain = flow_verbs_sync_domain,
+	.get_aged_flows = flow_null_get_aged_flows,
+	.counter_alloc = flow_null_counter_allocate,
+	.counter_free = flow_null_counter_free,
+	.counter_query = flow_null_counter_query,
 };
-- 
2.25.1

[PATCH v2] net/mlx5: add stub functions to null drv ops

[Asaf Penso]

There are several functions implementation that queries the drv type
to understand which fops function to use.
In case the type is DV, the function gets the concrete DV function and
calls it.
In case it’s not, the function returns an error.

The current implementation is not flexible enough and will not support
future concrete functions in a good way.

This patch adds more stub functions that include error handling and
are assigned to the null drv ops struct.
This allows the functions to always call the virtual function without
basing the decision on a specific drv type.

Signed-off-by: Asaf Penso <asafp@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c       | 88 +++++++++++++++++-------------
 drivers/net/mlx5/mlx5_flow.h       | 13 +++++
 drivers/net/mlx5/mlx5_flow_verbs.c |  4 ++
 3 files changed, 68 insertions(+), 37 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index a0cf677fb0..a0779d810d 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3786,6 +3786,46 @@ flow_null_sync_domain(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+int
+flow_null_get_aged_flows(struct rte_eth_dev *dev,
+		    void **context __rte_unused,
+		    uint32_t nb_contexts __rte_unused,
+		    struct rte_flow_error *error __rte_unused)
+{
+	DRV_LOG(ERR, "port %u get aged flows is not supported.",
+		dev->data->port_id);
+	return -ENOTSUP;
+}
+
+uint32_t
+flow_null_counter_allocate(struct rte_eth_dev *dev)
+{
+	DRV_LOG(ERR, "port %u counter allocate is not supported.",
+		dev->data->port_id);
+	return 0;
+}
+
+void
+flow_null_counter_free(struct rte_eth_dev *dev,
+			uint32_t counter __rte_unused)
+{
+	DRV_LOG(ERR, "port %u counter free is not supported.",
+		 dev->data->port_id);
+}
+
+int
+flow_null_counter_query(struct rte_eth_dev *dev,
+			uint32_t counter __rte_unused,
+			bool clear __rte_unused,
+			uint64_t *pkts __rte_unused,
+			uint64_t *bytes __rte_unused,
+			void **action __rte_unused)
+{
+	DRV_LOG(ERR, "port %u counter query is not supported.",
+		 dev->data->port_id);
+	return -ENOTSUP;
+}
+
 /* Void driver to protect from null pointer reference. */
 const struct mlx5_flow_driver_ops mlx5_flow_null_drv_ops = {
 	.validate = flow_null_validate,
@@ -3796,6 +3836,10 @@ const struct mlx5_flow_driver_ops mlx5_flow_null_drv_ops = {
 	.destroy = flow_null_destroy,
 	.query = flow_null_query,
 	.sync_domain = flow_null_sync_domain,
+	.get_aged_flows = flow_null_get_aged_flows,
+	.counter_alloc = flow_null_counter_allocate,
+	.counter_free = flow_null_counter_free,
+	.counter_query = flow_null_counter_query
 };
 
 /**
@@ -8352,17 +8396,10 @@ mlx5_flow_mtr_free(struct rte_eth_dev *dev, uint32_t mtr_idx)
 uint32_t
 mlx5_counter_alloc(struct rte_eth_dev *dev)
 {
-	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
 
-	if (flow_get_drv_type(dev, &attr) == MLX5_FLOW_TYPE_DV) {
-		fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV);
-		return fops->counter_alloc(dev);
-	}
-	DRV_LOG(ERR,
-		"port %u counter allocate is not supported.",
-		 dev->data->port_id);
-	return 0;
+	return flow_get_drv_ops(flow_get_drv_type(dev, &attr))->counter_alloc
+		(dev);
 }
 
 /**
@@ -8376,17 +8413,9 @@ mlx5_counter_alloc(struct rte_eth_dev *dev)
 void
 mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt)
 {
-	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
 
-	if (flow_get_drv_type(dev, &attr) == MLX5_FLOW_TYPE_DV) {
-		fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV);
-		fops->counter_free(dev, cnt);
-		return;
-	}
-	DRV_LOG(ERR,
-		"port %u counter free is not supported.",
-		 dev->data->port_id);
+	flow_get_drv_ops(flow_get_drv_type(dev, &attr))->counter_free(dev, cnt);
 }
 
 /**
@@ -8410,18 +8439,10 @@ int
 mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		   bool clear, uint64_t *pkts, uint64_t *bytes, void **action)
 {
-	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
 
-	if (flow_get_drv_type(dev, &attr) == MLX5_FLOW_TYPE_DV) {
-		fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV);
-		return fops->counter_query(dev, cnt, clear, pkts,
-					bytes, action);
-	}
-	DRV_LOG(ERR,
-		"port %u counter query is not supported.",
-		 dev->data->port_id);
-	return -ENOTSUP;
+	return flow_get_drv_ops(flow_get_drv_type(dev, &attr))->counter_query
+		(dev, cnt, clear, pkts, bytes, action);
 }
 
 /**
@@ -9889,17 +9910,10 @@ int
 mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error)
 {
-	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
-	enum mlx5_flow_drv_type type = flow_get_drv_type(dev, &attr);
 
-	if (type == MLX5_FLOW_TYPE_DV || type == MLX5_FLOW_TYPE_HW) {
-		fops = flow_get_drv_ops(type);
-		return fops->get_aged_flows(dev, contexts, nb_contexts, error);
-	}
-	DRV_LOG(ERR, "port %u get aged flows is not supported.",
-		dev->data->port_id);
-	return -ENOTSUP;
+	return flow_get_drv_ops(flow_get_drv_type(dev, &attr))->get_aged_flows
+		(dev, contexts, nb_contexts, error);
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 1f57ecd6e1..e376dcae93 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -2575,6 +2575,19 @@ int flow_dv_translate_items_hws(const struct rte_flow_item *items,
 int mlx5_flow_pick_transfer_proxy(struct rte_eth_dev *dev,
 				  uint16_t *proxy_port_id,
 				  struct rte_flow_error *error);
+int flow_null_get_aged_flows(struct rte_eth_dev *dev,
+		    void **context,
+		    uint32_t nb_contexts,
+		    struct rte_flow_error *error);
+uint32_t flow_null_counter_allocate(struct rte_eth_dev *dev);
+void flow_null_counter_free(struct rte_eth_dev *dev,
+			uint32_t counter);
+int flow_null_counter_query(struct rte_eth_dev *dev,
+			uint32_t counter,
+			bool clear,
+		    uint64_t *pkts,
+			uint64_t *bytes,
+			void **action);
 
 int mlx5_flow_hw_flush_ctrl_flows(struct rte_eth_dev *dev);
 
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 28ea28bfbe..c78479e61e 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -2132,4 +2132,8 @@ const struct mlx5_flow_driver_ops mlx5_flow_verbs_drv_ops = {
 	.query = flow_verbs_query,
 	.sync_domain = flow_verbs_sync_domain,
 	.discover_priorities = flow_verbs_discover_priorities,
+	.get_aged_flows = flow_null_get_aged_flows,
+	.counter_alloc = flow_null_counter_allocate,
+	.counter_free = flow_null_counter_free,
+	.counter_query = flow_null_counter_query,
 };
-- 
2.18.2

RE: [PATCH v2] net/mlx5: add stub functions to null drv ops

[Matan Azrad]

From: Asaf Penso <asafp@nvidia.com>
> There are several functions implementation that queries the drv type to
> understand which fops function to use.
> In case the type is DV, the function gets the concrete DV function and calls it.
> In case it’s not, the function returns an error.
> 
> The current implementation is not flexible enough and will not support future
> concrete functions in a good way.
> 
> This patch adds more stub functions that include error handling and are assigned
> to the null drv ops struct.
> This allows the functions to always call the virtual function without basing the
> decision on a specific drv type.
> 
> Signed-off-by: Asaf Penso <asafp@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>

RE: [PATCH v2] net/mlx5: add stub functions to null drv ops

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Asaf Penso <asafp@nvidia.com>
> Sent: Wednesday, December 28, 2022 8:37 PM
> To: dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>
> Subject: [PATCH v2] net/mlx5: add stub functions to null drv ops
> 
> There are several functions implementation that queries the drv type to
> understand which fops function to use.
> In case the type is DV, the function gets the concrete DV function and calls it.
> In case it’s not, the function returns an error.
> 
> The current implementation is not flexible enough and will not support
> future concrete functions in a good way.
> 
> This patch adds more stub functions that include error handling and are
> assigned to the null drv ops struct.
> This allows the functions to always call the virtual function without basing the
> decision on a specific drv type.
> 
> Signed-off-by: Asaf Penso <asafp@nvidia.com>

Patch applied to next-net-mlx,

Kindest regards
Raslan Darawsheh