On 8/29/21 3:17 PM, Xueming(Steven) Li wrote: > > >> -----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.
Very good. I see your point I'll send v4 tomorrow. >> >>>> >>>>> 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] >
