On 1/19/21 12:30 PM, Xueming(Steven) Li wrote:
> Hi Andrew,
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
>> Sent: Tuesday, January 19, 2021 4:01 PM
>> To: Xueming(Steven) Li <xuemi...@nvidia.com>
>> Cc: dev@dpdk.org; Slava Ovsiienko <viachesl...@nvidia.com>; Asaf Penso
>> <as...@nvidia.com>; NBU-Contact-Thomas Monjalon
>> <tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com>
>> Subject: Re: [PATCH v5 5/9] ethdev: support PF index in representor
>>
>> On 1/19/21 10:14 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
>>
>>
>> Don't we need
>> representor=pf3
>> i.e. without VF or sub-function?
>
> Standalone PF not used by Mellnaox PMD, but should be supported.
> Will update.
>>
>>>
>>>
>>> Signed-off-by: Xueming Li <xuemi...@nvidia.com>
>>> Acked-by: Viacheslav Ovsiienko <viachesl...@nvidia.com>
>>> Acked-by: Thomas Monjalon <tho...@monjalon.net>
>>> ---
>>> doc/guides/prog_guide/poll_mode_drv.rst | 2 ++
>>> lib/librte_ethdev/ethdev_private.c | 13 +++++++++++--
>>> 2 files changed, 13 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 d513f035d0..b9fdbd0f72 100644
>>> --- a/lib/librte_ethdev/ethdev_private.c
>>> +++ b/lib/librte_ethdev/ethdev_private.c
>>> @@ -120,8 +120,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
>>> @@ -133,6 +133,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;
>>> + str += 2;
>>> + str = rte_eth_devargs_process_list(str, eth_da->ports,
>>> + ð_da->nb_ports, RTE_MAX_ETHPORTS);
>>
>> May be RTE_MAX_ETHPORTS -> RTE_DIM(eth_da->ports) ?
>
> Same here, the dim could be different than MAX value.
Hold on, just for my understanding. The maximum says how
many entries could be added to the array. So, why?
>>
>>> + if (str == NULL)
>>> + goto err;
>>
>> Below we should not allow legacy VF syntax without "vf" prefix.
>
> For backward compatibility, default numbers to "vf", otherwise some existing
> app like OVS that working with VF will break.
I mean if new syntax is used (i.e. we have pfX prefix), we must
deny legacy syntax for VFs below.
>>
>>> + }
>>> if (str[0] == 'v' && str[1] == 'f') {
>>> eth_da->type = RTE_ETH_REPRESENTOR_VF;
>>> str += 2;
>>> @@ -144,6 +152,7 @@ rte_eth_devargs_parse_representor_ports(char *str,
>> void *data)
>>> }
>>> str = rte_eth_devargs_process_list(str, eth_da->representor_ports,
>>> ð_da->nb_representor_ports, RTE_MAX_ETHPORTS);
>>> +err:
>>> if (str == NULL)
>>> RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
>>> return str == NULL ? -1 : 0;
>>>
>
