> -----Original Message----- > From: Andrew Rybchenko <[email protected]> > Sent: Sunday, August 29, 2021 4:23 PM > To: Xueming(Steven) Li <[email protected]>; Viacheslav Galaktionov > <[email protected]> > Cc: Ajit Khaparde <[email protected]>; Somnath Kotur > <[email protected]>; John Daley > <[email protected]>; Hyong Youb Kim <[email protected]>; Beilei Xing > <[email protected]>; Qiming Yang > <[email protected]>; Qi Zhang <[email protected]>; Haiyue Wang > <[email protected]>; Matan Azrad > <[email protected]>; Shahaf Shuler <[email protected]>; Slava Ovsiienko > <[email protected]>; NBU-Contact-Thomas > Monjalon <[email protected]>; Ferruh Yigit <[email protected]>; > [email protected] > Subject: Re: [PATCH v2] ethdev: fix representor port ID search by name > > On 8/28/21 4:22 PM, Xueming(Steven) Li wrote: > > > > > >> -----Original Message----- > >> From: Viacheslav Galaktionov <[email protected]> > >> Sent: Friday, August 27, 2021 5:48 PM > >> To: Xueming(Steven) Li <[email protected]> > >> Cc: Andrew Rybchenko <[email protected]>; Ajit Khaparde > >> <[email protected]>; Somnath Kotur > >> <[email protected]>; John Daley <[email protected]>; Hyong > >> Youb Kim <[email protected]>; Beilei Xing <[email protected]>; > >> Qiming Yang <[email protected]>; Qi Zhang <[email protected]>; > >> Haiyue Wang <[email protected]>; Matan Azrad <[email protected]>; > >> Shahaf Shuler <[email protected]>; Slava Ovsiienko > >> <[email protected]>; NBU-Contact-Thomas Monjalon > >> <[email protected]>; Ferruh Yigit <[email protected]>; > >> [email protected] > >> 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 <[email protected]> > >>>> Sent: Wednesday, August 18, 2021 10:00 PM > >>>> To: Ajit Khaparde <[email protected]>; Somnath Kotur > >>>> <[email protected]>; John Daley <[email protected]>; > >>>> Hyong Youb Kim <[email protected]>; Beilei Xing > >>>> <[email protected]>; Qiming Yang <[email protected]>; Qi > >>>> Zhang <[email protected]>; Haiyue Wang <[email protected]>; > >>>> Matan Azrad <[email protected]>; Shahaf Shuler <[email protected]>; > >>>> Slava Ovsiienko <[email protected]>; NBU-Contact-Thomas > >>>> Monjalon <[email protected]>; Ferruh Yigit > >>>> <[email protected]> > >>>> Cc: [email protected]; Viacheslav Galaktionov > >>>> <[email protected]>; Xueming(Steven) Li > >>>> <[email protected]> > >>>> Subject: [PATCH v2] ethdev: fix representor port ID search by name > >>>> > >>>> From: Viacheslav Galaktionov <[email protected]> > >>>> > >>>> 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 > >>>> <[email protected]> > >>>> Signed-off-by: Andrew Rybchenko <[email protected]> > >>>> --- > >> > >> [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. > > May I clarify one question. Isn't bond will be a parent for the second PF VF > representors? Will above algorithm find it? > If yes, I think we don't need self fallback here.
Sorry maybe I was not clear enough. It's true that bond will be parent for second PF VF representor, the above algorithm can find it. But if second PF VF representor probe earlier than the bond, please note that bond get probed with first PF, bond won't be found. > If no, it looks a bit wrong to me but may be acceptable for so complicated > case. If it is acceptable to you, we can put self fallback here, > but in this case we don't need corresponding code which check self port_id > first. It would be even better this way since generic code > will be more clear. Agree, it's better to make generic ode more clear. > > >> > >>> 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]
