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 E326A2C36; Tue, 2 Apr 2019 09:36:46 +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-us2.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id B30B1280066; Tue, 2 Apr 2019 07:36:45 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 2 Apr 2019 08:36:38 +0100 To: Jerin Jacob Kollanukkaran , "stephen@networkplumber.org" , "thomas@monjalon.net" , Pavan Nikhilesh Bhagavatula , "ferruh.yigit@intel.com" CC: "dev@dpdk.org" , "stable@dpdk.org" References: <20190331162437.13048-1-pbhagavatula@marvell.com> <105f221f-0c64-e950-df9a-2c9eaa0991c3@solarflare.com> <819b2c86516855fda65403a8e94be9d03c4d7eeb.camel@marvell.com> From: Andrew Rybchenko Message-ID: <31ff511d-db25-7a64-63cb-97272f363dc1@solarflare.com> Date: Tue, 2 Apr 2019 10:36:12 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <819b2c86516855fda65403a8e94be9d03c4d7eeb.camel@marvell.com> Content-Language: en-GB X-Originating-IP: [91.220.146.112] 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-24526.003 X-TM-AS-Result: No-26.900700-8.000000-10 X-TMASE-MatchedRID: 9d2LtCNB3NIOwH4pD14DsPHkpkyUphL9+WzVGPiSY8gbVUVEY6U/r0oG fkhxJr6Wq1SffpAHzg582LSyel7BroBoGyFs+F1M84dsinZ5e1iH7D1bP/FcOhER5MrEGoh5oHM E123LGpOEBoUDDGMnVx6mHHBXlfPgsEBAuoaUqK+iAZ3zAhQYgrqGBW9J0Yqjh8BhJvgqWBkCkF PsvUlhGHILnXUBr+uuY32EbtFz/ckWiEoVBSOGHVVN8laWo90MWwKGivsEuI2il/DyAQi7pPbUh Pv66jYYFpa9AZMa1ccDxGMglREe2gGw2Tq5Z3SYMIiU395I8H0qkGpTIbOFAYFPumCibw8G0ZeB CmEdCbEdQ/Z33Vh06+GEuhchvL8o8lEDYmoBkrPbH8WaUL9qjJv8tNLRPUrWv8M96e6S1b0sWVd l3YcDsO3zSoJhBLBNapZ5uo8Kdw84c5PfPWZTtoQyuJF34BG6pNh/2/1+WiU7LF3pX3rdVLjxa5 EVBV1q585VzGMOFzABi3kqJOK62SAHAopEd76vR7xzOSs3wid9VSqq5I3ThR7y7JqRfJAVj8FM4 I+1Iq77Z5vTFHEXSA== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--26.900700-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24526.003 X-MDID: 1554190606-3NZuC5XLdiRM Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] ethdev: fix DMA zone reserve not honoring size 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: Tue, 02 Apr 2019 07:36:47 -0000 On 4/2/19 3:47 AM, Jerin Jacob Kollanukkaran wrote: > On Mon, 2019-04-01 at 10:30 +0300, Andrew Rybchenko wrote: >> External Email >> On 3/31/19 7:25 PM, Pavan Nikhilesh Bhagavatula wrote: >>> From: Pavan Nikhilesh >>> >>> The `rte_eth_dma_zone_reserve()` is generally used to create HW >>> rings. >>> In some scenarios when a driver needs to reconfigure the ring size >>> since the named memzone already exists it returns the previous >>> memzone >>> without checking if a different sized ring is requested. >>> >>> Introduce a check to see if the ring size requested is different >>> from the >>> previously created memzone length. >>> >>> Fixes: 719dbebceb81 ("xen: allow determining DOM0 at runtime") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Pavan Nikhilesh >>> --- >>> lib/librte_ethdev/rte_ethdev.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c >>> b/lib/librte_ethdev/rte_ethdev.c >>> index 12b66b68c..4ae12e43b 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.c >>> +++ b/lib/librte_ethdev/rte_ethdev.c >>> @@ -3604,9 +3604,12 @@ rte_eth_dma_zone_reserve(const struct >>> rte_eth_dev *dev, const char *ring_name, >>> } >>> >>> mz = rte_memzone_lookup(z_name); >>> - if (mz) >>> + if (mz && (mz->len == size)) >>> return mz; >>> >>> + if (mz) >>> + rte_memzone_free(mz); >> >> NACK >> I really don't like that API which should reserve does free if >> requested >> size does not match previously allocated. > Why? Is due to API name? 1. The problem really exists. The problem is bad and it very good that you     caught it and came up with a patch. Many thanks. 2. Silently free and reallocate memory is bad. Memory could be used/mapped etc. 3. As an absolute minimum if we accept the behaviour it must be documented     in the function description. > If so, > Can we have rte_eth_dma_zone_reservere_with_resize() then ? > or any another name, You would like to have? 4. I'd prefer an error if different size (or bigger) memzone is requested,     but I understand that it can break existing drivers. Thomas, Ferruh, what do you think? >> I understand the motivation, but I don't think the solution is >> correct. > What you think it has correct solution then? See above plus handling in drivers or dedicated function with better name as you suggest above. > Obviously, We can not allocate max ring size in init time. > If the NIC has support for 64K HW ring, We will be wasting too much as > it is per queue. Yes, I agree that it is an overkill. net/sfc tries to carefully free/reserve on NIC/queues reconfigure. Many thanks, Andrew. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 4FD8EA0679 for ; Tue, 2 Apr 2019 09:36:50 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8208F34F3; Tue, 2 Apr 2019 09:36:48 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id E326A2C36; Tue, 2 Apr 2019 09:36:46 +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-us2.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id B30B1280066; Tue, 2 Apr 2019 07:36:45 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 2 Apr 2019 08:36:38 +0100 To: Jerin Jacob Kollanukkaran , "stephen@networkplumber.org" , "thomas@monjalon.net" , Pavan Nikhilesh Bhagavatula , "ferruh.yigit@intel.com" CC: "dev@dpdk.org" , "stable@dpdk.org" References: <20190331162437.13048-1-pbhagavatula@marvell.com> <105f221f-0c64-e950-df9a-2c9eaa0991c3@solarflare.com> <819b2c86516855fda65403a8e94be9d03c4d7eeb.camel@marvell.com> From: Andrew Rybchenko Message-ID: <31ff511d-db25-7a64-63cb-97272f363dc1@solarflare.com> Date: Tue, 2 Apr 2019 10:36:12 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <819b2c86516855fda65403a8e94be9d03c4d7eeb.camel@marvell.com> Content-Language: en-GB X-Originating-IP: [91.220.146.112] 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-24526.003 X-TM-AS-Result: No-26.900700-8.000000-10 X-TMASE-MatchedRID: 9d2LtCNB3NIOwH4pD14DsPHkpkyUphL9+WzVGPiSY8gbVUVEY6U/r0oG fkhxJr6Wq1SffpAHzg582LSyel7BroBoGyFs+F1M84dsinZ5e1iH7D1bP/FcOhER5MrEGoh5oHM E123LGpOEBoUDDGMnVx6mHHBXlfPgsEBAuoaUqK+iAZ3zAhQYgrqGBW9J0Yqjh8BhJvgqWBkCkF PsvUlhGHILnXUBr+uuY32EbtFz/ckWiEoVBSOGHVVN8laWo90MWwKGivsEuI2il/DyAQi7pPbUh Pv66jYYFpa9AZMa1ccDxGMglREe2gGw2Tq5Z3SYMIiU395I8H0qkGpTIbOFAYFPumCibw8G0ZeB CmEdCbEdQ/Z33Vh06+GEuhchvL8o8lEDYmoBkrPbH8WaUL9qjJv8tNLRPUrWv8M96e6S1b0sWVd l3YcDsO3zSoJhBLBNapZ5uo8Kdw84c5PfPWZTtoQyuJF34BG6pNh/2/1+WiU7LF3pX3rdVLjxa5 EVBV1q585VzGMOFzABi3kqJOK62SAHAopEd76vR7xzOSs3wid9VSqq5I3ThR7y7JqRfJAVj8FM4 I+1Iq77Z5vTFHEXSA== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--26.900700-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24526.003 X-MDID: 1554190606-3NZuC5XLdiRM Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] ethdev: fix DMA zone reserve not honoring size 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190402073612.t1zTtIRA1w5OTKkLsbXZ8ItZq6dE-0ftlrdJB_aWhSQ@z> On 4/2/19 3:47 AM, Jerin Jacob Kollanukkaran wrote: > On Mon, 2019-04-01 at 10:30 +0300, Andrew Rybchenko wrote: >> External Email >> On 3/31/19 7:25 PM, Pavan Nikhilesh Bhagavatula wrote: >>> From: Pavan Nikhilesh >>> >>> The `rte_eth_dma_zone_reserve()` is generally used to create HW >>> rings. >>> In some scenarios when a driver needs to reconfigure the ring size >>> since the named memzone already exists it returns the previous >>> memzone >>> without checking if a different sized ring is requested. >>> >>> Introduce a check to see if the ring size requested is different >>> from the >>> previously created memzone length. >>> >>> Fixes: 719dbebceb81 ("xen: allow determining DOM0 at runtime") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Pavan Nikhilesh >>> --- >>> lib/librte_ethdev/rte_ethdev.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c >>> b/lib/librte_ethdev/rte_ethdev.c >>> index 12b66b68c..4ae12e43b 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.c >>> +++ b/lib/librte_ethdev/rte_ethdev.c >>> @@ -3604,9 +3604,12 @@ rte_eth_dma_zone_reserve(const struct >>> rte_eth_dev *dev, const char *ring_name, >>> } >>> >>> mz = rte_memzone_lookup(z_name); >>> - if (mz) >>> + if (mz && (mz->len == size)) >>> return mz; >>> >>> + if (mz) >>> + rte_memzone_free(mz); >> >> NACK >> I really don't like that API which should reserve does free if >> requested >> size does not match previously allocated. > Why? Is due to API name? 1. The problem really exists. The problem is bad and it very good that you     caught it and came up with a patch. Many thanks. 2. Silently free and reallocate memory is bad. Memory could be used/mapped etc. 3. As an absolute minimum if we accept the behaviour it must be documented     in the function description. > If so, > Can we have rte_eth_dma_zone_reservere_with_resize() then ? > or any another name, You would like to have? 4. I'd prefer an error if different size (or bigger) memzone is requested,     but I understand that it can break existing drivers. Thomas, Ferruh, what do you think? >> I understand the motivation, but I don't think the solution is >> correct. > What you think it has correct solution then? See above plus handling in drivers or dedicated function with better name as you suggest above. > Obviously, We can not allocate max ring size in init time. > If the NIC has support for 64K HW ring, We will be wasting too much as > it is per queue. Yes, I agree that it is an overkill. net/sfc tries to carefully free/reserve on NIC/queues reconfigure. Many thanks, Andrew.