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 215C2A0C50; Sun, 1 Aug 2021 10:50:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A1FC840143; Sun, 1 Aug 2021 10:50:32 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 4F4D840140; Sun, 1 Aug 2021 10:50:31 +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 E31A67F504; Sun, 1 Aug 2021 11:50:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru E31A67F504 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1627807830; bh=gSslE/ytqGBoFWbwDe6GWrwMYv311xrQg5BmDZzejRo=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=IBaDdjQAkZWZsOvDfIB18/P7b6mdKA1hZfz0ItPBE72bTZKRqixgKHE0MtL0Ey9xg 60VPoiz1DQpJBkW6pfpb4OpegCza70rWtZ4aWh9arQwMZ4n1u69La+A+LsAQ4ue36g ZGz4PljZ/RT7TzTe1C8aScx9nuXY6L6+Eon56so8= 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: <7b6d83eb-b26a-43ef-93e1-c9a18a247dad@oktetlabs.ru> Date: Sun, 1 Aug 2021 11:50:29 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.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/29/21 7:20 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] >> 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.