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

[Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>]

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]