[dpdk-dev] [PATCH v5 0/9] ethdev: support SubFunction representor

[Xueming(Steven) Li]

Hi Andrew,

>-----Original Message-----
>From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
>Sent: Friday, January 22, 2021 4:21 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>
>Subject: Re: [dpdk-dev] [PATCH v5 0/9] ethdev: support SubFunction
>representor
>
>On 1/19/21 5:24 PM, Xueming(Steven) Li wrote:
>>
>>> -----Original Message-----
>>> From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
>>> Sent: Tuesday, January 19, 2021 4:41 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>
>>> Subject: Re: [dpdk-dev] [PATCH v5 0/9] ethdev: support SubFunction
>>> representor
>>>
>>> On 1/19/21 10:13 AM, Xueming Li wrote:
>>>> dedicated queues(txq, rxq). A SF netdev supports E-Switch
>>>> representation offload similar to existing PF and VF representors. A
>>>> SF shares PCI level resources with other SFs and/or with its parent
>>>> PCI
>>> function.
>>>>
>>>> >From SmartNIC perspective, when PCI device is shared for
>>>> >multi-host,
>>>> representors for host controller and host PF is required.
>>>>
>>>> This patch set introduces new representor types in addtion to
>>>> existing VF representor. Syntax:
>>>>
>>>> [[c#]pf#]vf#: VF port representor/s from controller/pf
>>>> [[c#]pf#]sf#: SF port representor/s from controller/pf
>>>> #: VF representor - for backwards compatibility
>>>>
>>>> "#" is number instance, list or range, valid examples:
>>>>   1, [1,3,5], [0-3], [0,2-4,6]
>>>>
>>>> For backward compatibility, this patch also introduces new netdev
>>>> capability to indicate the capability of supportting SF representor.
>>>
>>> The patch series looks really nice. Thanks.
>>>
>>> As before, my biggest concern is making representor ID a bitmap. See
>>> my comments to a specific patch.
>>> Plus absence of defined semantics of caller function.
>>> Basically it is looks like it is assumed that controller #0 and PF #0 is the
>caller.
>>> However, it could be wrong.
>>
>> From devargs syntax perspective, a VF representor on PF1 could be
>referenced either way:
>>   1:  <PF0_BDF>,representor=pf1vfX
>>   2:  <PF1_BDF>,representor=vfX
>>   3:  <PF1_BDF>,representor=pf0vfX // works but not suggested, use PF0
>> BDF as caller If probe a device with PF0 BDF and locate it with PF1
>> BDF, the device iterator will fail, devargs is parsed by api, EAL
>> layer simply compare PCI BDF and then representor ID, i.e. "c#pf#vf#". The
>caller device should be consistent, better to use the first device.
>>
>> Devrg "<PF1_BDF>,representor=vfX" will work, representor controller ID
>> and pf ID default to #0, relative to caller context PF1 BDF.
>>
>> Is it good to add such suggestion/behavior on rte_eth_devargs comments?
>
>Sorry, but I disagree. Solution with vf1 and pf0vf1 making the same
>representor ID sounds not acceptable to me. It is definitely different things if I
>pass it to PF1.

My understanding is that representor is an "offset" to caller(context) PF, so the
caller PF has to be the lowest, i.e. PF0. The usage for mlx5 PMD is to probe VF 
representor on a kernel bonding:
1. PF0 is the only device for PMD to probe the bonding device, PMD detects underlay PFs.
2. PF0_BDF,reprensetor=pf1vf0, to probe the representor for first VF on PF1.
PF1_BDF,reprenstor=pf0vf0 works for the same representor, but not encouraged because 
it confuses EAL device iterator which can't tell the difference, as you said: "mainly for human"

Controller ID is not used by mlx5 PMD currently, just a place holder for now, but I think 
same policy applies: lowest as caller(context).

Your suggestion of reserving an c# and pf# ID for caller device which c# or pf# not specified looks
good, it makes the usage of the representor ID bitmap flexible, but considering some NIC with 4 PFs,
it's hard to choose, as you know, only 3 controller and 3 PFs left.

BTW, I guess your assumption is that  representor as "absolute" to caller PF, can't default to 0.
Is there a scenario for PF1 as caller?

>
>
>>>
>>> The next biggest concert is the absence of capability reporting API.
>>> How many controller are available?
>>> How many PFs on each controller are available?
>>> How many VFs on each controller/PF are available?
>>> How many SFs?
>>>
>>>From the first sight it sounds not that important right now and an
>>>extra  feature which could be added in the follow up patches, but IMHO
>>>addition of  the API would allow to avoid making representor ID a bitmap.
>>> Basically capabilities API can provide an array of available
>>>functions with  representor ID assigned to each entry. Also it could
>>>make the entire patch  series optional since it would allow to
>>>interpret numbers in representor=[....]  as representor IDs which are
>>>mapped to controller/PF/VF/SF by the  capabilities reporting API.
>>>
>>> I realize that sometimes it could be more convenient to use syntax
>>> suggested here. Mainly for human.
>>
>> Agree, mostly for human. Regarding to capability reporting API, how
>> about remove current one and enhance it later with a complete patch set?
>>
>
>Current one? Sorry I don't understand.

The patch of device SF capability, but seems I misunderstood your suggestion.
Let me explain process to create a SF:
1. SF can be created on the fly with scripts, unlike VF which is statically pre-created.
2. SF is created on a PF with a SF number. SF number is named per PF, different PF may have same SF number.
3. For standalone PF, hot plug to DPDK using "PF#_BDF,representor=sf#", no need to use pf#sf# here.
4. For bonding netdev, hot plug to DPDK using "PF0_BDF,representor=pf#sf#"
If using new api to return all representor IDs, need some way locate the new created SF by PF and SF number,
that's why "pf#sf#" is used in this patch set.

In the future, I think representor could be processed by PMD, so PMD could have enough flexibility
to support more device expressions and types. But that will introduce a fundamental change of devargs and 
device management, need a full plan.

>
>Andrew.