[dpdk-dev] [PATCH] net/mlx5: enhancement for flow dump value

[dpdk-dev] [PATCH] net/mlx5: enhancement for flow dump value

[Haifei Luo]

Multiple rules could use the same encap_decap/modify_hdr/counter action.
The flow dump data could be duplicated.

To avoid redundancy, flow dump value is based on the actions' pointer
instead of previous rules' pointer.

For counter, the data is stored in cmng of priv->sh.
For encap_decap/modify_hdr, the data stored in encaps_decaps/modify_cmds.
Traverse the fields and get action's pointer and information.

Formats are same for information in the dump except "id" stands for
actions' pointer:
    Counter:     rec_type,id,hits,bytes
    Modify_hdr:  rec_type,id,actions_number,actions
    Encap_decap: rec_type,id,buf

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx5/mlx5.h         |   2 +-
 drivers/net/mlx5/mlx5_flow.c    | 161 ++++++++++++++++++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow.h    |   5 ++
 drivers/net/mlx5/mlx5_flow_dv.c |  44 ++++++++++-
 4 files changed, 188 insertions(+), 24 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 5da5cea..5fad077 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1685,7 +1685,7 @@ int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 int mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow,
 			FILE *file, struct rte_flow_error *error);
 int save_dump_file(const unsigned char *data, uint32_t size,
-		uint32_t type, uint32_t id, void *arg, FILE *file);
+		uint32_t type, uint64_t id, void *arg, FILE *file);
 int mlx5_flow_query_counter(struct rte_eth_dev *dev, struct rte_flow *flow,
 	struct rte_flow_query_count *count, struct rte_flow_error *error);
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 5d19ef1..e746e20 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -8173,7 +8173,7 @@ struct mlx5_flow_workspace*
 
 int
 save_dump_file(const uint8_t *data, uint32_t size,
-	uint32_t type, uint32_t id, void *arg, FILE *file)
+	uint32_t type, uint64_t id, void *arg, FILE *file)
 {
 	char line[BUF_SIZE];
 	uint32_t out = 0;
@@ -8185,17 +8185,18 @@ struct mlx5_flow_workspace*
 	switch (type) {
 	case DR_DUMP_REC_TYPE_PMD_MODIFY_HDR:
 		actions_num = *(uint32_t *)(arg);
-		out += snprintf(line + out, BUF_SIZE - out, "%d,0x%x,%d,",
+		out += snprintf(line + out, BUF_SIZE - out, "%d,0x%" PRIx64 ",%d,",
 				type, id, actions_num);
 		break;
 	case DR_DUMP_REC_TYPE_PMD_PKT_REFORMAT:
-		out += snprintf(line + out, BUF_SIZE - out, "%d,0x%x,",
+		out += snprintf(line + out, BUF_SIZE - out, "%d,0x%" PRIx64 ",",
 				type, id);
 		break;
 	case DR_DUMP_REC_TYPE_PMD_COUNTER:
 		count = (struct rte_flow_query_count *)arg;
-		fprintf(file, "%d,0x%x,%" PRIu64 ",%" PRIu64 "\n", type,
-				id, count->hits, count->bytes);
+		fprintf(file,
+			"%d,0x%" PRIx64 ",%" PRIu64 ",%" PRIu64 "\n",
+			type, id, count->hits, count->bytes);
 		return 0;
 	default:
 		return -1;
@@ -8269,30 +8270,34 @@ struct mlx5_flow_workspace*
 	uint32_t actions_num;
 	const uint8_t *data;
 	size_t size;
-	uint32_t id;
+	uint64_t id;
 	uint32_t type;
+	void *action = NULL;
 
 	if (!flow) {
 		return rte_flow_error_set(error, ENOENT,
-			RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-			NULL,
-			"invalid flow handle");
+				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				NULL,
+				"invalid flow handle");
 	}
 	handle_idx = flow->dev_handles;
 	while (handle_idx) {
 		dh = mlx5_ipool_get(priv->sh->ipool
-			[MLX5_IPOOL_MLX5_FLOW], handle_idx);
+				[MLX5_IPOOL_MLX5_FLOW], handle_idx);
 		if (!dh)
 			continue;
 		handle_idx = dh->next.next;
-		id = (uint32_t)(uintptr_t)dh->drv_flow;
 
 		/* query counter */
 		type = DR_DUMP_REC_TYPE_PMD_COUNTER;
-		if (!mlx5_flow_query_counter(dev, flow, &count, error))
-			save_dump_file(NULL, 0, type,
-					id, (void *)&count, file);
-
+		flow_dv_query_count_ptr(dev, flow->counter,
+						&action, error);
+		if (action) {
+			id = (uint64_t)(uintptr_t)action;
+			if (!mlx5_flow_query_counter(dev, flow, &count, error))
+				save_dump_file(NULL, 0, type,
+						id, (void *)&count, file);
+		}
 		/* Get modify_hdr and encap_decap buf from ipools. */
 		encap_decap = NULL;
 		modify_hdr = dh->dvh.modify_hdr;
@@ -8305,14 +8310,16 @@ struct mlx5_flow_workspace*
 		if (modify_hdr) {
 			data = (const uint8_t *)modify_hdr->actions;
 			size = (size_t)(modify_hdr->actions_num) * 8;
+			id = (uint64_t)(uintptr_t)modify_hdr->action;
 			actions_num = modify_hdr->actions_num;
 			type = DR_DUMP_REC_TYPE_PMD_MODIFY_HDR;
 			save_dump_file(data, size, type, id,
-					(void *)(&actions_num), file);
+						(void *)(&actions_num), file);
 		}
 		if (encap_decap) {
 			data = encap_decap->buf;
 			size = encap_decap->size;
+			id = (uint64_t)(uintptr_t)encap_decap->action;
 			type = DR_DUMP_REC_TYPE_PMD_PKT_REFORMAT;
 			save_dump_file(data, size, type,
 						id, NULL, file);
@@ -8320,6 +8327,119 @@ struct mlx5_flow_workspace*
 	}
 	return 0;
 }
+
+/**
+ * Dump all flow's encap_decap/modify_hdr/counter data to file
+ *
+ * @param[in] dev
+ *   The pointer to Ethernet device.
+ * @param[in] file
+ *   A pointer to a file for output.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL. PMDs initialize this
+ *   structure in case of error only.
+ * @return
+ *   0 on success, a negative value otherwise.
+ */
+static int
+mlx5_flow_dev_dump_sh_all(struct rte_eth_dev *dev,
+	FILE *file, struct rte_flow_error *error)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_dev_ctx_shared *sh = priv->sh;
+	struct mlx5_hlist *h;
+	struct mlx5_flow_dv_modify_hdr_resource  *modify_hdr;
+	struct mlx5_flow_dv_encap_decap_resource *encap_decap;
+	struct rte_flow_query_count count;
+	uint32_t actions_num;
+	const uint8_t *data;
+	size_t size;
+	uint64_t id;
+	uint32_t type;
+	uint32_t i;
+	uint32_t j;
+	struct mlx5_list_inconst *l_inconst;
+	struct mlx5_list_entry *e;
+	int lcore_index;
+	struct mlx5_flow_counter_mng *cmng = &priv->sh->cmng;
+	uint32_t max;
+	void *action;
+
+	/* encap_decap hlist is lcore_share, get global core cache. */
+	i = MLX5_LIST_GLOBAL;
+	h = sh->encaps_decaps;
+	if (h) {
+		for (j = 0; j <= h->mask; j++) {
+			l_inconst = &h->buckets[j].l;
+			if (!l_inconst || !l_inconst->cache[i])
+				continue;
+
+			e = LIST_FIRST(&l_inconst->cache[i]->h);
+			while (e) {
+				encap_decap =
+				(struct mlx5_flow_dv_encap_decap_resource *)e;
+				data = encap_decap->buf;
+				size = encap_decap->size;
+				id = (uint64_t)(uintptr_t)encap_decap->action;
+				type = DR_DUMP_REC_TYPE_PMD_PKT_REFORMAT;
+				save_dump_file(data, size, type,
+					id, NULL, file);
+				e = LIST_NEXT(e, next);
+			}
+		}
+	}
+
+	/* get modify_hdr */
+	h = sh->modify_cmds;
+	if (h) {
+		lcore_index = rte_lcore_index(rte_lcore_id());
+		if (unlikely(lcore_index == -1)) {
+			lcore_index = MLX5_LIST_NLCORE;
+			rte_spinlock_lock(&h->l_const.lcore_lock);
+		}
+		i = lcore_index;
+
+		for (j = 0; j <= h->mask; j++) {
+			l_inconst = &h->buckets[j].l;
+			if (!l_inconst || !l_inconst->cache[i])
+				continue;
+
+			e = LIST_FIRST(&l_inconst->cache[i]->h);
+			while (e) {
+				modify_hdr =
+				(struct mlx5_flow_dv_modify_hdr_resource *)e;
+				data = (const uint8_t *)modify_hdr->actions;
+				size = (size_t)(modify_hdr->actions_num) * 8;
+				actions_num = modify_hdr->actions_num;
+				id = (uint64_t)(uintptr_t)modify_hdr->action;
+				type = DR_DUMP_REC_TYPE_PMD_MODIFY_HDR;
+				save_dump_file(data, size, type, id,
+						(void *)(&actions_num), file);
+				e = LIST_NEXT(e, next);
+			}
+		}
+
+		if (unlikely(lcore_index == MLX5_LIST_NLCORE))
+			rte_spinlock_unlock(&h->l_const.lcore_lock);
+	}
+
+	/* get counter */
+	MLX5_ASSERT(cmng->n_valid <= cmng->n);
+	max = MLX5_COUNTERS_PER_POOL * cmng->n_valid;
+	for (j = 1; j <= max; j++) {
+		action = NULL;
+		flow_dv_query_count_ptr(dev, j, &action, error);
+		if (action) {
+			if (!flow_dv_query_count(dev, j, &count, error)) {
+				type = DR_DUMP_REC_TYPE_PMD_COUNTER;
+				id = (uint64_t)(uintptr_t)action;
+				save_dump_file(NULL, 0, type,
+						id, (void *)&count, file);
+			}
+		}
+	}
+	return 0;
+}
 #endif
 
 /**
@@ -8346,9 +8466,6 @@ struct mlx5_flow_workspace*
 	int ret;
 	struct mlx5_flow_handle *dh;
 	struct rte_flow *flow;
-#ifdef HAVE_IBV_FLOW_DV_SUPPORT
-	uint32_t idx;
-#endif
 
 	if (!priv->config.dv_flow_en) {
 		if (fputs("device dv flow disabled\n", file) <= 0)
@@ -8359,8 +8476,8 @@ struct mlx5_flow_workspace*
 	/* dump all */
 	if (!flow_idx) {
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
-		MLX5_IPOOL_FOREACH(priv->flows[MLX5_FLOW_TYPE_GEN], idx, flow)
-			mlx5_flow_dev_dump_ipool(dev, flow, file, error);
+		if (mlx5_flow_dev_dump_sh_all(dev, file, error))
+			return -EINVAL;
 #endif
 		return mlx5_devx_cmd_flow_dump(sh->fdb_domain,
 					sh->rx_domain,
@@ -8370,7 +8487,7 @@ struct mlx5_flow_workspace*
 	flow = mlx5_ipool_get(priv->flows[MLX5_FLOW_TYPE_GEN],
 			(uintptr_t)(void *)flow_idx);
 	if (!flow)
-		return -ENOENT;
+		return -EINVAL;
 
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
 	mlx5_flow_dev_dump_ipool(dev, flow, file, error);
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 4a16f30..13a9043 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1674,6 +1674,11 @@ struct mlx5_list_entry *flow_dv_dest_array_clone_cb(void *tool_ctx,
 				   struct mlx5_list_entry *entry, void *cb_ctx);
 void flow_dv_dest_array_clone_free_cb(void *tool_ctx,
 				      struct mlx5_list_entry *entry);
+int flow_dv_query_count_ptr(struct rte_eth_dev *dev, uint32_t cnt_idx,
+				void **action, struct rte_flow_error *error);
+int
+flow_dv_query_count(struct rte_eth_dev *dev, uint32_t cnt_idx, void *data,
+		    struct rte_flow_error *error);
 
 struct mlx5_aso_age_action *flow_aso_age_get_by_idx(struct rte_eth_dev *dev,
 						    uint32_t age_idx);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 9cba22c..052a240 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -15648,7 +15648,7 @@ struct mlx5_list_entry *
  * @return
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
-static int
+int
 flow_dv_query_count(struct rte_eth_dev *dev, uint32_t cnt_idx, void *data,
 		    struct rte_flow_error *error)
 {
@@ -15686,6 +15686,48 @@ struct mlx5_list_entry *
 				  "counters are not available");
 }
 
+
+/**
+ * Query counter's action pointer for a DV flow rule via DevX.
+ *
+ * @param[in] dev
+ *   Pointer to Ethernet device.
+ * @param[in] cnt_idx
+ *   Index to the flow counter.
+ * @param[out] action_ptr
+ *   Action pointer for counter.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+int
+flow_dv_query_count_ptr(struct rte_eth_dev *dev, uint32_t cnt_idx,
+	void **action_ptr, struct rte_flow_error *error)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+
+	if (!priv->sh->devx || !action_ptr)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL,
+					  "counters are not supported");
+
+	if (cnt_idx) {
+		struct mlx5_flow_counter *cnt = NULL;
+		cnt = flow_dv_counter_get_by_idx(dev, cnt_idx, NULL);
+		if (cnt) {
+			*action_ptr = cnt->action;
+			return 0;
+		}
+	}
+	return rte_flow_error_set(error, EINVAL,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL,
+				  "counters are not available");
+}
+
 static int
 flow_dv_action_query(struct rte_eth_dev *dev,
 		     const struct rte_flow_action_handle *handle, void *data,
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] net/mlx5: enhancement for flow dump value

[Raslan Darawsheh]

Hi,
> -----Original Message-----
> From: Haifei Luo <haifeil@nvidia.com>
> Sent: Tuesday, October 26, 2021 6:57 AM
> To: Matan Azrad <matan@nvidia.com>; Ori Kam <orika@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>; ferruh.yigit@intel.com
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Raslan Darawsheh <rasland@nvidia.com>
> Subject: [PATCH] net/mlx5: enhancement for flow dump value
> 
> Multiple rules could use the same encap_decap/modify_hdr/counter action.
> The flow dump data could be duplicated.
> 
> To avoid redundancy, flow dump value is based on the actions' pointer
> instead of previous rules' pointer.
> 
> For counter, the data is stored in cmng of priv->sh.
> For encap_decap/modify_hdr, the data stored in
> encaps_decaps/modify_cmds.
> Traverse the fields and get action's pointer and information.
> 
> Formats are same for information in the dump except "id" stands for
> actions' pointer:
>     Counter:     rec_type,id,hits,bytes
>     Modify_hdr:  rec_type,id,actions_number,actions
>     Encap_decap: rec_type,id,buf
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh