> -----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?
