From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 9148D46089;
	Wed, 15 Jan 2025 07:28:03 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 63B794025F;
	Wed, 15 Jan 2025 07:28:03 +0100 (CET)
Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188])
 by mails.dpdk.org (Postfix) with ESMTP id B329E4003C
 for <dev@dpdk.org>; Wed, 15 Jan 2025 07:28:01 +0100 (CET)
Received: from mail.maildlp.com (unknown [172.19.88.105])
 by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4YXwvj61KMzbjw3;
 Wed, 15 Jan 2025 14:24:49 +0800 (CST)
Received: from kwepemo500011.china.huawei.com (unknown [7.202.195.194])
 by mail.maildlp.com (Postfix) with ESMTPS id 10DA5140159;
 Wed, 15 Jan 2025 14:27:58 +0800 (CST)
Received: from [10.67.121.193] (10.67.121.193) by
 kwepemo500011.china.huawei.com (7.202.195.194) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.2.1544.11; Wed, 15 Jan 2025 14:27:57 +0800
Message-ID: <27f57b15-68a0-4453-b044-be5baa0e8057@huawei.com>
Date: Wed, 15 Jan 2025 14:27:56 +0800
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/3] net/hns3: fix simple Tx path incorrect free the mbuf
To: Stephen Hemminger <stephen@networkplumber.org>
CC: Jie Hai <haijie1@huawei.com>, <dev@dpdk.org>, <thomas@monjalon.net>,
 <ferruh.yigit@amd.com>, <david.marchand@redhat.com>,
 <andrew.rybchenko@oktetlabs.ru>, Chengwen Feng <fengchengwen@huawei.com>,
 Huisong Li <lihuisong@huawei.com>
References: <20241230065405.18552-1-haijie1@huawei.com>
 <20241230065405.18552-2-haijie1@huawei.com> <20241230095530.7cb2f643@pi5>
 <29acb21d-a6d9-04bd-d528-10d1ccf3f7ce@huawei.com>
 <20250108085740.65815a35@pi5>
 <51df8d40-c158-4b67-98b0-ca4517f4bd2c@huawei.com>
 <20250109090319.261ac377@pi5>
Content-Language: en-US
From: huangdengdui <huangdengdui@huawei.com>
In-Reply-To: <20250109090319.261ac377@pi5>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
X-Originating-IP: [10.67.121.193]
X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To
 kwepemo500011.china.huawei.com (7.202.195.194)
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
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


On 2025/1/10 1:03, Stephen Hemminger wrote:
> On Thu, 9 Jan 2025 15:43:12 +0800
> huangdengdui <huangdengdui@huawei.com> wrote:
> 
>> On 2025/1/9 0:57, Stephen Hemminger wrote:
>>> On Wed, 8 Jan 2025 10:40:43 +0800
>>> Jie Hai <haijie1@huawei.com> wrote:
>>>   
>>>> On 2024/12/31 1:55, Stephen Hemminger wrote:  
>>>>> On Mon, 30 Dec 2024 14:54:03 +0800
>>>>> Jie Hai <haijie1@huawei.com> wrote:
>>>>>     
>>>>>> From: Jie Hai <haijie1@huawei.com>
>>>>>> To: <dev@dpdk.org>, <thomas@monjalon.net>, <ferruh.yigit@amd.com>,  <david.marchand@redhat.com>, <andrew.rybchenko@oktetlabs.ru>, Chengwen Feng  <fengchengwen@huawei.com>, "Wei Hu (Xavier)" <xavier.huwei@huawei.com>,  Huisong Li <lihuisong@huawei.com>
>>>>>> CC: <haijie1@huawei.com>, <huangdengdui@huawei.com>
>>>>>> Subject: [PATCH 1/3] net/hns3: fix simple Tx path incorrect free the mbuf
>>>>>> Date: Mon, 30 Dec 2024 14:54:03 +0800
>>>>>> X-Mailer: git-send-email 2.22.0
>>>>>>
>>>>>> From: Dengdui Huang <huangdengdui@huawei.com>
>>>>>>
>>>>>> When RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE offload is not set,
>>>>>> use rte_pktmbuf_free_seg() to free the mbuf.
>>>>>>
>>>>>> Fixes: 7ef933908f04 ("net/hns3: add simple Tx path")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>>>>>> Signed-off-by: Jie Hai <haijie1@huawei.com>    
>>>>>
>>>>> What about the fast free case which is using rte_mempool_put_bulk when
>>>>> it should use rte_pktmbuf_free_bulk instead?
>>>>>
>>>>>     
>>>> Hi, Stephen Hemminger,
>>>>
>>>> During the fast free case, the performance of using
>>>> rte_mempool_put_bulk is higher than that of using
>>>> rte_pktmbuf_free_bulk because other references
>>>> to mbuf do not need to be considered. So it's better
>>>>   to not change.
>>>>
>>>> Thanks,
>>>> Jie Hai  
>>>
>>> The problem is that having an open coded version of this buried in
>>> one driver is a long term potential proble>
>>> If you really think that optimizing free like this is noticeable, then
>>> why not make a new function "rte_pktmuf_fast_free_bulk" and put it in the
>>> regular mbuf library.
>>>   
>>
>> Do you mean to add the following functions to the library?
>>
>> void rte_pktmbuf_fast_free_bulk(struct rte_mbuf **mbufs, unsigned int count)
>> {
>> 	rte_mempool_put_bulk(mbufs[0]->pool, (void **)mbufs, count);
>> }
> 
> Yes something like that. Or call it rte_mbuf_raw_free_bulk to align with
> what rte_mbuf_raw_free(). And maybe add some debug assertions to make sure
> that mbuf is not cloned, indirect or has refcnt.
> 
> The concern is that this optimization might put an mbuf in the pool
> that has different properties than the normal free path.
> And that all semantics of allocation/free should be in rte_mbuf code to
> allow for future optimizations.

"Morten Brørup" has submitted a patch to implement this function.[1]
Currently, multiple drivers use rte_mempool_put_bulk to release mbufs.
We can modify them later so that all drivers use rte_mbuf_fast_free_bulk to free mbufs.

The current patch is a function problem, which has caused troubles to hns3 users. Can we merge it first?

[1]https://inbox.dpdk.org/dev/20250114163951.125667-1-mb@smartsharesystems.com/

> 
> 
>>
>> The driver uses rte_mempool_put_bulk only when the following conditions are met:
>> 1. All mbufs comes from the same mempool
>> 2. All mbufs have only one reference.
>> 3. All mbufs have only one segment.
>> So the rte_pktmbuf_fast_free_bulk function is just a wrapper around the rte_mempool_put_bulk function.
>