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

Andrew Rybchenko andrew.rybchenko at oktetlabs.ru
Sun Aug 1 10:50:29 CEST 2021


On 7/29/21 7:20 AM, Xueming(Steven) Li wrote:
> 
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
>> Sent: Tuesday, July 13, 2021 12:18 AM
>> 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>; Xueming(Steven) Li <xuemingl at nvidia.com>
>> Cc: dev at dpdk.org; Viacheslav Galaktionov <viacheslav.galaktionov at oktetlabs.ru>; stable at dpdk.org
>> Subject: [PATCH] ethdev: fix representor port ID search by name
>>
>> From: Viacheslav Galaktionov <viacheslav.galaktionov at oktetlabs.ru>
>>
>> Fix representor port ID search by name if the representor itself does not provide representors info. 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.
>>
>> Fixes: df7547a6a2cc ("ethdev: add helper function to get representor ID")
>> Cc: stable at dpdk.org
>>
>> Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov at oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
>> ---
>> The new field is added into the hole in rte_eth_dev_data structure.
>> The patch does not change ABI, but extra care is required since ABI check is disabled for the structure because of the libabigail bug [1].
>>
>> Potentially it is bad for out-of-tree drivers which implement representors but do not fill in a new parert_port_id field in
>> rte_eth_dev_data structure. Do we care?
>>
>> May be the patch should add lines to release notes, but I'd like to get initial feedback first.
>>
>> mlx5 changes should be reviwed by maintainers very carefully, since we are not sure if we patch it correctly.
>>
>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=28060

[snip]

>> b/drivers/net/mlx5/linux/mlx5_os.c
>> index be22d9cbd2..5550d30628 100644
>> --- a/drivers/net/mlx5/linux/mlx5_os.c
>> +++ b/drivers/net/mlx5/linux/mlx5_os.c
>> @@ -1511,6 +1511,17 @@ 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) {
>> +			const struct mlx5_priv *opriv =
>> +				rte_eth_devices[port_id].data->dev_private;
>> +
>> +			if (!opriv ||
>> +			    opriv->sh != priv->sh ||
>> +			    opriv->representor)
>> +				continue;
>> +			eth_dev->data->parent_port_id = port_id;
>> +			break;
>> +		}
> 
> At line 126, there is a logic that locate priv->domain_id, parent port_id could be found there.

Do you mean line 1260? The comment above says "Look for sibling devices 
in order to reuse their switch domain if any, otherwise allocate one.".
So, it is not a parent. Is the comment misleading and parent matches
the search criteria as well? But in any case, we should guarantee that
it is a parent port, not a sibling port. So, we need extra criteria to
match parent port only.


More information about the dev mailing list