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 9A271A0C49; Tue, 20 Jul 2021 12:35:36 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0A5ED40689; Tue, 20 Jul 2021 12:35:36 +0200 (CEST) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by mails.dpdk.org (Postfix) with ESMTP id BB17E40140 for ; Tue, 20 Jul 2021 12:35:34 +0200 (CEST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 493E05C00A5; Tue, 20 Jul 2021 06:35:34 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Tue, 20 Jul 2021 06:35:34 -0400 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=fm1; bh= Rqmqb534qxT0l99orggGhF+Bx5PeY17eoiNy9qe0IwI=; b=MZ9elS2JRSck7tZ7 Hn+hmFYbjl+/LbPC+MM22h16opln1yhb0yIPqf0dng0m5iYTakMYNdCQLgdW3dyh YDbK60c7fpIC3JoQFeurY8QP5OK7hy57ndGUjQ//e38/1VK8Gtx4pDfbkuwSIKqo 4lHfaQ1mwxD95LMDG0InHk79Y9NgkdY4DwYzoTaZWVepnHzuZf19vRs39PfqMv6a wdXqxUoD2iDRNaIk6C1C0d4YOgPsoYMSE/31gWfL/ZK7AaIxsCWA/f41q+wpbi8/ 68WUmBlNKlfQf+9vUSGSBKa5kDpqxJ5DjrxtlGha3BhaxXJWNBIJ9U30BijIRZX8 lJEqag== 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=fm3; bh=Rqmqb534qxT0l99orggGhF+Bx5PeY17eoiNy9qe0I wI=; b=tjZm2bCklFXQgwWHGU6FlMwiPxxuOeC5hr54B3kPLtqJGml7z2+w4UcX4 63Zj5PmU+kk6JMSgeZPVq9aCe/ldDrJQErKNJH969Lw2U2qRqlpOsMgZRSN1hcH9 Y5S/K6x5EzI8G59dgJhgc/1S5OQN1A9p3XamWPe3QJAJNQnUw7Ro5Yozm9wPOfCn eOgOhRmYXc/tNtJBmhhyxdHZE/wgEC0bH7jh85Vb6z1Tjz38SUYMDBGEpDKjwul+ BUu73Nw+xX6VrpsegU56icCQJ1z+xiE828qSDabbAAmSUeI6IDIOqofO1AQZQTpn mjKz7ypHnDqIDMDS3nIKBIh9FhsrA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrfedvgdeftdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei iedvffegheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh hmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 20 Jul 2021 06:35:30 -0400 (EDT) From: Thomas Monjalon To: Ferruh Yigit Cc: Paulis Gributs , xiaoyun.li@intel.com, anatoly.burakov@intel.com, Shahaf Shuler , dev@dpdk.org, Andrew Rybchenko , viacheslavo@nvidia.com, matan@nvidia.com, rasland@nvidia.com Date: Tue, 20 Jul 2021 12:35:40 +0200 Message-ID: <4976849.lnOlTO5uOn@thomas> In-Reply-To: <23ee1261-e6db-69d2-8a9e-423dc38a925a@intel.com> References: <20210715132015.1587544-1-paulis.gributs@intel.com> <23ee1261-e6db-69d2-8a9e-423dc38a925a@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] app/testpmd: remove most uses of rte_eth_devices 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" 19/07/2021 18:42, Ferruh Yigit: > On 7/15/2021 2:20 PM, Paulis Gributs wrote: > > This patch removes most uses of the global variable rte_eth_devices > > from testpmd. This was done to avoid using the object directly which > > applications should not do. > > > > Most uses have been replaced with standard function calls, however > > the use of it in the show_macs function could not be replaced as no > > function call exists to get all mac addresses of a given port. > > > > Signed-off-by: Paulis Gributs [...] > > @@ -857,16 +857,23 @@ dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused, > > int ret; > > > > RTE_ETH_FOREACH_DEV(pid) { > > - struct rte_eth_dev *dev = > > - &rte_eth_devices[pid]; > > + struct rte_eth_dev_info dev_info; > > > > - ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0, > > - memhdr->len); > > + ret = eth_dev_info_get_print_err(pid, &dev_info); > > + if (ret != 0) { > > + TESTPMD_LOG(DEBUG, > > + "unable to get device info for port %d on addr 0x%p," > > + "mempool unmapping will not be performed\n", > > + pid, memhdr->addr); > > + continue; > > + } > > + > > + ret = rte_dev_dma_unmap(dev_info.device, memhdr->addr, 0, memhdr->len); > > if (ret) { > > TESTPMD_LOG(DEBUG, > > "unable to DMA unmap addr 0x%p " > > "for device %s\n", > > - memhdr->addr, dev->data->name); > > + memhdr->addr, dev_info.device->name); > > } > > } > > ret = rte_extmem_unregister(memhdr->addr, memhdr->len); > > @@ -892,16 +899,22 @@ dma_map_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused, > > return; > > } > > RTE_ETH_FOREACH_DEV(pid) { > > - struct rte_eth_dev *dev = > > - &rte_eth_devices[pid]; > > + struct rte_eth_dev_info dev_info; > > > > - ret = rte_dev_dma_map(dev->device, memhdr->addr, 0, > > - memhdr->len); > > + ret = eth_dev_info_get_print_err(pid, &dev_info); > > + if (ret != 0) { > > + TESTPMD_LOG(DEBUG, > > + "unable to get device info for port %d on addr 0x%p," > > + "mempool mapping will not be performed\n", > > + pid, memhdr->addr); > > + continue; > > + } > > + ret = rte_dev_dma_map(dev_info.device, memhdr->addr, 0, memhdr->len); > > if (ret) { > > TESTPMD_LOG(DEBUG, > > "unable to DMA map addr 0x%p " > > "for device %s\n", > > - memhdr->addr, dev->data->name); > > + memhdr->addr, dev_info.device->name); > > } > > } > > } > > Hi Shahaf, > > These callbacks are used to map/unmap anon memory and added on commit [1]. > > Can you please elaborate why it is required? And does xmem covers this > functionality already? The external memory must be registered for DMA. It completes the feature of external memory, so yes it is required. > The concern I have is, it uses some DPDK details, like rte_device to implement > functionality in a test applications (testpmd). If this is a required > functionality for end user, it is very hard for them to implement this, and > perhaps we should have some APIs/wrappers to help the users in that case. > Or if it is not required, we can perhaps drop from testpmd. I agree the API is bad. It should be an API in every driver classes. > But first I am trying to understand what functionality it brings, if it is > something required by end user or not. We should deprecate the API and introduce a new one. Is it urgent to drop the API? Something you would like to do in 21.11?