From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <arybchenko@solarflare.com>
Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com
[67.231.154.164]) by dpdk.org (Postfix) with ESMTP id F0F982C36;
Tue, 2 Apr 2019 10:44:42 +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
CACBD140069; Tue, 2 Apr 2019 08:44:41 +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
09:44:35 +0100
To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"thomas@monjalon.net" <thomas@monjalon.net>, Pavan Nikhilesh Bhagavatula
<pbhagavatula@marvell.com>, "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
References: <20190331162437.13048-1-pbhagavatula@marvell.com>
<105f221f-0c64-e950-df9a-2c9eaa0991c3@solarflare.com>
<819b2c86516855fda65403a8e94be9d03c4d7eeb.camel@marvell.com>
<31ff511d-db25-7a64-63cb-97272f363dc1@solarflare.com>
<12cbd37b9f47b234459892f8374ac95616070638.camel@marvell.com>
From: Andrew Rybchenko <arybchenko@solarflare.com>
Message-ID: <a2c0d30a-fdfc-0538-b4e1-bf98272474a4@solarflare.com>
Date: Tue, 2 Apr 2019 11:44:32 +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: <12cbd37b9f47b234459892f8374ac95616070638.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-30.556500-8.000000-10
X-TMASE-MatchedRID: /77LoUQXvQ8OwH4pD14DsPHkpkyUphL9+WzVGPiSY8gbVUVEY6U/r0oG
fkhxJr6Wq1SffpAHzg582LSyel7BroBoGyFs+F1M84dsinZ5e1iH7D1bP/FcOhER5MrEGoh5oHM
E123LGpOEBoUDDGMnVx6mHHBXlfPgsEBAuoaUqK+iAZ3zAhQYgrqGBW9J0Yqjh8BhJvgqWBkCkF
PsvUlhGHILnXUBr+uu+X6JFUGKUhcTZlFtXatzcRWCVBr+Ay98bveZreOw8zZaW2Ktn+I8/vG39
9+Ui/tdanMRc/pu7fpQWd2CSdVLGuJFvnMb8Rr3zY7Fk1CTrPH/lBG+uXYJkBKs4drB12Rx0QxB
l5kNTScsrV1+ql6/GC24qsAWZ/7flvnH+pxJKEpjoaO27r+3fTQM9EfFyYyLTErrGhOiMLjSpsi
eekGtxDJ0lHjwzUMziSQL5szRvdHOrVNaM4+O1+J28Kqpjndpfuoqrh8mjZmY5NBG7YIbVywpAZ
mF/F0tHUP2d91YdOvhhLoXIby/KPJRA2JqAZKz2x/FmlC/aoyb/LTS0T1K1r/DPenuktW9LFlXZ
d2HA7Dt80qCYQSwTWqWebqPCncPOHOT3z1mU7a628cXbnOhTw3mvPKZj/z+Oyxd6V963VS48WuR
FQVdauLzNWBegCW2RYvisGWbbS+3sNbcHjySQTsAVzN+Ov/sLsfGyLodGYY4tx7U3hS+yIRXHC2
cqPxuAcI6j5I8aKhdUB/58JDjrw==
X-TM-AS-User-Approved-Sender: Yes
X-TM-AS-User-Blocked-Sender: No
X-TMASE-Result: 10--30.556500-8.000000
X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24526.003
X-MDID: 1554194682-MRFraz0vCuRn
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 02 Apr 2019 08:44:43 -0000
On 4/2/19 11:25 AM, Jerin Jacob Kollanukkaran wrote:
> On Tue, 2019-04-02 at 10:36 +0300, Andrew Rybchenko wrote:
>> 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 <pbhagavatula@marvell.com>
>>>>>
>>>>> 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 <pbhagavatula@marvell.com>
>>>>> ---
>>>>> 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.
> If I understand it correctly, Its been used while configuring
> the device and it is per queue, If so, Is there any case where
> memory in use in parallel in real world case with DPDK?
"in real world case with DPDK" is very fragile justification.
I simply don't want to dig in this way since it is very easy to make
a mistake or simply false assumption.
>> 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.
> Handling in driver means return error?
Yes.
> Regarding API, Yes, We can add new API. What we will do that exiting
> driver. Is up to driver maintainers to use the new API. I am fine with
> either approach, Just asking the opinion.
You have mine, but I'd like to know what other ethdev maintainers
think about it.
>>> 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: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by dpdk.space (Postfix) with ESMTP id 40780A0679
for <public@inbox.dpdk.org>; Tue, 2 Apr 2019 10:44:44 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 0B01C34F3;
Tue, 2 Apr 2019 10:44:44 +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 F0F982C36;
Tue, 2 Apr 2019 10:44:42 +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
CACBD140069; Tue, 2 Apr 2019 08:44:41 +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
09:44:35 +0100
To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"thomas@monjalon.net" <thomas@monjalon.net>, Pavan Nikhilesh Bhagavatula
<pbhagavatula@marvell.com>, "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
References: <20190331162437.13048-1-pbhagavatula@marvell.com>
<105f221f-0c64-e950-df9a-2c9eaa0991c3@solarflare.com>
<819b2c86516855fda65403a8e94be9d03c4d7eeb.camel@marvell.com>
<31ff511d-db25-7a64-63cb-97272f363dc1@solarflare.com>
<12cbd37b9f47b234459892f8374ac95616070638.camel@marvell.com>
From: Andrew Rybchenko <arybchenko@solarflare.com>
Message-ID: <a2c0d30a-fdfc-0538-b4e1-bf98272474a4@solarflare.com>
Date: Tue, 2 Apr 2019 11:44:32 +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: <12cbd37b9f47b234459892f8374ac95616070638.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-30.556500-8.000000-10
X-TMASE-MatchedRID: /77LoUQXvQ8OwH4pD14DsPHkpkyUphL9+WzVGPiSY8gbVUVEY6U/r0oG
fkhxJr6Wq1SffpAHzg582LSyel7BroBoGyFs+F1M84dsinZ5e1iH7D1bP/FcOhER5MrEGoh5oHM
E123LGpOEBoUDDGMnVx6mHHBXlfPgsEBAuoaUqK+iAZ3zAhQYgrqGBW9J0Yqjh8BhJvgqWBkCkF
PsvUlhGHILnXUBr+uu+X6JFUGKUhcTZlFtXatzcRWCVBr+Ay98bveZreOw8zZaW2Ktn+I8/vG39
9+Ui/tdanMRc/pu7fpQWd2CSdVLGuJFvnMb8Rr3zY7Fk1CTrPH/lBG+uXYJkBKs4drB12Rx0QxB
l5kNTScsrV1+ql6/GC24qsAWZ/7flvnH+pxJKEpjoaO27r+3fTQM9EfFyYyLTErrGhOiMLjSpsi
eekGtxDJ0lHjwzUMziSQL5szRvdHOrVNaM4+O1+J28Kqpjndpfuoqrh8mjZmY5NBG7YIbVywpAZ
mF/F0tHUP2d91YdOvhhLoXIby/KPJRA2JqAZKz2x/FmlC/aoyb/LTS0T1K1r/DPenuktW9LFlXZ
d2HA7Dt80qCYQSwTWqWebqPCncPOHOT3z1mU7a628cXbnOhTw3mvPKZj/z+Oyxd6V963VS48WuR
FQVdauLzNWBegCW2RYvisGWbbS+3sNbcHjySQTsAVzN+Ov/sLsfGyLodGYY4tx7U3hS+yIRXHC2
cqPxuAcI6j5I8aKhdUB/58JDjrw==
X-TM-AS-User-Approved-Sender: Yes
X-TM-AS-User-Blocked-Sender: No
X-TMASE-Result: 10--30.556500-8.000000
X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24526.003
X-MDID: 1554194682-MRFraz0vCuRn
Content-Type: text/plain; charset="UTF-8"; format="flowed"
Content-Transfer-Encoding: 7bit
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190402084432.nbOtJN9SlIZu25FbA6zZsJ7VMKcWLqO97S88BgsB4yY@z>
On 4/2/19 11:25 AM, Jerin Jacob Kollanukkaran wrote:
> On Tue, 2019-04-02 at 10:36 +0300, Andrew Rybchenko wrote:
>> 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 <pbhagavatula@marvell.com>
>>>>>
>>>>> 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 <pbhagavatula@marvell.com>
>>>>> ---
>>>>> 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.
> If I understand it correctly, Its been used while configuring
> the device and it is per queue, If so, Is there any case where
> memory in use in parallel in real world case with DPDK?
"in real world case with DPDK" is very fragile justification.
I simply don't want to dig in this way since it is very easy to make
a mistake or simply false assumption.
>> 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.
> Handling in driver means return error?
Yes.
> Regarding API, Yes, We can add new API. What we will do that exiting
> driver. Is up to driver maintainers to use the new API. I am fine with
> either approach, Just asking the opinion.
You have mine, but I'd like to know what other ethdev maintainers
think about it.
>>> 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.