From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7E9C7A0547 for ; Mon, 19 Jul 2021 14:36:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6C2D7410DD; Mon, 19 Jul 2021 14:36:23 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 0224C4003E; Mon, 19 Jul 2021 14:36:20 +0200 (CEST) Received: from [192.168.100.116] (unknown [37.139.99.76]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id F02E27F53F; Mon, 19 Jul 2021 15:36:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru F02E27F53F DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1626698180; bh=C0fxbBTD61gjtEyfQELiPPlrASI6o1MhuOrpqGhdRoY=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=d/iyyDIu6BWHSTHqleetld5Uurnu3Wm0KztTw0lvajXQH1Rn/ME9yACIlP08uxHRc BmUoumYD+fDcunPBgVTN7YQuwmVSJWRssl9EXKje7YLOFatDMZi4TZRjhVinrCLVD0 pzAthukQ0NM9iA64bvALpbu7kqaqmNtFOSiB5Cvw= To: "Xueming(Steven) Li" , Ajit Khaparde , Somnath Kotur , John Daley , Hyong Youb Kim , Beilei Xing , Qiming Yang , Qi Zhang , Haiyue Wang , Matan Azrad , Shahaf Shuler , Slava Ovsiienko , NBU-Contact-Thomas Monjalon , Ferruh Yigit Cc: "dev@dpdk.org" , Viacheslav Galaktionov , "stable@dpdk.org" References: <20210712161747.958019-1-andrew.rybchenko@oktetlabs.ru> <498b2d48-4c04-24f5-12a7-b1da3a55fc51@oktetlabs.ru> From: Andrew Rybchenko Message-ID: <0164acb5-9644-4058-ee4c-8eeead751f58@oktetlabs.ru> Date: Mon, 19 Jul 2021 15:36:06 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-stable] [PATCH] ethdev: fix representor port ID search by name X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 7/19/21 2:54 PM, Xueming(Steven) Li wrote: > > >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Monday, July 19, 2021 4:46 PM >> To: Xueming(Steven) Li ; Ajit Khaparde ; Somnath Kotur >> ; John Daley ; Hyong Youb Kim ; Beilei Xing >> ; Qiming Yang ; Qi Zhang ; Haiyue Wang >> ; Matan Azrad ; Shahaf Shuler ; Slava Ovsiienko >> ; NBU-Contact-Thomas Monjalon ; Ferruh Yigit >> Cc: dev@dpdk.org; Viacheslav Galaktionov ; stable@dpdk.org >> Subject: Re: [PATCH] ethdev: fix representor port ID search by name >> >> On 7/19/21 9:58 AM, Xueming(Steven) Li wrote: >>> >>> >>>> -----Original Message----- >>>> From: Andrew Rybchenko >>>> Sent: Tuesday, July 13, 2021 12:18 AM >>>> To: Ajit Khaparde ; Somnath Kotur >>>> ; John Daley ; Hyong >>>> Youb Kim ; Beilei Xing ; >>>> Qiming Yang ; Qi Zhang ; >>>> Haiyue Wang ; Matan Azrad ; >>>> Shahaf Shuler ; Slava Ovsiienko >>>> ; NBU-Contact-Thomas Monjalon >>>> ; Ferruh Yigit ; >>>> Xueming(Steven) Li >>>> Cc: dev@dpdk.org; Viacheslav Galaktionov >>>> ; stable@dpdk.org >>>> Subject: [PATCH] ethdev: fix representor port ID search by name >>>> >>>> From: Viacheslav Galaktionov >>>> >>>> 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@dpdk.org >>>> >>>> Signed-off-by: Viacheslav Galaktionov >>>> >>>> Signed-off-by: Andrew Rybchenko >>>> --- >>>> 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] >> >>>> --- a/lib/ethdev/ethdev_driver.h >>>> +++ b/lib/ethdev/ethdev_driver.h >>>> @@ -1248,8 +1248,8 @@ struct rte_eth_devargs { >>>> * For backward compatibility, if no representor info, direct >>>> * map legacy VF (no controller and pf). >>>> * >>>> - * @param ethdev >>>> - * Handle of ethdev port. >>>> + * @param parent_port_id >>>> + * Port ID of the backing device. >>>> * @param type >>>> * Representor type. >>>> * @param controller >>>> @@ -1266,7 +1266,7 @@ struct rte_eth_devargs { >>>> */ >>>> __rte_internal >>>> int >>>> -rte_eth_representor_id_get(const struct rte_eth_dev *ethdev, >>>> +rte_eth_representor_id_get(uint16_t parent_port_id, >>> >>> It make more sense to get representor info from parent port. >>> Representor is a member of switch domain, PMD owns the information of >>> the representor owner port and info of representors. This change looks better, but not sure whether it valuable to introduce a new >> member to the EAL data structure. >> >> IMHO, it is simply incorrect to return representors info on a representor itself. Representor info is an information which representors >> may be populated using the device. >> >> If above statement is correct, we need a way to get parent device by representor to do name to representor ID mapping. I see two >> options to do it: >> A. Dedicated field in rte_eth_dev_data as the patch does. >> B. Dedicated ethdev op (since representor knows parent port ID anyway). >> We have chosen (A) because of simplicity. > > Just recalled that representor port could be probed w/o owner PF, is a force for parent port? I thought that it is impossible and parent port is absolutely required for a representor. Could you provide an example and explain how will it work?