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

From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: "Singh, Aman Deep" <aman.deep.singh@intel.com>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Somnath Kotur <somnath.kotur@broadcom.com>,
	John Daley <johndale@cisco.com>,
	Hyong Youb Kim <hyonkim@cisco.com>,
	Beilei Xing <beilei.xing@intel.com>,
	Qiming Yang <qiming.yang@intel.com>,
	Qi Zhang <qi.z.zhang@intel.com>,
	Haiyue Wang <haiyue.wang@intel.com>,
	Matan Azrad <matan@nvidia.com>,
	Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@intel.com>
Cc: dev@dpdk.org,
	Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
Subject: Re: [dpdk-dev] [PATCH v5] ethdev: fix representor port ID search by name
Date: Thu, 30 Sep 2021 16:40:54 +0300	[thread overview]
Message-ID: <d15d7d86-d3a4-1d6b-61e7-78ef20f8b41f@oktetlabs.ru> (raw)
In-Reply-To: <99404cce-3571-b73e-6736-4723ff067af8@intel.com>

On 9/30/21 3:51 PM, Singh, Aman Deep wrote:
> 
> On 9/30/2021 5:33 PM, Andrew Rybchenko wrote:
>> On 9/29/21 2:13 PM, Singh, Aman Deep wrote:
>>> On 9/13/2021 4:56 PM, Andrew Rybchenko wrote:
>>>> From: Viacheslav Galaktionov <viacheslav.galaktionov@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 backing device for representors.
>>>>
>>>> Signed-off-by: Viacheslav Galaktionov
>>>> <viacheslav.galaktionov@oktetlabs.ru>
>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Acked-by: Haiyue Wang <haiyue.wang@intel.com>
>>>> Acked-by: Beilei Xing <beilei.xing@intel.com>
>>>> ---
>>>> 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].
>>>> It should not be a problem anyway since 21.11 is a ABI breaking
>>>> release.
>>>>
>>>> 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. Get ID by name will not work.
>>> Did we change name of new field from parert_port_id to backer_port_id.
>> Yes, see v5 changelog below.
>> It is done to address review notes from Ferruh on v4.
> 
> Maybe I did not put it clearly, my bad. Just wanted, in above lines also
> the usage
> of "parert_port_id" should be changed.

Thanks, I'll fix it in v6, but I think it does not worse to
respin it since it is not a part of description. Just extra
information.

>>
>>>> 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
>>>>
>>>> v5:
>>>>       - try to improve name: backer_port_id instead of parent_port_id
>>>>       - init new field to RTE_MAX_ETHPORTS on allocation to avoid
>>>>         zero port usage by default
>>>>
>>>> v4:
>>>>       - apply mlx5 review notes: remove fallback from generic ethdev
>>>>         code and add fallback to mlx5 code to handle legacy usecase
>>>>
>>>> v3:
>>>>       - fix mlx5 build breakage
>>>>
>>>> v2:
>>>>       - fix mlx5 review notes
>>>>       - try device port ID first before parent in order to address
>>>>         backward compatibility issue
>> [snip]
>>