[dpdk-dev] [PATCH 0/4] *** Support for one flow dump ***

[dpdk-dev] [PATCH 0/4] *** Support for one flow dump ***

[Haifei Luo]

Dump all the flows is supported.Some customers require to dump info 
for one flow. To implement this requirement, 
add the CLI to dump one rule: flow dump PORT rule ID
         the CLI to dump all: flow dump PORT all
Examples:
    testpmd> flow dump 0 all
    testpmd> flow dump 0 rule 0
Also new APIs(rte_flow_dump,mlx5_flow_dump_rule, and etc) are added 
in rte and drivers to support it.

Haifei Luo (4):
  ethdev: add rte API for single flow dump
  app/testpmd: add CLIs for single flow dump feature
  common/mlx5: add mlx5 APIs for single flow dump feature
  net/mlx5: add mlx5 APIs for single flow dump feature

 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/nics/mlx5.rst                    | 10 ++++--
 doc/guides/prog_guide/rte_flow.rst          | 44 +++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 drivers/common/mlx5/linux/meson.build       |  4 +++
 drivers/common/mlx5/linux/mlx5_glue.c       | 13 +++++++
 drivers/common/mlx5/linux/mlx5_glue.h       |  1 +
 drivers/common/mlx5/mlx5_devx_cmds.c        | 10 ++++++
 drivers/common/mlx5/mlx5_devx_cmds.h        |  2 ++
 drivers/common/mlx5/version.map             |  1 +
 drivers/net/mlx5/linux/mlx5_socket.c        | 27 ++++++++++----
 drivers/net/mlx5/mlx5.h                     |  3 ++
 drivers/net/mlx5/mlx5_flow.c                | 38 ++++++++++++++++++++
 lib/librte_ethdev/rte_flow.c                | 21 +++++++++++
 lib/librte_ethdev/rte_flow.h                | 24 +++++++++++++
 lib/librte_ethdev/rte_flow_driver.h         |  6 ++++
 lib/librte_ethdev/version.map               |  1 +
 19 files changed, 286 insertions(+), 21 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH 1/4] ethdev: add rte API for single flow dump

[Haifei Luo]

Previous implementations support dump all the flows.Add new ones
to dump one flow.
New API added: rte_flow_dump.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 doc/guides/nics/mlx5.rst            | 10 +++++++--
 doc/guides/prog_guide/rte_flow.rst  | 44 +++++++++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_flow.c        | 21 ++++++++++++++++++
 lib/librte_ethdev/rte_flow.h        | 24 ++++++++++++++++++++
 lib/librte_ethdev/rte_flow_driver.h |  6 +++++
 lib/librte_ethdev/version.map       |  1 +
 6 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 7c50497..b8b6b02 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1778,13 +1778,19 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       testpmd> flow dump <port> <output_file>
+       To dump all flows:
+       testpmd> flow dump <port> all <output_file>
+       and dump one flow:
+       testpmd> flow dump <port> rule <rule_id> <output_file>
 
    - call rte_flow_dev_dump api:
 
    .. code-block:: console
 
+       To dump all flows:
        rte_flow_dev_dump(port, file, NULL);
+       and dump one flow:
+       rte_flow_dump(port, flow, file, NULL);
 
 #. Dump human-readable flows from raw file:
 
@@ -1792,4 +1798,4 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       mlx_steering_dump.py -f <output_file>
+       mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 62a5791..17e4351 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3023,6 +3023,50 @@ Return values:
 
 - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
 
+Dump
+~~~~~
+
+Dump information for all or one flows.
+
+This Function rte_flow_dev_dump will dump the information for all the flows.
+
+.. code-block:: c
+
+   int
+   rte_flow_dev_dump(uint16_t port_id, FILE *file,
+                     struct rte_flow_error *error);
+
+Arguments:
+
+- ``port_id``: port identifier of Ethernet device.
+- ``file``: a pointer to a file for output
+- ``error``: perform verbose error reporting if not NULL. PMDs initialize
+  this structure in case of error only.
+
+Return values:
+
+- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
+
+This Function rte_flow_dump will dump the information for one flow.
+
+.. code-block:: c
+
+   int
+   rte_flow_dump(uint16_t port_id, struct rte_flow *flow, FILE *file,
+                 struct rte_flow_error *error);
+
+Arguments:
+
+- ``port_id``: port identifier of Ethernet device.
+- ``file``: a pointer to a file for output
+- ``flow``: flow rule handle to dump.
+- ``error``: perform verbose error reporting if not NULL. PMDs initialize
+  this structure in case of error only.
+
+Return values:
+
+- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
+
 Query
 ~~~~~
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 241af6c..ff051e7 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -1044,6 +1044,27 @@ enum rte_flow_conv_item_spec_type {
 }
 
 int
+rte_flow_dump(uint16_t port_id, struct rte_flow *flow,
+			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_single_dump)) {
+		fts_enter(dev);
+		ret = ops->dev_single_dump(dev, flow, 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));
+}
+
+int
 rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 		    uint32_t nb_contexts, struct rte_flow_error *error)
 {
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 68c68cd..aac9e6c 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -3214,6 +3214,30 @@ enum rte_flow_conv_op {
 rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Dump hardware internal representation information of
+ * one rte flow to file.
+ *
+ * @param[in] port_id
+ *   The port identifier of the Ethernet device.
+ * @param[in] flow
+ *   The pointer of rte flow.
+ * @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 nagative value otherwise.
+ */
+__rte_experimental
+int
+rte_flow_dump(uint16_t port_id, struct rte_flow *flow,
+			FILE *file, struct rte_flow_error *error);
+
+/**
  * Check if mbuf dynamic field for metadata is registered.
  *
  * @return
diff --git a/lib/librte_ethdev/rte_flow_driver.h b/lib/librte_ethdev/rte_flow_driver.h
index dabd819..8aa4510 100644
--- a/lib/librte_ethdev/rte_flow_driver.h
+++ b/lib/librte_ethdev/rte_flow_driver.h
@@ -102,6 +102,12 @@ struct rte_flow_ops {
 		(struct rte_eth_dev *dev,
 		 FILE *file,
 		 struct rte_flow_error *error);
+	/** See rte_flow_dump(). */
+	int (*dev_single_dump)
+		(struct rte_eth_dev *dev,
+		 struct rte_flow *flow,
+		 FILE *file,
+		 struct rte_flow_error *error);
 	/** See rte_flow_get_aged_flows() */
 	int (*get_aged_flows)
 		(struct rte_eth_dev *dev,
diff --git a/lib/librte_ethdev/version.map b/lib/librte_ethdev/version.map
index a124e1e..234798e 100644
--- a/lib/librte_ethdev/version.map
+++ b/lib/librte_ethdev/version.map
@@ -231,6 +231,7 @@ EXPERIMENTAL {
 	rte_eth_fec_get_capability;
 	rte_eth_fec_get;
 	rte_eth_fec_set;
+	rte_flow_dump;
 	rte_flow_shared_action_create;
 	rte_flow_shared_action_destroy;
 	rte_flow_shared_action_query;
-- 
1.8.3.1

[dpdk-dev] [PATCH 2/4] app/testpmd: add CLIs for single flow dump feature

[Haifei Luo]

Add support for single flow dump.
The CLIs to dump one rule: flow dump PORT rule ID
to dump all: flow dump PORT all
Examples:
testpmd> flow dump 0 all
testpmd> flow dump 0 rule 0

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 4 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 49d9f9c..7f53b83 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -108,6 +108,10 @@ enum index {
 	TUNNEL_SET,
 	TUNNEL_MATCH,
 
+	/* Dump arguments */
+	DUMP_ALL,
+	DUMP_ONE,
+
 	/* Shared action arguments */
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -791,6 +795,8 @@ struct buffer {
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
+			bool mode;
+			uint32_t rule;
 		} dump; /**< Dump arguments. */
 		struct {
 			uint32_t rule;
@@ -842,6 +848,12 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index next_dump_subcmd[] = {
+	DUMP_ALL,
+	DUMP_ONE,
+	ZERO,
+};
+
 static const enum index next_sa_subcmd[] = {
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -2028,10 +2040,9 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	},
 	[DUMP] = {
 		.name = "dump",
-		.help = "dump all flow rules to file",
-		.next = NEXT(next_dump_attr, NEXT_ENTRY(PORT_ID)),
-		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
-			     ARGS_ENTRY(struct buffer, port)),
+		.help = "dump single/all flow rules to file",
+		.next = NEXT(next_dump_subcmd, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_dump,
 	},
 	[QUERY] = {
@@ -2121,6 +2132,22 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.destroy.rule)),
 		.call = parse_destroy,
 	},
+	/* Dump arguments. */
+	[DUMP_ALL] = {
+		.name = "all",
+		.help = "dump all",
+		.next = NEXT(next_dump_attr),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file)),
+		.call = parse_dump,
+	},
+	[DUMP_ONE] = {
+		.name = "rule",
+		.help = "dump one rule",
+		.next = NEXT(next_dump_attr, NEXT_ENTRY(RULE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
+				ARGS_ENTRY(struct buffer, args.dump.rule)),
+		.call = parse_dump,
+	},
 	/* Query arguments. */
 	[QUERY_ACTION] = {
 		.name = "{action}",
@@ -6342,8 +6369,20 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		ctx->objdata = 0;
 		ctx->object = out;
 		ctx->objmask = NULL;
+		return len;
+	}
+	switch (ctx->curr) {
+	case DUMP_ALL:
+	case DUMP_ONE:
+		out->args.dump.mode = (ctx->curr == DUMP_ALL) ? true : false;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+		return len;
+	default:
+		return -1;
 	}
-	return len;
 }
 
 /** Parse tokens for query command. */
@@ -7637,8 +7676,10 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
-	case DUMP:
-		port_flow_dump(in->port, in->args.dump.file);
+	case DUMP_ONE:
+	case DUMP_ALL:
+		port_flow_dump(in->port, in->args.dump.mode,
+				in->args.dump.rule, in->args.dump.file);
 		break;
 	case QUERY:
 		port_flow_query(in->port, in->args.query.rule,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 576d5ac..c6e54b6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1915,13 +1915,41 @@ struct rte_flow_shared_action *
 	return ret;
 }
 
-/** Dump all flow rules. */
+/** Dump flow rules. */
 int
-port_flow_dump(portid_t port_id, const char *file_name)
+port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
+		const char *file_name)
 {
 	int ret = 0;
 	FILE *file = stdout;
 	struct rte_flow_error error;
+	struct rte_port *port;
+	struct port_flow *pflow;
+	struct rte_flow *tmpFlow = NULL;
+	bool found = false;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+
+	if (!dump_all) {
+		port = &ports[port_id];
+		pflow = port->flow_list;
+		while (pflow) {
+			if (rule_id != pflow->id) {
+				pflow = pflow->next;
+			} else {
+				tmpFlow = pflow->flow;
+				if (tmpFlow)
+					found = true;
+				break;
+			}
+		}
+		if (found == false) {
+			printf("Failed to dump to flow %d\n", rule_id);
+			return -EINVAL;
+		}
+	}
 
 	if (file_name && strlen(file_name)) {
 		file = fopen(file_name, "w");
@@ -1931,7 +1959,11 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, file, &error);
+	if (!dump_all)
+		ret = rte_flow_dump(port_id, tmpFlow, file, &error);
+	else
+		ret = rte_flow_dev_dump(port_id, file, &error);
+
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index ce83f31..3dcbac1 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -824,7 +824,8 @@ void update_age_action_context(const struct rte_flow_action *actions,
 		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
-int port_flow_dump(portid_t port_id, const char *file_name);
+int port_flow_dump(portid_t port_id, bool dump_all,
+			uint32_t rule, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_t rule,
 		    const struct rte_flow_action *action);
 void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f59eb8a..278e1ea 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3314,7 +3314,11 @@ following sections.
 
 - Dump internal representation information of all flows in hardware::
 
-   flow dump {port_id} {output_file}
+   flow dump {port_id} all {output_file}
+
+   for one flow :
+
+   flow dump {port_id} rule {rule_id} {output_file}
 
 - List and destroy aged flow rules::
 
-- 
1.8.3.1

[dpdk-dev] [PATCH 3/4] common/mlx5: add mlx5 APIs for single flow dump feature

[Haifei Luo]

add mlx5 APIs for single flow dump feature

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 drivers/common/mlx5/linux/meson.build |  4 ++++
 drivers/common/mlx5/linux/mlx5_glue.c | 13 +++++++++++++
 drivers/common/mlx5/linux/mlx5_glue.h |  1 +
 drivers/common/mlx5/mlx5_devx_cmds.c  | 10 ++++++++++
 drivers/common/mlx5/mlx5_devx_cmds.h  |  2 ++
 drivers/common/mlx5/version.map       |  1 +
 6 files changed, 31 insertions(+)

diff --git a/drivers/common/mlx5/linux/meson.build b/drivers/common/mlx5/linux/meson.build
index 220de35..e8fcbd9 100644
--- a/drivers/common/mlx5/linux/meson.build
+++ b/drivers/common/mlx5/linux/meson.build
@@ -186,6 +186,10 @@ has_sym_args = [
 	'mlx5dv_dr_action_create_aso' ],
 	[ 'HAVE_INFINIBAND_VERBS_H', 'infiniband/verbs.h',
 	'INFINIBAND_VERBS_H' ],
+	[ 'HAVE_MLX5_UMR_IMKEY', 'infiniband/mlx5dv.h',
+	'MLX5_WQE_UMR_CTRL_FLAG_INLINE' ],
+	[ 'HAVE_MLX5_DR_FLOW_DUMP_RULE', 'infiniband/mlx5dv.h',
+	'mlx5dv_dump_dr_rule' ],
 ]
 config = configuration_data()
 foreach arg:has_sym_args
diff --git a/drivers/common/mlx5/linux/mlx5_glue.c b/drivers/common/mlx5/linux/mlx5_glue.c
index 964f7e7..d3bd645 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.c
+++ b/drivers/common/mlx5/linux/mlx5_glue.c
@@ -1101,6 +1101,18 @@
 }
 
 static int
+mlx5_glue_dr_dump_single_rule(FILE *file, void *rule)
+{
+#ifdef HAVE_MLX5_DR_FLOW_DUMP_RULE
+	return mlx5dv_dump_dr_rule(file, rule);
+#else
+	RTE_SET_USED(file);
+	RTE_SET_USED(rule);
+	return -ENOTSUP;
+#endif
+}
+
+static int
 mlx5_glue_dr_dump_domain(FILE *file, void *domain)
 {
 #ifdef HAVE_MLX5_DR_FLOW_DUMP
@@ -1423,6 +1435,7 @@
 	.devx_wq_query = mlx5_glue_devx_wq_query,
 	.devx_port_query = mlx5_glue_devx_port_query,
 	.dr_dump_domain = mlx5_glue_dr_dump_domain,
+	.dr_dump_rule = mlx5_glue_dr_dump_single_rule,
 	.dr_reclaim_domain_memory = mlx5_glue_dr_reclaim_domain_memory,
 	.dr_create_flow_action_sampler =
 		mlx5_glue_dr_create_flow_action_sampler,
diff --git a/drivers/common/mlx5/linux/mlx5_glue.h b/drivers/common/mlx5/linux/mlx5_glue.h
index 9e385be..97462e9 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.h
+++ b/drivers/common/mlx5/linux/mlx5_glue.h
@@ -313,6 +313,7 @@ struct mlx5_glue {
 			       uint32_t port_num,
 			       struct mlx5dv_devx_port *mlx5_devx_port);
 	int (*dr_dump_domain)(FILE *file, void *domain);
+	int (*dr_dump_rule)(FILE *file, void *rule);
 	int (*devx_query_eqn)(struct ibv_context *context, uint32_t cpus,
 			      uint32_t *eqn);
 	struct mlx5dv_devx_event_channel *(*devx_create_event_channel)
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c b/drivers/common/mlx5/mlx5_devx_cmds.c
index 0060c37..7ddd6a7 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.c
+++ b/drivers/common/mlx5/mlx5_devx_cmds.c
@@ -1551,6 +1551,16 @@ struct mlx5_devx_obj *
 	return -ret;
 }
 
+int
+mlx5_devx_cmd_flow_single_dump(void *rule_info __rte_unused,
+			FILE *file __rte_unused)
+{
+	int ret = 0;
+	if (rule_info)
+		ret = mlx5_glue->dr_dump_rule(file, rule_info);
+	return -ret;
+}
+
 /*
  * Create CQ using DevX API.
  *
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h b/drivers/common/mlx5/mlx5_devx_cmds.h
index bc66d28..1ea4d27 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.h
+++ b/drivers/common/mlx5/mlx5_devx_cmds.h
@@ -467,6 +467,8 @@ struct mlx5_devx_obj *mlx5_devx_cmd_create_tis(void *ctx,
 int mlx5_devx_cmd_flow_dump(void *fdb_domain, void *rx_domain, void *tx_domain,
 			    FILE *file);
 __rte_internal
+int mlx5_devx_cmd_flow_single_dump(void *rule, FILE *file);
+__rte_internal
 struct mlx5_devx_obj *mlx5_devx_cmd_create_cq(void *ctx,
 					      struct mlx5_devx_cq_attr *attr);
 __rte_internal
diff --git a/drivers/common/mlx5/version.map b/drivers/common/mlx5/version.map
index 91f3fa5..4d49011 100644
--- a/drivers/common/mlx5/version.map
+++ b/drivers/common/mlx5/version.map
@@ -28,6 +28,7 @@ INTERNAL {
 	mlx5_devx_cmd_flow_counter_alloc;
 	mlx5_devx_cmd_flow_counter_query;
 	mlx5_devx_cmd_flow_dump;
+	mlx5_devx_cmd_flow_single_dump;
 	mlx5_devx_cmd_mkey_create;
 	mlx5_devx_cmd_modify_qp_state;
 	mlx5_devx_cmd_modify_rq;
-- 
1.8.3.1

[dpdk-dev] [PATCH 4/4] net/mlx5: add mlx5 APIs for single flow dump feature

[Haifei Luo]

Add API mlx5_flow_dump_rule to support the feature.
Modify mlx5_socket since one extra arg flow_ptr is added.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_socket.c | 27 ++++++++++++++++++-------
 drivers/net/mlx5/mlx5.h              |  3 +++
 drivers/net/mlx5/mlx5_flow.c         | 38 ++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index 1938453..88ca8e1 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -32,12 +32,15 @@
 mlx5_pmd_socket_handle(void *cb __rte_unused)
 {
 	int conn_sock;
-	int ret;
+	int ret, j;
 	struct cmsghdr *cmsg = NULL;
-	int data;
+	#define LENGTH 9
+	/*The first byte for port_id and the rest for flowptr.*/
+	int data[LENGTH];
+	uint64_t flow_ptr = 0;
 	char buf[CMSG_SPACE(sizeof(int))] = { 0 };
 	struct iovec io = {
-		.iov_base = &data,
+		.iov_base = &data[0],
 		.iov_len = sizeof(data),
 	};
 	struct msghdr msg = {
@@ -50,7 +53,9 @@
 	int fd;
 	FILE *file = NULL;
 	struct rte_eth_dev *dev;
+	struct rte_flow_error err;
 
+	memset(data, 0, sizeof(data));
 	/* Accept the connection from the client. */
 	conn_sock = accept(server_socket, NULL, NULL);
 	if (conn_sock < 0) {
@@ -88,15 +93,23 @@
 	}
 	/* Dump flow. */
 	dev = &rte_eth_devices[port_id];
-	ret = mlx5_flow_dev_dump(dev, file, NULL);
+	/*The first byte in data for port_id and the following 8 for flowptr*/
+	for (j = 1; j < LENGTH; j++)
+		flow_ptr = (flow_ptr << 8) + data[j];
+	if (flow_ptr == 0)
+		ret = mlx5_flow_dev_dump(dev, file, NULL);
+	else
+		ret = mlx5_flow_dump_rule(dev,
+			(struct rte_flow *)((uintptr_t)flow_ptr), file, &err);
+
 	/* Set-up the ancillary data and reply. */
 	msg.msg_controllen = 0;
 	msg.msg_control = NULL;
 	msg.msg_iovlen = 1;
 	msg.msg_iov = &io;
-	data = -ret;
-	io.iov_len = sizeof(data);
-	io.iov_base = &data;
+	data[0] = -ret;
+	io.iov_len = sizeof(data[0]);
+	io.iov_base = &data[0];
 	do {
 		ret = sendmsg(conn_sock, &msg, 0);
 	} while (ret < 0 && errno == EINTR);
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index a281fd2..c5d4cee 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1222,6 +1222,9 @@ int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		       bool clear, uint64_t *pkts, uint64_t *bytes);
 int mlx5_flow_dev_dump(struct rte_eth_dev *dev, FILE *file,
 		       struct rte_flow_error *error);
+int mlx5_flow_dump_rule(struct rte_eth_dev *dev, struct rte_flow *flow,
+			FILE *file,
+			struct rte_flow_error *error);
 void mlx5_flow_rxq_dynf_metadata_set(struct rte_eth_dev *dev);
 int mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index ab5be3d..6d031ba 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -635,6 +635,7 @@ enum mlx5_expansion {
 	.isolate = mlx5_flow_isolate,
 	.query = mlx5_flow_query,
 	.dev_dump = mlx5_flow_dev_dump,
+	.dev_single_dump = mlx5_flow_dump_rule,
 	.get_aged_flows = mlx5_flow_get_aged_flows,
 	.shared_action_create = mlx5_shared_action_create,
 	.shared_action_destroy = mlx5_shared_action_destroy,
@@ -7200,6 +7201,43 @@ struct mlx5_meter_domains_infos *
 				       sh->tx_domain, file);
 }
 
+int
+mlx5_flow_dump_rule(struct rte_eth_dev *dev, struct rte_flow *flow_idx,
+			FILE *file,
+			struct rte_flow_error *error __rte_unused)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	uint32_t handle_idx;
+	int ret;
+	struct mlx5_flow_handle *dh;
+	struct rte_flow *flow = mlx5_ipool_get(priv->sh->ipool
+			[MLX5_IPOOL_RTE_FLOW], (uintptr_t)(void *)flow_idx);
+
+	if (!priv->config.dv_flow_en) {
+		if (fputs("device dv flow disabled\n", file) <= 0)
+			return -errno;
+		return -ENOTSUP;
+	}
+
+	if (!flow)
+		return -ENOENT;
+	handle_idx = flow->dev_handles;
+	while (handle_idx) {
+		dh = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MLX5_FLOW],
+				handle_idx);
+		if (!dh)
+			return -ENOENT;
+		if (dh->drv_flow) {
+			ret = mlx5_devx_cmd_flow_single_dump(dh->drv_flow,
+					file);
+			if (ret)
+				return -ENOENT;
+		}
+		handle_idx = dh->next.next;
+	}
+	return 0;
+}
+
 /**
  * Get aged-out flows.
  *
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 1/4] ethdev: add rte API for single flow dump

[Stephen Hemminger]

On Tue,  9 Mar 2021 10:15:13 +0200
Haifei Luo <haifeil@nvidia.com> wrote:

> +__rte_experimental
> +int
> +rte_flow_dump(uint16_t port_id, struct rte_flow *flow,
> +			FILE *file, struct rte_flow_error *error);
> +

The flow argument should be const since dumping does not change
its state.

Re: [dpdk-dev] [PATCH 1/4] ethdev: add rte API for single flow dump

[Ori Kam]

Hi Haifei,

PSB

> -----Original Message-----
> From: Haifei Luo <haifeil@nvidia.com>
> Subject: [PATCH 1/4] ethdev: add rte API for single flow dump
> 
> Previous implementations support dump all the flows.Add new ones
> to dump one flow.
> New API added: rte_flow_dump.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>  doc/guides/nics/mlx5.rst            | 10 +++++++--
>  doc/guides/prog_guide/rte_flow.rst  | 44
> +++++++++++++++++++++++++++++++++++++
>  lib/librte_ethdev/rte_flow.c        | 21 ++++++++++++++++++
>  lib/librte_ethdev/rte_flow.h        | 24 ++++++++++++++++++++
>  lib/librte_ethdev/rte_flow_driver.h |  6 +++++
>  lib/librte_ethdev/version.map       |  1 +
>  6 files changed, 104 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
> index 7c50497..b8b6b02 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -1778,13 +1778,19 @@ all flows with assistance of external tools.
> 
>     .. code-block:: console
> 
> -       testpmd> flow dump <port> <output_file>
> +       To dump all flows:
> +       testpmd> flow dump <port> all <output_file>
> +       and dump one flow:
> +       testpmd> flow dump <port> rule <rule_id> <output_file>
> 
>     - call rte_flow_dev_dump api:
> 
>     .. code-block:: console
> 
> +       To dump all flows:
>         rte_flow_dev_dump(port, file, NULL);
> +       and dump one flow:
> +       rte_flow_dump(port, flow, file, NULL);
> 
>  #. Dump human-readable flows from raw file:
> 
> @@ -1792,4 +1798,4 @@ all flows with assistance of external tools.
> 
>     .. code-block:: console
> 
> -       mlx_steering_dump.py -f <output_file>
> +       mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 62a5791..17e4351 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -3023,6 +3023,50 @@ Return values:
> 
>  - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
> 
> +Dump
> +~~~~~
> +
> +Dump information for all or one flows.
> +
> +This Function rte_flow_dev_dump will dump the information for all the flows.
> +
> +.. code-block:: c
> +
> +   int
> +   rte_flow_dev_dump(uint16_t port_id, FILE *file,
> +                     struct rte_flow_error *error);
> +
> +Arguments:
> +
> +- ``port_id``: port identifier of Ethernet device.
> +- ``file``: a pointer to a file for output
> +- ``error``: perform verbose error reporting if not NULL. PMDs initialize
> +  this structure in case of error only.
> +
> +Return values:
> +
> +- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
> +
> +This Function rte_flow_dump will dump the information for one flow.
> +
> +.. code-block:: c
> +
> +   int
> +   rte_flow_dump(uint16_t port_id, struct rte_flow *flow, FILE *file,
> +                 struct rte_flow_error *error);
> +
> +Arguments:
> +
> +- ``port_id``: port identifier of Ethernet device.
> +- ``file``: a pointer to a file for output
> +- ``flow``: flow rule handle to dump.
> +- ``error``: perform verbose error reporting if not NULL. PMDs initialize
> +  this structure in case of error only.
> +
> +Return values:
> +
> +- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
> +
>  Query
>  ~~~~~
> 
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 241af6c..ff051e7 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -1044,6 +1044,27 @@ enum rte_flow_conv_item_spec_type {
>  }
> 
>  int
> +rte_flow_dump(uint16_t port_id, struct rte_flow *flow,
> +			FILE *file, struct rte_flow_error *error)

Why not update the current dump function to support dumping one flow?


Best,
Ori

[dpdk-dev] [PATCH v2 0/5] support single flow dump

[Haifei Luo]

Dump internal representation information of all flows is supported.
It is useful to dump one flow. To implement this requirement,
add this CLI to dump one rule: flow dump PORT rule ID
      and the CLI to dump all: flow dump PORT all
Examples:
    testpmd> flow dump 0 all
    testpmd> flow dump 0 rule 0

For RTE API, add one arg rte_flow in rte_flow_dev_dump.
Accordingly, add this arg in related dev_dump and driver APIs.

Haifei Luo (5):
  ethdev: modify rte API for single flow dump
  app/testpmd: add CLIs for single flow dump feature
  common/mlx5: add mlx5 APIs for single flow dump feature
  net/mlx5: add mlx5 APIs for single flow dump feature
  doc: add single flow dump to guides

 app/test-pmd/cmdline_flow.c                     | 55 +++++++++++++++++++++----
 app/test-pmd/config.c                           | 38 +++++++++++++++--
 app/test-pmd/testpmd.h                          |  3 +-
 doc/guides/nics/features/default.ini            |  1 +
 doc/guides/nics/features/mlx5.ini               |  1 +
 doc/guides/nics/mlx5.rst                        |  9 ++--
 doc/guides/prog_guide/rte_flow.rst              | 24 +++++++++++
 doc/guides/rel_notes/release_21_05.rst          |  5 ++-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst     |  6 ++-
 drivers/common/mlx5/linux/meson.build           |  2 +
 drivers/common/mlx5/linux/mlx5_glue.c           | 13 ++++++
 drivers/common/mlx5/linux/mlx5_glue.h           |  1 +
 drivers/common/mlx5/mlx5_devx_cmds.c            | 14 +++++++
 drivers/common/mlx5/mlx5_devx_cmds.h            |  2 +
 drivers/common/mlx5/rte_common_mlx5_exports.def |  1 +
 drivers/common/mlx5/version.map                 |  1 +
 drivers/net/mlx5/linux/mlx5_socket.c            | 30 +++++++++++---
 drivers/net/mlx5/mlx5.h                         |  4 +-
 drivers/net/mlx5/mlx5_flow.c                    | 34 +++++++++++++--
 drivers/net/octeontx2/otx2_flow.c               |  9 +++-
 lib/librte_ethdev/rte_flow.c                    |  5 ++-
 lib/librte_ethdev/rte_flow.h                    |  5 ++-
 lib/librte_ethdev/rte_flow_driver.h             |  1 +
 23 files changed, 233 insertions(+), 31 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v2 1/5] ethdev: modify rte API for single flow dump

[Haifei Luo]

Previous implementations support dump all the flows. Add new arg
rte_flow in rte_flow_dev_dump to dump one flow.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 app/test-pmd/config.c                |  2 +-
 doc/guides/nics/mlx5.rst             |  9 ++++++---
 doc/guides/prog_guide/rte_flow.rst   | 24 ++++++++++++++++++++++++
 drivers/net/mlx5/linux/mlx5_socket.c |  2 +-
 drivers/net/mlx5/mlx5.h              |  4 ++--
 drivers/net/mlx5/mlx5_flow.c         |  9 ++++++---
 drivers/net/octeontx2/otx2_flow.c    |  9 ++++++++-
 lib/librte_ethdev/rte_flow.c         |  5 +++--
 lib/librte_ethdev/rte_flow.h         |  5 ++++-
 lib/librte_ethdev/rte_flow_driver.h  |  1 +
 10 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index ef0b978..2bfa8fa 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1931,7 +1931,7 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, file, &error);
+	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 8703435..17e6ada 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1829,13 +1829,16 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       testpmd> flow dump <port> <output_file>
+       To dump all flows:
+       testpmd> flow dump <port> all <output_file>
+       and dump one flow:
+       testpmd> flow dump <port> rule <rule_id> <output_file>
 
    - call rte_flow_dev_dump api:
 
    .. code-block:: console
 
-       rte_flow_dev_dump(port, file, NULL);
+       rte_flow_dev_dump(port, flow, file, NULL);
 
 #. Dump human-readable flows from raw file:
 
@@ -1843,4 +1846,4 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       mlx_steering_dump.py -f <output_file>
+       mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index aec2ba1..3bff7c3 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3018,6 +3018,30 @@ Return values:
 
 - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
 
+Dump
+~~~~~
+
+Dump information for all or one flows.
+
+.. code-block:: c
+
+   int
+   rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+                     FILE *file,
+                     struct rte_flow_error *error);
+
+Arguments:
+
+- ``port_id``: port identifier of Ethernet device.
+- ``flow``: flow rule handle to dump. NULL to dump all.
+- ``file``: a pointer to a file for output
+- ``error``: perform verbose error reporting if not NULL. PMDs initialize
+  this structure in case of error only.
+
+Return values:
+
+- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
+
 Query
 ~~~~~
 
diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index b1f41bc..6e354f4 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -84,7 +84,7 @@
 	}
 	/* Dump flow. */
 	dev = &rte_eth_devices[port_id];
-	ret = mlx5_flow_dev_dump(dev, file, NULL);
+	ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
 	/* Set-up the ancillary data and reply. */
 	msg.msg_controllen = 0;
 	msg.msg_control = NULL;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 6faba4f..d0b7908 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1245,8 +1245,8 @@ void mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
 void mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt);
 int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		       bool clear, uint64_t *pkts, uint64_t *bytes);
-int mlx5_flow_dev_dump(struct rte_eth_dev *dev, FILE *file,
-		       struct rte_flow_error *error);
+int mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow,
+			FILE *file, struct rte_flow_error *error);
 void mlx5_flow_rxq_dynf_metadata_set(struct rte_eth_dev *dev);
 int mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index c347f81..bce6ab2 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7170,7 +7170,7 @@ struct mlx5_meter_domains_infos *
  *   0 on success, a nagative value otherwise.
  */
 int
-mlx5_flow_dev_dump(struct rte_eth_dev *dev,
+mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow_idx,
 		   FILE *file,
 		   struct rte_flow_error *error __rte_unused)
 {
@@ -7182,8 +7182,11 @@ struct mlx5_meter_domains_infos *
 			return -errno;
 		return -ENOTSUP;
 	}
-	return mlx5_devx_cmd_flow_dump(sh->fdb_domain, sh->rx_domain,
-				       sh->tx_domain, file);
+
+	if (!flow_idx)
+		return mlx5_devx_cmd_flow_dump(sh->fdb_domain,
+				sh->rx_domain, sh->tx_domain, file);
+	return -ENOTSUP;
 }
 
 /**
diff --git a/drivers/net/octeontx2/otx2_flow.c b/drivers/net/octeontx2/otx2_flow.c
index 14ac9bc..1c90d75 100644
--- a/drivers/net/octeontx2/otx2_flow.c
+++ b/drivers/net/octeontx2/otx2_flow.c
@@ -807,7 +807,7 @@
 
 static int
 otx2_flow_dev_dump(struct rte_eth_dev *dev,
-		  FILE *file,
+		  struct rte_flow *flow, FILE *file,
 		  struct rte_flow_error *error)
 {
 	struct otx2_eth_dev *hw = dev->data->dev_private;
@@ -822,6 +822,13 @@
 				   "Invalid file");
 		return -EINVAL;
 	}
+	if (flow != NULL) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE,
+				   NULL,
+				   "Invalid argument");
+		return -EINVAL;
+	}
 
 	max_prio = hw->npc_flow.flow_max_priority;
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index e07e617..7241f00 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -1027,7 +1027,8 @@ enum rte_flow_conv_item_spec_type {
 }
 
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error)
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+			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);
@@ -1037,7 +1038,7 @@ enum rte_flow_conv_item_spec_type {
 		return -rte_errno;
 	if (likely(!!ops->dev_dump)) {
 		fts_enter(dev);
-		ret = ops->dev_dump(dev, file, error);
+		ret = ops->dev_dump(dev, flow, file, error);
 		fts_exit(dev);
 		return flow_err(port_id, ret, error);
 	}
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 6cc5713..a763af5 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -3232,6 +3232,8 @@ enum rte_flow_conv_op {
  *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
+ * @param[in] flow
+ *   The pointer of rte flow.
  * @param[in] file
  *   A pointer to a file for output.
  * @param[out] error
@@ -3242,7 +3244,8 @@ enum rte_flow_conv_op {
  */
 __rte_experimental
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error);
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+		FILE *file, struct rte_flow_error *error);
 
 /**
  * Check if mbuf dynamic field for metadata is registered.
diff --git a/lib/librte_ethdev/rte_flow_driver.h b/lib/librte_ethdev/rte_flow_driver.h
index da594d9..6ae1f8c 100644
--- a/lib/librte_ethdev/rte_flow_driver.h
+++ b/lib/librte_ethdev/rte_flow_driver.h
@@ -75,6 +75,7 @@ struct rte_flow_ops {
 	/** See rte_flow_dev_dump(). */
 	int (*dev_dump)
 		(struct rte_eth_dev *dev,
+		 struct rte_flow *flow,
 		 FILE *file,
 		 struct rte_flow_error *error);
 	/** See rte_flow_get_aged_flows() */
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/5] app/testpmd: add CLIs for single flow dump feature

[Haifei Luo]

Add support for single flow dump.
The CLIs to dump one rule: flow dump PORT rule ID
to dump all: flow dump PORT all
Examples:
testpmd> flow dump 0 all
testpmd> flow dump 0 rule 0

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 4 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 2c40c69..44fe9b6 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -108,6 +108,10 @@ enum index {
 	TUNNEL_SET,
 	TUNNEL_MATCH,
 
+	/* Dump arguments */
+	DUMP_ALL,
+	DUMP_ONE,
+
 	/* Shared action arguments */
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -791,6 +795,8 @@ struct buffer {
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
+			bool mode;
+			uint32_t rule;
 		} dump; /**< Dump arguments. */
 		struct {
 			uint32_t rule;
@@ -842,6 +848,12 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index next_dump_subcmd[] = {
+	DUMP_ALL,
+	DUMP_ONE,
+	ZERO,
+};
+
 static const enum index next_sa_subcmd[] = {
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -2028,10 +2040,9 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	},
 	[DUMP] = {
 		.name = "dump",
-		.help = "dump all flow rules to file",
-		.next = NEXT(next_dump_attr, NEXT_ENTRY(PORT_ID)),
-		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
-			     ARGS_ENTRY(struct buffer, port)),
+		.help = "dump single/all flow rules to file",
+		.next = NEXT(next_dump_subcmd, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_dump,
 	},
 	[QUERY] = {
@@ -2121,6 +2132,22 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.destroy.rule)),
 		.call = parse_destroy,
 	},
+	/* Dump arguments. */
+	[DUMP_ALL] = {
+		.name = "all",
+		.help = "dump all",
+		.next = NEXT(next_dump_attr),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file)),
+		.call = parse_dump,
+	},
+	[DUMP_ONE] = {
+		.name = "rule",
+		.help = "dump one rule",
+		.next = NEXT(next_dump_attr, NEXT_ENTRY(RULE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
+				ARGS_ENTRY(struct buffer, args.dump.rule)),
+		.call = parse_dump,
+	},
 	/* Query arguments. */
 	[QUERY_ACTION] = {
 		.name = "{action}",
@@ -6344,8 +6371,20 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		ctx->objdata = 0;
 		ctx->object = out;
 		ctx->objmask = NULL;
+		return len;
+	}
+	switch (ctx->curr) {
+	case DUMP_ALL:
+	case DUMP_ONE:
+		out->args.dump.mode = (ctx->curr == DUMP_ALL) ? true : false;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+		return len;
+	default:
+		return -1;
 	}
-	return len;
 }
 
 /** Parse tokens for query command. */
@@ -7639,8 +7678,10 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
-	case DUMP:
-		port_flow_dump(in->port, in->args.dump.file);
+	case DUMP_ONE:
+	case DUMP_ALL:
+		port_flow_dump(in->port, in->args.dump.mode,
+				in->args.dump.rule, in->args.dump.file);
 		break;
 	case QUERY:
 		port_flow_query(in->port, in->args.query.rule,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 2bfa8fa..a860608 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1915,13 +1915,41 @@ struct rte_flow_shared_action *
 	return ret;
 }
 
-/** Dump all flow rules. */
+/** Dump flow rules. */
 int
-port_flow_dump(portid_t port_id, const char *file_name)
+port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
+		const char *file_name)
 {
 	int ret = 0;
 	FILE *file = stdout;
 	struct rte_flow_error error;
+	struct rte_port *port;
+	struct port_flow *pflow;
+	struct rte_flow *tmpFlow = NULL;
+	bool found = false;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+
+	if (!dump_all) {
+		port = &ports[port_id];
+		pflow = port->flow_list;
+		while (pflow) {
+			if (rule_id != pflow->id) {
+				pflow = pflow->next;
+			} else {
+				tmpFlow = pflow->flow;
+				if (tmpFlow)
+					found = true;
+				break;
+			}
+		}
+		if (found == false) {
+			printf("Failed to dump to flow %d\n", rule_id);
+			return -EINVAL;
+		}
+	}
 
 	if (file_name && strlen(file_name)) {
 		file = fopen(file_name, "w");
@@ -1931,7 +1959,11 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
+
+	if (!dump_all)
+		ret = rte_flow_dev_dump(port_id, tmpFlow, file, &error);
+	else
+		ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a87ccb0..36d8535 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -825,7 +825,8 @@ void update_age_action_context(const struct rte_flow_action *actions,
 		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
-int port_flow_dump(portid_t port_id, const char *file_name);
+int port_flow_dump(portid_t port_id, bool dump_all,
+			uint32_t rule, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_t rule,
 		    const struct rte_flow_action *action);
 void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f59eb8a..278e1ea 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3314,7 +3314,11 @@ following sections.
 
 - Dump internal representation information of all flows in hardware::
 
-   flow dump {port_id} {output_file}
+   flow dump {port_id} all {output_file}
+
+   for one flow :
+
+   flow dump {port_id} rule {rule_id} {output_file}
 
 - List and destroy aged flow rules::
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump feature

[Haifei Luo]

add mlx5 APIs for single flow dump feature

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 drivers/common/mlx5/linux/meson.build           |  2 ++
 drivers/common/mlx5/linux/mlx5_glue.c           | 13 +++++++++++++
 drivers/common/mlx5/linux/mlx5_glue.h           |  1 +
 drivers/common/mlx5/mlx5_devx_cmds.c            | 14 ++++++++++++++
 drivers/common/mlx5/mlx5_devx_cmds.h            |  2 ++
 drivers/common/mlx5/rte_common_mlx5_exports.def |  1 +
 drivers/common/mlx5/version.map                 |  1 +
 7 files changed, 34 insertions(+)

diff --git a/drivers/common/mlx5/linux/meson.build b/drivers/common/mlx5/linux/meson.build
index 220de35..3f42163 100644
--- a/drivers/common/mlx5/linux/meson.build
+++ b/drivers/common/mlx5/linux/meson.build
@@ -186,6 +186,8 @@ has_sym_args = [
 	'mlx5dv_dr_action_create_aso' ],
 	[ 'HAVE_INFINIBAND_VERBS_H', 'infiniband/verbs.h',
 	'INFINIBAND_VERBS_H' ],
+	[ 'HAVE_MLX5_DR_FLOW_DUMP_RULE', 'infiniband/mlx5dv.h',
+	'mlx5dv_dump_dr_rule' ],
 ]
 config = configuration_data()
 foreach arg:has_sym_args
diff --git a/drivers/common/mlx5/linux/mlx5_glue.c b/drivers/common/mlx5/linux/mlx5_glue.c
index 964f7e7..d3bd645 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.c
+++ b/drivers/common/mlx5/linux/mlx5_glue.c
@@ -1101,6 +1101,18 @@
 }
 
 static int
+mlx5_glue_dr_dump_single_rule(FILE *file, void *rule)
+{
+#ifdef HAVE_MLX5_DR_FLOW_DUMP_RULE
+	return mlx5dv_dump_dr_rule(file, rule);
+#else
+	RTE_SET_USED(file);
+	RTE_SET_USED(rule);
+	return -ENOTSUP;
+#endif
+}
+
+static int
 mlx5_glue_dr_dump_domain(FILE *file, void *domain)
 {
 #ifdef HAVE_MLX5_DR_FLOW_DUMP
@@ -1423,6 +1435,7 @@
 	.devx_wq_query = mlx5_glue_devx_wq_query,
 	.devx_port_query = mlx5_glue_devx_port_query,
 	.dr_dump_domain = mlx5_glue_dr_dump_domain,
+	.dr_dump_rule = mlx5_glue_dr_dump_single_rule,
 	.dr_reclaim_domain_memory = mlx5_glue_dr_reclaim_domain_memory,
 	.dr_create_flow_action_sampler =
 		mlx5_glue_dr_create_flow_action_sampler,
diff --git a/drivers/common/mlx5/linux/mlx5_glue.h b/drivers/common/mlx5/linux/mlx5_glue.h
index 9e385be..97462e9 100644
--- a/drivers/common/mlx5/linux/mlx5_glue.h
+++ b/drivers/common/mlx5/linux/mlx5_glue.h
@@ -313,6 +313,7 @@ struct mlx5_glue {
 			       uint32_t port_num,
 			       struct mlx5dv_devx_port *mlx5_devx_port);
 	int (*dr_dump_domain)(FILE *file, void *domain);
+	int (*dr_dump_rule)(FILE *file, void *rule);
 	int (*devx_query_eqn)(struct ibv_context *context, uint32_t cpus,
 			      uint32_t *eqn);
 	struct mlx5dv_devx_event_channel *(*devx_create_event_channel)
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c b/drivers/common/mlx5/mlx5_devx_cmds.c
index c90e020..c0b6fdb 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.c
+++ b/drivers/common/mlx5/mlx5_devx_cmds.c
@@ -1579,6 +1579,20 @@ struct mlx5_devx_obj *
 	return -ret;
 }
 
+int
+mlx5_devx_cmd_flow_single_dump(void *rule_info __rte_unused,
+			FILE *file __rte_unused)
+{
+	int ret = 0;
+#ifdef HAVE_MLX5_DR_FLOW_DUMP_RULE
+	if (rule_info)
+		ret = mlx5_glue->dr_dump_rule(file, rule_info);
+#else
+	ret = ENOTSUP;
+#endif
+	return -ret;
+}
+
 /*
  * Create CQ using DevX API.
  *
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h b/drivers/common/mlx5/mlx5_devx_cmds.h
index 2826c0b..f587d0c 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.h
+++ b/drivers/common/mlx5/mlx5_devx_cmds.h
@@ -474,6 +474,8 @@ struct mlx5_devx_obj *mlx5_devx_cmd_create_tis(void *ctx,
 int mlx5_devx_cmd_flow_dump(void *fdb_domain, void *rx_domain, void *tx_domain,
 			    FILE *file);
 __rte_internal
+int mlx5_devx_cmd_flow_single_dump(void *rule, FILE *file);
+__rte_internal
 struct mlx5_devx_obj *mlx5_devx_cmd_create_cq(void *ctx,
 					      struct mlx5_devx_cq_attr *attr);
 __rte_internal
diff --git a/drivers/common/mlx5/rte_common_mlx5_exports.def b/drivers/common/mlx5/rte_common_mlx5_exports.def
index fd62b80..0e6d6d3 100644
--- a/drivers/common/mlx5/rte_common_mlx5_exports.def
+++ b/drivers/common/mlx5/rte_common_mlx5_exports.def
@@ -20,6 +20,7 @@ EXPORTS
 	mlx5_devx_cmd_flow_counter_alloc
 	mlx5_devx_cmd_flow_counter_query
 	mlx5_devx_cmd_flow_dump
+	mlx5_devx_cmd_flow_single_dump
 	mlx5_devx_cmd_mkey_create
 	mlx5_devx_cmd_modify_qp_state
 	mlx5_devx_cmd_modify_rq
diff --git a/drivers/common/mlx5/version.map b/drivers/common/mlx5/version.map
index 91f3fa5..4d49011 100644
--- a/drivers/common/mlx5/version.map
+++ b/drivers/common/mlx5/version.map
@@ -28,6 +28,7 @@ INTERNAL {
 	mlx5_devx_cmd_flow_counter_alloc;
 	mlx5_devx_cmd_flow_counter_query;
 	mlx5_devx_cmd_flow_dump;
+	mlx5_devx_cmd_flow_single_dump;
 	mlx5_devx_cmd_mkey_create;
 	mlx5_devx_cmd_modify_qp_state;
 	mlx5_devx_cmd_modify_rq;
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

[Haifei Luo]

Modify API mlx5_flow_dev_dump to support the feature.
Modify mlx5_socket since one extra arg flow_ptr is added.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_socket.c | 30 ++++++++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow.c         | 29 +++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index 6e354f4..2ef837c 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -2,6 +2,10 @@
  * Copyright 2019 Mellanox Technologies, Ltd
  */
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
@@ -29,11 +33,15 @@
 {
 	int conn_sock;
 	int ret;
+	int j;
 	struct cmsghdr *cmsg = NULL;
-	int data;
+	#define LENGTH 9
+	/* The first byte for port_id and the rest for flowptr. */
+	int data[LENGTH];
+	uint64_t flow_ptr = 0;
 	char buf[CMSG_SPACE(sizeof(int))] = { 0 };
 	struct iovec io = {
-		.iov_base = &data,
+		.iov_base = &data[0],
 		.iov_len = sizeof(data),
 	};
 	struct msghdr msg = {
@@ -46,7 +54,9 @@
 	int fd;
 	FILE *file = NULL;
 	struct rte_eth_dev *dev;
+	struct rte_flow_error err;
 
+	memset(data, 0, sizeof(data));
 	/* Accept the connection from the client. */
 	conn_sock = accept(server_socket, NULL, NULL);
 	if (conn_sock < 0) {
@@ -84,15 +94,23 @@
 	}
 	/* Dump flow. */
 	dev = &rte_eth_devices[port_id];
-	ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
+	/* The first byte in data for port_id and the following 8 for flowptr */
+	for (j = 1; j < LENGTH; j++)
+		flow_ptr = (flow_ptr << 8) + data[j];
+	if (flow_ptr == 0)
+		ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
+	else
+		ret = mlx5_flow_dev_dump(dev,
+			(struct rte_flow *)((uintptr_t)flow_ptr), file, &err);
+
 	/* Set-up the ancillary data and reply. */
 	msg.msg_controllen = 0;
 	msg.msg_control = NULL;
 	msg.msg_iovlen = 1;
 	msg.msg_iov = &io;
-	data = -ret;
-	io.iov_len = sizeof(data);
-	io.iov_base = &data;
+	data[0] = -ret;
+	io.iov_len = sizeof(data[0]);
+	io.iov_base = &data[0];
 	do {
 		ret = sendmsg(conn_sock, &msg, 0);
 	} while (ret < 0 && errno == EINTR);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index bce6ab2..b68909f 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7183,10 +7183,35 @@ struct mlx5_meter_domains_infos *
 		return -ENOTSUP;
 	}
 
+	/*dump all*/
 	if (!flow_idx)
 		return mlx5_devx_cmd_flow_dump(sh->fdb_domain,
-				sh->rx_domain, sh->tx_domain, file);
-	return -ENOTSUP;
+					sh->rx_domain,
+					sh->tx_domain, file);
+	/*dump one*/
+	uint32_t handle_idx;
+	int ret;
+	struct mlx5_flow_handle *dh;
+	struct rte_flow *flow = mlx5_ipool_get(priv->sh->ipool
+		[MLX5_IPOOL_RTE_FLOW], (uintptr_t)(void *)flow_idx);
+
+	if (!flow)
+		return -ENOENT;
+	handle_idx = flow->dev_handles;
+	while (handle_idx) {
+		dh = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MLX5_FLOW],
+				handle_idx);
+		if (!dh)
+			return -ENOENT;
+		if (dh->drv_flow) {
+			ret = mlx5_devx_cmd_flow_single_dump(dh->drv_flow,
+					file);
+			if (ret)
+				return -ENOENT;
+		}
+		handle_idx = dh->next.next;
+	}
+	return 0;
 }
 
 /**
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 5/5] doc: add single flow dump to guides

[Haifei Luo]

Add "Flow dump" in features/default.ini and features/mlx5.ini.
Add testpmd CLI and API changes in release_notes.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 doc/guides/nics/features/default.ini   | 1 +
 doc/guides/nics/features/mlx5.ini      | 1 +
 doc/guides/rel_notes/release_21_05.rst | 5 ++++-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index 8046bd1..49aaf17 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -39,6 +39,7 @@ DCB                  =
 VLAN filter          =
 Flow control         =
 Flow API             =
+Flow dump            =
 Rate limitation      =
 Traffic mirroring    =
 Inline crypto        =
diff --git a/doc/guides/nics/features/mlx5.ini b/doc/guides/nics/features/mlx5.ini
index ddd131d..3c5fcff 100644
--- a/doc/guides/nics/features/mlx5.ini
+++ b/doc/guides/nics/features/mlx5.ini
@@ -29,6 +29,7 @@ SR-IOV               = Y
 VLAN filter          = Y
 Flow control         = Y
 Flow API             = Y
+Flow dump            = Y
 CRC offload          = Y
 VLAN offload         = Y
 L3 checksum offload  = Y
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index 873140b..f256324 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -125,7 +125,8 @@ New Features
     ``dpdk-testpmd -- --eth-link-speed N``
   * Added command to display Rx queue used descriptor count.
     ``show port (port_id) rxq (queue_id) desc used count``
-
+  * Added command to dump internal representation information of single flow.
+    ``flow dump (port_id) rule (rule_id)``
 
 Removed Items
 -------------
@@ -155,6 +156,8 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* ethdev: Added a rte_flow pointer parameter to the function
+  ``rte_flow_dev_dump()`` allowing dump for single flow.
 
 ABI Changes
 -----------
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

[Slava Ovsiienko]

> -----Original Message-----
> From: Haifei Luo <haifeil@nvidia.com>
> Sent: Wednesday, April 7, 2021 9:09
> To: dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li
> <xuemingl@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
> Subject: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump
> feature
> 
> Modify API mlx5_flow_dev_dump to support the feature.
> Modify mlx5_socket since one extra arg flow_ptr is added.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump feature

[Slava Ovsiienko]

> -----Original Message-----
> From: Haifei Luo <haifeil@nvidia.com>
> Sent: Wednesday, April 7, 2021 9:09
> To: dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li
> <xuemingl@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Ray Kinsella
> <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
> Subject: [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump
> feature
> 
> add mlx5 APIs for single flow dump feature
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

[Slava Ovsiienko]

> -----Original Message-----
> From: Haifei Luo <haifeil@nvidia.com>
> Sent: Wednesday, April 7, 2021 9:09
> To: dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li
> <xuemingl@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
> Subject: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump
> feature
> 
> Modify API mlx5_flow_dev_dump to support the feature.
> Modify mlx5_socket since one extra arg flow_ptr is added.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>

Sorry, this patch is errorneously acked instead of the 
"common/mlx5: add mlx5 APIs for single flow dump feature"

I have comment for this one.

> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> +
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <sys/un.h>
> @@ -29,11 +33,15 @@
>  {
>  	int conn_sock;
>  	int ret;
> +	int j;
>  	struct cmsghdr *cmsg = NULL;
> -	int data;
> +	#define LENGTH 9
> +	/* The first byte for port_id and the rest for flowptr. */
> +	int data[LENGTH];

So, we define 36/72 bytes array? And then use each int as byte to save flow_idx value?
I suppose the correct way would be to define the structure of message in stead of using ints array,
something likle this:

struct mlx5_ipc_msg {
  int status;
  void* flow_idx;
}

> +	/* The first byte in data for port_id and the following 8 for flowptr */
> +	for (j = 1; j < LENGTH; j++)
> +		flow_ptr = (flow_ptr << 8) + data[j];
    If structure is define, there should be:
        flow_ptr =  msg->flow_idx

> +	if (flow_ptr == 0)
> +		ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
> +	else
> +		ret = mlx5_flow_dev_dump(dev,
> +			(struct rte_flow *)((uintptr_t)flow_ptr), file, &err);
> +

> +	/*dump one*/
> +	uint32_t handle_idx;
> +	int ret;
> +	struct mlx5_flow_handle *dh;
> +	struct rte_flow *flow = mlx5_ipool_get(priv->sh->ipool
> +		[MLX5_IPOOL_RTE_FLOW], (uintptr_t)(void *)flow_idx);
> +
Please, move variable declarations to the routine beginning, to others

With best regards, Slava

Re: [dpdk-dev] [PATCH v2 1/5] ethdev: modify rte API for single flow dump

[Ori Kam]

Hi Haifei,

> -----Original Message-----
> From: Haifei Luo <haifeil@nvidia.com>
> Sent: Wednesday, April 7, 2021 9:09 AM
> <andrew.rybchenko@oktetlabs.ru>
> Subject: [PATCH v2 1/5] ethdev: modify rte API for single flow dump
> 
> Previous implementations support dump all the flows. Add new arg
> rte_flow in rte_flow_dev_dump to dump one flow.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>  app/test-pmd/config.c                |  2 +-
>  doc/guides/nics/mlx5.rst             |  9 ++++++---
>  doc/guides/prog_guide/rte_flow.rst   | 24 ++++++++++++++++++++++++
>  drivers/net/mlx5/linux/mlx5_socket.c |  2 +-
>  drivers/net/mlx5/mlx5.h              |  4 ++--
>  drivers/net/mlx5/mlx5_flow.c         |  9 ++++++---
>  drivers/net/octeontx2/otx2_flow.c    |  9 ++++++++-
>  lib/librte_ethdev/rte_flow.c         |  5 +++--
>  lib/librte_ethdev/rte_flow.h         |  5 ++++-
>  lib/librte_ethdev/rte_flow_driver.h  |  1 +
>  10 files changed, 56 insertions(+), 14 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index ef0b978..2bfa8fa 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1931,7 +1931,7 @@ struct rte_flow_shared_action *
>  			return -errno;
>  		}
>  	}
> -	ret = rte_flow_dev_dump(port_id, file, &error);
> +	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
>  	if (ret) {
>  		port_flow_complain(&error);
>  		printf("Failed to dump flow: %s\n", strerror(-ret));
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
> index 8703435..17e6ada 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -1829,13 +1829,16 @@ all flows with assistance of external tools.
> 
>     .. code-block:: console
> 
> -       testpmd> flow dump <port> <output_file>
> +       To dump all flows:
> +       testpmd> flow dump <port> all <output_file>
> +       and dump one flow:
> +       testpmd> flow dump <port> rule <rule_id> <output_file>
> 
>     - call rte_flow_dev_dump api:
> 
>     .. code-block:: console
> 
> -       rte_flow_dev_dump(port, file, NULL);
> +       rte_flow_dev_dump(port, flow, file, NULL);
> 
>  #. Dump human-readable flows from raw file:
> 
> @@ -1843,4 +1846,4 @@ all flows with assistance of external tools.
> 
>     .. code-block:: console
> 
> -       mlx_steering_dump.py -f <output_file>
> +       mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index aec2ba1..3bff7c3 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -3018,6 +3018,30 @@ Return values:
> 
>  - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
> 
> +Dump
> +~~~~~
> +
> +Dump information for all or one flows.
> +
> +.. code-block:: c
> +
> +   int
> +   rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
> +                     FILE *file,
> +                     struct rte_flow_error *error);
> +
> +Arguments:
> +
> +- ``port_id``: port identifier of Ethernet device.
> +- ``flow``: flow rule handle to dump. NULL to dump all.
> +- ``file``: a pointer to a file for output
> +- ``error``: perform verbose error reporting if not NULL. PMDs initialize
> +  this structure in case of error only.
> +
> +Return values:
> +
> +- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
> +
>  Query
>  ~~~~~
> 
> diff --git a/drivers/net/mlx5/linux/mlx5_socket.c
> b/drivers/net/mlx5/linux/mlx5_socket.c
> index b1f41bc..6e354f4 100644
> --- a/drivers/net/mlx5/linux/mlx5_socket.c
> +++ b/drivers/net/mlx5/linux/mlx5_socket.c
> @@ -84,7 +84,7 @@
>  	}
>  	/* Dump flow. */
>  	dev = &rte_eth_devices[port_id];
> -	ret = mlx5_flow_dev_dump(dev, file, NULL);
> +	ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
>  	/* Set-up the ancillary data and reply. */
>  	msg.msg_controllen = 0;
>  	msg.msg_control = NULL;
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index 6faba4f..d0b7908 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -1245,8 +1245,8 @@ void mlx5_flow_async_pool_query_handle(struct
> mlx5_dev_ctx_shared *sh,
>  void mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt);
>  int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
>  		       bool clear, uint64_t *pkts, uint64_t *bytes);
> -int mlx5_flow_dev_dump(struct rte_eth_dev *dev, FILE *file,
> -		       struct rte_flow_error *error);
> +int mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow,
> +			FILE *file, struct rte_flow_error *error);
>  void mlx5_flow_rxq_dynf_metadata_set(struct rte_eth_dev *dev);
>  int mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
>  			uint32_t nb_contexts, struct rte_flow_error *error);
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index c347f81..bce6ab2 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -7170,7 +7170,7 @@ struct mlx5_meter_domains_infos *
>   *   0 on success, a nagative value otherwise.
>   */
>  int
> -mlx5_flow_dev_dump(struct rte_eth_dev *dev,
> +mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow_idx,
>  		   FILE *file,
>  		   struct rte_flow_error *error __rte_unused)
>  {
> @@ -7182,8 +7182,11 @@ struct mlx5_meter_domains_infos *
>  			return -errno;
>  		return -ENOTSUP;
>  	}
> -	return mlx5_devx_cmd_flow_dump(sh->fdb_domain, sh->rx_domain,
> -				       sh->tx_domain, file);
> +
> +	if (!flow_idx)
> +		return mlx5_devx_cmd_flow_dump(sh->fdb_domain,
> +				sh->rx_domain, sh->tx_domain, file);
> +	return -ENOTSUP;
>  }
> 
>  /**
> diff --git a/drivers/net/octeontx2/otx2_flow.c
> b/drivers/net/octeontx2/otx2_flow.c
> index 14ac9bc..1c90d75 100644
> --- a/drivers/net/octeontx2/otx2_flow.c
> +++ b/drivers/net/octeontx2/otx2_flow.c
> @@ -807,7 +807,7 @@
> 
>  static int
>  otx2_flow_dev_dump(struct rte_eth_dev *dev,
> -		  FILE *file,
> +		  struct rte_flow *flow, FILE *file,
>  		  struct rte_flow_error *error)
>  {
>  	struct otx2_eth_dev *hw = dev->data->dev_private;
> @@ -822,6 +822,13 @@
>  				   "Invalid file");
>  		return -EINVAL;
>  	}
> +	if (flow != NULL) {
> +		rte_flow_error_set(error, EINVAL,
> +				   RTE_FLOW_ERROR_TYPE_HANDLE,
> +				   NULL,
> +				   "Invalid argument");
> +		return -EINVAL;
> +	}
> 
>  	max_prio = hw->npc_flow.flow_max_priority;
> 
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index e07e617..7241f00 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -1027,7 +1027,8 @@ enum rte_flow_conv_item_spec_type {
>  }
> 
>  int
> -rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error)
> +rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
> +			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);
> @@ -1037,7 +1038,7 @@ enum rte_flow_conv_item_spec_type {
>  		return -rte_errno;
>  	if (likely(!!ops->dev_dump)) {
>  		fts_enter(dev);
> -		ret = ops->dev_dump(dev, file, error);
> +		ret = ops->dev_dump(dev, flow, file, error);
>  		fts_exit(dev);
>  		return flow_err(port_id, ret, error);
>  	}
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 6cc5713..a763af5 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -3232,6 +3232,8 @@ enum rte_flow_conv_op {
>   *
>   * @param[in] port_id
>   *    The port identifier of the Ethernet device.
> + * @param[in] flow
> + *   The pointer of rte flow.
>   * @param[in] file
>   *   A pointer to a file for output.
>   * @param[out] error
> @@ -3242,7 +3244,8 @@ enum rte_flow_conv_op {
>   */
>  __rte_experimental
>  int
> -rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error);
> +rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
> +		FILE *file, struct rte_flow_error *error);
> 
>  /**
>   * Check if mbuf dynamic field for metadata is registered.
> diff --git a/lib/librte_ethdev/rte_flow_driver.h
> b/lib/librte_ethdev/rte_flow_driver.h
> index da594d9..6ae1f8c 100644
> --- a/lib/librte_ethdev/rte_flow_driver.h
> +++ b/lib/librte_ethdev/rte_flow_driver.h
> @@ -75,6 +75,7 @@ struct rte_flow_ops {
>  	/** See rte_flow_dev_dump(). */
>  	int (*dev_dump)
>  		(struct rte_eth_dev *dev,
> +		 struct rte_flow *flow,
>  		 FILE *file,
>  		 struct rte_flow_error *error);
>  	/** See rte_flow_get_aged_flows() */
> --
> 1.8.3.1

Acked-by: Ori Kam <orika@nvidia.com>
Thanks,
Ori

Re: [dpdk-dev] [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

[Haifei Luo]

Hi Slava,
      For #1, The steering tool send messages to DPDK to request dump. Server/Client  use data structure "struct msghdr"
to communicate. It has " msg_iov " ," msg_iovlen" and etc. 
      In the tool side, Msg_iov is constructed as 1 byte for port_id,  8 bytes for flowptr. In DPDK, then we parse the message this way.  
      For #2,  I will move them to the beginning.

-----Original Message-----
From: Slava Ovsiienko <viacheslavo@nvidia.com> 
Sent: Monday, April 12, 2021 3:38 PM
To: Haifei Luo <haifeil@nvidia.com>; dev@dpdk.org
Cc: Ori Kam <orika@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
Subject: RE: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

> -----Original Message-----
> From: Haifei Luo <haifeil@nvidia.com>
> Sent: Wednesday, April 7, 2021 9:09
> To: dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko 
> <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; 
> Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo 
> <haifeil@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler 
> <shahafs@nvidia.com>
> Subject: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump 
> feature
> 
> Modify API mlx5_flow_dev_dump to support the feature.
> Modify mlx5_socket since one extra arg flow_ptr is added.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>

Sorry, this patch is errorneously acked instead of the
"common/mlx5: add mlx5 APIs for single flow dump feature"

I have comment for this one.

> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> +
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <sys/un.h>
> @@ -29,11 +33,15 @@
>  {
>  	int conn_sock;
>  	int ret;
> +	int j;
>  	struct cmsghdr *cmsg = NULL;
> -	int data;
> +	#define LENGTH 9
> +	/* The first byte for port_id and the rest for flowptr. */
> +	int data[LENGTH];

So, we define 36/72 bytes array? And then use each int as byte to save flow_idx value?
I suppose the correct way would be to define the structure of message in stead of using ints array, something likle this:

struct mlx5_ipc_msg {
  int status;
  void* flow_idx;
}

> +	/* The first byte in data for port_id and the following 8 for flowptr */
> +	for (j = 1; j < LENGTH; j++)
> +		flow_ptr = (flow_ptr << 8) + data[j];
    If structure is define, there should be:
        flow_ptr =  msg->flow_idx

> +	if (flow_ptr == 0)
> +		ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
> +	else
> +		ret = mlx5_flow_dev_dump(dev,
> +			(struct rte_flow *)((uintptr_t)flow_ptr), file, &err);
> +

> +	/*dump one*/
> +	uint32_t handle_idx;
> +	int ret;
> +	struct mlx5_flow_handle *dh;
> +	struct rte_flow *flow = mlx5_ipool_get(priv->sh->ipool
> +		[MLX5_IPOOL_RTE_FLOW], (uintptr_t)(void *)flow_idx);
> +
Please, move variable declarations to the routine beginning, to others

With best regards, Slava

Re: [dpdk-dev] [PATCH v2 1/5] ethdev: modify rte API for single flow dump

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On Tue, Apr 6, 2021 at 11:09 PM Haifei Luo <haifeil@nvidia.com> wrote:
>
> Previous implementations support dump all the flows. Add new arg
> rte_flow in rte_flow_dev_dump to dump one flow.
>
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>  app/test-pmd/config.c                |  2 +-
>  doc/guides/nics/mlx5.rst             |  9 ++++++---
>  doc/guides/prog_guide/rte_flow.rst   | 24 ++++++++++++++++++++++++
>  drivers/net/mlx5/linux/mlx5_socket.c |  2 +-
>  drivers/net/mlx5/mlx5.h              |  4 ++--
>  drivers/net/mlx5/mlx5_flow.c         |  9 ++++++---
>  drivers/net/octeontx2/otx2_flow.c    |  9 ++++++++-
>  lib/librte_ethdev/rte_flow.c         |  5 +++--
>  lib/librte_ethdev/rte_flow.h         |  5 ++++-
>  lib/librte_ethdev/rte_flow_driver.h  |  1 +
>  10 files changed, 56 insertions(+), 14 deletions(-)
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

Re: [dpdk-dev] [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

[Haifei Luo]

HI Slava,
     Yes for   "we define 36/72 bytes array?". 
     Correction for my last comment,   not byte , and it is  one "int" for port_id , 8 "int" for flowptr.
Sorry for the possible confusion.


-----Original Message-----
From: Haifei Luo 
Sent: Tuesday, April 13, 2021 9:29 AM
To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org
Cc: Ori Kam <orika@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
Subject: RE: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

Hi Slava,
      For #1, The steering tool send messages to DPDK to request dump. Server/Client  use data structure "struct msghdr"
to communicate. It has " msg_iov " ," msg_iovlen" and etc. 
      In the tool side, Msg_iov is constructed as 1 byte for port_id,  8 bytes for flowptr. In DPDK, then we parse the message this way.  
      For #2,  I will move them to the beginning.

-----Original Message-----
From: Slava Ovsiienko <viacheslavo@nvidia.com>
Sent: Monday, April 12, 2021 3:38 PM
To: Haifei Luo <haifeil@nvidia.com>; dev@dpdk.org
Cc: Ori Kam <orika@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
Subject: RE: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

> -----Original Message-----
> From: Haifei Luo <haifeil@nvidia.com>
> Sent: Wednesday, April 7, 2021 9:09
> To: dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko 
> <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo 
> <haifeil@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler 
> <shahafs@nvidia.com>
> Subject: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump 
> feature
> 
> Modify API mlx5_flow_dev_dump to support the feature.
> Modify mlx5_socket since one extra arg flow_ptr is added.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>

Sorry, this patch is errorneously acked instead of the
"common/mlx5: add mlx5 APIs for single flow dump feature"

I have comment for this one.

> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> +
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <sys/un.h>
> @@ -29,11 +33,15 @@
>  {
>  	int conn_sock;
>  	int ret;
> +	int j;
>  	struct cmsghdr *cmsg = NULL;
> -	int data;
> +	#define LENGTH 9
> +	/* The first byte for port_id and the rest for flowptr. */
> +	int data[LENGTH];

So, we define 36/72 bytes array? And then use each int as byte to save flow_idx value?
I suppose the correct way would be to define the structure of message in stead of using ints array, something likle this:

struct mlx5_ipc_msg {
  int status;
  void* flow_idx;
}

> +	/* The first byte in data for port_id and the following 8 for flowptr */
> +	for (j = 1; j < LENGTH; j++)
> +		flow_ptr = (flow_ptr << 8) + data[j];
    If structure is define, there should be:
        flow_ptr =  msg->flow_idx

> +	if (flow_ptr == 0)
> +		ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
> +	else
> +		ret = mlx5_flow_dev_dump(dev,
> +			(struct rte_flow *)((uintptr_t)flow_ptr), file, &err);
> +

> +	/*dump one*/
> +	uint32_t handle_idx;
> +	int ret;
> +	struct mlx5_flow_handle *dh;
> +	struct rte_flow *flow = mlx5_ipool_get(priv->sh->ipool
> +		[MLX5_IPOOL_RTE_FLOW], (uintptr_t)(void *)flow_idx);
> +
Please, move variable declarations to the routine beginning, to others

With best regards, Slava

Re: [dpdk-dev] [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature

[Slava Ovsiienko]

> -----Original Message-----
> From: Haifei Luo
> Sent: Tuesday, April 13, 2021 9:29 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> Xueming(Steven) Li <xuemingl@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
> Subject: RE: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump
> feature
> 
> Hi Slava,
>       For #1, The steering tool send messages to DPDK to request dump.
> Server/Client  use data structure "struct msghdr"
> to communicate. It has " msg_iov " ," msg_iovlen" and etc.
>       In the tool side, Msg_iov is constructed as 1 byte for port_id,  8 bytes for
> flowptr. In DPDK, then we parse the message this way.

Yes, it is clear. In my opinion we should not use byte array and parse
we should present the some structure instead:

struct mlx5_flow_dump_req {
	uint8_t port_id;
              void *flow_id;
} __rte_packed;

BTW, why port_id is 1 byte? port_id in DPDK is 16-bit value.

If the request dump tool uses somr structure - it should be defined in some
common file. IMO, it is not a good practice to rely on raw byte array layout.

With best regards, Slava

>       For #2,  I will move them to the beginning.
> 
> -----Original Message-----
> From: Slava Ovsiienko <viacheslavo@nvidia.com>
> Sent: Monday, April 12, 2021 3:38 PM
> To: Haifei Luo <haifeil@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo
> <haifeil@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler
> <shahafs@nvidia.com>
> Subject: RE: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump
> feature
> 
> > -----Original Message-----
> > From: Haifei Luo <haifeil@nvidia.com>
> > Sent: Wednesday, April 7, 2021 9:09
> > To: dev@dpdk.org
> > Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko
> > <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> > Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo
> > <haifeil@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler
> > <shahafs@nvidia.com>
> > Subject: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump
> > feature
> >
> > Modify API mlx5_flow_dev_dump to support the feature.
> > Modify mlx5_socket since one extra arg flow_ptr is added.
> >
> > Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> 
> Sorry, this patch is errorneously acked instead of the
> "common/mlx5: add mlx5 APIs for single flow dump feature"
> 
> I have comment for this one.
> 
> > +#ifndef _GNU_SOURCE
> > +#define _GNU_SOURCE
> > +#endif
> > +
> >  #include <sys/types.h>
> >  #include <sys/socket.h>
> >  #include <sys/un.h>
> > @@ -29,11 +33,15 @@
> >  {
> >  	int conn_sock;
> >  	int ret;
> > +	int j;
> >  	struct cmsghdr *cmsg = NULL;
> > -	int data;
> > +	#define LENGTH 9
> > +	/* The first byte for port_id and the rest for flowptr. */
> > +	int data[LENGTH];
> 
> So, we define 36/72 bytes array? And then use each int as byte to save
> flow_idx value?
> I suppose the correct way would be to define the structure of message in
> stead of using ints array, something likle this:
> 
> struct mlx5_ipc_msg {
>   int status;
>   void* flow_idx;
> }
> 
> > +	/* The first byte in data for port_id and the following 8 for flowptr */
> > +	for (j = 1; j < LENGTH; j++)
> > +		flow_ptr = (flow_ptr << 8) + data[j];
>     If structure is define, there should be:
>         flow_ptr =  msg->flow_idx
> 
> > +	if (flow_ptr == 0)
> > +		ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
> > +	else
> > +		ret = mlx5_flow_dev_dump(dev,
> > +			(struct rte_flow *)((uintptr_t)flow_ptr), file, &err);
> > +
> 
> > +	/*dump one*/
> > +	uint32_t handle_idx;
> > +	int ret;
> > +	struct mlx5_flow_handle *dh;
> > +	struct rte_flow *flow = mlx5_ipool_get(priv->sh->ipool
> > +		[MLX5_IPOOL_RTE_FLOW], (uintptr_t)(void *)flow_idx);
> > +
> Please, move variable declarations to the routine beginning, to others
> 
> With best regards, Slava

Re: [dpdk-dev] [PATCH v2 2/5] app/testpmd: add CLIs for single flow dump feature

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]

On Tue, Apr 6, 2021 at 11:09 PM Haifei Luo <haifeil@nvidia.com> wrote:
>
> Add support for single flow dump.
> The CLIs to dump one rule: flow dump PORT rule ID
> to dump all: flow dump PORT all
> Examples:
> testpmd> flow dump 0 all
> testpmd> flow dump 0 rule 0
>
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

Re: [dpdk-dev] [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump feature

[Kinsella, Ray]

On 07/04/2021 07:09, Haifei Luo wrote:
> add mlx5 APIs for single flow dump feature
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>  drivers/common/mlx5/linux/meson.build           |  2 ++
>  drivers/common/mlx5/linux/mlx5_glue.c           | 13 +++++++++++++
>  drivers/common/mlx5/linux/mlx5_glue.h           |  1 +
>  drivers/common/mlx5/mlx5_devx_cmds.c            | 14 ++++++++++++++
>  drivers/common/mlx5/mlx5_devx_cmds.h            |  2 ++
>  drivers/common/mlx5/rte_common_mlx5_exports.def |  1 +

Perhaps check with David Marchand on this, I suspect amendments to the exports file will soon be redundant.

>  drivers/common/mlx5/version.map                 |  1 +
>  7 files changed, 34 insertions(+)
> 
> diff --git a/drivers/common/mlx5/linux/meson.build b/drivers/common/mlx5/linux/meson.build
> index 220de35..3f42163 100644
> --- a/drivers/common/mlx5/linux/meson.build
> +++ b/drivers/common/mlx5/linux/meson.build
> @@ -186,6 +186,8 @@ has_sym_args = [
>  	'mlx5dv_dr_action_create_aso' ],
>  	[ 'HAVE_INFINIBAND_VERBS_H', 'infiniband/verbs.h',
>  	'INFINIBAND_VERBS_H' ],
> +	[ 'HAVE_MLX5_DR_FLOW_DUMP_RULE', 'infiniband/mlx5dv.h',
> +	'mlx5dv_dump_dr_rule' ],
>  ]
>  config = configuration_data()
>  foreach arg:has_sym_args
> diff --git a/drivers/common/mlx5/linux/mlx5_glue.c b/drivers/common/mlx5/linux/mlx5_glue.c
> index 964f7e7..d3bd645 100644
> --- a/drivers/common/mlx5/linux/mlx5_glue.c
> +++ b/drivers/common/mlx5/linux/mlx5_glue.c
> @@ -1101,6 +1101,18 @@
>  }
>  
>  static int
> +mlx5_glue_dr_dump_single_rule(FILE *file, void *rule)
> +{
> +#ifdef HAVE_MLX5_DR_FLOW_DUMP_RULE
> +	return mlx5dv_dump_dr_rule(file, rule);
> +#else
> +	RTE_SET_USED(file);
> +	RTE_SET_USED(rule);
> +	return -ENOTSUP;
> +#endif
> +}
> +
> +static int
>  mlx5_glue_dr_dump_domain(FILE *file, void *domain)
>  {
>  #ifdef HAVE_MLX5_DR_FLOW_DUMP
> @@ -1423,6 +1435,7 @@
>  	.devx_wq_query = mlx5_glue_devx_wq_query,
>  	.devx_port_query = mlx5_glue_devx_port_query,
>  	.dr_dump_domain = mlx5_glue_dr_dump_domain,
> +	.dr_dump_rule = mlx5_glue_dr_dump_single_rule,
>  	.dr_reclaim_domain_memory = mlx5_glue_dr_reclaim_domain_memory,
>  	.dr_create_flow_action_sampler =
>  		mlx5_glue_dr_create_flow_action_sampler,
> diff --git a/drivers/common/mlx5/linux/mlx5_glue.h b/drivers/common/mlx5/linux/mlx5_glue.h
> index 9e385be..97462e9 100644
> --- a/drivers/common/mlx5/linux/mlx5_glue.h
> +++ b/drivers/common/mlx5/linux/mlx5_glue.h
> @@ -313,6 +313,7 @@ struct mlx5_glue {
>  			       uint32_t port_num,
>  			       struct mlx5dv_devx_port *mlx5_devx_port);
>  	int (*dr_dump_domain)(FILE *file, void *domain);
> +	int (*dr_dump_rule)(FILE *file, void *rule);
>  	int (*devx_query_eqn)(struct ibv_context *context, uint32_t cpus,
>  			      uint32_t *eqn);
>  	struct mlx5dv_devx_event_channel *(*devx_create_event_channel)
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c b/drivers/common/mlx5/mlx5_devx_cmds.c
> index c90e020..c0b6fdb 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.c
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.c
> @@ -1579,6 +1579,20 @@ struct mlx5_devx_obj *
>  	return -ret;
>  }
>  
> +int
> +mlx5_devx_cmd_flow_single_dump(void *rule_info __rte_unused,
> +			FILE *file __rte_unused)
> +{
> +	int ret = 0;
> +#ifdef HAVE_MLX5_DR_FLOW_DUMP_RULE
> +	if (rule_info)
> +		ret = mlx5_glue->dr_dump_rule(file, rule_info);
> +#else
> +	ret = ENOTSUP;
> +#endif
> +	return -ret;
> +}
> +
>  /*
>   * Create CQ using DevX API.
>   *
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h b/drivers/common/mlx5/mlx5_devx_cmds.h
> index 2826c0b..f587d0c 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.h
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.h
> @@ -474,6 +474,8 @@ struct mlx5_devx_obj *mlx5_devx_cmd_create_tis(void *ctx,
>  int mlx5_devx_cmd_flow_dump(void *fdb_domain, void *rx_domain, void *tx_domain,
>  			    FILE *file);
>  __rte_internal
> +int mlx5_devx_cmd_flow_single_dump(void *rule, FILE *file);
> +__rte_internal
>  struct mlx5_devx_obj *mlx5_devx_cmd_create_cq(void *ctx,
>  					      struct mlx5_devx_cq_attr *attr);
>  __rte_internal
> diff --git a/drivers/common/mlx5/rte_common_mlx5_exports.def b/drivers/common/mlx5/rte_common_mlx5_exports.def
> index fd62b80..0e6d6d3 100644
> --- a/drivers/common/mlx5/rte_common_mlx5_exports.def
> +++ b/drivers/common/mlx5/rte_common_mlx5_exports.def
> @@ -20,6 +20,7 @@ EXPORTS
>  	mlx5_devx_cmd_flow_counter_alloc
>  	mlx5_devx_cmd_flow_counter_query
>  	mlx5_devx_cmd_flow_dump
> +	mlx5_devx_cmd_flow_single_dump
>  	mlx5_devx_cmd_mkey_create
>  	mlx5_devx_cmd_modify_qp_state
>  	mlx5_devx_cmd_modify_rq
> diff --git a/drivers/common/mlx5/version.map b/drivers/common/mlx5/version.map
> index 91f3fa5..4d49011 100644
> --- a/drivers/common/mlx5/version.map
> +++ b/drivers/common/mlx5/version.map
> @@ -28,6 +28,7 @@ INTERNAL {
>  	mlx5_devx_cmd_flow_counter_alloc;
>  	mlx5_devx_cmd_flow_counter_query;
>  	mlx5_devx_cmd_flow_dump;
> +	mlx5_devx_cmd_flow_single_dump;
>  	mlx5_devx_cmd_mkey_create;
>  	mlx5_devx_cmd_modify_qp_state;
>  	mlx5_devx_cmd_modify_rq;
> 

Re: [dpdk-dev] [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump feature

[Haifei Luo]

HI David,
      In current release, is the exports file necessary? I will keep the modification if yes. Thank you.

drivers/common/mlx5/rte_common_mlx5_exports.def 

-----Original Message-----
From: Kinsella, Ray <mdr@ashroe.eu> 
Sent: Wednesday, April 14, 2021 12:44 AM
To: Haifei Luo <haifeil@nvidia.com>; dev@dpdk.org
Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Neil Horman <nhorman@tuxdriver.com>; David Marchand <david.marchand@redhat.com>
Subject: Re: [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump feature

External email: Use caution opening links or attachments


On 07/04/2021 07:09, Haifei Luo wrote:
> add mlx5 APIs for single flow dump feature
>
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>  drivers/common/mlx5/linux/meson.build           |  2 ++
>  drivers/common/mlx5/linux/mlx5_glue.c           | 13 +++++++++++++
>  drivers/common/mlx5/linux/mlx5_glue.h           |  1 +
>  drivers/common/mlx5/mlx5_devx_cmds.c            | 14 ++++++++++++++
>  drivers/common/mlx5/mlx5_devx_cmds.h            |  2 ++
>  drivers/common/mlx5/rte_common_mlx5_exports.def |  1 +

Perhaps check with David Marchand on this, I suspect amendments to the exports file will soon be redundant.

>  drivers/common/mlx5/version.map                 |  1 +
>  7 files changed, 34 insertions(+)
>
> diff --git a/drivers/common/mlx5/linux/meson.build 
> b/drivers/common/mlx5/linux/meson.build
> index 220de35..3f42163 100644
> --- a/drivers/common/mlx5/linux/meson.build
> +++ b/drivers/common/mlx5/linux/meson.build
> @@ -186,6 +186,8 @@ has_sym_args = [
>       'mlx5dv_dr_action_create_aso' ],
>       [ 'HAVE_INFINIBAND_VERBS_H', 'infiniband/verbs.h',
>       'INFINIBAND_VERBS_H' ],
> +     [ 'HAVE_MLX5_DR_FLOW_DUMP_RULE', 'infiniband/mlx5dv.h',
> +     'mlx5dv_dump_dr_rule' ],
>  ]
>  config = configuration_data()
>  foreach arg:has_sym_args
> diff --git a/drivers/common/mlx5/linux/mlx5_glue.c 
> b/drivers/common/mlx5/linux/mlx5_glue.c
> index 964f7e7..d3bd645 100644
> --- a/drivers/common/mlx5/linux/mlx5_glue.c
> +++ b/drivers/common/mlx5/linux/mlx5_glue.c
> @@ -1101,6 +1101,18 @@
>  }
>
>  static int
> +mlx5_glue_dr_dump_single_rule(FILE *file, void *rule) { #ifdef 
> +HAVE_MLX5_DR_FLOW_DUMP_RULE
> +     return mlx5dv_dump_dr_rule(file, rule); #else
> +     RTE_SET_USED(file);
> +     RTE_SET_USED(rule);
> +     return -ENOTSUP;
> +#endif
> +}
> +
> +static int
>  mlx5_glue_dr_dump_domain(FILE *file, void *domain)  {  #ifdef 
> HAVE_MLX5_DR_FLOW_DUMP @@ -1423,6 +1435,7 @@
>       .devx_wq_query = mlx5_glue_devx_wq_query,
>       .devx_port_query = mlx5_glue_devx_port_query,
>       .dr_dump_domain = mlx5_glue_dr_dump_domain,
> +     .dr_dump_rule = mlx5_glue_dr_dump_single_rule,
>       .dr_reclaim_domain_memory = mlx5_glue_dr_reclaim_domain_memory,
>       .dr_create_flow_action_sampler =
>               mlx5_glue_dr_create_flow_action_sampler,
> diff --git a/drivers/common/mlx5/linux/mlx5_glue.h 
> b/drivers/common/mlx5/linux/mlx5_glue.h
> index 9e385be..97462e9 100644
> --- a/drivers/common/mlx5/linux/mlx5_glue.h
> +++ b/drivers/common/mlx5/linux/mlx5_glue.h
> @@ -313,6 +313,7 @@ struct mlx5_glue {
>                              uint32_t port_num,
>                              struct mlx5dv_devx_port *mlx5_devx_port);
>       int (*dr_dump_domain)(FILE *file, void *domain);
> +     int (*dr_dump_rule)(FILE *file, void *rule);
>       int (*devx_query_eqn)(struct ibv_context *context, uint32_t cpus,
>                             uint32_t *eqn);
>       struct mlx5dv_devx_event_channel *(*devx_create_event_channel) 
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c 
> b/drivers/common/mlx5/mlx5_devx_cmds.c
> index c90e020..c0b6fdb 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.c
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.c
> @@ -1579,6 +1579,20 @@ struct mlx5_devx_obj *
>       return -ret;
>  }
>
> +int
> +mlx5_devx_cmd_flow_single_dump(void *rule_info __rte_unused,
> +                     FILE *file __rte_unused) {
> +     int ret = 0;
> +#ifdef HAVE_MLX5_DR_FLOW_DUMP_RULE
> +     if (rule_info)
> +             ret = mlx5_glue->dr_dump_rule(file, rule_info); #else
> +     ret = ENOTSUP;
> +#endif
> +     return -ret;
> +}
> +
>  /*
>   * Create CQ using DevX API.
>   *
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h b/drivers/common/mlx5/mlx5_devx_cmds.h
> index 2826c0b..f587d0c 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.h
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.h
> @@ -474,6 +474,8 @@ struct mlx5_devx_obj *mlx5_devx_cmd_create_tis(void *ctx,
>  int mlx5_devx_cmd_flow_dump(void *fdb_domain, void *rx_domain, void *tx_domain,
>                           FILE *file);
>  __rte_internal
> +int mlx5_devx_cmd_flow_single_dump(void *rule, FILE *file);
> +__rte_internal
>  struct mlx5_devx_obj *mlx5_devx_cmd_create_cq(void *ctx,
>                                             struct mlx5_devx_cq_attr *attr);
>  __rte_internal
> diff --git a/drivers/common/mlx5/rte_common_mlx5_exports.def b/drivers/common/mlx5/rte_common_mlx5_exports.def
> index fd62b80..0e6d6d3 100644
> --- a/drivers/common/mlx5/rte_common_mlx5_exports.def
> +++ b/drivers/common/mlx5/rte_common_mlx5_exports.def
> @@ -20,6 +20,7 @@ EXPORTS
>       mlx5_devx_cmd_flow_counter_alloc
>       mlx5_devx_cmd_flow_counter_query
>       mlx5_devx_cmd_flow_dump
> +     mlx5_devx_cmd_flow_single_dump
>       mlx5_devx_cmd_mkey_create
>       mlx5_devx_cmd_modify_qp_state
>       mlx5_devx_cmd_modify_rq
> diff --git a/drivers/common/mlx5/version.map b/drivers/common/mlx5/version.map
> index 91f3fa5..4d49011 100644
> --- a/drivers/common/mlx5/version.map
> +++ b/drivers/common/mlx5/version.map
> @@ -28,6 +28,7 @@ INTERNAL {
>       mlx5_devx_cmd_flow_counter_alloc;
>       mlx5_devx_cmd_flow_counter_query;
>       mlx5_devx_cmd_flow_dump;
> +     mlx5_devx_cmd_flow_single_dump;
>       mlx5_devx_cmd_mkey_create;
>       mlx5_devx_cmd_modify_qp_state;
>       mlx5_devx_cmd_modify_rq;
>

[dpdk-dev] [PATCH v3 0/3] support single flow dump

[Haifei Luo]

Dump internal representation information of all flows is supported.
It is useful to dump one flow. To implement this requirement,
add this CLI to dump one rule: flow dump PORT rule ID
      and the CLI to dump all: flow dump PORT all
Examples:
    testpmd> flow dump 0 all
    testpmd> flow dump 0 rule 0
The first 0 is for port. The second one is for rule id.

For RTE API, add one arg rte_flow in rte_flow_dev_dump.
If rte_flow is null, it will dump information for all flows.
Otherwise, it will dump one.
Accordingly, add this arg in related dev_dump and driver APIs.

V2: fix comments about rte API. V1 has one API rte_flow_dump,
remove it and update rte_flow_dev_dump by adding one arg rte_flow.

V3: split into two series. One is for ethdev/testpmd/doc, the other is
for drivers.

Haifei Luo (3):
  ethdev: modify rte API for single flow dump
  app/testpmd: add CLIs for single flow dump feature
  doc: add single flow dump to guides

 app/test-pmd/cmdline_flow.c                 | 55 ++++++++++++++++++++---
 app/test-pmd/config.c                       | 38 ++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/nics/features/default.ini        |  1 +
 doc/guides/nics/features/mlx5.ini           |  1 +
 doc/guides/nics/mlx5.rst                    |  9 ++--
 doc/guides/prog_guide/rte_flow.rst          | 24 ++++++++++
 doc/guides/rel_notes/release_21_05.rst      | 69 ++---------------------------
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 ++-
 drivers/net/mlx5/linux/mlx5_socket.c        |  2 +-
 drivers/net/mlx5/mlx5.h                     |  4 +-
 drivers/net/mlx5/mlx5_flow.c                |  9 ++--
 drivers/net/octeontx2/otx2_flow.c           |  9 +++-
 lib/librte_ethdev/rte_flow.c                |  5 ++-
 lib/librte_ethdev/rte_flow.h                |  5 ++-
 lib/librte_ethdev/rte_flow_driver.h         |  1 +
 16 files changed, 151 insertions(+), 90 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v3 1/3] ethdev: modify rte API for single flow dump

[Haifei Luo]

Previous implementations support dump all the flows. Add new arg
rte_flow in rte_flow_dev_dump to dump one flow.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/config.c                |  2 +-
 doc/guides/nics/mlx5.rst             |  9 ++++++---
 doc/guides/prog_guide/rte_flow.rst   | 24 ++++++++++++++++++++++++
 drivers/net/mlx5/linux/mlx5_socket.c |  2 +-
 drivers/net/mlx5/mlx5.h              |  4 ++--
 drivers/net/mlx5/mlx5_flow.c         |  9 ++++++---
 drivers/net/octeontx2/otx2_flow.c    |  9 ++++++++-
 lib/librte_ethdev/rte_flow.c         |  5 +++--
 lib/librte_ethdev/rte_flow.h         |  5 ++++-
 lib/librte_ethdev/rte_flow_driver.h  |  1 +
 10 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a5e82b7..ca34a63 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1932,7 +1932,7 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, file, &error);
+	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 490329a..7ff92b0 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1837,13 +1837,16 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       testpmd> flow dump <port> <output_file>
+       To dump all flows:
+       testpmd> flow dump <port> all <output_file>
+       and dump one flow:
+       testpmd> flow dump <port> rule <rule_id> <output_file>
 
    - call rte_flow_dev_dump api:
 
    .. code-block:: console
 
-       rte_flow_dev_dump(port, file, NULL);
+       rte_flow_dev_dump(port, flow, file, NULL);
 
 #. Dump human-readable flows from raw file:
 
@@ -1851,4 +1854,4 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       mlx_steering_dump.py -f <output_file>
+       mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index e1b93ec..2ed149e 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3025,6 +3025,30 @@ Return values:
 
 - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
 
+Dump
+~~~~~
+
+Dump information for all or one flows.
+
+.. code-block:: c
+
+   int
+   rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+                     FILE *file,
+                     struct rte_flow_error *error);
+
+Arguments:
+
+- ``port_id``: port identifier of Ethernet device.
+- ``flow``: flow rule handle to dump. NULL to dump all.
+- ``file``: a pointer to a file for output
+- ``error``: perform verbose error reporting if not NULL. PMDs initialize
+  this structure in case of error only.
+
+Return values:
+
+- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
+
 Query
 ~~~~~
 
diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index b1f41bc..6e354f4 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -84,7 +84,7 @@
 	}
 	/* Dump flow. */
 	dev = &rte_eth_devices[port_id];
-	ret = mlx5_flow_dev_dump(dev, file, NULL);
+	ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
 	/* Set-up the ancillary data and reply. */
 	msg.msg_controllen = 0;
 	msg.msg_control = NULL;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 0f69f9d..e0f7101 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1245,8 +1245,8 @@ void mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
 void mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt);
 int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		       bool clear, uint64_t *pkts, uint64_t *bytes);
-int mlx5_flow_dev_dump(struct rte_eth_dev *dev, FILE *file,
-		       struct rte_flow_error *error);
+int mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow,
+			FILE *file, struct rte_flow_error *error);
 void mlx5_flow_rxq_dynf_metadata_set(struct rte_eth_dev *dev);
 int mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 668c32c..a8cf674 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7154,7 +7154,7 @@ struct mlx5_meter_domains_infos *
  *   0 on success, a nagative value otherwise.
  */
 int
-mlx5_flow_dev_dump(struct rte_eth_dev *dev,
+mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow_idx,
 		   FILE *file,
 		   struct rte_flow_error *error __rte_unused)
 {
@@ -7166,8 +7166,11 @@ struct mlx5_meter_domains_infos *
 			return -errno;
 		return -ENOTSUP;
 	}
-	return mlx5_devx_cmd_flow_dump(sh->fdb_domain, sh->rx_domain,
-				       sh->tx_domain, file);
+
+	if (!flow_idx)
+		return mlx5_devx_cmd_flow_dump(sh->fdb_domain,
+				sh->rx_domain, sh->tx_domain, file);
+	return -ENOTSUP;
 }
 
 /**
diff --git a/drivers/net/octeontx2/otx2_flow.c b/drivers/net/octeontx2/otx2_flow.c
index 14ac9bc..1c90d75 100644
--- a/drivers/net/octeontx2/otx2_flow.c
+++ b/drivers/net/octeontx2/otx2_flow.c
@@ -807,7 +807,7 @@
 
 static int
 otx2_flow_dev_dump(struct rte_eth_dev *dev,
-		  FILE *file,
+		  struct rte_flow *flow, FILE *file,
 		  struct rte_flow_error *error)
 {
 	struct otx2_eth_dev *hw = dev->data->dev_private;
@@ -822,6 +822,13 @@
 				   "Invalid file");
 		return -EINVAL;
 	}
+	if (flow != NULL) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE,
+				   NULL,
+				   "Invalid argument");
+		return -EINVAL;
+	}
 
 	max_prio = hw->npc_flow.flow_max_priority;
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index e07e617..7241f00 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -1027,7 +1027,8 @@ enum rte_flow_conv_item_spec_type {
 }
 
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error)
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+			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);
@@ -1037,7 +1038,7 @@ enum rte_flow_conv_item_spec_type {
 		return -rte_errno;
 	if (likely(!!ops->dev_dump)) {
 		fts_enter(dev);
-		ret = ops->dev_dump(dev, file, error);
+		ret = ops->dev_dump(dev, flow, file, error);
 		fts_exit(dev);
 		return flow_err(port_id, ret, error);
 	}
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index c476a0f..53d1aa5 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -3232,6 +3232,8 @@ enum rte_flow_conv_op {
  *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
+ * @param[in] flow
+ *   The pointer of rte flow.
  * @param[in] file
  *   A pointer to a file for output.
  * @param[out] error
@@ -3242,7 +3244,8 @@ enum rte_flow_conv_op {
  */
 __rte_experimental
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error);
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+		FILE *file, struct rte_flow_error *error);
 
 /**
  * Check if mbuf dynamic field for metadata is registered.
diff --git a/lib/librte_ethdev/rte_flow_driver.h b/lib/librte_ethdev/rte_flow_driver.h
index da594d9..6ae1f8c 100644
--- a/lib/librte_ethdev/rte_flow_driver.h
+++ b/lib/librte_ethdev/rte_flow_driver.h
@@ -75,6 +75,7 @@ struct rte_flow_ops {
 	/** See rte_flow_dev_dump(). */
 	int (*dev_dump)
 		(struct rte_eth_dev *dev,
+		 struct rte_flow *flow,
 		 FILE *file,
 		 struct rte_flow_error *error);
 	/** See rte_flow_get_aged_flows() */
-- 
1.8.3.1

[dpdk-dev] [PATCH v3 2/3] app/testpmd: add CLIs for single flow dump feature

[Haifei Luo]

Add support for single flow dump.
The CLIs to dump one rule: flow dump PORT rule ID
to dump all: flow dump PORT all
Examples:
testpmd> flow dump 0 all
testpmd> flow dump 0 rule 0

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 4 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index fb7a3a8..0127d9e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -108,6 +108,10 @@ enum index {
 	TUNNEL_SET,
 	TUNNEL_MATCH,
 
+	/* Dump arguments */
+	DUMP_ALL,
+	DUMP_ONE,
+
 	/* Shared action arguments */
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -793,6 +797,8 @@ struct buffer {
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
+			bool mode;
+			uint32_t rule;
 		} dump; /**< Dump arguments. */
 		struct {
 			uint32_t rule;
@@ -844,6 +850,12 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index next_dump_subcmd[] = {
+	DUMP_ALL,
+	DUMP_ONE,
+	ZERO,
+};
+
 static const enum index next_sa_subcmd[] = {
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -2032,10 +2044,9 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	},
 	[DUMP] = {
 		.name = "dump",
-		.help = "dump all flow rules to file",
-		.next = NEXT(next_dump_attr, NEXT_ENTRY(PORT_ID)),
-		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
-			     ARGS_ENTRY(struct buffer, port)),
+		.help = "dump single/all flow rules to file",
+		.next = NEXT(next_dump_subcmd, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_dump,
 	},
 	[QUERY] = {
@@ -2125,6 +2136,22 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.destroy.rule)),
 		.call = parse_destroy,
 	},
+	/* Dump arguments. */
+	[DUMP_ALL] = {
+		.name = "all",
+		.help = "dump all",
+		.next = NEXT(next_dump_attr),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file)),
+		.call = parse_dump,
+	},
+	[DUMP_ONE] = {
+		.name = "rule",
+		.help = "dump one rule",
+		.next = NEXT(next_dump_attr, NEXT_ENTRY(RULE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
+				ARGS_ENTRY(struct buffer, args.dump.rule)),
+		.call = parse_dump,
+	},
 	/* Query arguments. */
 	[QUERY_ACTION] = {
 		.name = "{action}",
@@ -6364,8 +6391,20 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		ctx->objdata = 0;
 		ctx->object = out;
 		ctx->objmask = NULL;
+		return len;
+	}
+	switch (ctx->curr) {
+	case DUMP_ALL:
+	case DUMP_ONE:
+		out->args.dump.mode = (ctx->curr == DUMP_ALL) ? true : false;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+		return len;
+	default:
+		return -1;
 	}
-	return len;
 }
 
 /** Parse tokens for query command. */
@@ -7659,8 +7698,10 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
-	case DUMP:
-		port_flow_dump(in->port, in->args.dump.file);
+	case DUMP_ONE:
+	case DUMP_ALL:
+		port_flow_dump(in->port, in->args.dump.mode,
+				in->args.dump.rule, in->args.dump.file);
 		break;
 	case QUERY:
 		port_flow_query(in->port, in->args.query.rule,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index ca34a63..40b2b29 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1916,13 +1916,41 @@ struct rte_flow_shared_action *
 	return ret;
 }
 
-/** Dump all flow rules. */
+/** Dump flow rules. */
 int
-port_flow_dump(portid_t port_id, const char *file_name)
+port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
+		const char *file_name)
 {
 	int ret = 0;
 	FILE *file = stdout;
 	struct rte_flow_error error;
+	struct rte_port *port;
+	struct port_flow *pflow;
+	struct rte_flow *tmpFlow = NULL;
+	bool found = false;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+
+	if (!dump_all) {
+		port = &ports[port_id];
+		pflow = port->flow_list;
+		while (pflow) {
+			if (rule_id != pflow->id) {
+				pflow = pflow->next;
+			} else {
+				tmpFlow = pflow->flow;
+				if (tmpFlow)
+					found = true;
+				break;
+			}
+		}
+		if (found == false) {
+			printf("Failed to dump to flow %d\n", rule_id);
+			return -EINVAL;
+		}
+	}
 
 	if (file_name && strlen(file_name)) {
 		file = fopen(file_name, "w");
@@ -1932,7 +1960,11 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
+
+	if (!dump_all)
+		ret = rte_flow_dev_dump(port_id, tmpFlow, file, &error);
+	else
+		ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a87ccb0..36d8535 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -825,7 +825,8 @@ void update_age_action_context(const struct rte_flow_action *actions,
 		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
-int port_flow_dump(portid_t port_id, const char *file_name);
+int port_flow_dump(portid_t port_id, bool dump_all,
+			uint32_t rule, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_t rule,
 		    const struct rte_flow_action *action);
 void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 36f0a32..5454524 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3332,7 +3332,11 @@ following sections.
 
 - Dump internal representation information of all flows in hardware::
 
-   flow dump {port_id} {output_file}
+   flow dump {port_id} all {output_file}
+
+   for one flow :
+
+   flow dump {port_id} rule {rule_id} {output_file}
 
 - List and destroy aged flow rules::
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v3 3/3] doc: add single flow dump to guides

[Haifei Luo]

Add "Flow dump" in features/default.ini and features/mlx5.ini.
Add testpmd CLI and API changes in release_notes.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 doc/guides/nics/features/default.ini   | 1 +
 doc/guides/nics/features/mlx5.ini      | 1 +
 doc/guides/rel_notes/release_21_05.rst | 5 +++++
 3 files changed, 7 insertions(+)

diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index 8046bd1..49aaf17 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -39,6 +39,7 @@ DCB                  =
 VLAN filter          =
 Flow control         =
 Flow API             =
+Flow dump            =
 Rate limitation      =
 Traffic mirroring    =
 Inline crypto        =
diff --git a/doc/guides/nics/features/mlx5.ini b/doc/guides/nics/features/mlx5.ini
index ddd131d..3c5fcff 100644
--- a/doc/guides/nics/features/mlx5.ini
+++ b/doc/guides/nics/features/mlx5.ini
@@ -29,6 +29,7 @@ SR-IOV               = Y
 VLAN filter          = Y
 Flow control         = Y
 Flow API             = Y
+Flow dump            = Y
 CRC offload          = Y
 VLAN offload         = Y
 L3 checksum offload  = Y
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index a0b9079..e3ae470 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -174,6 +174,8 @@ New Features
     ``dpdk-testpmd -- --eth-link-speed N``
   * Added command to display Rx queue used descriptor count.
     ``show port (port_id) rxq (queue_id) desc used count``
+  * Added command to dump internal representation information of single flow.
+    ``flow dump (port_id) rule (rule_id)``
 
 
 Removed Items
@@ -207,6 +209,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* ethdev: Added a rte_flow pointer parameter to the function
+  ``rte_flow_dev_dump()`` allowing dump for single flow.
+
 * eal: The experimental TLS API added in ``rte_thread.h`` has been renamed
   from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
   and confusion with the transport layer security term.
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump feature

[David Marchand]

On Wed, Apr 14, 2021 at 4:40 AM Haifei Luo <haifeil@nvidia.com> wrote:
>
> HI David,
>       In current release, is the exports file necessary? I will keep the modification if yes. Thank you.
>
> drivers/common/mlx5/rte_common_mlx5_exports.def

It depends on the tree against which you send your patches.

If this subtree (like next-net, or next-net-mlx) has been rebased on
56ea803e87 - build: remove Windows export symbol list (6 days ago)
<David Marchand>

Then no .def file is needed.

Else, you should keep this .def update.


In any case, this is easy to fix for maintainers who take the patch,
since in your case, the .def update will trigger a conflict which can
be just dropped.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump feature

[Haifei Luo]

HI David, 
   Thank you for the description and I will modify accordingly.

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Wednesday, April 14, 2021 2:21 PM
To: Haifei Luo <haifeil@nvidia.com>
Cc: Kinsella, Ray <mdr@ashroe.eu>; dev@dpdk.org; Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Neil Horman <nhorman@tuxdriver.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; NBU-Contact-Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2 3/5] common/mlx5: add mlx5 APIs for single flow dump feature

External email: Use caution opening links or attachments


On Wed, Apr 14, 2021 at 4:40 AM Haifei Luo <haifeil@nvidia.com> wrote:
>
> HI David,
>       In current release, is the exports file necessary? I will keep the modification if yes. Thank you.
>
> drivers/common/mlx5/rte_common_mlx5_exports.def

It depends on the tree against which you send your patches.

If this subtree (like next-net, or next-net-mlx) has been rebased on
56ea803e87 - build: remove Windows export symbol list (6 days ago) <David Marchand>

Then no .def file is needed.

Else, you should keep this .def update.


In any case, this is easy to fix for maintainers who take the patch, since in your case, the .def update will trigger a conflict which can be just dropped.


--
David Marchand

Re: [dpdk-dev] [PATCH v3 3/3] doc: add single flow dump to guides

[Ferruh Yigit]

On 4/14/2021 7:13 AM, Haifei Luo wrote:
> Add "Flow dump" in features/default.ini and features/mlx5.ini.
> Add testpmd CLI and API changes in release_notes.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>   doc/guides/nics/features/default.ini   | 1 +
>   doc/guides/nics/features/mlx5.ini      | 1 +
>   doc/guides/rel_notes/release_21_05.rst | 5 +++++
>   3 files changed, 7 insertions(+)
> 
> diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
> index 8046bd1..49aaf17 100644
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> @@ -39,6 +39,7 @@ DCB                  =
>   VLAN filter          =
>   Flow control         =
>   Flow API             =
> +Flow dump            =

Hi Haifei,

I don't think "flow dump" is important enough to be listed in the feature list, 
I am for not adding it.
Comparing the one line above item, "Flow API", this one is small and indeed 
subset of it.

>   Rate limitation      =
>   Traffic mirroring    =
>   Inline crypto        =
> diff --git a/doc/guides/nics/features/mlx5.ini b/doc/guides/nics/features/mlx5.ini
> index ddd131d..3c5fcff 100644
> --- a/doc/guides/nics/features/mlx5.ini
> +++ b/doc/guides/nics/features/mlx5.ini
> @@ -29,6 +29,7 @@ SR-IOV               = Y
>   VLAN filter          = Y
>   Flow control         = Y
>   Flow API             = Y
> +Flow dump            = Y
>   CRC offload          = Y
>   VLAN offload         = Y
>   L3 checksum offload  = Y
> diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
> index a0b9079..e3ae470 100644
> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> @@ -174,6 +174,8 @@ New Features
>       ``dpdk-testpmd -- --eth-link-speed N``
>     * Added command to display Rx queue used descriptor count.
>       ``show port (port_id) rxq (queue_id) desc used count``
> +  * Added command to dump internal representation information of single flow.
> +    ``flow dump (port_id) rule (rule_id)``
>   
>   
>   Removed Items
> @@ -207,6 +209,9 @@ API Changes
>      Also, make sure to start the actual text at the margin.
>      =======================================================
>   
> +* ethdev: Added a rte_flow pointer parameter to the function
> +  ``rte_flow_dev_dump()`` allowing dump for single flow.
> +
>   * eal: The experimental TLS API added in ``rte_thread.h`` has been renamed
>     from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
>     and confusion with the transport layer security term.
> 

Can you please distribute the release notes updates to the patches that 
introduces the change? So there should be a separate release notes update patch.

Thanks.

Re: [dpdk-dev] [PATCH v3 3/3] doc: add single flow dump to guides

[Haifei Luo]

Hi Ferruh,
   Okay ,  I will remove it from feature list. Thank you.

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Wednesday, April 14, 2021 4:24 PM
To: Haifei Luo <haifeil@nvidia.com>; dev@dpdk.org
Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; ajit.khaparde@broadcom.com; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
Subject: Re: [PATCH v3 3/3] doc: add single flow dump to guides

External email: Use caution opening links or attachments


On 4/14/2021 7:13 AM, Haifei Luo wrote:
> Add "Flow dump" in features/default.ini and features/mlx5.ini.
> Add testpmd CLI and API changes in release_notes.
>
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>   doc/guides/nics/features/default.ini   | 1 +
>   doc/guides/nics/features/mlx5.ini      | 1 +
>   doc/guides/rel_notes/release_21_05.rst | 5 +++++
>   3 files changed, 7 insertions(+)
>
> diff --git a/doc/guides/nics/features/default.ini 
> b/doc/guides/nics/features/default.ini
> index 8046bd1..49aaf17 100644
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> @@ -39,6 +39,7 @@ DCB                  =
>   VLAN filter          =
>   Flow control         =
>   Flow API             =
> +Flow dump            =

Hi Haifei,

I don't think "flow dump" is important enough to be listed in the feature list, I am for not adding it.
Comparing the one line above item, "Flow API", this one is small and indeed subset of it.

>   Rate limitation      =
>   Traffic mirroring    =
>   Inline crypto        =
> diff --git a/doc/guides/nics/features/mlx5.ini 
> b/doc/guides/nics/features/mlx5.ini
> index ddd131d..3c5fcff 100644
> --- a/doc/guides/nics/features/mlx5.ini
> +++ b/doc/guides/nics/features/mlx5.ini
> @@ -29,6 +29,7 @@ SR-IOV               = Y
>   VLAN filter          = Y
>   Flow control         = Y
>   Flow API             = Y
> +Flow dump            = Y
>   CRC offload          = Y
>   VLAN offload         = Y
>   L3 checksum offload  = Y
> diff --git a/doc/guides/rel_notes/release_21_05.rst 
> b/doc/guides/rel_notes/release_21_05.rst
> index a0b9079..e3ae470 100644
> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> @@ -174,6 +174,8 @@ New Features
>       ``dpdk-testpmd -- --eth-link-speed N``
>     * Added command to display Rx queue used descriptor count.
>       ``show port (port_id) rxq (queue_id) desc used count``
> +  * Added command to dump internal representation information of single flow.
> +    ``flow dump (port_id) rule (rule_id)``
>
>
>   Removed Items
> @@ -207,6 +209,9 @@ API Changes
>      Also, make sure to start the actual text at the margin.
>      =======================================================
>
> +* ethdev: Added a rte_flow pointer parameter to the function
> +  ``rte_flow_dev_dump()`` allowing dump for single flow.
> +
>   * eal: The experimental TLS API added in ``rte_thread.h`` has been renamed
>     from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
>     and confusion with the transport layer security term.
>

Can you please distribute the release notes updates to the patches that introduces the change? So there should be a separate release notes update patch.

Thanks.

[dpdk-dev] [PATCH v4 0/3] support single flow dump

[Haifei Luo]

Dump internal representation information of all flows is supported.
It is useful to dump one flow. To implement this requirement,
add this CLI to dump one rule: flow dump PORT rule ID
      and the CLI to dump all: flow dump PORT all
Examples:
    testpmd> flow dump 0 all
    testpmd> flow dump 0 rule 0
The first 0 is for port. The second one is for rule id.

For RTE API, add one arg rte_flow in rte_flow_dev_dump.
If rte_flow is null, it will dump information for all flows.
Otherwise, it will dump one.
Accordingly, add this arg in related dev_dump and driver APIs.

V2: fix comments about rte API. V1 has one API rte_flow_dump,
remove it and update rte_flow_dev_dump by adding one arg rte_flow.

V3: split into two series. One is for ethdev/testpmd/doc, the other is
for drivers.

V4: Fix comments. Remove "Flow dump" from features/default.ini and
features/mlx5.ini.

Haifei Luo (3):
  ethdev: modify rte API for single flow dump
  app/testpmd: add CLIs for single flow dump feature
  doc: add single flow dump to guides

 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/nics/mlx5.rst                    |  9 +++--
 doc/guides/prog_guide/rte_flow.rst          | 24 +++++++++++++
 doc/guides/rel_notes/release_21_05.rst      |  5 +++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 drivers/net/mlx5/linux/mlx5_socket.c        |  2 +-
 drivers/net/mlx5/mlx5.h                     |  4 +--
 drivers/net/mlx5/mlx5_flow.c                |  9 +++--
 drivers/net/octeontx2/otx2_flow.c           |  9 ++++-
 lib/librte_ethdev/rte_flow.c                |  5 +--
 lib/librte_ethdev/rte_flow.h                |  5 ++-
 lib/librte_ethdev/rte_flow_driver.h         |  1 +
 14 files changed, 150 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v4 1/3] ethdev: modify rte API for single flow dump

[Haifei Luo]

Previous implementations support dump all the flows. Add new arg
rte_flow in rte_flow_dev_dump to dump one flow.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/config.c                |  2 +-
 doc/guides/nics/mlx5.rst             |  9 ++++++---
 doc/guides/prog_guide/rte_flow.rst   | 24 ++++++++++++++++++++++++
 drivers/net/mlx5/linux/mlx5_socket.c |  2 +-
 drivers/net/mlx5/mlx5.h              |  4 ++--
 drivers/net/mlx5/mlx5_flow.c         |  9 ++++++---
 drivers/net/octeontx2/otx2_flow.c    |  9 ++++++++-
 lib/librte_ethdev/rte_flow.c         |  5 +++--
 lib/librte_ethdev/rte_flow.h         |  5 ++++-
 lib/librte_ethdev/rte_flow_driver.h  |  1 +
 10 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a5e82b7..ca34a63 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1932,7 +1932,7 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, file, &error);
+	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 490329a..7ff92b0 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1837,13 +1837,16 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       testpmd> flow dump <port> <output_file>
+       To dump all flows:
+       testpmd> flow dump <port> all <output_file>
+       and dump one flow:
+       testpmd> flow dump <port> rule <rule_id> <output_file>
 
    - call rte_flow_dev_dump api:
 
    .. code-block:: console
 
-       rte_flow_dev_dump(port, file, NULL);
+       rte_flow_dev_dump(port, flow, file, NULL);
 
 #. Dump human-readable flows from raw file:
 
@@ -1851,4 +1854,4 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       mlx_steering_dump.py -f <output_file>
+       mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index e1b93ec..2ed149e 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3025,6 +3025,30 @@ Return values:
 
 - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
 
+Dump
+~~~~~
+
+Dump information for all or one flows.
+
+.. code-block:: c
+
+   int
+   rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+                     FILE *file,
+                     struct rte_flow_error *error);
+
+Arguments:
+
+- ``port_id``: port identifier of Ethernet device.
+- ``flow``: flow rule handle to dump. NULL to dump all.
+- ``file``: a pointer to a file for output
+- ``error``: perform verbose error reporting if not NULL. PMDs initialize
+  this structure in case of error only.
+
+Return values:
+
+- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
+
 Query
 ~~~~~
 
diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index b1f41bc..6e354f4 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -84,7 +84,7 @@
 	}
 	/* Dump flow. */
 	dev = &rte_eth_devices[port_id];
-	ret = mlx5_flow_dev_dump(dev, file, NULL);
+	ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
 	/* Set-up the ancillary data and reply. */
 	msg.msg_controllen = 0;
 	msg.msg_control = NULL;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 0f69f9d..e0f7101 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1245,8 +1245,8 @@ void mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
 void mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt);
 int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		       bool clear, uint64_t *pkts, uint64_t *bytes);
-int mlx5_flow_dev_dump(struct rte_eth_dev *dev, FILE *file,
-		       struct rte_flow_error *error);
+int mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow,
+			FILE *file, struct rte_flow_error *error);
 void mlx5_flow_rxq_dynf_metadata_set(struct rte_eth_dev *dev);
 int mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 668c32c..a8cf674 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7154,7 +7154,7 @@ struct mlx5_meter_domains_infos *
  *   0 on success, a nagative value otherwise.
  */
 int
-mlx5_flow_dev_dump(struct rte_eth_dev *dev,
+mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow_idx,
 		   FILE *file,
 		   struct rte_flow_error *error __rte_unused)
 {
@@ -7166,8 +7166,11 @@ struct mlx5_meter_domains_infos *
 			return -errno;
 		return -ENOTSUP;
 	}
-	return mlx5_devx_cmd_flow_dump(sh->fdb_domain, sh->rx_domain,
-				       sh->tx_domain, file);
+
+	if (!flow_idx)
+		return mlx5_devx_cmd_flow_dump(sh->fdb_domain,
+				sh->rx_domain, sh->tx_domain, file);
+	return -ENOTSUP;
 }
 
 /**
diff --git a/drivers/net/octeontx2/otx2_flow.c b/drivers/net/octeontx2/otx2_flow.c
index 14ac9bc..1c90d75 100644
--- a/drivers/net/octeontx2/otx2_flow.c
+++ b/drivers/net/octeontx2/otx2_flow.c
@@ -807,7 +807,7 @@
 
 static int
 otx2_flow_dev_dump(struct rte_eth_dev *dev,
-		  FILE *file,
+		  struct rte_flow *flow, FILE *file,
 		  struct rte_flow_error *error)
 {
 	struct otx2_eth_dev *hw = dev->data->dev_private;
@@ -822,6 +822,13 @@
 				   "Invalid file");
 		return -EINVAL;
 	}
+	if (flow != NULL) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE,
+				   NULL,
+				   "Invalid argument");
+		return -EINVAL;
+	}
 
 	max_prio = hw->npc_flow.flow_max_priority;
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index e07e617..7241f00 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -1027,7 +1027,8 @@ enum rte_flow_conv_item_spec_type {
 }
 
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error)
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+			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);
@@ -1037,7 +1038,7 @@ enum rte_flow_conv_item_spec_type {
 		return -rte_errno;
 	if (likely(!!ops->dev_dump)) {
 		fts_enter(dev);
-		ret = ops->dev_dump(dev, file, error);
+		ret = ops->dev_dump(dev, flow, file, error);
 		fts_exit(dev);
 		return flow_err(port_id, ret, error);
 	}
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index c476a0f..53d1aa5 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -3232,6 +3232,8 @@ enum rte_flow_conv_op {
  *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
+ * @param[in] flow
+ *   The pointer of rte flow.
  * @param[in] file
  *   A pointer to a file for output.
  * @param[out] error
@@ -3242,7 +3244,8 @@ enum rte_flow_conv_op {
  */
 __rte_experimental
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error);
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+		FILE *file, struct rte_flow_error *error);
 
 /**
  * Check if mbuf dynamic field for metadata is registered.
diff --git a/lib/librte_ethdev/rte_flow_driver.h b/lib/librte_ethdev/rte_flow_driver.h
index da594d9..6ae1f8c 100644
--- a/lib/librte_ethdev/rte_flow_driver.h
+++ b/lib/librte_ethdev/rte_flow_driver.h
@@ -75,6 +75,7 @@ struct rte_flow_ops {
 	/** See rte_flow_dev_dump(). */
 	int (*dev_dump)
 		(struct rte_eth_dev *dev,
+		 struct rte_flow *flow,
 		 FILE *file,
 		 struct rte_flow_error *error);
 	/** See rte_flow_get_aged_flows() */
-- 
1.8.3.1

[dpdk-dev] [PATCH v4 2/3] app/testpmd: add CLIs for single flow dump feature

[Haifei Luo]

Add support for single flow dump.
The CLIs to dump one rule: flow dump PORT rule ID
to dump all: flow dump PORT all
Examples:
testpmd> flow dump 0 all
testpmd> flow dump 0 rule 0

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 4 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index fb7a3a8..0127d9e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -108,6 +108,10 @@ enum index {
 	TUNNEL_SET,
 	TUNNEL_MATCH,
 
+	/* Dump arguments */
+	DUMP_ALL,
+	DUMP_ONE,
+
 	/* Shared action arguments */
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -793,6 +797,8 @@ struct buffer {
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
+			bool mode;
+			uint32_t rule;
 		} dump; /**< Dump arguments. */
 		struct {
 			uint32_t rule;
@@ -844,6 +850,12 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index next_dump_subcmd[] = {
+	DUMP_ALL,
+	DUMP_ONE,
+	ZERO,
+};
+
 static const enum index next_sa_subcmd[] = {
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -2032,10 +2044,9 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	},
 	[DUMP] = {
 		.name = "dump",
-		.help = "dump all flow rules to file",
-		.next = NEXT(next_dump_attr, NEXT_ENTRY(PORT_ID)),
-		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
-			     ARGS_ENTRY(struct buffer, port)),
+		.help = "dump single/all flow rules to file",
+		.next = NEXT(next_dump_subcmd, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_dump,
 	},
 	[QUERY] = {
@@ -2125,6 +2136,22 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.destroy.rule)),
 		.call = parse_destroy,
 	},
+	/* Dump arguments. */
+	[DUMP_ALL] = {
+		.name = "all",
+		.help = "dump all",
+		.next = NEXT(next_dump_attr),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file)),
+		.call = parse_dump,
+	},
+	[DUMP_ONE] = {
+		.name = "rule",
+		.help = "dump one rule",
+		.next = NEXT(next_dump_attr, NEXT_ENTRY(RULE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
+				ARGS_ENTRY(struct buffer, args.dump.rule)),
+		.call = parse_dump,
+	},
 	/* Query arguments. */
 	[QUERY_ACTION] = {
 		.name = "{action}",
@@ -6364,8 +6391,20 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		ctx->objdata = 0;
 		ctx->object = out;
 		ctx->objmask = NULL;
+		return len;
+	}
+	switch (ctx->curr) {
+	case DUMP_ALL:
+	case DUMP_ONE:
+		out->args.dump.mode = (ctx->curr == DUMP_ALL) ? true : false;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+		return len;
+	default:
+		return -1;
 	}
-	return len;
 }
 
 /** Parse tokens for query command. */
@@ -7659,8 +7698,10 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
-	case DUMP:
-		port_flow_dump(in->port, in->args.dump.file);
+	case DUMP_ONE:
+	case DUMP_ALL:
+		port_flow_dump(in->port, in->args.dump.mode,
+				in->args.dump.rule, in->args.dump.file);
 		break;
 	case QUERY:
 		port_flow_query(in->port, in->args.query.rule,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index ca34a63..40b2b29 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1916,13 +1916,41 @@ struct rte_flow_shared_action *
 	return ret;
 }
 
-/** Dump all flow rules. */
+/** Dump flow rules. */
 int
-port_flow_dump(portid_t port_id, const char *file_name)
+port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
+		const char *file_name)
 {
 	int ret = 0;
 	FILE *file = stdout;
 	struct rte_flow_error error;
+	struct rte_port *port;
+	struct port_flow *pflow;
+	struct rte_flow *tmpFlow = NULL;
+	bool found = false;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+
+	if (!dump_all) {
+		port = &ports[port_id];
+		pflow = port->flow_list;
+		while (pflow) {
+			if (rule_id != pflow->id) {
+				pflow = pflow->next;
+			} else {
+				tmpFlow = pflow->flow;
+				if (tmpFlow)
+					found = true;
+				break;
+			}
+		}
+		if (found == false) {
+			printf("Failed to dump to flow %d\n", rule_id);
+			return -EINVAL;
+		}
+	}
 
 	if (file_name && strlen(file_name)) {
 		file = fopen(file_name, "w");
@@ -1932,7 +1960,11 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
+
+	if (!dump_all)
+		ret = rte_flow_dev_dump(port_id, tmpFlow, file, &error);
+	else
+		ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a87ccb0..36d8535 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -825,7 +825,8 @@ void update_age_action_context(const struct rte_flow_action *actions,
 		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
-int port_flow_dump(portid_t port_id, const char *file_name);
+int port_flow_dump(portid_t port_id, bool dump_all,
+			uint32_t rule, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_t rule,
 		    const struct rte_flow_action *action);
 void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 36f0a32..5454524 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3332,7 +3332,11 @@ following sections.
 
 - Dump internal representation information of all flows in hardware::
 
-   flow dump {port_id} {output_file}
+   flow dump {port_id} all {output_file}
+
+   for one flow :
+
+   flow dump {port_id} rule {rule_id} {output_file}
 
 - List and destroy aged flow rules::
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v4 3/3] doc: add single flow dump to guides

[Haifei Luo]

Add testpmd CLI and API changes in release_notes.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 doc/guides/rel_notes/release_21_05.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index a0b9079..e3ae470 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -174,6 +174,8 @@ New Features
     ``dpdk-testpmd -- --eth-link-speed N``
   * Added command to display Rx queue used descriptor count.
     ``show port (port_id) rxq (queue_id) desc used count``
+  * Added command to dump internal representation information of single flow.
+    ``flow dump (port_id) rule (rule_id)``
 
 
 Removed Items
@@ -207,6 +209,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* ethdev: Added a rte_flow pointer parameter to the function
+  ``rte_flow_dev_dump()`` allowing dump for single flow.
+
 * eal: The experimental TLS API added in ``rte_thread.h`` has been renamed
   from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
   and confusion with the transport layer security term.
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v4 1/3] ethdev: modify rte API for single flow dump

[Thomas Monjalon]

About the title, what is "rte API"?
I guess you mean DPDK API with rte prefix.
But given all DPDK API have rte prefix,
and this patch is for DPDK,
you can just say "API".

So the title can be:
	ethdev: modify API for single flow dump

But it can look as single flow dump was possible before,
which is wrong because it is a new feature.

Another idea is wrong:
The packets of the flow are not dumped,
it is only dumping the HW representation of the flow rule.

I propose to simply describe the new feature:
	ethdev: dump single flow rule


> + * @param[in] flow
> + *   The pointer of rte flow.

Please replace "rte flow" with 'flow rule".
Is it allowed to pass NULL? Will it dump all?
If yes, you can change to:
	Flow rule to dump.
	Dump all rules if NULL.

Re: [dpdk-dev] [PATCH v4 1/3] ethdev: modify rte API for single flow dump

[Thomas Monjalon]

14/04/2021 10:41, Haifei Luo:
> Previous implementations support dump all the flows. Add new arg
> rte_flow in rte_flow_dev_dump to dump one flow.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Ori Kam <orika@nvidia.com>
[...]
> +Dump
> +~~~~~
> +
> +Dump information for all or one flows.
> +
> +.. code-block:: c
> +
> +   int
> +   rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
> +                     FILE *file,
> +                     struct rte_flow_error *error);
> +
> +Arguments:
> +
> +- ``port_id``: port identifier of Ethernet device.
> +- ``flow``: flow rule handle to dump. NULL to dump all.
> +- ``file``: a pointer to a file for output
> +- ``error``: perform verbose error reporting if not NULL. PMDs initialize
> +  this structure in case of error only.
> +
> +Return values:
> +
> +- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.

I really don't understand why we continue duplicating rte_flow doxygen
in the guide. Please stop.
Do one good API description in doxygen,
and describe only the big picture and how-to in the guide.

Re: [dpdk-dev] [PATCH v4 1/3] ethdev: modify rte API for single flow dump

[Haifei Luo]

Hi Thomas,
    I will remove it from prog_guide/rte_flow.rst. Is it okay ? Thank you.

-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net> 
Sent: Wednesday, April 14, 2021 5:01 PM
To: Ori Kam <orika@nvidia.com>
Cc: dev@dpdk.org; Haifei Luo <haifeil@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; ajit.khaparde@broadcom.com; Xiaoyun Li <xiaoyun.li@intel.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Jerin Jacob <jerinj@marvell.com>; Nithin Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v4 1/3] ethdev: modify rte API for single flow dump

External email: Use caution opening links or attachments


14/04/2021 10:41, Haifei Luo:
> Previous implementations support dump all the flows. Add new arg 
> rte_flow in rte_flow_dev_dump to dump one flow.
>
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Ori Kam <orika@nvidia.com>
[...]
> +Dump
> +~~~~~
> +
> +Dump information for all or one flows.
> +
> +.. code-block:: c
> +
> +   int
> +   rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
> +                     FILE *file,
> +                     struct rte_flow_error *error);
> +
> +Arguments:
> +
> +- ``port_id``: port identifier of Ethernet device.
> +- ``flow``: flow rule handle to dump. NULL to dump all.
> +- ``file``: a pointer to a file for output
> +- ``error``: perform verbose error reporting if not NULL. PMDs 
> +initialize
> +  this structure in case of error only.
> +
> +Return values:
> +
> +- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.

I really don't understand why we continue duplicating rte_flow doxygen in the guide. Please stop.
Do one good API description in doxygen,
and describe only the big picture and how-to in the guide.

Re: [dpdk-dev] [PATCH v4 1/3] ethdev: modify rte API for single flow dump

[Haifei Luo]

HI Thomas,
    #1, okay , will modify title as " ethdev: dump single flow rule " .
    #2, yes, it can pass NULL. Will modify as you described.
Thank you so much for the comments.

-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net> 
Sent: Wednesday, April 14, 2021 4:58 PM
To: Haifei Luo <haifeil@nvidia.com>
Cc: dev@dpdk.org; Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; ajit.khaparde@broadcom.com; Xiaoyun Li <xiaoyun.li@intel.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Jerin Jacob <jerinj@marvell.com>; Nithin Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v4 1/3] ethdev: modify rte API for single flow dump

External email: Use caution opening links or attachments


About the title, what is "rte API"?
I guess you mean DPDK API with rte prefix.
But given all DPDK API have rte prefix,
and this patch is for DPDK,
you can just say "API".

So the title can be:
        ethdev: modify API for single flow dump

But it can look as single flow dump was possible before, which is wrong because it is a new feature.

Another idea is wrong:
The packets of the flow are not dumped,
it is only dumping the HW representation of the flow rule.

I propose to simply describe the new feature:
        ethdev: dump single flow rule


> + * @param[in] flow
> + *   The pointer of rte flow.

Please replace "rte flow" with 'flow rule".
Is it allowed to pass NULL? Will it dump all?
If yes, you can change to:
        Flow rule to dump.
        Dump all rules if NULL.

Re: [dpdk-dev] [PATCH v4 3/3] doc: add single flow dump to guides

[Ferruh Yigit]

On 4/14/2021 9:41 AM, Haifei Luo wrote:
> Add testpmd CLI and API changes in release_notes.
> 
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>   doc/guides/rel_notes/release_21_05.rst | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
> index a0b9079..e3ae470 100644
> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> @@ -174,6 +174,8 @@ New Features
>       ``dpdk-testpmd -- --eth-link-speed N``
>     * Added command to display Rx queue used descriptor count.
>       ``show port (port_id) rxq (queue_id) desc used count``
> +  * Added command to dump internal representation information of single flow.
> +    ``flow dump (port_id) rule (rule_id)``
>   
>   
>   Removed Items
> @@ -207,6 +209,9 @@ API Changes
>      Also, make sure to start the actual text at the margin.
>      =======================================================
>   
> +* ethdev: Added a rte_flow pointer parameter to the function
> +  ``rte_flow_dev_dump()`` allowing dump for single flow.
> +
>   * eal: The experimental TLS API added in ``rte_thread.h`` has been renamed
>     from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
>     and confusion with the transport layer security term.
> 

Hi Haifei,

I guess you missed the following comment to v3, can you please check it:

Can you please distribute the release notes updates to the patches that 
introduces the change? So there should be a separate release notes update patch.

Thanks.

Re: [dpdk-dev] [PATCH v4 1/3] ethdev: modify rte API for single flow dump

[Thomas Monjalon]

14/04/2021 11:07, Haifei Luo:
> Hi Thomas,
>     I will remove it from prog_guide/rte_flow.rst. Is it okay ? Thank you.

This is more a question for Ori.
In case he is not available, yes I think it is better to skip
rte_flow.rst in this patch as it doesn't bring added value.

Note: please avoid top-post.



> From: Thomas Monjalon <thomas@monjalon.net> 
> 14/04/2021 10:41, Haifei Luo:
> > Previous implementations support dump all the flows. Add new arg 
> > rte_flow in rte_flow_dev_dump to dump one flow.
> >
> > Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Acked-by: Ori Kam <orika@nvidia.com>
> [...]
> > +Dump
> > +~~~~~
> > +
> > +Dump information for all or one flows.
> > +
> > +.. code-block:: c
> > +
> > +   int
> > +   rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
> > +                     FILE *file,
> > +                     struct rte_flow_error *error);
> > +
> > +Arguments:
> > +
> > +- ``port_id``: port identifier of Ethernet device.
> > +- ``flow``: flow rule handle to dump. NULL to dump all.
> > +- ``file``: a pointer to a file for output
> > +- ``error``: perform verbose error reporting if not NULL. PMDs 
> > +initialize
> > +  this structure in case of error only.
> > +
> > +Return values:
> > +
> > +- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
> 
> I really don't understand why we continue duplicating rte_flow doxygen in the guide. Please stop.
> Do one good API description in doxygen,
> and describe only the big picture and how-to in the guide.

[dpdk-dev] [PATCH v5 0/3] support single flow dump

[Haifei Luo]

Dump internal representation information of all flows is supported.
It is useful to dump one flow. To implement this requirement,
add this CLI to dump one rule: flow dump PORT rule ID
      and the CLI to dump all: flow dump PORT all
Examples:
    testpmd> flow dump 0 all
    testpmd> flow dump 0 rule 0
The first 0 is for port. The second one is for rule id.

For RTE API, add one arg rte_flow in rte_flow_dev_dump.
If rte_flow is null, it will dump information for all flows.
Otherwise, it will dump one.
Accordingly, add this arg in related dev_dump and driver APIs.

V2: fix comments about rte API. V1 has one API rte_flow_dump,
remove it and update rte_flow_dev_dump by adding one arg rte_flow.

V3: split into two series. One is for ethdev/testpmd/doc, the other is
for drivers.

V4: Fix comments. Remove "Flow dump" from features/default.ini and
features/mlx5.ini.

V5: Fix comments. Modify title and enhance API's description. 

Haifei Luo (3):
  ethdev: dump single flow rule
  app/testpmd: add CLIs for single flow dump feature
  doc: add single flow dump to guides

 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/nics/mlx5.rst                    |  9 +++--
 doc/guides/rel_notes/release_21_05.rst      |  5 +++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 drivers/net/mlx5/linux/mlx5_socket.c        |  2 +-
 drivers/net/mlx5/mlx5.h                     |  4 +--
 drivers/net/mlx5/mlx5_flow.c                |  9 +++--
 drivers/net/octeontx2/otx2_flow.c           |  9 ++++-
 lib/librte_ethdev/rte_flow.c                |  5 +--
 lib/librte_ethdev/rte_flow.h                |  5 ++-
 lib/librte_ethdev/rte_flow_driver.h         |  1 +
 13 files changed, 126 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v5 1/3] ethdev: dump single flow rule

[Haifei Luo]

Previous implementations support dump all the flows. Add new arg
rte_flow in rte_flow_dev_dump to dump one flow.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/config.c                | 2 +-
 doc/guides/nics/mlx5.rst             | 9 ++++++---
 drivers/net/mlx5/linux/mlx5_socket.c | 2 +-
 drivers/net/mlx5/mlx5.h              | 4 ++--
 drivers/net/mlx5/mlx5_flow.c         | 9 ++++++---
 drivers/net/octeontx2/otx2_flow.c    | 9 ++++++++-
 lib/librte_ethdev/rte_flow.c         | 5 +++--
 lib/librte_ethdev/rte_flow.h         | 5 ++++-
 lib/librte_ethdev/rte_flow_driver.h  | 1 +
 9 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a5e82b7..ca34a63 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1932,7 +1932,7 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, file, &error);
+	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 490329a..7ff92b0 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1837,13 +1837,16 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       testpmd> flow dump <port> <output_file>
+       To dump all flows:
+       testpmd> flow dump <port> all <output_file>
+       and dump one flow:
+       testpmd> flow dump <port> rule <rule_id> <output_file>
 
    - call rte_flow_dev_dump api:
 
    .. code-block:: console
 
-       rte_flow_dev_dump(port, file, NULL);
+       rte_flow_dev_dump(port, flow, file, NULL);
 
 #. Dump human-readable flows from raw file:
 
@@ -1851,4 +1854,4 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       mlx_steering_dump.py -f <output_file>
+       mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index b1f41bc..6e354f4 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -84,7 +84,7 @@
 	}
 	/* Dump flow. */
 	dev = &rte_eth_devices[port_id];
-	ret = mlx5_flow_dev_dump(dev, file, NULL);
+	ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
 	/* Set-up the ancillary data and reply. */
 	msg.msg_controllen = 0;
 	msg.msg_control = NULL;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 0f69f9d..e0f7101 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1245,8 +1245,8 @@ void mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
 void mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt);
 int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		       bool clear, uint64_t *pkts, uint64_t *bytes);
-int mlx5_flow_dev_dump(struct rte_eth_dev *dev, FILE *file,
-		       struct rte_flow_error *error);
+int mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow,
+			FILE *file, struct rte_flow_error *error);
 void mlx5_flow_rxq_dynf_metadata_set(struct rte_eth_dev *dev);
 int mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 668c32c..a8cf674 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7154,7 +7154,7 @@ struct mlx5_meter_domains_infos *
  *   0 on success, a nagative value otherwise.
  */
 int
-mlx5_flow_dev_dump(struct rte_eth_dev *dev,
+mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow_idx,
 		   FILE *file,
 		   struct rte_flow_error *error __rte_unused)
 {
@@ -7166,8 +7166,11 @@ struct mlx5_meter_domains_infos *
 			return -errno;
 		return -ENOTSUP;
 	}
-	return mlx5_devx_cmd_flow_dump(sh->fdb_domain, sh->rx_domain,
-				       sh->tx_domain, file);
+
+	if (!flow_idx)
+		return mlx5_devx_cmd_flow_dump(sh->fdb_domain,
+				sh->rx_domain, sh->tx_domain, file);
+	return -ENOTSUP;
 }
 
 /**
diff --git a/drivers/net/octeontx2/otx2_flow.c b/drivers/net/octeontx2/otx2_flow.c
index 14ac9bc..1c90d75 100644
--- a/drivers/net/octeontx2/otx2_flow.c
+++ b/drivers/net/octeontx2/otx2_flow.c
@@ -807,7 +807,7 @@
 
 static int
 otx2_flow_dev_dump(struct rte_eth_dev *dev,
-		  FILE *file,
+		  struct rte_flow *flow, FILE *file,
 		  struct rte_flow_error *error)
 {
 	struct otx2_eth_dev *hw = dev->data->dev_private;
@@ -822,6 +822,13 @@
 				   "Invalid file");
 		return -EINVAL;
 	}
+	if (flow != NULL) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE,
+				   NULL,
+				   "Invalid argument");
+		return -EINVAL;
+	}
 
 	max_prio = hw->npc_flow.flow_max_priority;
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index e07e617..7241f00 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -1027,7 +1027,8 @@ enum rte_flow_conv_item_spec_type {
 }
 
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error)
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+			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);
@@ -1037,7 +1038,7 @@ enum rte_flow_conv_item_spec_type {
 		return -rte_errno;
 	if (likely(!!ops->dev_dump)) {
 		fts_enter(dev);
-		ret = ops->dev_dump(dev, file, error);
+		ret = ops->dev_dump(dev, flow, file, error);
 		fts_exit(dev);
 		return flow_err(port_id, ret, error);
 	}
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index c476a0f..5eba79d 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -3232,6 +3232,8 @@ enum rte_flow_conv_op {
  *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
+ * @param[in] flow
+ *   The pointer of flow rule to dump. Dump all rules if NULL.
  * @param[in] file
  *   A pointer to a file for output.
  * @param[out] error
@@ -3242,7 +3244,8 @@ enum rte_flow_conv_op {
  */
 __rte_experimental
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error);
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+		FILE *file, struct rte_flow_error *error);
 
 /**
  * Check if mbuf dynamic field for metadata is registered.
diff --git a/lib/librte_ethdev/rte_flow_driver.h b/lib/librte_ethdev/rte_flow_driver.h
index da594d9..6ae1f8c 100644
--- a/lib/librte_ethdev/rte_flow_driver.h
+++ b/lib/librte_ethdev/rte_flow_driver.h
@@ -75,6 +75,7 @@ struct rte_flow_ops {
 	/** See rte_flow_dev_dump(). */
 	int (*dev_dump)
 		(struct rte_eth_dev *dev,
+		 struct rte_flow *flow,
 		 FILE *file,
 		 struct rte_flow_error *error);
 	/** See rte_flow_get_aged_flows() */
-- 
1.8.3.1

[dpdk-dev] [PATCH v5 2/3] app/testpmd: add CLIs for single flow dump feature

[Haifei Luo]

Add support for single flow dump.
The CLIs to dump one rule: flow dump PORT rule ID
to dump all: flow dump PORT all
Examples:
testpmd> flow dump 0 all
testpmd> flow dump 0 rule 0

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 4 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index fb7a3a8..0127d9e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -108,6 +108,10 @@ enum index {
 	TUNNEL_SET,
 	TUNNEL_MATCH,
 
+	/* Dump arguments */
+	DUMP_ALL,
+	DUMP_ONE,
+
 	/* Shared action arguments */
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -793,6 +797,8 @@ struct buffer {
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
+			bool mode;
+			uint32_t rule;
 		} dump; /**< Dump arguments. */
 		struct {
 			uint32_t rule;
@@ -844,6 +850,12 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index next_dump_subcmd[] = {
+	DUMP_ALL,
+	DUMP_ONE,
+	ZERO,
+};
+
 static const enum index next_sa_subcmd[] = {
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -2032,10 +2044,9 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	},
 	[DUMP] = {
 		.name = "dump",
-		.help = "dump all flow rules to file",
-		.next = NEXT(next_dump_attr, NEXT_ENTRY(PORT_ID)),
-		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
-			     ARGS_ENTRY(struct buffer, port)),
+		.help = "dump single/all flow rules to file",
+		.next = NEXT(next_dump_subcmd, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_dump,
 	},
 	[QUERY] = {
@@ -2125,6 +2136,22 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.destroy.rule)),
 		.call = parse_destroy,
 	},
+	/* Dump arguments. */
+	[DUMP_ALL] = {
+		.name = "all",
+		.help = "dump all",
+		.next = NEXT(next_dump_attr),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file)),
+		.call = parse_dump,
+	},
+	[DUMP_ONE] = {
+		.name = "rule",
+		.help = "dump one rule",
+		.next = NEXT(next_dump_attr, NEXT_ENTRY(RULE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
+				ARGS_ENTRY(struct buffer, args.dump.rule)),
+		.call = parse_dump,
+	},
 	/* Query arguments. */
 	[QUERY_ACTION] = {
 		.name = "{action}",
@@ -6364,8 +6391,20 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		ctx->objdata = 0;
 		ctx->object = out;
 		ctx->objmask = NULL;
+		return len;
+	}
+	switch (ctx->curr) {
+	case DUMP_ALL:
+	case DUMP_ONE:
+		out->args.dump.mode = (ctx->curr == DUMP_ALL) ? true : false;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+		return len;
+	default:
+		return -1;
 	}
-	return len;
 }
 
 /** Parse tokens for query command. */
@@ -7659,8 +7698,10 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
-	case DUMP:
-		port_flow_dump(in->port, in->args.dump.file);
+	case DUMP_ONE:
+	case DUMP_ALL:
+		port_flow_dump(in->port, in->args.dump.mode,
+				in->args.dump.rule, in->args.dump.file);
 		break;
 	case QUERY:
 		port_flow_query(in->port, in->args.query.rule,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index ca34a63..40b2b29 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1916,13 +1916,41 @@ struct rte_flow_shared_action *
 	return ret;
 }
 
-/** Dump all flow rules. */
+/** Dump flow rules. */
 int
-port_flow_dump(portid_t port_id, const char *file_name)
+port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
+		const char *file_name)
 {
 	int ret = 0;
 	FILE *file = stdout;
 	struct rte_flow_error error;
+	struct rte_port *port;
+	struct port_flow *pflow;
+	struct rte_flow *tmpFlow = NULL;
+	bool found = false;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+
+	if (!dump_all) {
+		port = &ports[port_id];
+		pflow = port->flow_list;
+		while (pflow) {
+			if (rule_id != pflow->id) {
+				pflow = pflow->next;
+			} else {
+				tmpFlow = pflow->flow;
+				if (tmpFlow)
+					found = true;
+				break;
+			}
+		}
+		if (found == false) {
+			printf("Failed to dump to flow %d\n", rule_id);
+			return -EINVAL;
+		}
+	}
 
 	if (file_name && strlen(file_name)) {
 		file = fopen(file_name, "w");
@@ -1932,7 +1960,11 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
+
+	if (!dump_all)
+		ret = rte_flow_dev_dump(port_id, tmpFlow, file, &error);
+	else
+		ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a87ccb0..36d8535 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -825,7 +825,8 @@ void update_age_action_context(const struct rte_flow_action *actions,
 		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
-int port_flow_dump(portid_t port_id, const char *file_name);
+int port_flow_dump(portid_t port_id, bool dump_all,
+			uint32_t rule, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_t rule,
 		    const struct rte_flow_action *action);
 void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 36f0a32..5454524 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3332,7 +3332,11 @@ following sections.
 
 - Dump internal representation information of all flows in hardware::
 
-   flow dump {port_id} {output_file}
+   flow dump {port_id} all {output_file}
+
+   for one flow :
+
+   flow dump {port_id} rule {rule_id} {output_file}
 
 - List and destroy aged flow rules::
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v5 3/3] doc: add single flow dump to guides

[Haifei Luo]

Add testpmd CLIs and APIs changes in release_notes.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
---
 doc/guides/rel_notes/release_21_05.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index a0b9079..e3ae470 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -174,6 +174,8 @@ New Features
     ``dpdk-testpmd -- --eth-link-speed N``
   * Added command to display Rx queue used descriptor count.
     ``show port (port_id) rxq (queue_id) desc used count``
+  * Added command to dump internal representation information of single flow.
+    ``flow dump (port_id) rule (rule_id)``
 
 
 Removed Items
@@ -207,6 +209,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* ethdev: Added a rte_flow pointer parameter to the function
+  ``rte_flow_dev_dump()`` allowing dump for single flow.
+
 * eal: The experimental TLS API added in ``rte_thread.h`` has been renamed
   from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
   and confusion with the transport layer security term.
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v3 3/3] doc: add single flow dump to guides

[Haifei Luo]

Hi Ferruh,
      "Can you please distribute the release notes updates to the patches that introduces the change?"
okay

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Wednesday, April 14, 2021 4:24 PM
To: Haifei Luo <haifeil@nvidia.com>; dev@dpdk.org
Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; ajit.khaparde@broadcom.com; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
Subject: Re: [PATCH v3 3/3] doc: add single flow dump to guides

External email: Use caution opening links or attachments


On 4/14/2021 7:13 AM, Haifei Luo wrote:
> Add "Flow dump" in features/default.ini and features/mlx5.ini.
> Add testpmd CLI and API changes in release_notes.
>
> Signed-off-by: Haifei Luo <haifeil@nvidia.com>
> ---
>   doc/guides/nics/features/default.ini   | 1 +
>   doc/guides/nics/features/mlx5.ini      | 1 +
>   doc/guides/rel_notes/release_21_05.rst | 5 +++++
>   3 files changed, 7 insertions(+)
>
> diff --git a/doc/guides/nics/features/default.ini 
> b/doc/guides/nics/features/default.ini
> index 8046bd1..49aaf17 100644
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> @@ -39,6 +39,7 @@ DCB                  =
>   VLAN filter          =
>   Flow control         =
>   Flow API             =
> +Flow dump            =

Hi Haifei,

I don't think "flow dump" is important enough to be listed in the feature list, I am for not adding it.
Comparing the one line above item, "Flow API", this one is small and indeed subset of it.

>   Rate limitation      =
>   Traffic mirroring    =
>   Inline crypto        =
> diff --git a/doc/guides/nics/features/mlx5.ini 
> b/doc/guides/nics/features/mlx5.ini
> index ddd131d..3c5fcff 100644
> --- a/doc/guides/nics/features/mlx5.ini
> +++ b/doc/guides/nics/features/mlx5.ini
> @@ -29,6 +29,7 @@ SR-IOV               = Y
>   VLAN filter          = Y
>   Flow control         = Y
>   Flow API             = Y
> +Flow dump            = Y
>   CRC offload          = Y
>   VLAN offload         = Y
>   L3 checksum offload  = Y
> diff --git a/doc/guides/rel_notes/release_21_05.rst 
> b/doc/guides/rel_notes/release_21_05.rst
> index a0b9079..e3ae470 100644
> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> @@ -174,6 +174,8 @@ New Features
>       ``dpdk-testpmd -- --eth-link-speed N``
>     * Added command to display Rx queue used descriptor count.
>       ``show port (port_id) rxq (queue_id) desc used count``
> +  * Added command to dump internal representation information of single flow.
> +    ``flow dump (port_id) rule (rule_id)``
>
>
>   Removed Items
> @@ -207,6 +209,9 @@ API Changes
>      Also, make sure to start the actual text at the margin.
>      =======================================================
>
> +* ethdev: Added a rte_flow pointer parameter to the function
> +  ``rte_flow_dev_dump()`` allowing dump for single flow.
> +
>   * eal: The experimental TLS API added in ``rte_thread.h`` has been renamed
>     from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
>     and confusion with the transport layer security term.
>

Can you please distribute the release notes updates to the patches that introduces the change? So there should be a separate release notes update patch.

Thanks.

[dpdk-dev] [PATCH v6 0/2] single flow dump

[Haifei Luo]

Dump internal representation information of all flows is supported.
It is useful to dump one flow. To implement this requirement,
add this CLI to dump one rule: flow dump PORT rule ID
      and the CLI to dump all: flow dump PORT all
Examples:
    testpmd> flow dump 0 all
    testpmd> flow dump 0 rule 0
The first 0 is for port. The second one is for rule id.

For RTE API, add one arg rte_flow in rte_flow_dev_dump.
If rte_flow is null, it will dump information for all flows.
Otherwise, it will dump one.
Accordingly, add this arg in related dev_dump and driver APIs.

V2: fix comments about rte API. V1 has one API rte_flow_dump,
remove it and update rte_flow_dev_dump by adding one arg rte_flow.

V3: split into two series. One is for ethdev/testpmd/doc, the other is
for drivers.

V4: Fix comments. Remove "Flow dump" from features/default.ini and
features/mlx5.ini.

V5: Fix comments. Modify title and enhance API's description.

V6: Distribute the release notes updates to the patches that 
introduces the change. 

Haifei Luo (2):
  ethdev: dump single flow rule
  app/testpmd: add CLIs for single flow dump feature

 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/nics/mlx5.rst                    |  9 +++--
 doc/guides/rel_notes/release_21_05.rst      |  5 +++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 drivers/net/mlx5/linux/mlx5_socket.c        |  2 +-
 drivers/net/mlx5/mlx5.h                     |  4 +--
 drivers/net/mlx5/mlx5_flow.c                |  9 +++--
 drivers/net/octeontx2/otx2_flow.c           |  9 ++++-
 lib/librte_ethdev/rte_flow.c                |  5 +--
 lib/librte_ethdev/rte_flow.h                |  5 ++-
 lib/librte_ethdev/rte_flow_driver.h         |  1 +
 13 files changed, 126 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v6 1/2] ethdev: dump single flow rule

[Haifei Luo]

Previous implementations support dump all the flows. Add new arg
rte_flow in rte_flow_dev_dump to dump one flow.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/config.c                  | 2 +-
 doc/guides/nics/mlx5.rst               | 9 ++++++---
 doc/guides/rel_notes/release_21_05.rst | 3 +++
 drivers/net/mlx5/linux/mlx5_socket.c   | 2 +-
 drivers/net/mlx5/mlx5.h                | 4 ++--
 drivers/net/mlx5/mlx5_flow.c           | 9 ++++++---
 drivers/net/octeontx2/otx2_flow.c      | 9 ++++++++-
 lib/librte_ethdev/rte_flow.c           | 5 +++--
 lib/librte_ethdev/rte_flow.h           | 5 ++++-
 lib/librte_ethdev/rte_flow_driver.h    | 1 +
 10 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a5e82b7..ca34a63 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1932,7 +1932,7 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, file, &error);
+	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 490329a..7ff92b0 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1837,13 +1837,16 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       testpmd> flow dump <port> <output_file>
+       To dump all flows:
+       testpmd> flow dump <port> all <output_file>
+       and dump one flow:
+       testpmd> flow dump <port> rule <rule_id> <output_file>
 
    - call rte_flow_dev_dump api:
 
    .. code-block:: console
 
-       rte_flow_dev_dump(port, file, NULL);
+       rte_flow_dev_dump(port, flow, file, NULL);
 
 #. Dump human-readable flows from raw file:
 
@@ -1851,4 +1854,4 @@ all flows with assistance of external tools.
 
    .. code-block:: console
 
-       mlx_steering_dump.py -f <output_file>
+       mlx_steering_dump.py -f <output_file> -flowptr <flow_ptr>
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index a0b9079..6d209a2 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -207,6 +207,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* ethdev: Added a rte_flow pointer parameter to the function
+  ``rte_flow_dev_dump()`` allowing dump for single flow.
+
 * eal: The experimental TLS API added in ``rte_thread.h`` has been renamed
   from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
   and confusion with the transport layer security term.
diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index b1f41bc..6e354f4 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -84,7 +84,7 @@
 	}
 	/* Dump flow. */
 	dev = &rte_eth_devices[port_id];
-	ret = mlx5_flow_dev_dump(dev, file, NULL);
+	ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
 	/* Set-up the ancillary data and reply. */
 	msg.msg_controllen = 0;
 	msg.msg_control = NULL;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 0f69f9d..e0f7101 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1245,8 +1245,8 @@ void mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
 void mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt);
 int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		       bool clear, uint64_t *pkts, uint64_t *bytes);
-int mlx5_flow_dev_dump(struct rte_eth_dev *dev, FILE *file,
-		       struct rte_flow_error *error);
+int mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow,
+			FILE *file, struct rte_flow_error *error);
 void mlx5_flow_rxq_dynf_metadata_set(struct rte_eth_dev *dev);
 int mlx5_flow_get_aged_flows(struct rte_eth_dev *dev, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 668c32c..a8cf674 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7154,7 +7154,7 @@ struct mlx5_meter_domains_infos *
  *   0 on success, a nagative value otherwise.
  */
 int
-mlx5_flow_dev_dump(struct rte_eth_dev *dev,
+mlx5_flow_dev_dump(struct rte_eth_dev *dev, struct rte_flow *flow_idx,
 		   FILE *file,
 		   struct rte_flow_error *error __rte_unused)
 {
@@ -7166,8 +7166,11 @@ struct mlx5_meter_domains_infos *
 			return -errno;
 		return -ENOTSUP;
 	}
-	return mlx5_devx_cmd_flow_dump(sh->fdb_domain, sh->rx_domain,
-				       sh->tx_domain, file);
+
+	if (!flow_idx)
+		return mlx5_devx_cmd_flow_dump(sh->fdb_domain,
+				sh->rx_domain, sh->tx_domain, file);
+	return -ENOTSUP;
 }
 
 /**
diff --git a/drivers/net/octeontx2/otx2_flow.c b/drivers/net/octeontx2/otx2_flow.c
index 14ac9bc..1c90d75 100644
--- a/drivers/net/octeontx2/otx2_flow.c
+++ b/drivers/net/octeontx2/otx2_flow.c
@@ -807,7 +807,7 @@
 
 static int
 otx2_flow_dev_dump(struct rte_eth_dev *dev,
-		  FILE *file,
+		  struct rte_flow *flow, FILE *file,
 		  struct rte_flow_error *error)
 {
 	struct otx2_eth_dev *hw = dev->data->dev_private;
@@ -822,6 +822,13 @@
 				   "Invalid file");
 		return -EINVAL;
 	}
+	if (flow != NULL) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE,
+				   NULL,
+				   "Invalid argument");
+		return -EINVAL;
+	}
 
 	max_prio = hw->npc_flow.flow_max_priority;
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index e07e617..7241f00 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -1027,7 +1027,8 @@ enum rte_flow_conv_item_spec_type {
 }
 
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error)
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+			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);
@@ -1037,7 +1038,7 @@ enum rte_flow_conv_item_spec_type {
 		return -rte_errno;
 	if (likely(!!ops->dev_dump)) {
 		fts_enter(dev);
-		ret = ops->dev_dump(dev, file, error);
+		ret = ops->dev_dump(dev, flow, file, error);
 		fts_exit(dev);
 		return flow_err(port_id, ret, error);
 	}
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index c476a0f..5eba79d 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -3232,6 +3232,8 @@ enum rte_flow_conv_op {
  *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
+ * @param[in] flow
+ *   The pointer of flow rule to dump. Dump all rules if NULL.
  * @param[in] file
  *   A pointer to a file for output.
  * @param[out] error
@@ -3242,7 +3244,8 @@ enum rte_flow_conv_op {
  */
 __rte_experimental
 int
-rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error);
+rte_flow_dev_dump(uint16_t port_id, struct rte_flow *flow,
+		FILE *file, struct rte_flow_error *error);
 
 /**
  * Check if mbuf dynamic field for metadata is registered.
diff --git a/lib/librte_ethdev/rte_flow_driver.h b/lib/librte_ethdev/rte_flow_driver.h
index da594d9..6ae1f8c 100644
--- a/lib/librte_ethdev/rte_flow_driver.h
+++ b/lib/librte_ethdev/rte_flow_driver.h
@@ -75,6 +75,7 @@ struct rte_flow_ops {
 	/** See rte_flow_dev_dump(). */
 	int (*dev_dump)
 		(struct rte_eth_dev *dev,
+		 struct rte_flow *flow,
 		 FILE *file,
 		 struct rte_flow_error *error);
 	/** See rte_flow_get_aged_flows() */
-- 
1.8.3.1

[dpdk-dev] [PATCH v6 2/2] app/testpmd: add CLIs for single flow dump feature

[Haifei Luo]

Add support for single flow dump.
The CLIs to dump one rule: flow dump PORT rule ID
to dump all: flow dump PORT all
Examples:
testpmd> flow dump 0 all
testpmd> flow dump 0 rule 0

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 app/test-pmd/cmdline_flow.c                 | 55 +++++++++++++++++++++++++----
 app/test-pmd/config.c                       | 38 ++++++++++++++++++--
 app/test-pmd/testpmd.h                      |  3 +-
 doc/guides/rel_notes/release_21_05.rst      |  2 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 +++-
 5 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index fb7a3a8..0127d9e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -108,6 +108,10 @@ enum index {
 	TUNNEL_SET,
 	TUNNEL_MATCH,
 
+	/* Dump arguments */
+	DUMP_ALL,
+	DUMP_ONE,
+
 	/* Shared action arguments */
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -793,6 +797,8 @@ struct buffer {
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
+			bool mode;
+			uint32_t rule;
 		} dump; /**< Dump arguments. */
 		struct {
 			uint32_t rule;
@@ -844,6 +850,12 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index next_dump_subcmd[] = {
+	DUMP_ALL,
+	DUMP_ONE,
+	ZERO,
+};
+
 static const enum index next_sa_subcmd[] = {
 	SHARED_ACTION_CREATE,
 	SHARED_ACTION_UPDATE,
@@ -2032,10 +2044,9 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	},
 	[DUMP] = {
 		.name = "dump",
-		.help = "dump all flow rules to file",
-		.next = NEXT(next_dump_attr, NEXT_ENTRY(PORT_ID)),
-		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
-			     ARGS_ENTRY(struct buffer, port)),
+		.help = "dump single/all flow rules to file",
+		.next = NEXT(next_dump_subcmd, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_dump,
 	},
 	[QUERY] = {
@@ -2125,6 +2136,22 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.destroy.rule)),
 		.call = parse_destroy,
 	},
+	/* Dump arguments. */
+	[DUMP_ALL] = {
+		.name = "all",
+		.help = "dump all",
+		.next = NEXT(next_dump_attr),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file)),
+		.call = parse_dump,
+	},
+	[DUMP_ONE] = {
+		.name = "rule",
+		.help = "dump one rule",
+		.next = NEXT(next_dump_attr, NEXT_ENTRY(RULE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.dump.file),
+				ARGS_ENTRY(struct buffer, args.dump.rule)),
+		.call = parse_dump,
+	},
 	/* Query arguments. */
 	[QUERY_ACTION] = {
 		.name = "{action}",
@@ -6364,8 +6391,20 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 		ctx->objdata = 0;
 		ctx->object = out;
 		ctx->objmask = NULL;
+		return len;
+	}
+	switch (ctx->curr) {
+	case DUMP_ALL:
+	case DUMP_ONE:
+		out->args.dump.mode = (ctx->curr == DUMP_ALL) ? true : false;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+		return len;
+	default:
+		return -1;
 	}
-	return len;
 }
 
 /** Parse tokens for query command. */
@@ -7659,8 +7698,10 @@ static int comp_set_modify_field_id(struct context *, const struct token *,
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
-	case DUMP:
-		port_flow_dump(in->port, in->args.dump.file);
+	case DUMP_ONE:
+	case DUMP_ALL:
+		port_flow_dump(in->port, in->args.dump.mode,
+				in->args.dump.rule, in->args.dump.file);
 		break;
 	case QUERY:
 		port_flow_query(in->port, in->args.query.rule,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index ca34a63..40b2b29 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1916,13 +1916,41 @@ struct rte_flow_shared_action *
 	return ret;
 }
 
-/** Dump all flow rules. */
+/** Dump flow rules. */
 int
-port_flow_dump(portid_t port_id, const char *file_name)
+port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
+		const char *file_name)
 {
 	int ret = 0;
 	FILE *file = stdout;
 	struct rte_flow_error error;
+	struct rte_port *port;
+	struct port_flow *pflow;
+	struct rte_flow *tmpFlow = NULL;
+	bool found = false;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+
+	if (!dump_all) {
+		port = &ports[port_id];
+		pflow = port->flow_list;
+		while (pflow) {
+			if (rule_id != pflow->id) {
+				pflow = pflow->next;
+			} else {
+				tmpFlow = pflow->flow;
+				if (tmpFlow)
+					found = true;
+				break;
+			}
+		}
+		if (found == false) {
+			printf("Failed to dump to flow %d\n", rule_id);
+			return -EINVAL;
+		}
+	}
 
 	if (file_name && strlen(file_name)) {
 		file = fopen(file_name, "w");
@@ -1932,7 +1960,11 @@ struct rte_flow_shared_action *
 			return -errno;
 		}
 	}
-	ret = rte_flow_dev_dump(port_id, NULL, file, &error);
+
+	if (!dump_all)
+		ret = rte_flow_dev_dump(port_id, tmpFlow, file, &error);
+	else
+		ret = rte_flow_dev_dump(port_id, NULL, file, &error);
 	if (ret) {
 		port_flow_complain(&error);
 		printf("Failed to dump flow: %s\n", strerror(-ret));
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a87ccb0..36d8535 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -825,7 +825,8 @@ void update_age_action_context(const struct rte_flow_action *actions,
 		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
-int port_flow_dump(portid_t port_id, const char *file_name);
+int port_flow_dump(portid_t port_id, bool dump_all,
+			uint32_t rule, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_t rule,
 		    const struct rte_flow_action *action);
 void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index 6d209a2..e3ae470 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -174,6 +174,8 @@ New Features
     ``dpdk-testpmd -- --eth-link-speed N``
   * Added command to display Rx queue used descriptor count.
     ``show port (port_id) rxq (queue_id) desc used count``
+  * Added command to dump internal representation information of single flow.
+    ``flow dump (port_id) rule (rule_id)``
 
 
 Removed Items
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 36f0a32..5454524 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3332,7 +3332,11 @@ following sections.
 
 - Dump internal representation information of all flows in hardware::
 
-   flow dump {port_id} {output_file}
+   flow dump {port_id} all {output_file}
+
+   for one flow :
+
+   flow dump {port_id} rule {rule_id} {output_file}
 
 - List and destroy aged flow rules::
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v5 1/3] ethdev: dump single flow rule

[Thomas Monjalon]

14/04/2021 11:51, Haifei Luo:
> Previous implementations support dump all the flows. Add new arg
> rte_flow in rte_flow_dev_dump to dump one flow.

Rewording:
The API supported dumping all the flow rules.
A parameter is added to allow dumping a specific flow rule.

> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -1837,13 +1837,16 @@ all flows with assistance of external tools.
>  
>     .. code-block:: console
>  
> -       testpmd> flow dump <port> <output_file>
> +       To dump all flows:
> +       testpmd> flow dump <port> all <output_file>
> +       and dump one flow:
> +       testpmd> flow dump <port> rule <rule_id> <output_file>

Comments should not be in the code block.

> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -3232,6 +3232,8 @@ enum rte_flow_conv_op {
>   *
>   * @param[in] port_id
>   *    The port identifier of the Ethernet device.
> + * @param[in] flow
> + *   The pointer of flow rule to dump. Dump all rules if NULL.

Sorry for not making it explicit,
when fixing "rte flow" I thought you would replace it also
above in the existing function description.

I think  "The pointer of" is useless in general but I'm OK
if you want to keep it.
To Ferruh and Andrew: we could make the ethdev API description
lighter and more pleasant to read.

Re: [dpdk-dev] [PATCH v5 1/3] ethdev: dump single flow rule

[Haifei Luo]

HI Thomas,
   #1,  okay. Will change the place for comments.
   #2,  yes, I can change the description.

Thank you so much.

-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net> 
Sent: Wednesday, April 14, 2021 6:33 PM
To: Haifei Luo <haifeil@nvidia.com>
Cc: dev@dpdk.org; Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; ajit.khaparde@broadcom.com; Xiaoyun Li <xiaoyun.li@intel.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Jerin Jacob <jerinj@marvell.com>; Nithin Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v5 1/3] ethdev: dump single flow rule

External email: Use caution opening links or attachments


14/04/2021 11:51, Haifei Luo:
> Previous implementations support dump all the flows. Add new arg 
> rte_flow in rte_flow_dev_dump to dump one flow.

Rewording:
The API supported dumping all the flow rules.
A parameter is added to allow dumping a specific flow rule.

> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -1837,13 +1837,16 @@ all flows with assistance of external tools.
>
>     .. code-block:: console
>
> -       testpmd> flow dump <port> <output_file>
> +       To dump all flows:
> +       testpmd> flow dump <port> all <output_file>
> +       and dump one flow:
> +       testpmd> flow dump <port> rule <rule_id> <output_file>

Comments should not be in the code block.

> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -3232,6 +3232,8 @@ enum rte_flow_conv_op {
>   *
>   * @param[in] port_id
>   *    The port identifier of the Ethernet device.
> + * @param[in] flow
> + *   The pointer of flow rule to dump. Dump all rules if NULL.

Sorry for not making it explicit,
when fixing "rte flow" I thought you would replace it also above in the existing function description.

I think  "The pointer of" is useless in general but I'm OK if you want to keep it.
To Ferruh and Andrew: we could make the ethdev API description lighter and more pleasant to read.

Re: [dpdk-dev] [PATCH v6 0/2] single flow dump

[Ferruh Yigit]

On 4/14/2021 11:19 AM, Haifei Luo wrote:
> Dump internal representation information of all flows is supported.
> It is useful to dump one flow. To implement this requirement,
> add this CLI to dump one rule: flow dump PORT rule ID
>        and the CLI to dump all: flow dump PORT all
> Examples:
>      testpmd> flow dump 0 all
>      testpmd> flow dump 0 rule 0
> The first 0 is for port. The second one is for rule id.
> 
> For RTE API, add one arg rte_flow in rte_flow_dev_dump.
> If rte_flow is null, it will dump information for all flows.
> Otherwise, it will dump one.
> Accordingly, add this arg in related dev_dump and driver APIs.
> 
> V2: fix comments about rte API. V1 has one API rte_flow_dump,
> remove it and update rte_flow_dev_dump by adding one arg rte_flow.
> 
> V3: split into two series. One is for ethdev/testpmd/doc, the other is
> for drivers.
> 
> V4: Fix comments. Remove "Flow dump" from features/default.ini and
> features/mlx5.ini.
> 
> V5: Fix comments. Modify title and enhance API's description.
> 
> V6: Distribute the release notes updates to the patches that
> introduces the change.
> 
> Haifei Luo (2):
>    ethdev: dump single flow rule
>    app/testpmd: add CLIs for single flow dump feature
> 

Series applied to dpdk-next-net/main, thanks.