On 8/29/21 3:17 PM, Xueming(Steven) Li wrote:
>
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
>> Sent: Sunday, August 29, 2021 4:23 PM
>> To: Xueming(Steven) Li <xuemi...@nvidia.com>; Viacheslav Galaktionov
>> <viacheslav.galaktio...@oktetlabs.ru>
>> Cc: 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>;
>> dev@dpdk.org
>> 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 <viacheslav.galaktio...@oktetlabs.ru>
>>>> Sent: Friday, August 27, 2021 5:48 PM
>>>> To: Xueming(Steven) Li <xuemi...@nvidia.com>
>>>> Cc: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>; 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>;
>>>> dev@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.rybche...@oktetlabs.ru>
>>>>>> Sent: Wednesday, August 18, 2021 10:00 PM
>>>>>> 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>
>>>>>> Cc: dev@dpdk.org; Viacheslav Galaktionov
>>>>>> <viacheslav.galaktio...@oktetlabs.ru>; Xueming(Steven) Li
>>>>>> <xuemi...@nvidia.com>
>>>>>> Subject: [PATCH v2] ethdev: fix representor port ID search by name
>>>>>>
>>>>>> From: Viacheslav Galaktionov <viacheslav.galaktio...@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.galaktio...@oktetlabs.ru>
>>>>>> Signed-off-by: Andrew Rybchenko <andrew.rybche...@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.
>>
>> 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]
>
