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 44E0445B1C; Sat, 12 Oct 2024 11:14:40 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DB2DA40268; Sat, 12 Oct 2024 11:14:39 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 439E340265 for ; Sat, 12 Oct 2024 11:14:37 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.19.88.194]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4XQd7M72dNz10N05; Sat, 12 Oct 2024 17:12:47 +0800 (CST) Received: from dggpeml500024.china.huawei.com (unknown [7.185.36.10]) by mail.maildlp.com (Postfix) with ESMTPS id 6F787140257; Sat, 12 Oct 2024 17:14:35 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Sat, 12 Oct 2024 17:14:35 +0800 Message-ID: Date: Sat, 12 Oct 2024 17:14:34 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/4] ethdev: verify queue ID when Tx done cleanup To: Ferruh Yigit , Andrew Rybchenko , Jie Hai , CC: , , Keith Wiles , Billy McFall References: <20240905064638.17980-1-haijie1@huawei.com> <20240905064638.17980-2-haijie1@huawei.com> <20681877-7c2a-4b54-bf32-0de7779df55c@oktetlabs.ru> <78d72ce0-88b7-4e0b-8acc-4da7b7a527d8@amd.com> Content-Language: en-US From: fengchengwen In-Reply-To: <78d72ce0-88b7-4e0b-8acc-4da7b7a527d8@amd.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggpeml500024.china.huawei.com (7.185.36.10) 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 2024/10/12 10:27, Ferruh Yigit wrote: > On 9/5/2024 8:33 AM, Andrew Rybchenko wrote: >> On 9/5/24 09:46, Jie Hai wrote: >>> From: Chengwen Feng >>> >>> Verify queue_id for rte_eth_tx_done_cleanup API. >> >> If I'm not mistaken the function is considered data path API (fast). >> If so, it should not validate it's parameters as in rte_eth_tx_burst(). >> It may be done under RTE_ETHDEV_DEBUG_TX only. >> >> May be documentation should be fixed to highlight it. >> >> And yes, current implementation looks inconsistent from this point of >> view since port_id and presence of callback are checked. >> >> Anyway motivation in the patch description is insufficient. >> > > Hi Chengwen, > > I agree with Andrew, to not add checks to the datapath function. > > Can you please explain more why this check is needed at first place? > Is it for a specific usecase? Hi Ferruh, It was triggered by our internal security review. I think it's okay to add the check under RTE_ETHDEV_DEBUG_TX. and Haijie will send v2. Thanks > >>> >>> Fixes: 44a718c457b5 ("ethdev: add API to free consumed buffers in Tx >>> ring") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Chengwen Feng >>> --- >>>   lib/ethdev/rte_ethdev.c | 4 ++++ >>>   1 file changed, 4 insertions(+) >>> >>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >>> index f1c658f49e80..998deb5ab101 100644 >>> --- a/lib/ethdev/rte_ethdev.c >>> +++ b/lib/ethdev/rte_ethdev.c >>> @@ -2823,6 +2823,10 @@ rte_eth_tx_done_cleanup(uint16_t port_id, >>> uint16_t queue_id, uint32_t free_cnt) >>>       RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >>>       dev = &rte_eth_devices[port_id]; >>>   +    ret = eth_dev_validate_tx_queue(dev, queue_id); >>> +    if (ret != 0) >>> +        return ret; >>> + >>>       if (*dev->dev_ops->tx_done_cleanup == NULL) >>>           return -ENOTSUP; >>>   >> >