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 EB651A00C3; Tue, 18 Jan 2022 13:31:39 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8344F42719; Tue, 18 Jan 2022 13:31:39 +0100 (CET) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by mails.dpdk.org (Postfix) with ESMTP id 5A92E42714 for ; Tue, 18 Jan 2022 13:31:38 +0100 (CET) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id A76765C0106; Tue, 18 Jan 2022 07:31:37 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Tue, 18 Jan 2022 07:31:37 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm3; bh= EgPsXZ9cyr24qRbZW7Q5Oq9SkCOSjCI72kTKLgg8dGw=; b=VdafyGJcMxHPQt5v Z55usg1Ova5wga0e8H/dCkCTBcU6OaKzIsC3VNGZo/USHu/XaG4LiRJfRoq5U6J/ P303M6R4h1QUD/Hl3Wehy11ehBQs0Vj9msgv3F72iiHNR1x2J2P6epYLX6r9BRTZ kzgxHR5MRN1+zyFq7n740L1LYX8J3jlKDyvhaAm3JRYyn/pMP7x0+0oEEtRnEpEf qupFIS+qHlCpQC2HSspJNmKtq2A4vSdKo5zTnnkW6WU8ALJ2DHGtkI9vpIF7OxvZ 1QAkJDEDiu8KCEYm/3cl4BWPwRj8Vaa7IMtyIbB1BVg4TUh/qr0J3x2TF8l6WtkR d68grQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=EgPsXZ9cyr24qRbZW7Q5Oq9SkCOSjCI72kTKLgg8d Gw=; b=h8eQYrOz0SHNMebHmnvg6XYsXPRxtayvayEiJxixj3c0zBFG7ceNNL47R L8cmmB1GDqliN2IomCHX3pOQ+HRvS0bnbGoZyU2HEXL8cAwV7f5Is8qxWS7NKJOr Q4IHkC1ezs/NAm5jTSjlexRnh+pyanwmTJi4cfJ3+Fg3dInwM0IPb6lHYyeITlz4 rZQWyLOAmyfNnha0jnG15riMSIMV0qMw92nLDyOOllQ2mzSdfVseeCpz/U9FDm4p 5msaQTehhzMdAZfUl7IBLLO2PqP9tuKqvdGekmP1YgC0AC+yXmOKLD/gSHkcyVEY 9GiiCkVfQuBzk+uWftnLzqC4xC8tg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrudefgdegtdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei iedvffegheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh hmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 18 Jan 2022 07:31:36 -0500 (EST) From: Thomas Monjalon To: kumaraparameshwaran rathinavel , Ferruh Yigit Cc: "Wiles, Keith" , dev@dpdk.org, Kumara Parameshwaran , Andrew Rybchenko , David Marchand Subject: Re: [PATCH] net/tap: Bug fix to populate fds in secondary process Date: Tue, 18 Jan 2022 13:31:34 +0100 Message-ID: <2206724.IFkqi6BYcA@thomas> In-Reply-To: <923832f2-c8a0-84e9-360c-e79549a480a6@intel.com> References: <20211126041515.96259-1-kumaraparamesh92@gmail.com> <923832f2-c8a0-84e9-360c-e79549a480a6@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 18/01/2022 13:12, Ferruh Yigit: > On 1/18/2022 11:21 AM, kumaraparameshwaran rathinavel wrote: > > Comment moved down. > > Please don't top post, it makes very hard to follow the discussion and bad > for archives to visit discussion later. > > > > > On Tue, Jan 18, 2022 at 3:17 PM Ferruh Yigit > wrote: > > > > On 1/17/2022 6:33 PM, Thomas Monjalon wrote: > > > 17/01/2022 19:28, Ferruh Yigit: > > >>> + ret = rte_eth_dev_get_port_by_name(request_param->port_name, &port_id); > > >>> + if (ret) { > > >>> + TAP_LOG(ERR, "Failed to get port id for %s", > > >>> + request_param->port_name); > > >>> + return -1; > > >>> + } > > >>> + dev = &rte_eth_devices[port_id]; > > >> > > >> Since this is not really related with your patch, I want to have a separate thread for it. > > >> > > >> It is not good to access the 'rte_eth_devices' global variable directly from a driver, that > > >> is error prone. > > >> > > >> Btw, what 'peer' supposed to contain? > > >> > > >> It can be solved by adding an internal API, only for drivers to get eth_dev from the name, > > >> like: 'rte_eth_dev_get_by_name()'. > > >> This way a few other usage can be converted to this API. > > >> > > >> @Thomas and @Andrew what do you think about the new API proposal? > > > > > > It looks similar to rte_eth_dev_get_port_by_name() which returns a port_id. > > > > Exactly, but get eth_dev directly for drivers. For drivers no need to work with port_id > > handler, they can use eth_dev directly. > > > > Another solution can be an getter function for drivers, which gets port_id and returns > > the eth_dev. > > > > > It is a bit strange for an ethdev driver to not have access to its own ethdev struct. > > > Isn't there something broken in the logic? > > > > > > > This is callback function between primary and secondary applications sync. So port name > > will be same for both, but eth_dev will be different and port_id may be different. > > Driver finds its own eth_dev from the shared port name. > > > > > Just wanted to bring it to your attention, > > > > In Mellanox driver there is a requirement to exchange fds between primary and secondary and similar usage is seen, the primary sends the port_id and the secondary refers to the rte_eth_devices in the driver, > > The functions are > > - mlx5_mp_secondary_handle in secondary > > - mlx5_mp_req_start_rxtx in primary which is invoked from mlx5_dev_start. > > > > In my implementation I have used the name and invoked get_port_by_name, I can also pass the port_id from the primary to make it uniform. So with similar usage in Mellanox is there a problem there as well on referring to the rte_eth_devices from the PMD ? > > > > It would be same, still will be accessing to the 'rte_eth_devices'. > That is why a new API for drivers may help. I agree to add a new API if needed to remove those direct access to rte_eth_devices.