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

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

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;
>