From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 CC: Jie Hai , , , , , , Chengwen Feng , Huisong Li 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 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 wrote: > >> On 2025/1/9 0:57, Stephen Hemminger wrote: >>> On Wed, 8 Jan 2025 10:40:43 +0800 >>> Jie Hai wrote: >>> >>>> On 2024/12/31 1:55, Stephen Hemminger wrote: >>>>> On Mon, 30 Dec 2024 14:54:03 +0800 >>>>> Jie Hai wrote: >>>>> >>>>>> From: Jie Hai >>>>>> To: , , , , , Chengwen Feng , "Wei Hu (Xavier)" , Huisong Li >>>>>> CC: , >>>>>> 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 >>>>>> >>>>>> 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 >>>>>> Signed-off-by: Jie Hai >>>>> >>>>> 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. >