Re: [dpdk-dev] [PATCH 4/4] ethdev: check support for rx_queue_count and descriptor_done fns

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Jun 12, 2015 at 01:32:56PM -0400, Roger B. Melton wrote:
> Hi Bruce,  Comment in-line.  Regards, Roger
> 
> On 6/12/15 7:28 AM, Bruce Richardson wrote:
> >The functions rte_eth_rx_queue_count and rte_eth_descriptor_done are
> >supported by very few PMDs. Therefore, it is best to check for support
> >for the functions in the ethdev library, so as to avoid crashes
> >at run-time if the application goes to use those APIs. The performance
> >impact of this change should be very small as this is a predictable
> >branch in the function.
> >
> >Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >---
> >  lib/librte_ether/rte_ethdev.h | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> >index 827ca3e..9ad1b6a 100644
> >--- a/lib/librte_ether/rte_ethdev.h
> >+++ b/lib/librte_ether/rte_ethdev.h
> >@@ -2496,6 +2496,8 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
> >   *  The queue id on the specific port.
> >   * @return
> >   *  The number of used descriptors in the specific queue.
> >+ *  NOTE: if function is not supported by device this call
> >+ *        returns (uint32_t)-ENOTSUP
> >   */
> >  static inline uint32_t
> 
> Why not change the return type to int32_t?
> In this way, the caller isn't required to make the assumption that a large
> queue count indicates an error.  < 0 means error, other wise it's a valid
> queue count.
> 
> This approach would be consistent with other APIs.
> 

Yes, good point, I should see about that. One thing I'm unsure of, though, is
does this count as ABI breakage? I don't see how it should break any older
apps, since the return type is the same size, but I'm not sure as we are 
changing the return type of the function.

Neil, can you perhaps comment here? Is changing uint32_t to int32_t ok, from
an ABI point of view?

Regards,
/Bruce