[dpdk-dev] [PATCH v2] ethdev: fix representor port ID search by name

[Xueming(Steven) Li]

> -----Original Message-----
> From: Viacheslav Galaktionov <viacheslav.galaktionov at oktetlabs.ru>
> Sent: Friday, August 27, 2021 5:48 PM
> To: Xueming(Steven) Li <xuemingl at nvidia.com>
> Cc: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>; Ajit Khaparde <ajit.khaparde at broadcom.com>; Somnath Kotur
> <somnath.kotur at broadcom.com>; John Daley <johndale at cisco.com>; Hyong Youb Kim <hyonkim at cisco.com>; Beilei Xing
> <beilei.xing at intel.com>; Qiming Yang <qiming.yang at intel.com>; Qi Zhang <qi.z.zhang at intel.com>; Haiyue Wang
> <haiyue.wang at intel.com>; Matan Azrad <matan at nvidia.com>; Shahaf Shuler <shahafs at nvidia.com>; Slava Ovsiienko
> <viacheslavo at nvidia.com>; NBU-Contact-Thomas Monjalon <thomas at monjalon.net>; Ferruh Yigit <ferruh.yigit at intel.com>;
> dev at dpdk.org
> Subject: Re: [PATCH v2] ethdev: fix representor port ID search by name
> 
> On 2021-08-27 12:18, Xueming(Steven) Li wrote:
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
> >> Sent: Wednesday, August 18, 2021 10:00 PM
> >> To: Ajit Khaparde <ajit.khaparde at broadcom.com>; Somnath Kotur
> >> <somnath.kotur at broadcom.com>; John Daley <johndale at cisco.com>; Hyong
> >> Youb Kim <hyonkim at cisco.com>; Beilei Xing <beilei.xing at intel.com>;
> >> Qiming Yang <qiming.yang at intel.com>; Qi Zhang <qi.z.zhang at intel.com>;
> >> Haiyue Wang <haiyue.wang at intel.com>; Matan Azrad <matan at nvidia.com>;
> >> Shahaf Shuler <shahafs at nvidia.com>; Slava Ovsiienko
> >> <viacheslavo at nvidia.com>; NBU-Contact-Thomas Monjalon
> >> <thomas at monjalon.net>; Ferruh Yigit <ferruh.yigit at intel.com>
> >> Cc: dev at dpdk.org; Viacheslav Galaktionov
> >> <viacheslav.galaktionov at oktetlabs.ru>; Xueming(Steven) Li
> >> <xuemingl at nvidia.com>
> >> Subject: [PATCH v2] ethdev: fix representor port ID search by name
> >>
> >> From: Viacheslav Galaktionov <viacheslav.galaktionov at oktetlabs.ru>
> >>
> >> 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.
> >>
> >> Signed-off-by: Viacheslav Galaktionov
> >> <viacheslav.galaktionov at oktetlabs.ru>
> >> Signed-off-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
> >> ---
> 
> [snip]
> 
> >> b/drivers/net/mlx5/windows/mlx5_os.c
> >> index 7e1df1c751..0c5a02bfcb 100644
> >> --- a/drivers/net/mlx5/windows/mlx5_os.c
> >> +++ b/drivers/net/mlx5/windows/mlx5_os.c
> >> @@ -543,6 +543,23 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
> >>  	if (priv->representor) {
> >>  		eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;
> >>  		eth_dev->data->representor_id = priv->representor_id;
> >> +		MLX5_ETH_FOREACH_DEV(port_id, priv->pci_dev) {
> >> +			struct mlx5_priv *opriv =
> >> +				rte_eth_devices[port_id].data->dev_private;
> >> +			if (opriv &&
> >> +			    opriv->master &&
> >> +			    opriv->domain_id == priv->domain_id &&
> >> +			    opriv->sh == priv->sh) {
> >> +				eth_dev->data->parent_port_id =
> >> +					rte_eth_devices[port_id].data->port_id;
> >
> > Could this value different than port_id?
> 
> Oh, yes, that's an oversight. Thank you!
> 
> >> +				break;
> >> +			}
> >> +		}
> >> +		if (port_id >= RTE_MAX_ETHPORTS) {
> >> +			DRV_LOG(ERR, "no master device for representor");
> >> +			err = ENODEV;
> >> +			goto error;
> >
> > Here shouldn't be an error.
> 
> What do you mean? Is it normal not to have a master device for a representor?

As discussed before, representor could exists w/o master device, special case.

> 
> > Parent port ID default to 0, it could be wrong if multiple PF probed,
> > let's default to current port ID.
> 
> What is the "current" port ID here? Do you mean the representor's port ID?

Representor port ID.

> If you are talking about the value of the port_id variable, then I suppose it could be set to RTE_MAX_ETHPORTS explicitly if a master
> device isn't found.
> 
> [snip]