On 9/5/2024 8:33 AM, Andrew Rybchenko wrote:
> On 9/5/24 09:46, Jie Hai wrote:
>> From: Chengwen Feng <fengcheng...@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.
>
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?
>>
>> Fixes: 44a718c457b5 ("ethdev: add API to free consumed buffers in Tx
>> ring")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengcheng...@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;
>>
>
