[dpdk-dev] [PATCH] app/testpmd: add command to print representor info

[dpdk-dev] [PATCH] app/testpmd: add command to print representor info

[Andrew Rybchenko]

From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>

Make it simpler to debug configurations and code related to the representor
info API.

Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 app/test-pmd/cmdline.c | 117 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 82253bc751..49e6e63471 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Show port supported ptypes"
 			" for a specific port\n\n"
 
+			"show port (port_id) representors\n"
+			"    Show supported representors"
+			" for a specific port\n\n"
+
 			"show device info (<identifier>|all)"
 			"       Show general information about devices probed.\n\n"
 
@@ -16962,6 +16966,118 @@ cmdline_parse_inst_t cmd_show_capability = {
 	},
 };
 
+/* *** show port representors information *** */
+struct cmd_representor_info_result {
+	cmdline_fixed_string_t cmd_show;
+	cmdline_fixed_string_t cmd_port;
+	cmdline_fixed_string_t cmd_keyword;
+	portid_t cmd_pid;
+};
+
+static void
+cmd_representor_info_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_representor_info_result *res = parsed_result;
+	struct rte_eth_representor_info *info;
+	uint32_t i;
+	int ret;
+	int num;
+
+	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+		fprintf(stderr, "Invalid port id %u\n", res->cmd_pid);
+		return;
+	}
+
+	ret = rte_eth_representor_info_get(res->cmd_pid, NULL);
+	if (ret < 0) {
+		fprintf(stderr,
+			"Failed to get the number of representor info ranges for port %hu: %s\n",
+			res->cmd_pid, rte_strerror(-ret));
+		return;
+	}
+	num = ret;
+
+	info = calloc(1, sizeof(*info) + num * sizeof(info->ranges[0]));
+	if (info == NULL) {
+		fprintf(stderr,
+			"Failed to allocate memory for representor info for port %hu\n",
+			res->cmd_pid);
+		return;
+	}
+	info->nb_ranges_alloc = num;
+
+	ret = rte_eth_representor_info_get(res->cmd_pid, info);
+	if (ret < 0) {
+		fprintf(stderr,
+			"Failed to get the representor info for port %hu: %s\n",
+			res->cmd_pid, rte_strerror(-ret));
+		free(info);
+		return;
+	}
+
+	printf("Port controller: %hu\n", info->controller);
+	printf("Port PF: %hu\n", info->pf);
+	printf("Ranges: %u\n", info->nb_ranges);
+	for (i = 0; i < info->nb_ranges; i++) {
+		printf("%u:\n", i);
+		printf("  Name: %s\n", info->ranges[i].name);
+		printf("  Controller: %d\n", info->ranges[i].controller);
+		printf("  PF: %d\n", info->ranges[i].pf);
+		switch (info->ranges[i].type) {
+		case RTE_ETH_REPRESENTOR_NONE:
+			printf("  Type: NONE\n");
+			break;
+		case RTE_ETH_REPRESENTOR_VF:
+			printf("  Type: VF\n");
+			printf("  VF: %d\n", info->ranges[i].vf);
+			break;
+		case RTE_ETH_REPRESENTOR_SF:
+			printf("  Type: SF\n");
+			printf("  SF: %d\n", info->ranges[i].sf);
+			break;
+		case RTE_ETH_REPRESENTOR_PF:
+			printf("  Type: PF\n");
+			break;
+		default:
+			printf("  Type: UNKNOWN VALUE %d\n",
+			       info->ranges[i].type);
+			break;
+		}
+		printf("  Range: [%u-%u]\n", info->ranges[i].id_base,
+		       info->ranges[i].id_end);
+	}
+
+	free(info);
+}
+
+cmdline_parse_token_string_t cmd_representor_info_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_show, "show");
+cmdline_parse_token_string_t cmd_representor_info_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_port, "port");
+cmdline_parse_token_num_t cmd_representor_info_pid =
+	TOKEN_NUM_INITIALIZER(struct cmd_representor_info_result,
+			cmd_pid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_representor_info_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_keyword, "representors");
+
+cmdline_parse_inst_t cmd_representor_info = {
+	.f = cmd_representor_info_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> representors",
+	.tokens = {
+		(void *)&cmd_representor_info_show,
+		(void *)&cmd_representor_info_port,
+		(void *)&cmd_representor_info_pid,
+		(void *)&cmd_representor_info_keyword,
+		NULL,
+	},
+};
+
 /* *** show fec mode per port configuration *** */
 struct cmd_show_fec_metadata_result {
 	cmdline_fixed_string_t cmd_show;
@@ -17816,6 +17932,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_show_fec_mode,
 	(cmdline_parse_inst_t *)&cmd_set_fec_mode,
 	(cmdline_parse_inst_t *)&cmd_show_capability,
+	(cmdline_parse_inst_t *)&cmd_representor_info,
 	NULL,
 };
 
-- 
2.30.2

Re: [dpdk-dev] [PATCH] app/testpmd: add command to print representor info

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Friday, August 27, 2021 14:40
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: dev@dpdk.org; Xueming Li <xuemingl@nvidia.com>; Viacheslav Galaktionov
> <viacheslav.galaktionov@oktetlabs.ru>; Andy Moreton <amoreton@xilinx.com>
> Subject: [PATCH] app/testpmd: add command to print representor info
> 
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> 
> Make it simpler to debug configurations and code related to the representor
> info API.
> 
> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
>  app/test-pmd/cmdline.c | 117
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
> 
<snip>
> 2.30.2

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

Re: [dpdk-dev] [PATCH] app/testpmd: add command to print representor info

[Xueming(Steven) Li]

Thanks for getting this feature more complete!

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Friday, August 27, 2021 2:40 PM
> To: Xiaoyun Li <xiaoyun.li@intel.com>
> Cc: dev@dpdk.org; Xueming(Steven) Li <xuemingl@nvidia.com>; Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>; Andy
> Moreton <amoreton@xilinx.com>
> Subject: [PATCH] app/testpmd: add command to print representor info
> 
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> 
> Make it simpler to debug configurations and code related to the representor info API.
> 
> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
>  app/test-pmd/cmdline.c | 117 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 82253bc751..49e6e63471 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"    Show port supported ptypes"
>  			" for a specific port\n\n"
> 
> +			"show port (port_id) representors\n"
> +			"    Show supported representors"
> +			" for a specific port\n\n"

Confusing name, code below shows representor info, not probed representor devices.
How about "show port (port_id) representor info"?

> +
>  			"show device info (<identifier>|all)"
>  			"       Show general information about devices probed.\n\n"
> 
> @@ -16962,6 +16966,118 @@ cmdline_parse_inst_t cmd_show_capability = {
>  	},
>  };
> 
> +/* *** show port representors information *** */ struct
> +cmd_representor_info_result {
> +	cmdline_fixed_string_t cmd_show;
> +	cmdline_fixed_string_t cmd_port;
> +	cmdline_fixed_string_t cmd_keyword;
> +	portid_t cmd_pid;
> +};
> +
> +static void
> +cmd_representor_info_parsed(void *parsed_result,
> +		__rte_unused struct cmdline *cl,
> +		__rte_unused void *data)
> +{
> +	struct cmd_representor_info_result *res = parsed_result;
> +	struct rte_eth_representor_info *info;
> +	uint32_t i;
> +	int ret;
> +	int num;
> +
> +	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {

Seems port "all" not supported, is there a check?

> +		fprintf(stderr, "Invalid port id %u\n", res->cmd_pid);
> +		return;
> +	}
> +
> +	ret = rte_eth_representor_info_get(res->cmd_pid, NULL);
> +	if (ret < 0) {
> +		fprintf(stderr,
> +			"Failed to get the number of representor info ranges for port %hu: %s\n",
> +			res->cmd_pid, rte_strerror(-ret));
> +		return;
> +	}
> +	num = ret;
> +
> +	info = calloc(1, sizeof(*info) + num * sizeof(info->ranges[0]));
> +	if (info == NULL) {
> +		fprintf(stderr,
> +			"Failed to allocate memory for representor info for port %hu\n",
> +			res->cmd_pid);
> +		return;
> +	}
> +	info->nb_ranges_alloc = num;
> +
> +	ret = rte_eth_representor_info_get(res->cmd_pid, info);
> +	if (ret < 0) {
> +		fprintf(stderr,
> +			"Failed to get the representor info for port %hu: %s\n",
> +			res->cmd_pid, rte_strerror(-ret));
> +		free(info);
> +		return;
> +	}
> +
> +	printf("Port controller: %hu\n", info->controller);
> +	printf("Port PF: %hu\n", info->pf);
> +	printf("Ranges: %u\n", info->nb_ranges);
> +	for (i = 0; i < info->nb_ranges; i++) {
> +		printf("%u:\n", i);

Normally there will be certain amount of ranges here, how about output as a table?

> +		printf("  Name: %s\n", info->ranges[i].name);
> +		printf("  Controller: %d\n", info->ranges[i].controller);
> +		printf("  PF: %d\n", info->ranges[i].pf);
> +		switch (info->ranges[i].type) {
> +		case RTE_ETH_REPRESENTOR_NONE:
> +			printf("  Type: NONE\n");
> +			break;
> +		case RTE_ETH_REPRESENTOR_VF:
> +			printf("  Type: VF\n");
> +			printf("  VF: %d\n", info->ranges[i].vf);
> +			break;
> +		case RTE_ETH_REPRESENTOR_SF:
> +			printf("  Type: SF\n");
> +			printf("  SF: %d\n", info->ranges[i].sf);
> +			break;
> +		case RTE_ETH_REPRESENTOR_PF:
> +			printf("  Type: PF\n");
> +			break;
> +		default:
> +			printf("  Type: UNKNOWN VALUE %d\n",
> +			       info->ranges[i].type);
> +			break;
> +		}
> +		printf("  Range: [%u-%u]\n", info->ranges[i].id_base,
> +		       info->ranges[i].id_end);
> +	}
> +
> +	free(info);
> +}
> +
> +cmdline_parse_token_string_t cmd_representor_info_show =
> +	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_show, "show");
> +cmdline_parse_token_string_t cmd_representor_info_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_port, "port");
> +cmdline_parse_token_num_t cmd_representor_info_pid =
> +	TOKEN_NUM_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_pid, RTE_UINT16);
> +cmdline_parse_token_string_t cmd_representor_info_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_keyword, "representors");
> +
> +cmdline_parse_inst_t cmd_representor_info = {
> +	.f = cmd_representor_info_parsed,
> +	.data = NULL,
> +	.help_str = "show port <port_id> representors",
> +	.tokens = {
> +		(void *)&cmd_representor_info_show,
> +		(void *)&cmd_representor_info_port,
> +		(void *)&cmd_representor_info_pid,
> +		(void *)&cmd_representor_info_keyword,
> +		NULL,
> +	},
> +};
> +
>  /* *** show fec mode per port configuration *** */  struct cmd_show_fec_metadata_result {
>  	cmdline_fixed_string_t cmd_show;
> @@ -17816,6 +17932,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_show_fec_mode,
>  	(cmdline_parse_inst_t *)&cmd_set_fec_mode,
>  	(cmdline_parse_inst_t *)&cmd_show_capability,
> +	(cmdline_parse_inst_t *)&cmd_representor_info,
>  	NULL,
>  };
> 
> --
> 2.30.2

Re: [dpdk-dev] [PATCH] app/testpmd: add command to print representor info

[Viacheslav Galaktionov]

On 2021-08-27 11:20, Xueming(Steven) Li wrote:
> Thanks for getting this feature more complete!
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Friday, August 27, 2021 2:40 PM
>> To: Xiaoyun Li <xiaoyun.li@intel.com>
>> Cc: dev@dpdk.org; Xueming(Steven) Li <xuemingl@nvidia.com>; Viacheslav 
>> Galaktionov <viacheslav.galaktionov@oktetlabs.ru>; Andy
>> Moreton <amoreton@xilinx.com>
>> Subject: [PATCH] app/testpmd: add command to print representor info
>> 
>> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>> 
>> Make it simpler to debug configurations and code related to the 
>> representor info API.
>> 
>> Signed-off-by: Viacheslav Galaktionov 
>> <viacheslav.galaktionov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>> ---
>>  app/test-pmd/cmdline.c | 117 
>> +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 117 insertions(+)
>> 
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 
>> 82253bc751..49e6e63471 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void 
>> *parsed_result,
>>  			"    Show port supported ptypes"
>>  			" for a specific port\n\n"
>> 
>> +			"show port (port_id) representors\n"
>> +			"    Show supported representors"
>> +			" for a specific port\n\n"
> 
> Confusing name, code below shows representor info, not probed
> representor devices.
> How about "show port (port_id) representor info"?

That's a good point, thank you!

[snip]

>> +static void
>> +cmd_representor_info_parsed(void *parsed_result,
>> +		__rte_unused struct cmdline *cl,
>> +		__rte_unused void *data)
>> +{
>> +	struct cmd_representor_info_result *res = parsed_result;
>> +	struct rte_eth_representor_info *info;
>> +	uint32_t i;
>> +	int ret;
>> +	int num;
>> +
>> +	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
> 
> Seems port "all" not supported, is there a check?

What kind of check do you mean?
I suppose support for "all" can be added. In such a case, testpmd can go 
over
all available ports that aren't representors and print their representor 
info.
Does this sound good to you?

[snip]

>> +	printf("Port controller: %hu\n", info->controller);
>> +	printf("Port PF: %hu\n", info->pf);
>> +	printf("Ranges: %u\n", info->nb_ranges);
>> +	for (i = 0; i < info->nb_ranges; i++) {
>> +		printf("%u:\n", i);
> 
> Normally there will be certain amount of ranges here, how about output
> as a table?

I'm not sure what the layout of this table should be, could you provide 
an
example?

[snip]

Re: [dpdk-dev] [PATCH] app/testpmd: add command to print representor info

[Xueming(Steven) Li]

> -----Original Message-----
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> Sent: Friday, August 27, 2021 5:21 PM
> To: Xueming(Steven) Li <xuemingl@nvidia.com>
> Cc: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Xiaoyun Li <xiaoyun.li@intel.com>; dev@dpdk.org; Andy Moreton
> <amoreton@xilinx.com>
> Subject: Re: [PATCH] app/testpmd: add command to print representor info
> 
> On 2021-08-27 11:20, Xueming(Steven) Li wrote:
> > Thanks for getting this feature more complete!
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Friday, August 27, 2021 2:40 PM
> >> To: Xiaoyun Li <xiaoyun.li@intel.com>
> >> Cc: dev@dpdk.org; Xueming(Steven) Li <xuemingl@nvidia.com>;
> >> Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>; Andy
> >> Moreton <amoreton@xilinx.com>
> >> Subject: [PATCH] app/testpmd: add command to print representor info
> >>
> >> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> >>
> >> Make it simpler to debug configurations and code related to the
> >> representor info API.
> >>
> >> Signed-off-by: Viacheslav Galaktionov
> >> <viacheslav.galaktionov@oktetlabs.ru>
> >> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> >> ---
> >>  app/test-pmd/cmdline.c | 117
> >> +++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 117 insertions(+)
> >>
> >> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> >> 82253bc751..49e6e63471 100644
> >> --- a/app/test-pmd/cmdline.c
> >> +++ b/app/test-pmd/cmdline.c
> >> @@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void
> >> *parsed_result,
> >>  			"    Show port supported ptypes"
> >>  			" for a specific port\n\n"
> >>
> >> +			"show port (port_id) representors\n"
> >> +			"    Show supported representors"
> >> +			" for a specific port\n\n"
> >
> > Confusing name, code below shows representor info, not probed
> > representor devices.
> > How about "show port (port_id) representor info"?
> 
> That's a good point, thank you!
> 
> [snip]
> 
> >> +static void
> >> +cmd_representor_info_parsed(void *parsed_result,
> >> +		__rte_unused struct cmdline *cl,
> >> +		__rte_unused void *data)
> >> +{
> >> +	struct cmd_representor_info_result *res = parsed_result;
> >> +	struct rte_eth_representor_info *info;
> >> +	uint32_t i;
> >> +	int ret;
> >> +	int num;
> >> +
> >> +	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
> >
> > Seems port "all" not supported, is there a check?
> 
> What kind of check do you mean?
> I suppose support for "all" can be added. In such a case, testpmd can go over all available ports that aren't representors and print
> their representor info.
> Does this sound good to you?

Sorry, "all" will not pass the check, please ignore my comment.
Dump a single port is good enough.

> 
> [snip]
> 
> >> +	printf("Port controller: %hu\n", info->controller);
> >> +	printf("Port PF: %hu\n", info->pf);
> >> +	printf("Ranges: %u\n", info->nb_ranges);
> >> +	for (i = 0; i < info->nb_ranges; i++) {
> >> +		printf("%u:\n", i);
> >
> > Normally there will be certain amount of ranges here, how about output
> > as a table?
> 
> I'm not sure what the layout of this table should be, could you provide an example?

Range entry fields is dump in vertical here, how about dump range entry as a row? No need to align columns.

> 
> [snip]

[dpdk-dev] [PATCH v2] app/testpmd: add command to print representor info

[Andrew Rybchenko]

From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>

Make it simpler to debug configurations and code related to the representor
info API.

Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
v2:
    - change output format to log just one line per range

 app/test-pmd/cmdline.c | 135 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 82253bc751..ae700f9dd1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Show port supported ptypes"
 			" for a specific port\n\n"
 
+			"show port (port_id) representor info\n"
+			"    Show supported representors"
+			" for a specific port\n\n"
+
 			"show device info (<identifier>|all)"
 			"       Show general information about devices probed.\n\n"
 
@@ -16962,6 +16966,136 @@ cmdline_parse_inst_t cmd_show_capability = {
 	},
 };
 
+/* *** show port representors information *** */
+struct cmd_representor_info_result {
+	cmdline_fixed_string_t cmd_show;
+	cmdline_fixed_string_t cmd_port;
+	cmdline_fixed_string_t cmd_keyword;
+	portid_t cmd_pid;
+};
+
+static void
+cmd_representor_info_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_representor_info_result *res = parsed_result;
+	struct rte_eth_representor_info *info;
+	struct rte_eth_representor_range *range;
+	uint32_t range_diff;
+	uint32_t i;
+	int ret;
+	int num;
+
+	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+		fprintf(stderr, "Invalid port id %u\n", res->cmd_pid);
+		return;
+	}
+
+	ret = rte_eth_representor_info_get(res->cmd_pid, NULL);
+	if (ret < 0) {
+		fprintf(stderr,
+			"Failed to get the number of representor info ranges for port %hu: %s\n",
+			res->cmd_pid, rte_strerror(-ret));
+		return;
+	}
+	num = ret;
+
+	info = calloc(1, sizeof(*info) + num * sizeof(info->ranges[0]));
+	if (info == NULL) {
+		fprintf(stderr,
+			"Failed to allocate memory for representor info for port %hu\n",
+			res->cmd_pid);
+		return;
+	}
+	info->nb_ranges_alloc = num;
+
+	ret = rte_eth_representor_info_get(res->cmd_pid, info);
+	if (ret < 0) {
+		fprintf(stderr,
+			"Failed to get the representor info for port %hu: %s\n",
+			res->cmd_pid, rte_strerror(-ret));
+		free(info);
+		return;
+	}
+
+	printf("Port controller: %hu\n", info->controller);
+	printf("Port PF: %hu\n", info->pf);
+
+	printf("Ranges: %u\n", info->nb_ranges);
+	for (i = 0; i < info->nb_ranges; i++) {
+		range = &info->ranges[i];
+		range_diff = range->id_end - range->id_base;
+
+		printf("%u. ", i + 1);
+		printf("'%s' ", range->name);
+		if (range_diff > 0)
+			printf("[%u-%u]: ", range->id_base, range->id_end);
+		else
+			printf("[%u]: ", range->id_base);
+
+		printf("Controller %d, PF %d", range->controller, range->pf);
+
+		switch (range->type) {
+		case RTE_ETH_REPRESENTOR_NONE:
+			printf(", NONE\n");
+			break;
+		case RTE_ETH_REPRESENTOR_VF:
+			if (range_diff > 0) {
+				printf(", VF %d..%d\n", range->vf,
+				       range->vf + range_diff);
+			} else {
+				printf(", VF %d\n", range->vf);
+			}
+			break;
+		case RTE_ETH_REPRESENTOR_SF:
+			printf(", SF %d\n", range->sf);
+			break;
+		case RTE_ETH_REPRESENTOR_PF:
+			if (range_diff > 0)
+				printf("..%d\n", range->pf + range_diff);
+			else
+				printf("\n");
+			break;
+		default:
+			printf(", UNKNOWN TYPE %d\n", range->type);
+			break;
+		}
+	}
+
+	free(info);
+}
+
+cmdline_parse_token_string_t cmd_representor_info_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_show, "show");
+cmdline_parse_token_string_t cmd_representor_info_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_port, "port");
+cmdline_parse_token_num_t cmd_representor_info_pid =
+	TOKEN_NUM_INITIALIZER(struct cmd_representor_info_result,
+			cmd_pid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_representor_info_keyword1 =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_keyword, "representor");
+cmdline_parse_token_string_t cmd_representor_info_keyword2 =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_keyword, "info");
+
+cmdline_parse_inst_t cmd_representor_info = {
+	.f = cmd_representor_info_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> representor info",
+	.tokens = {
+		(void *)&cmd_representor_info_show,
+		(void *)&cmd_representor_info_port,
+		(void *)&cmd_representor_info_pid,
+		(void *)&cmd_representor_info_keyword1,
+		(void *)&cmd_representor_info_keyword2,
+		NULL,
+	},
+};
+
 /* *** show fec mode per port configuration *** */
 struct cmd_show_fec_metadata_result {
 	cmdline_fixed_string_t cmd_show;
@@ -17816,6 +17950,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_show_fec_mode,
 	(cmdline_parse_inst_t *)&cmd_set_fec_mode,
 	(cmdline_parse_inst_t *)&cmd_show_capability,
+	(cmdline_parse_inst_t *)&cmd_representor_info,
 	NULL,
 };
 
-- 
2.30.2

Re: [dpdk-dev] [PATCH v2] app/testpmd: add command to print representor info

[Xueming(Steven) Li]

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Wednesday, September 1, 2021 12:13 AM
> To: Xiaoyun Li <xiaoyun.li@intel.com>
> Cc: dev@dpdk.org; Xueming(Steven) Li <xuemingl@nvidia.com>; Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>; Andy
> Moreton <amoreton@xilinx.com>
> Subject: [PATCH v2] app/testpmd: add command to print representor info
> 
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> 
> Make it simpler to debug configurations and code related to the representor info API.
> 
> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
> v2:
>     - change output format to log just one line per range
> 
>  app/test-pmd/cmdline.c | 135 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 135 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 82253bc751..ae700f9dd1 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"    Show port supported ptypes"
>  			" for a specific port\n\n"
> 
> +			"show port (port_id) representor info\n"
> +			"    Show supported representors"
> +			" for a specific port\n\n"
> +
>  			"show device info (<identifier>|all)"
>  			"       Show general information about devices probed.\n\n"
> 
> @@ -16962,6 +16966,136 @@ cmdline_parse_inst_t cmd_show_capability = {
>  	},
>  };
> 
> +/* *** show port representors information *** */ struct
> +cmd_representor_info_result {
> +	cmdline_fixed_string_t cmd_show;
> +	cmdline_fixed_string_t cmd_port;
> +	cmdline_fixed_string_t cmd_keyword;
> +	portid_t cmd_pid;
> +};
> +
> +static void
> +cmd_representor_info_parsed(void *parsed_result,
> +		__rte_unused struct cmdline *cl,
> +		__rte_unused void *data)
> +{
> +	struct cmd_representor_info_result *res = parsed_result;
> +	struct rte_eth_representor_info *info;
> +	struct rte_eth_representor_range *range;
> +	uint32_t range_diff;
> +	uint32_t i;
> +	int ret;
> +	int num;
> +
> +	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
> +		fprintf(stderr, "Invalid port id %u\n", res->cmd_pid);
> +		return;
> +	}
> +
> +	ret = rte_eth_representor_info_get(res->cmd_pid, NULL);
> +	if (ret < 0) {
> +		fprintf(stderr,
> +			"Failed to get the number of representor info ranges for port %hu: %s\n",
> +			res->cmd_pid, rte_strerror(-ret));
> +		return;
> +	}
> +	num = ret;
> +
> +	info = calloc(1, sizeof(*info) + num * sizeof(info->ranges[0]));
> +	if (info == NULL) {
> +		fprintf(stderr,
> +			"Failed to allocate memory for representor info for port %hu\n",
> +			res->cmd_pid);
> +		return;
> +	}
> +	info->nb_ranges_alloc = num;
> +
> +	ret = rte_eth_representor_info_get(res->cmd_pid, info);
> +	if (ret < 0) {
> +		fprintf(stderr,
> +			"Failed to get the representor info for port %hu: %s\n",
> +			res->cmd_pid, rte_strerror(-ret));
> +		free(info);
> +		return;
> +	}
> +
> +	printf("Port controller: %hu\n", info->controller);
> +	printf("Port PF: %hu\n", info->pf);
> +
> +	printf("Ranges: %u\n", info->nb_ranges);
> +	for (i = 0; i < info->nb_ranges; i++) {
> +		range = &info->ranges[i];
> +		range_diff = range->id_end - range->id_base;
> +
> +		printf("%u. ", i + 1);
> +		printf("'%s' ", range->name);
> +		if (range_diff > 0)
> +			printf("[%u-%u]: ", range->id_base, range->id_end);
> +		else
> +			printf("[%u]: ", range->id_base);
> +
> +		printf("Controller %d, PF %d", range->controller, range->pf);
> +
> +		switch (range->type) {
> +		case RTE_ETH_REPRESENTOR_NONE:
> +			printf(", NONE\n");
> +			break;
> +		case RTE_ETH_REPRESENTOR_VF:
> +			if (range_diff > 0) {
> +				printf(", VF %d..%d\n", range->vf,
> +				       range->vf + range_diff);
> +			} else {
> +				printf(", VF %d\n", range->vf);
> +			}
> +			break;
> +		case RTE_ETH_REPRESENTOR_SF:
> +			printf(", SF %d\n", range->sf);
> +			break;
> +		case RTE_ETH_REPRESENTOR_PF:
> +			if (range_diff > 0)
> +				printf("..%d\n", range->pf + range_diff);
> +			else
> +				printf("\n");
> +			break;
> +		default:
> +			printf(", UNKNOWN TYPE %d\n", range->type);
> +			break;
> +		}
> +	}
> +
> +	free(info);
> +}
> +
> +cmdline_parse_token_string_t cmd_representor_info_show =
> +	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_show, "show");
> +cmdline_parse_token_string_t cmd_representor_info_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_port, "port");
> +cmdline_parse_token_num_t cmd_representor_info_pid =
> +	TOKEN_NUM_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_pid, RTE_UINT16);
> +cmdline_parse_token_string_t cmd_representor_info_keyword1 =
> +	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_keyword, "representor");
> +cmdline_parse_token_string_t cmd_representor_info_keyword2 =
> +	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
> +			cmd_keyword, "info");
> +
> +cmdline_parse_inst_t cmd_representor_info = {
> +	.f = cmd_representor_info_parsed,
> +	.data = NULL,
> +	.help_str = "show port <port_id> representor info",
> +	.tokens = {
> +		(void *)&cmd_representor_info_show,
> +		(void *)&cmd_representor_info_port,
> +		(void *)&cmd_representor_info_pid,
> +		(void *)&cmd_representor_info_keyword1,
> +		(void *)&cmd_representor_info_keyword2,
> +		NULL,
> +	},
> +};
> +
>  /* *** show fec mode per port configuration *** */  struct cmd_show_fec_metadata_result {
>  	cmdline_fixed_string_t cmd_show;
> @@ -17816,6 +17950,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_show_fec_mode,
>  	(cmdline_parse_inst_t *)&cmd_set_fec_mode,
>  	(cmdline_parse_inst_t *)&cmd_show_capability,
> +	(cmdline_parse_inst_t *)&cmd_representor_info,
>  	NULL,
>  };
> 
> --
> 2.30.2

Reviewed-by: Xueming(Steven) Li <xuemingl@nvidia.com>

Re: [dpdk-dev] [PATCH v2] app/testpmd: add command to print representor info

[Ferruh Yigit]

On 8/31/2021 5:12 PM, Andrew Rybchenko wrote:
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> 
> Make it simpler to debug configurations and code related to the representor
> info API.
> 
> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
> v2:
>     - change output format to log just one line per range
> 
>  app/test-pmd/cmdline.c | 135 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 135 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 82253bc751..ae700f9dd1 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"    Show port supported ptypes"
>  			" for a specific port\n\n"
>  
> +			"show port (port_id) representor info\n"
> +			"    Show supported representors"
> +			" for a specific port\n\n"
> +

What do you think extending existing "show port info #" command instead of
creating a new command for it?

Since "show port info #" is a well known command, it can simplify the usage.
When port is representor port it can display additional info.

Re: [dpdk-dev] [PATCH v2] app/testpmd: add command to print representor info

[Andrew Rybchenko]

On 9/14/21 6:52 PM, Ferruh Yigit wrote:
> On 8/31/2021 5:12 PM, Andrew Rybchenko wrote:
>> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>
>> Make it simpler to debug configurations and code related to the representor
>> info API.
>>
>> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>> ---
>> v2:
>>     - change output format to log just one line per range
>>
>>  app/test-pmd/cmdline.c | 135 +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 135 insertions(+)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 82253bc751..ae700f9dd1 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>>  			"    Show port supported ptypes"
>>  			" for a specific port\n\n"
>>  
>> +			"show port (port_id) representor info\n"
>> +			"    Show supported representors"
>> +			" for a specific port\n\n"
>> +
> 
> What do you think extending existing "show port info #" command instead of
> creating a new command for it?

My fear with such approach is that output of the "show port
info #" is already too long and adding representors info
there will make it even much longer.

> Since "show port info #" is a well known command, it can simplify the usage.
> When port is representor port it can display additional info.
> 

Just to be clear: it will output information for "backer"
(or parent) port which should be used to create representors.

Re: [dpdk-dev] [PATCH v2] app/testpmd: add command to print representor info

[Ferruh Yigit]

On 9/14/2021 5:17 PM, Andrew Rybchenko wrote:
> On 9/14/21 6:52 PM, Ferruh Yigit wrote:
>> On 8/31/2021 5:12 PM, Andrew Rybchenko wrote:
>>> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>>
>>> Make it simpler to debug configurations and code related to the representor
>>> info API.
>>>
>>> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>>> ---
>>> v2:
>>>     - change output format to log just one line per range
>>>
>>>  app/test-pmd/cmdline.c | 135 +++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 135 insertions(+)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>> index 82253bc751..ae700f9dd1 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>  			"    Show port supported ptypes"
>>>  			" for a specific port\n\n"
>>>  
>>> +			"show port (port_id) representor info\n"
>>> +			"    Show supported representors"
>>> +			" for a specific port\n\n"
>>> +
>>
>> What do you think extending existing "show port info #" command instead of
>> creating a new command for it?
> 
> My fear with such approach is that output of the "show port
> info #" is already too long and adding representors info
> there will make it even much longer.
> 

That is fair concern, what about extend existing command with a new keyword to
just print representor info:
"show port info # representor"

>> Since "show port info #" is a well known command, it can simplify the usage.
>> When port is representor port it can display additional info.
>>
> 
> Just to be clear: it will output information for "backer"
> (or parent) port which should be used to create representors.
> 

[dpdk-dev] [PATCH v3] app/testpmd: add command to print representor info

[Andrew Rybchenko]

From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>

Make it simpler to debug configurations and code related to the representor
info API.

Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Reviewed-by: Xueming(Steven) Li <xuemingl@nvidia.com>
---
v3:
    - change command to "show port info (port_id) representor"

v2:
    - change output format to log just one line per range

 app/test-pmd/cmdline.c | 137 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 137 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7dd3965d6f..2f24d33337 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -162,6 +162,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|dcb_tc) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port info (port_id) representor\n"
+			"    Show supported representors"
+			" for a specific port\n\n"
+
 			"show port port_id (module_eeprom|eeprom)\n"
 			"    Display the module EEPROM or EEPROM information for port_id.\n\n"
 
@@ -7904,6 +7908,138 @@ cmdline_parse_inst_t cmd_showport = {
 	},
 };
 
+/* *** show port representors information *** */
+struct cmd_representor_info_result {
+	cmdline_fixed_string_t cmd_show;
+	cmdline_fixed_string_t cmd_port;
+	cmdline_fixed_string_t cmd_info;
+	cmdline_fixed_string_t cmd_keyword;
+	portid_t cmd_pid;
+};
+
+static void
+cmd_representor_info_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_representor_info_result *res = parsed_result;
+	struct rte_eth_representor_info *info;
+	struct rte_eth_representor_range *range;
+	uint32_t range_diff;
+	uint32_t i;
+	int ret;
+	int num;
+
+	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+		fprintf(stderr, "Invalid port id %u\n", res->cmd_pid);
+		return;
+	}
+
+	ret = rte_eth_representor_info_get(res->cmd_pid, NULL);
+	if (ret < 0) {
+		fprintf(stderr,
+			"Failed to get the number of representor info ranges for port %hu: %s\n",
+			res->cmd_pid, rte_strerror(-ret));
+		return;
+	}
+	num = ret;
+
+	info = calloc(1, sizeof(*info) + num * sizeof(info->ranges[0]));
+	if (info == NULL) {
+		fprintf(stderr,
+			"Failed to allocate memory for representor info for port %hu\n",
+			res->cmd_pid);
+		return;
+	}
+	info->nb_ranges_alloc = num;
+
+	ret = rte_eth_representor_info_get(res->cmd_pid, info);
+	if (ret < 0) {
+		fprintf(stderr,
+			"Failed to get the representor info for port %hu: %s\n",
+			res->cmd_pid, rte_strerror(-ret));
+		free(info);
+		return;
+	}
+
+	printf("Port controller: %hu\n", info->controller);
+	printf("Port PF: %hu\n", info->pf);
+
+	printf("Ranges: %u\n", info->nb_ranges);
+	for (i = 0; i < info->nb_ranges; i++) {
+		range = &info->ranges[i];
+		range_diff = range->id_end - range->id_base;
+
+		printf("%u. ", i + 1);
+		printf("'%s' ", range->name);
+		if (range_diff > 0)
+			printf("[%u-%u]: ", range->id_base, range->id_end);
+		else
+			printf("[%u]: ", range->id_base);
+
+		printf("Controller %d, PF %d", range->controller, range->pf);
+
+		switch (range->type) {
+		case RTE_ETH_REPRESENTOR_NONE:
+			printf(", NONE\n");
+			break;
+		case RTE_ETH_REPRESENTOR_VF:
+			if (range_diff > 0) {
+				printf(", VF %d..%d\n", range->vf,
+				       range->vf + range_diff);
+			} else {
+				printf(", VF %d\n", range->vf);
+			}
+			break;
+		case RTE_ETH_REPRESENTOR_SF:
+			printf(", SF %d\n", range->sf);
+			break;
+		case RTE_ETH_REPRESENTOR_PF:
+			if (range_diff > 0)
+				printf("..%d\n", range->pf + range_diff);
+			else
+				printf("\n");
+			break;
+		default:
+			printf(", UNKNOWN TYPE %d\n", range->type);
+			break;
+		}
+	}
+
+	free(info);
+}
+
+cmdline_parse_token_string_t cmd_representor_info_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_show, "show");
+cmdline_parse_token_string_t cmd_representor_info_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_port, "port");
+cmdline_parse_token_string_t cmd_representor_info_info =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_info, "info");
+cmdline_parse_token_num_t cmd_representor_info_pid =
+	TOKEN_NUM_INITIALIZER(struct cmd_representor_info_result,
+			cmd_pid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_representor_info_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_keyword, "representor");
+
+cmdline_parse_inst_t cmd_representor_info = {
+	.f = cmd_representor_info_parsed,
+	.data = NULL,
+	.help_str = "show port info <port_id> representor",
+	.tokens = {
+		(void *)&cmd_representor_info_show,
+		(void *)&cmd_representor_info_port,
+		(void *)&cmd_representor_info_info,
+		(void *)&cmd_representor_info_pid,
+		(void *)&cmd_representor_info_keyword,
+		NULL,
+	},
+};
+
+
 /* *** SHOW DEVICE INFO *** */
 struct cmd_showdevice_result {
 	cmdline_fixed_string_t show;
@@ -17553,6 +17689,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_showqueue,
 	(cmdline_parse_inst_t *)&cmd_showeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
+	(cmdline_parse_inst_t *)&cmd_representor_info,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
 	(cmdline_parse_inst_t *)&cmd_showfwdall,
-- 
2.30.2

Re: [dpdk-dev] [PATCH v2] app/testpmd: add command to print representor info

[Andrew Rybchenko]

On 9/14/21 7:36 PM, Ferruh Yigit wrote:
> On 9/14/2021 5:17 PM, Andrew Rybchenko wrote:
>> On 9/14/21 6:52 PM, Ferruh Yigit wrote:
>>> On 8/31/2021 5:12 PM, Andrew Rybchenko wrote:
>>>> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>>>
>>>> Make it simpler to debug configurations and code related to the representor
>>>> info API.
>>>>
>>>> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>>>> ---
>>>> v2:
>>>>     - change output format to log just one line per range
>>>>
>>>>  app/test-pmd/cmdline.c | 135 +++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 135 insertions(+)
>>>>
>>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>>> index 82253bc751..ae700f9dd1 100644
>>>> --- a/app/test-pmd/cmdline.c
>>>> +++ b/app/test-pmd/cmdline.c
>>>> @@ -236,6 +236,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>>>>  			"    Show port supported ptypes"
>>>>  			" for a specific port\n\n"
>>>>  
>>>> +			"show port (port_id) representor info\n"
>>>> +			"    Show supported representors"
>>>> +			" for a specific port\n\n"
>>>> +
>>>
>>> What do you think extending existing "show port info #" command instead of
>>> creating a new command for it?
>>
>> My fear with such approach is that output of the "show port
>> info #" is already too long and adding representors info
>> there will make it even much longer.
>>
> 
> That is fair concern, what about extend existing command with a new keyword to
> just print representor info:
> "show port info # representor"

Good idea, see v3.

>>> Since "show port info #" is a well known command, it can simplify the usage.
>>> When port is representor port it can display additional info.
>>>
>>
>> Just to be clear: it will output information for "backer"
>> (or parent) port which should be used to create representors.
>>

Re: [dpdk-dev] [PATCH v3] app/testpmd: add command to print representor info

[Ajit Khaparde]

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

On Wed, Sep 15, 2021 at 6:48 AM Andrew Rybchenko
<andrew.rybchenko@oktetlabs.ru> wrote:
>
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>
> Make it simpler to debug configurations and code related to the representor
> info API.
>
> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> Reviewed-by: Xueming(Steven) Li <xuemingl@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

Re: [dpdk-dev] [PATCH v3] app/testpmd: add command to print representor info

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Wednesday, September 15, 2021 21:48
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: dev@dpdk.org; Xueming Li <xuemingl@nvidia.com>; Viacheslav Galaktionov
> <viacheslav.galaktionov@oktetlabs.ru>; Andy Moreton <amoreton@xilinx.com>
> Subject: [PATCH v3] app/testpmd: add command to print representor info
> 
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> 
> Make it simpler to debug configurations and code related to the representor
> info API.
> 
> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> Reviewed-by: Xueming(Steven) Li <xuemingl@nvidia.com>
> ---
> v3:
>     - change command to "show port info (port_id) representor"
> 
> v2:
>     - change output format to log just one line per range
> 
>  app/test-pmd/cmdline.c | 137
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 137 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 7dd3965d6f..2f24d33337 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -162,6 +162,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"show port (info|stats|summary|xstats|fdir|dcb_tc)
> (port_id|all)\n"
>  			"    Display information for port_id, or all.\n\n"
> 
> +			"show port info (port_id) representor\n"
> +			"    Show supported representors"
> +			" for a specific port\n\n"

Why not just put "    Show supported representors for a specific port\n\n" in one sentence?

> +
>  			"show port port_id (module_eeprom|eeprom)\n"
>  			"    Display the module EEPROM or EEPROM
> information for port_id.\n\n"
> 
> @@ -7904,6 +7908,138 @@ cmdline_parse_inst_t cmd_showport = {
>  	},
>  };
> 
<snip>
> +		printf("Controller %d, PF %d", range->controller, range->pf);
> +
> +		switch (range->type) {
> +		case RTE_ETH_REPRESENTOR_NONE:
> +			printf(", NONE\n");
> +			break;
> +		case RTE_ETH_REPRESENTOR_VF:
> +			if (range_diff > 0) {
> +				printf(", VF %d..%d\n", range->vf,
> +				       range->vf + range_diff);
> +			} else {
> +				printf(", VF %d\n", range->vf);
> +			}

There's no need of braces for this "if else" like others.

BRs
Xiaoyun

> +			break;
> +		case RTE_ETH_REPRESENTOR_SF:
> +			printf(", SF %d\n", range->sf);
> +			break;
> +		case RTE_ETH_REPRESENTOR_PF:
> +			if (range_diff > 0)
> +				printf("..%d\n", range->pf + range_diff);
> +			else
> +				printf("\n");
> +			break;
> +		default:
> +			printf(", UNKNOWN TYPE %d\n", range->type);
> +			break;
> +		}
> +	}
> +
> +	free(info);
> +}
> +
<snip>
> --
> 2.30.2

[dpdk-dev] [PATCH v4] app/testpmd: add command to print representor info

[Andrew Rybchenko]

From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>

Make it simpler to debug configurations and code related to the representor
info API.

Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
Reviewed-by: Xueming(Steven) Li <xuemingl@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
v4:
    - apply review notes from Xiaoyun Li

v3:
    - change command to "show port info (port_id) representor"

v2:
    - change output format to log just one line per range

 app/test-pmd/cmdline.c | 135 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 826256b0b3..f86a3c03e3 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -162,6 +162,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (info|stats|summary|xstats|fdir|dcb_tc) (port_id|all)\n"
 			"    Display information for port_id, or all.\n\n"
 
+			"show port info (port_id) representor\n"
+			"    Show supported representors for a specific port\n\n"
+
 			"show port port_id (module_eeprom|eeprom)\n"
 			"    Display the module EEPROM or EEPROM information for port_id.\n\n"
 
@@ -7888,6 +7891,137 @@ cmdline_parse_inst_t cmd_showport = {
 	},
 };
 
+/* *** show port representors information *** */
+struct cmd_representor_info_result {
+	cmdline_fixed_string_t cmd_show;
+	cmdline_fixed_string_t cmd_port;
+	cmdline_fixed_string_t cmd_info;
+	cmdline_fixed_string_t cmd_keyword;
+	portid_t cmd_pid;
+};
+
+static void
+cmd_representor_info_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_representor_info_result *res = parsed_result;
+	struct rte_eth_representor_info *info;
+	struct rte_eth_representor_range *range;
+	uint32_t range_diff;
+	uint32_t i;
+	int ret;
+	int num;
+
+	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+		fprintf(stderr, "Invalid port id %u\n", res->cmd_pid);
+		return;
+	}
+
+	ret = rte_eth_representor_info_get(res->cmd_pid, NULL);
+	if (ret < 0) {
+		fprintf(stderr,
+			"Failed to get the number of representor info ranges for port %hu: %s\n",
+			res->cmd_pid, rte_strerror(-ret));
+		return;
+	}
+	num = ret;
+
+	info = calloc(1, sizeof(*info) + num * sizeof(info->ranges[0]));
+	if (info == NULL) {
+		fprintf(stderr,
+			"Failed to allocate memory for representor info for port %hu\n",
+			res->cmd_pid);
+		return;
+	}
+	info->nb_ranges_alloc = num;
+
+	ret = rte_eth_representor_info_get(res->cmd_pid, info);
+	if (ret < 0) {
+		fprintf(stderr,
+			"Failed to get the representor info for port %hu: %s\n",
+			res->cmd_pid, rte_strerror(-ret));
+		free(info);
+		return;
+	}
+
+	printf("Port controller: %hu\n", info->controller);
+	printf("Port PF: %hu\n", info->pf);
+
+	printf("Ranges: %u\n", info->nb_ranges);
+	for (i = 0; i < info->nb_ranges; i++) {
+		range = &info->ranges[i];
+		range_diff = range->id_end - range->id_base;
+
+		printf("%u. ", i + 1);
+		printf("'%s' ", range->name);
+		if (range_diff > 0)
+			printf("[%u-%u]: ", range->id_base, range->id_end);
+		else
+			printf("[%u]: ", range->id_base);
+
+		printf("Controller %d, PF %d", range->controller, range->pf);
+
+		switch (range->type) {
+		case RTE_ETH_REPRESENTOR_NONE:
+			printf(", NONE\n");
+			break;
+		case RTE_ETH_REPRESENTOR_VF:
+			if (range_diff > 0)
+				printf(", VF %d..%d\n", range->vf,
+				       range->vf + range_diff);
+			else
+				printf(", VF %d\n", range->vf);
+			break;
+		case RTE_ETH_REPRESENTOR_SF:
+			printf(", SF %d\n", range->sf);
+			break;
+		case RTE_ETH_REPRESENTOR_PF:
+			if (range_diff > 0)
+				printf("..%d\n", range->pf + range_diff);
+			else
+				printf("\n");
+			break;
+		default:
+			printf(", UNKNOWN TYPE %d\n", range->type);
+			break;
+		}
+	}
+
+	free(info);
+}
+
+cmdline_parse_token_string_t cmd_representor_info_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_show, "show");
+cmdline_parse_token_string_t cmd_representor_info_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_port, "port");
+cmdline_parse_token_string_t cmd_representor_info_info =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_info, "info");
+cmdline_parse_token_num_t cmd_representor_info_pid =
+	TOKEN_NUM_INITIALIZER(struct cmd_representor_info_result,
+			cmd_pid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_representor_info_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_representor_info_result,
+			cmd_keyword, "representor");
+
+cmdline_parse_inst_t cmd_representor_info = {
+	.f = cmd_representor_info_parsed,
+	.data = NULL,
+	.help_str = "show port info <port_id> representor",
+	.tokens = {
+		(void *)&cmd_representor_info_show,
+		(void *)&cmd_representor_info_port,
+		(void *)&cmd_representor_info_info,
+		(void *)&cmd_representor_info_pid,
+		(void *)&cmd_representor_info_keyword,
+		NULL,
+	},
+};
+
+
 /* *** SHOW DEVICE INFO *** */
 struct cmd_showdevice_result {
 	cmdline_fixed_string_t show;
@@ -17275,6 +17409,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_showqueue,
 	(cmdline_parse_inst_t *)&cmd_showeeprom,
 	(cmdline_parse_inst_t *)&cmd_showportall,
+	(cmdline_parse_inst_t *)&cmd_representor_info,
 	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showcfg,
 	(cmdline_parse_inst_t *)&cmd_showfwdall,
-- 
2.30.2

Re: [dpdk-dev] [PATCH v3] app/testpmd: add command to print representor info

[Andrew Rybchenko]

Hi Xiaoyun,

On 10/11/21 11:32 AM, Li, Xiaoyun wrote:
> Hi
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Wednesday, September 15, 2021 21:48
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>
>> Cc: dev@dpdk.org; Xueming Li <xuemingl@nvidia.com>; Viacheslav Galaktionov
>> <viacheslav.galaktionov@oktetlabs.ru>; Andy Moreton <amoreton@xilinx.com>
>> Subject: [PATCH v3] app/testpmd: add command to print representor info
>>
>> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>
>> Make it simpler to debug configurations and code related to the representor
>> info API.
>>
>> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>> Reviewed-by: Xueming(Steven) Li <xuemingl@nvidia.com>
>> ---
>> v3:
>>     - change command to "show port info (port_id) representor"
>>
>> v2:
>>     - change output format to log just one line per range
>>
>>  app/test-pmd/cmdline.c | 137
>> +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 137 insertions(+)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
>> 7dd3965d6f..2f24d33337 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -162,6 +162,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>>  			"show port (info|stats|summary|xstats|fdir|dcb_tc)
>> (port_id|all)\n"
>>  			"    Display information for port_id, or all.\n\n"
>>
>> +			"show port info (port_id) representor\n"
>> +			"    Show supported representors"
>> +			" for a specific port\n\n"
> 
> Why not just put "    Show supported representors for a specific port\n\n" in one sentence?
> 

Thanks, done in v4.

>> +
>>  			"show port port_id (module_eeprom|eeprom)\n"
>>  			"    Display the module EEPROM or EEPROM
>> information for port_id.\n\n"
>>
>> @@ -7904,6 +7908,138 @@ cmdline_parse_inst_t cmd_showport = {
>>  	},
>>  };
>>
> <snip>
>> +		printf("Controller %d, PF %d", range->controller, range->pf);
>> +
>> +		switch (range->type) {
>> +		case RTE_ETH_REPRESENTOR_NONE:
>> +			printf(", NONE\n");
>> +			break;
>> +		case RTE_ETH_REPRESENTOR_VF:
>> +			if (range_diff > 0) {
>> +				printf(", VF %d..%d\n", range->vf,
>> +				       range->vf + range_diff);
>> +			} else {
>> +				printf(", VF %d\n", range->vf);
>> +			}
> 
> There's no need of braces for this "if else" like others.

Fixed in v4.

> 
> BRs
> Xiaoyun
> 
>> +			break;
>> +		case RTE_ETH_REPRESENTOR_SF:
>> +			printf(", SF %d\n", range->sf);
>> +			break;
>> +		case RTE_ETH_REPRESENTOR_PF:
>> +			if (range_diff > 0)
>> +				printf("..%d\n", range->pf + range_diff);
>> +			else
>> +				printf("\n");
>> +			break;
>> +		default:
>> +			printf(", UNKNOWN TYPE %d\n", range->type);
>> +			break;
>> +		}
>> +	}
>> +
>> +	free(info);
>> +}
>> +
> <snip>
>> --
>> 2.30.2

Many thanks for review notes,
Andrew.

Re: [dpdk-dev] [PATCH v4] app/testpmd: add command to print representor info

[Li, Xiaoyun]

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, October 11, 2021 21:09
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: dev@dpdk.org; Viacheslav Galaktionov
> <viacheslav.galaktionov@oktetlabs.ru>; Andy Moreton <amoreton@xilinx.com>;
> Xueming Li <xuemingl@nvidia.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>
> Subject: [PATCH v4] app/testpmd: add command to print representor info
> 
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> 
> Make it simpler to debug configurations and code related to the representor
> info API.
> 
> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> Reviewed-by: Xueming(Steven) Li <xuemingl@nvidia.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
> v4:
>     - apply review notes from Xiaoyun Li
> 
> v3:
>     - change command to "show port info (port_id) representor"
> 
> v2:
>     - change output format to log just one line per range
> 
>  app/test-pmd/cmdline.c | 135
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 135 insertions(+)
> 
Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

Re: [dpdk-dev] [PATCH v4] app/testpmd: add command to print representor info

[Ferruh Yigit]

On 10/12/2021 2:50 AM, Li, Xiaoyun wrote:
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, October 11, 2021 21:09
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>
>> Cc: dev@dpdk.org; Viacheslav Galaktionov
>> <viacheslav.galaktionov@oktetlabs.ru>; Andy Moreton <amoreton@xilinx.com>;
>> Xueming Li <xuemingl@nvidia.com>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>
>> Subject: [PATCH v4] app/testpmd: add command to print representor info
>>
>> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>
>> Make it simpler to debug configurations and code related to the representor
>> info API.
>>
>> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>> Reviewed-by: Xueming(Steven) Li <xuemingl@nvidia.com>
>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> ---
>> v4:
>>      - apply review notes from Xiaoyun Li
>>
>> v3:
>>      - change command to "show port info (port_id) representor"
>>
>> v2:
>>      - change output format to log just one line per range
>>
>>   app/test-pmd/cmdline.c | 135
>> +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 135 insertions(+)
>>
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 

Applied to dpdk-next-net/main, thanks.