From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v4 1/4] ethdev: rename memzones allocated for DMA Date: Fri, 12 Oct 2018 19:21:17 +0200 Message-ID: <8911296.15KZnjE6xt@xps> References: <20180907230958.21402-1-thomas@monjalon.net> <2096271.VzXizFIdQq@xps> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, gaetan.rivet@6wind.com, ophirmu@mellanox.com, qi.z.zhang@intel.com, ferruh.yigit@intel.com, "Ananyev, Konstantin" , Ajit Khaparde , Somnath Kotur , Rahul Lakkireddy To: Andrew Rybchenko Return-path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id B9B7C1BB2E for ; Fri, 12 Oct 2018 19:21:21 +0200 (CEST) In-Reply-To: <2096271.VzXizFIdQq@xps> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 12/10/2018 19:18, Thomas Monjalon: > 12/10/2018 18:46, Andrew Rybchenko: > > On 10/12/18 7:42 PM, Andrew Rybchenko wrote: > > > On 10/12/18 7:40 PM, Thomas Monjalon wrote: > > >> 12/10/2018 09:53, Andrew Rybchenko: > > >>> On 10/12/18 12:02 AM, Thomas Monjalon wrote: > > >>>> The helper rte_eth_dma_zone_reserve() is called by PMDs > > >>>> when probing a new port. > > >>>> It creates a new memzone with an unique name. > > >>>> The name of this memzone was using the name of the driver > > >>>> doing the probe. > > >>>> > > >>>> In order to avoid assigning the driver before the end of the probing > > >>>> (next patch), the driver name is removed from these memzone names. > > >>>> The ethdev name (data->name) is not used because it may be too long > > >>>> and may be not set at this stage of probing. > > >>>> > > >>>> Syntax of old name: ___ > > >>>> Syntax of new name: eth_p_q_ > > >>>> > > >>>> Signed-off-by: Thomas Monjalon > > >>>> --- > > >>>> lib/librte_ethdev/rte_ethdev.c | 5 ++--- > > >>>> 1 file changed, 2 insertions(+), 3 deletions(-) > > >>>> > > >>>> diff --git a/lib/librte_ethdev/rte_ethdev.c > > >>>> b/lib/librte_ethdev/rte_ethdev.c > > >>>> index ef99f7068..ec443def5 100644 > > >>>> --- a/lib/librte_ethdev/rte_ethdev.c > > >>>> +++ b/lib/librte_ethdev/rte_ethdev.c > > >>>> @@ -3441,9 +3441,8 @@ rte_eth_dma_zone_reserve(const struct > > >>>> rte_eth_dev *dev, const char *ring_name, > > >>>> char z_name[RTE_MEMZONE_NAMESIZE]; > > >>>> const struct rte_memzone *mz; > > >>>> - snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d", > > >>>> - dev->device->driver->name, ring_name, > > >>>> - dev->data->port_id, queue_id); > > >>>> + snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s", > > >>>> + dev->data->port_id, queue_id, ring_name); > > >>>> mz = rte_memzone_lookup(z_name); > > >>>> if (mz) > > >>> LGTM, but I've found more places where the pattern is duplicate > > >>> and testpmd frightens me: > > >>> - app/test-pmd/config.c ring_dma_zone_lookup() which is used > > >>> to look at descriptors (looks like Intel specific since has > > >>> RTE_LIBRTE_I40E_16BYTE_RX_DESC conditional code) > > >> >From what I see there is no access to rte_device.driver here, > > >> except one in exit function. > > > > > > Yes, but testpmd will fail to find the memzone and command to > > > take a look at descriptors will be broken. > > > > > > May be it is already broken etc. I think someone from Intel should > > > comment it. > > > > Potentially the following drivers may use it: > > $ git grep -w \"rx_ring\" drivers/net/ > > drivers/net/avf/avf_rxtx.c:380: mz = rte_eth_dma_zone_reserve(dev, > > "rx_ring", queue_idx, > > drivers/net/axgbe/axgbe_rxtx.c:91: dma = > > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size, 12 > > drivers/net/cxgbe/sge.c:1878: fwevtq ? "fwq_ring" : "rx_ring", > > drivers/net/e1000/em_rxtx.c:1437: rz = > > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, rsize, > > drivers/net/e1000/igb_rxtx.c:1734: rz = > > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size, > > drivers/net/fm10k/fm10k_ethdev.c:1878: mz = > > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_id, > > drivers/net/i40e/i40e_rxtx.c:1853: rz = > > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, > > drivers/net/ixgbe/ixgbe_rxtx.c:2966: rz = > > rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, > > drivers/net/nfp/nfp_net.c:1516: tz = rte_eth_dma_zone_reserve(dev, > > "rx_ring", queue_idx, > > Excuse me, I really don't understand what you mean. Oh, got it! You mean the call to rte_memzone_lookup() must expect the new memzone name. So I must update it here too, right?