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
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
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
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
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
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.
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
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
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
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
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
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
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
> -----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>
> -----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>
> -----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
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
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
[-- 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>
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
> -----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
[-- 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>
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; >
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; >
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
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
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
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
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
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
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.
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.
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
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
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
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
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.
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.
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.
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.
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.
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.
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
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
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
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
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.
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
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
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
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.
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.
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.