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

["Xueming(Steven) Li" <xuemingl@nvidia.com>]

> -----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]