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 43D2CA0547; Mon, 19 Jul 2021 10:46:36 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 21788410F2; Mon, 19 Jul 2021 10:46:36 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 718E640E28; Mon, 19 Jul 2021 10:46:34 +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 E3A2E7F4FD; Mon, 19 Jul 2021 11:46:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru E3A2E7F4FD DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1626684393; bh=A9cUDBU+4MbBM2rI87wVlSCI3a5oHXKJnL7BWQHf0mg=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=Dkb0A4YOy7T3bExGwyMfr4wOD5LPnzssLojdXwNSc0/P7roRkIiAUmcuBXkeSptew MnEn498QV2EaxjVT3TEtojT1lPUa7qA5SHM1BGx4kS6++M6NrNMyNXFbF3HFHGn2cP pBfy+QJmfbI7P7gBqQ9kcbRB3wBiKv25113q2/hs= 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> From: Andrew Rybchenko Message-ID: <498b2d48-4c04-24f5-12a7-b1da3a55fc51@oktetlabs.ru> Date: Mon, 19 Jul 2021 11:46:29 +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-dev] [PATCH] ethdev: fix representor port ID search by name X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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.