On 1/27/21 6:04 AM, Xueming(Steven) Li wrote:

Hi Andrew,

quoted

-----Original Message-----
From: Andrew Rybchenko <redacted>
Sent: Friday, January 22, 2021 4:21 PM
To: Xueming(Steven) Li <redacted>
Cc: dev@dpdk.org; Slava Ovsiienko <redacted>; Asaf Penso
[off-list ref]
Subject: Re: [dpdk-dev] [PATCH v5 0/9] ethdev: support SubFunction
representor

On 1/19/21 5:24 PM, Xueming(Steven) Li wrote:

quoted

quoted

-----Original Message-----
From: Andrew Rybchenko <redacted>
Sent: Tuesday, January 19, 2021 4:41 PM
To: Xueming(Steven) Li <redacted>
Cc: dev@dpdk.org; Slava Ovsiienko <redacted>; Asaf
Penso [off-list ref]
Subject: Re: [dpdk-dev] [PATCH v5 0/9] ethdev: support SubFunction
representor

On 1/19/21 10:13 AM, Xueming Li wrote:

quoted

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.

quoted

quoted

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.

quoted

quoted

However, it could be wrong.

From devargs syntax perspective, a VF representor on PF1 could be

referenced either way:

quoted

  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.

quoted

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.

Sorry, how to address PF0 from PF1?

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.

I'd prefer the option if we make a choice between these two.
At least it a bit more consistent.
But IMHO anyway bitmap is a road to wrong direction.

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?

Yes.

quoted

quoted

quoted

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.

Is there a maximum index and maximum total number of SF's
created? How to find it?

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.

I think the API should simply reserve/report space for maximum
number of SFs. So, IDs are stable across restart/reboot in
assumption that NIC is not reconfigured (changed maximum number
of VF or maximum number of SFs of any PF).

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.

quoted

Andrew.