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

Andrew Rybchenko andrew.rybchenko at oktetlabs.ru
Sun Aug 29 10:23:05 CEST 2021


On 8/28/21 4:22 PM, Xueming(Steven) Li wrote:
> 
> 
>> -----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.

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

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



More information about the dev mailing list