Re: [PATCH v2 1/2] ethdev: remove unnecessary type conversion

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

On 5/12/25 19:04, Morten Brørup wrote:
>> From: Sunil Kumar Kori <skori@marvell.com>
>> Sent: Monday, 12 May 2025 17.07
>>
>> As rte_eth_rx_queue_count() returns signed value to represent
>> the error cases but internally invoked callback is returning
>> unsigned value. Hence unnecessary type conversion is done.
>>
>> To avoid this typecasting from signed to unsigned, fixed
>> return type of callback functions.
>>
>> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> 
> Note for other reviewers:
> The tx_queue_count callback already returns int, and doesn't need to be updated.
> 
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index ea7f8c4a1a..b3031ab9e6 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -6401,7 +6401,7 @@ rte_eth_rx_queue_count(uint16_t port_id, uint16_t
>> queue_id)
>>
>>   	if (p->rx_queue_count == NULL)
>>   		return -ENOTSUP;
>> -	return (int)p->rx_queue_count(qd);
>> +	return p->rx_queue_count(qd);
>>   }
>>
>>   /**@{@name Rx hardware descriptor states
>> diff --git a/lib/ethdev/rte_ethdev_core.h
>> b/lib/ethdev/rte_ethdev_core.h
>> index e55fb42996..4ffae4921a 100644
>> --- a/lib/ethdev/rte_ethdev_core.h
>> +++ b/lib/ethdev/rte_ethdev_core.h
>> @@ -45,7 +45,7 @@ typedef uint16_t (*eth_tx_prep_t)(void *txq,
>>
>>
>>   /** @internal Get number of used descriptors on a receive queue. */
>> -typedef uint32_t (*eth_rx_queue_count_t)(void *rxq);
>> +typedef int (*eth_rx_queue_count_t)(void *rxq);
>>
>>   /** @internal Check the status of a Rx descriptor */
>>   typedef int (*eth_rx_descriptor_status_t)(void *rxq, uint16_t offset);
>> --
>> 2.43.0
> 
> For the ethdev library changes:
> Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
> 

IMHO API consistency vs Tx is more important here. Moreover, since it
helps to avoid weird type casts in the next patch, it proves that it is
a step in a right direction. Basically I see no point to keep the
difference in return value of ethdev API and driver callback, it does
not add value. If driver callback always successful it can simply always
return non-negative values - that's it.

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>