From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruce Richardson Subject: Re: [PATCH 4/4] ethdev: check support for rx_queue_count and descriptor_done fns Date: Mon, 15 Jun 2015 11:14:18 +0100 Message-ID: <20150615101418.GA3872@bricha3-MOBL3> References: <1434108496-1993-1-git-send-email-bruce.richardson@intel.com> <1434108496-1993-5-git-send-email-bruce.richardson@intel.com> <557B17C8.3090300@cisco.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: "Roger B. Melton" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id A5081FFA for ; Mon, 15 Jun 2015 12:14:22 +0200 (CEST) Content-Disposition: inline In-Reply-To: <557B17C8.3090300@cisco.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > >--- > > 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