Hi Xiaoyun,
On 10/11/21 11:32 AM, Li, Xiaoyun wrote:
> Hi
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
>> Sent: Wednesday, September 15, 2021 21:48
>> To: Li, Xiaoyun <xiaoyun...@intel.com>
>> Cc: dev@dpdk.org; Xueming Li <xuemi...@nvidia.com>; Viacheslav Galaktionov
>> <viacheslav.galaktio...@oktetlabs.ru>; Andy Moreton <amore...@xilinx.com>
>> Subject: [PATCH v3] app/testpmd: add command to print representor info
>>
>> From: Viacheslav Galaktionov <viacheslav.galaktio...@oktetlabs.ru>
>>
>> Make it simpler to debug configurations and code related to the representor
>> info API.
>>
>> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktio...@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
>> Reviewed-by: Andy Moreton <amore...@xilinx.com>
>> Reviewed-by: Xueming(Steven) Li <xuemi...@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.
