On 7/29/21 7:13 AM, Xueming(Steven) Li wrote:
-----Original Message-----
From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
Sent: Tuesday, July 20, 2021 5:00 PM
To: Xueming(Steven) Li <xuemi...@nvidia.com>; Ajit Khaparde
<ajit.khapa...@broadcom.com>; Somnath Kotur
<somnath.ko...@broadcom.com>; John Daley <johnd...@cisco.com>; Hyong Youb Kim
<hyon...@cisco.com>; Beilei Xing
<beilei.x...@intel.com>; Qiming Yang <qiming.y...@intel.com>; Qi Zhang
<qi.z.zh...@intel.com>; Haiyue Wang
<haiyue.w...@intel.com>; Matan Azrad <ma...@nvidia.com>; Shahaf Shuler
<shah...@nvidia.com>; Slava Ovsiienko
<viachesl...@nvidia.com>; NBU-Contact-Thomas Monjalon <tho...@monjalon.net>; Ferruh
Yigit <ferruh.yi...@intel.com>
Cc: dev@dpdk.org; Viacheslav Galaktionov <viacheslav.galaktio...@oktetlabs.ru>;
sta...@dpdk.org
Subject: Re: [PATCH] ethdev: fix representor port ID search by name
On 7/19/21 3:50 PM, Xueming(Steven) Li wrote:
-----Original Message-----
From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
Sent: Monday, July 19, 2021 8:36 PM
To: Xueming(Steven) Li <xuemi...@nvidia.com>; Ajit Khaparde
<ajit.khapa...@broadcom.com>; Somnath Kotur
<somnath.ko...@broadcom.com>; John Daley <johnd...@cisco.com>; Hyong
Youb Kim <hyon...@cisco.com>; Beilei Xing <beilei.x...@intel.com>;
Qiming Yang <qiming.y...@intel.com>; Qi Zhang <qi.z.zh...@intel.com>;
Haiyue Wang <haiyue.w...@intel.com>; Matan Azrad <ma...@nvidia.com>;
Shahaf Shuler <shah...@nvidia.com>; Slava Ovsiienko
<viachesl...@nvidia.com>; NBU-Contact-Thomas Monjalon
<tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com>
Cc: dev@dpdk.org; Viacheslav Galaktionov
<viacheslav.galaktio...@oktetlabs.ru>; sta...@dpdk.org
Subject: Re: [PATCH] ethdev: fix representor port ID search by name
On 7/19/21 2:54 PM, Xueming(Steven) Li wrote:
-----Original Message-----
From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
Sent: Monday, July 19, 2021 4:46 PM
To: Xueming(Steven) Li <xuemi...@nvidia.com>; Ajit Khaparde
<ajit.khapa...@broadcom.com>; Somnath Kotur
<somnath.ko...@broadcom.com>; John Daley <johnd...@cisco.com>;
Hyong Youb Kim <hyon...@cisco.com>; Beilei Xing
<beilei.x...@intel.com>; Qiming Yang <qiming.y...@intel.com>; Qi
Zhang <qi.z.zh...@intel.com>; Haiyue Wang <haiyue.w...@intel.com>;
Matan Azrad <ma...@nvidia.com>; Shahaf Shuler <shah...@nvidia.com>;
Slava Ovsiienko <viachesl...@nvidia.com>; NBU-Contact-Thomas
Monjalon <tho...@monjalon.net>; Ferruh Yigit
<ferruh.yi...@intel.com>
Cc: dev@dpdk.org; Viacheslav Galaktionov
<viacheslav.galaktio...@oktetlabs.ru>; sta...@dpdk.org
Subject: Re: [PATCH] ethdev: fix representor port ID search by name
On 7/19/21 9:58 AM, Xueming(Steven) Li wrote:
-----Original Message-----
From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
Sent: Tuesday, July 13, 2021 12:18 AM
To: Ajit Khaparde <ajit.khapa...@broadcom.com>; Somnath Kotur
<somnath.ko...@broadcom.com>; John Daley <johnd...@cisco.com>;
Hyong Youb Kim <hyon...@cisco.com>; Beilei Xing
<beilei.x...@intel.com>; Qiming Yang <qiming.y...@intel.com>; Qi
Zhang <qi.z.zh...@intel.com>; Haiyue Wang
<haiyue.w...@intel.com>; Matan Azrad <ma...@nvidia.com>; Shahaf
Shuler <shah...@nvidia.com>; Slava Ovsiienko
<viachesl...@nvidia.com>; NBU-Contact-Thomas Monjalon
<tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com>;
Xueming(Steven) Li <xuemi...@nvidia.com>
Cc: dev@dpdk.org; Viacheslav Galaktionov
<viacheslav.galaktio...@oktetlabs.ru>; sta...@dpdk.org
Subject: [PATCH] ethdev: fix representor port ID search by name
From: Viacheslav Galaktionov
<viacheslav.galaktio...@oktetlabs.ru>
Fix representor port ID search by name if the representor itself
does not provide representors info. Getting a list of
representors from a representor does not make sense. Instead, a
parent device
should be used.
To this end, extend the rte_eth_dev_data structure to include the port ID of
the parent device for representors.
Fixes: df7547a6a2cc ("ethdev: add helper function to get
representor
ID")
Cc: sta...@dpdk.org
Signed-off-by: Viacheslav Galaktionov
<viacheslav.galaktio...@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
---
The new field is added into the hole in rte_eth_dev_data structure.
The patch does not change ABI, but extra care is required since
ABI check is disabled for the structure because of the libabigail
bug
[1].
Potentially it is bad for out-of-tree drivers which implement
representors but do not fill in a new parert_port_id field in rte_eth_dev_data
structure. Do we care?
May be the patch should add lines to release notes, but I'd like to get initial
feedback first.
mlx5 changes should be reviwed by maintainers very carefully, since we are not
sure if we patch it correctly.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=28060
[snip]
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1248,8 +1248,8 @@ struct rte_eth_devargs {
* For backward compatibility, if no representor info, direct
* map legacy VF (no controller and pf).
*
- * @param ethdev
- * Handle of ethdev port.
+ * @param parent_port_id
+ * Port ID of the backing device.
* @param type
* Representor type.
* @param controller
@@ -1266,7 +1266,7 @@ struct rte_eth_devargs {
*/
__rte_internal
int
-rte_eth_representor_id_get(const struct rte_eth_dev *ethdev,
+rte_eth_representor_id_get(uint16_t parent_port_id,
It make more sense to get representor info from parent port.
Representor is a member of switch domain, PMD owns the information
of the representor owner port and info of representors. This
change looks better, but not sure whether it valuable to introduce
a new
member to the EAL data structure.
IMHO, it is simply incorrect to return representors info on a
representor itself. Representor info is an information which representors may
be populated using the device.
If above statement is correct, we need a way to get parent device
by representor to do name to representor ID mapping. I see two options to do it:
A. Dedicated field in rte_eth_dev_data as the patch does.
B. Dedicated ethdev op (since representor knows parent port ID anyway).
We have chosen (A) because of simplicity.
Just recalled that representor port could be probed w/o owner PF, is a force
for parent port?
I thought that it is impossible and parent port is absolutely
required for a representor. Could you provide an example and explain how will
it work?
In case of bonding, PF0 and PF1 become one PF port `bond0`, PCI address is PF0.
-a <PF0>,representor=pf[0-1]vf[0-99] // this is the syntax we proposed.
Is it net/bonding or vendor-specific bonding in HW?
If I remember correctly in the case of net/bonding we have ethdev ports for
bonded devices.
Not net/bonding pmd, it's Linux bonding, supported by hw driver.
Got it.
To be backward compatible, also support the following 2 devargs:
-a <pf0>,representor=[0-99] // probe bond0 and representor on pf0
-a <pf1>,representor=[0-99] // probe representors on pf1.
If devargs start with PF1 devargs, no owner PF1 created as it disabled
in bonding. Can't create bond0(PF0) automatically here as device is located by
PCI address(PF1) from devargs.
So, I guess the problem is vendor-specific bonding in HW. Anyway legacy
backward compatible representor spec should not require
representors info since it worked before without it. So, it does not sound like
a reason to have representors info on a representor
itself.
Legacy backward logic could be something like this: if PF owner port found, use
it, fallback to current representor.
This won't break anything I guess, how do you think?
Logically even in legacy backward compatibility PF1 VFs representors
have parent port ID - PF0 which is a bond of PF0 & PF1. So,
parent_port_id should be filled in. In this case eth_representor_cmp()
will do rte_eth_representor_id_get(PF0-bond-id, -1, -1, VF, &id) which
will return PF0 VF representor ID. Most likely it will even match and
everything works, but it is still incorrect.
In fact, I have another idea. Try to do:
rte_eth_representor_id_get(representor-port-id, ...) first
for the backward compatibility case and, if not supported, do
it on parent port ID.
