From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id AE6AD1B56A for ; Fri, 12 Oct 2018 19:51:33 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 213D5400085; Fri, 12 Oct 2018 17:51:32 +0000 (UTC) Received: from [100.75.19.169] (188.170.81.179) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Fri, 12 Oct 2018 18:51:13 +0100 From: Andrew Rybchenko To: Thomas Monjalon CC: , , , , , "Ananyev, Konstantin" , Ajit Khaparde , Somnath Kotur , Rahul Lakkireddy Date: Fri, 12 Oct 2018 20:51:11 +0300 Message-ID: <16669675298.27dc.d74a604c1342fc6d922a87b3e5a1c1e3@solarflare.com> In-Reply-To: <8911296.15KZnjE6xt@xps> References: <20180907230958.21402-1-thomas@monjalon.net> <2096271.VzXizFIdQq@xps> <8911296.15KZnjE6xt@xps> User-Agent: AquaMail/1.17.0-1306 (build: 101700006) MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset="us-ascii" Content-Transfer-Encoding: 8bit X-Originating-IP: [188.170.81.179] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24150.003 X-TM-AS-Result: No-15.224800-8.000000-10 X-TMASE-MatchedRID: VfovoVrt/obao4CRCsO6Y/VFR4sC8dPyjLOy13Cgb4+qvcIF1TcLYLxM KamuTJuMx7KF/vz8hkPzK0TBaI5SozbcsMCH+ZLFnu1HSadECDVMkOX0UoduuX5h6y4KCSJcR3D 0YLhX33xYWIm0XA3Pvq99ALCdtMLaATOUpyqPMZxIRA38P/dwborogmbAtARIVz8J52OVy+T29H rpAfPGC5x+js/TWVcSiTfXjBinr3a77rpLLmM7ApGzIhDiMWXrh+w9Wz/xXDqTMTaQzhvoev5QP McorVJCdYdFTob1BsKsJ2NEVUY760OrZJUSTvYoVnRXm1iHN1bEQdG7H66TyH4gKq42LRYkqUwt o0C8gEA8tsQsYmb9US+pwhy4Xu5Cm/Yfa9xE0YN+3BndfXUhXQ== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--15.224800-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24150.003 X-MDID: 1539366693-Dkj9Q26R6pZ1 Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename memzones allocated for DMA X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Oct 2018 17:51:34 -0000 On October 12, 2018 20:21:32 Thomas Monjalon wrote: > 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? Yes