[dpdk-dev] [PATCH v6 5/9] ethdev: support PF index in representor

[Xueming(Steven) Li]

>-----Original Message-----
>From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
>Sent: Monday, February 15, 2021 4:35 PM
>To: Xueming(Steven) Li <xuemingl at nvidia.com>
>Cc: dev at dpdk.org; Slava Ovsiienko <viacheslavo at nvidia.com>; Asaf Penso <asafp at nvidia.com>; NBU-Contact-Thomas Monjalon
><thomas at monjalon.net>; Ferruh Yigit <ferruh.yigit at intel.com>
>Subject: Re: [PATCH v6 5/9] ethdev: support PF index in representor
>
>On 2/14/21 6:21 AM, Xueming Li wrote:
>> With Kernel bonding, multiple underlying PFs are bonded, VFs come from
>> different PF, need to identify representor of VFs unambiguously by
>> adding PF index.
>>
>> This patch introduces optional 'pf' section to representor devargs
>> syntax, examples:
>>  representor=pf0vf0             - single VF representor
>>  representor=pf[0-1]sf[0-1023]  - SF representors from 2 PFs
>>
>> PF type representor is supported by using standalone 'pf' section:
>>  representor=pf1                - PF representor
>>
>>
>> Signed-off-by: Xueming Li <xuemingl at nvidia.com>
>> Acked-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>
>> Acked-by: Thomas Monjalon <thomas at monjalon.net>
>> ---
>>  doc/guides/prog_guide/poll_mode_drv.rst |  2 ++
>>  lib/librte_ethdev/ethdev_private.c      | 18 ++++++++++++++++--
>>  lib/librte_ethdev/rte_ethdev.h          |  1 +
>>  3 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst
>> b/doc/guides/prog_guide/poll_mode_drv.rst
>> index 86e5867f1b..b2147aad30 100644
>> --- a/doc/guides/prog_guide/poll_mode_drv.rst
>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
>> @@ -382,6 +382,8 @@ parameters to those ports.
>>     -a DBDF,representor=sf[1,3,5]
>>     -a DBDF,representor=sf[0-1023]
>>     -a DBDF,representor=sf[0,2-4,7,9-11]
>> +   -a DBDF,representor=pf1vf0
>> +   -a DBDF,representor=pf[0-1]sf[0-127]
>>
>>  Note: PMDs are not required to support the standard device arguments
>> and users  should consult the relevant PMD documentation to see support devargs.
>> diff --git a/lib/librte_ethdev/ethdev_private.c
>> b/lib/librte_ethdev/ethdev_private.c
>> index 13c191192e..eea0686020 100644
>> --- a/lib/librte_ethdev/ethdev_private.c
>> +++ b/lib/librte_ethdev/ethdev_private.c
>> @@ -118,8 +118,8 @@ rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
>>   *
>>   * Representor format:
>>   *   #: range or single number of VF representor - legacy
>> - *   vf#: VF port representor/s
>> - *   sf#: SF port representor/s
>> + *   [pf#]vf#: VF port representor/s
>> + *   [pf#]sf#: SF port representor/s
>>   *
>>   * Examples of #:
>>   *  2               - single
>> @@ -131,6 +131,14 @@ rte_eth_devargs_parse_representor_ports(char
>> *str, void *data)  {
>>  	struct rte_eth_devargs *eth_da = data;
>>
>> +	if (str[0] == 'p' && str[1] == 'f') {
>> +		eth_da->type = RTE_ETH_REPRESENTOR_PF;
>
>Is it actually supported? Above documentation does not say so, i.e. -a DBDF,representor=pf1 is missing.

Yes, will update.

>
>> +		str += 2;
>> +		str = rte_eth_devargs_process_list(str, eth_da->ports,
>> +				&eth_da->nb_ports, RTE_DIM(eth_da->ports));
>> +		if (str == NULL)
>> +			goto err;
>> +	}
>>  	if (str[0] == 'v' && str[1] == 'f') {
>>  		eth_da->type = RTE_ETH_REPRESENTOR_VF;
>>  		str += 2;
>> @@ -138,11 +146,17 @@ rte_eth_devargs_parse_representor_ports(char *str, void *data)
>>  		eth_da->type = RTE_ETH_REPRESENTOR_SF;
>>  		str += 2;
>>  	} else {
>
>If PF representors are actually supported, shouldn't it be if (str[0] != '\0') plus rte_eth_devargs_process_list() after the body should be
>inside in fact?

Yes, need this check, I'll add it after 'pf' parsing.
Also, 'c' should be followed by 'pf', 'pf' should followed by 'vf' or 'sf', I'll update check.

>
>> +		/* Don't mix legacy syntax with 'pf' section. */
>> +		if (eth_da->type == RTE_ETH_REPRESENTOR_PF) {
>> +			str = NULL;
>> +			goto err;
>> +		}
>>  		eth_da->type = RTE_ETH_REPRESENTOR_VF;
>>  	}
>>  	str = rte_eth_devargs_process_list(str, eth_da->representor_ports,
>>  		&eth_da->nb_representor_ports,
>>  		RTE_DIM(eth_da->representor_ports));
>> +err:
>>  	if (str == NULL)
>>  		RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
>>  	return str == NULL ? -1 : 0;
>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>> b/lib/librte_ethdev/rte_ethdev.h index 26b5e109c3..9cd519bf59 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1513,6 +1513,7 @@ enum rte_eth_representor_type {
>>  	RTE_ETH_REPRESENTOR_NONE, /**< not a representor. */
>>  	RTE_ETH_REPRESENTOR_VF,   /**< representor of Virtual Function. */
>>  	RTE_ETH_REPRESENTOR_SF,   /**< representor of Sub Function. */
>> +	RTE_ETH_REPRESENTOR_PF,   /**< representor of Physical Function. */
>>  };
>>
>>  /**
>>