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

["Li, Xiaoyun" <xiaoyun.li@intel.com>]

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