diff mbox series

[1/4] ethdev: verify queue ID when Tx done cleanup

Message ID	20240905064638.17980-2-haijie1@huawei.com (mailing list archive)
State	New
Delegated to:	Thomas Monjalon
Headers	From: Jie Hai <haijie1@huawei.com> To: <dev@dpdk.org>, <thomas@monjalon.net>, <ferruh.yigit@amd.com>, Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>, Keith Wiles <keith.wiles@intel.com>, Billy McFall <bmcfall@redhat.com> CC: <lihuisong@huawei.com>, <fengchengwen@huawei.com>, <haijie1@huawei.com> Subject: [PATCH 1/4] ethdev: verify queue ID when Tx done cleanup Date: Thu, 5 Sep 2024 14:46:35 +0800 Message-ID: <20240905064638.17980-2-haijie1@huawei.com> In-Reply-To: <20240905064638.17980-1-haijie1@huawei.com> References: <20240905064638.17980-1-haijie1@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain Precedence: list Errors-To: dev-bounces@dpdk.org
Series	clean code for dmadev and hns3 \| [0/4] clean code for dmadev and hns3 [1/4] ethdev: verify queue ID when Tx done cleanup [2/4] net/hns3: verify reset type from firmware [3/4] dmadev: fix potential null pointer access [4/4] dmadev: clean code for verify parameter

Checks

Context	Check	Description
ci/checkpatch	success	coding style OK

Commit Message

Jie Hai Sept. 5, 2024, 6:46 a.m. UTC

From: Chengwen Feng <fengchengwen@huawei.com>

Verify queue_id for rte_eth_tx_done_cleanup API.

Fixes: 44a718c457b5 ("ethdev: add API to free consumed buffers in Tx ring")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Andrew Rybchenko Sept. 5, 2024, 7:33 a.m. UTC | #1

On 9/5/24 09:46, Jie Hai wrote:
> From: Chengwen Feng <fengchengwen@huawei.com>
> 
> 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.

> 
> Fixes: 44a718c457b5 ("ethdev: add API to free consumed buffers in Tx ring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>   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;
>

diff mbox series

Patch

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;