From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH 4/4] ethdev: check support for rx_queue_count and descriptor_done fns Date: Mon, 06 Jul 2015 17:11:33 +0200 Message-ID: <2329102.ivJhQypGCc@xps13> References: <1434108496-1993-1-git-send-email-bruce.richardson@intel.com> <557B17C8.3090300@cisco.com> <20150615101418.GA3872@bricha3-MOBL3> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org To: nhorman@tuxdriver.com Return-path: Received: from mail-wi0-f174.google.com (mail-wi0-f174.google.com [209.85.212.174]) by dpdk.org (Postfix) with ESMTP id 5AEF711C5 for ; Mon, 6 Jul 2015 17:12:43 +0200 (CEST) Received: by wiwl6 with SMTP id l6so287895958wiw.0 for ; Mon, 06 Jul 2015 08:12:43 -0700 (PDT) In-Reply-To: <20150615101418.GA3872@bricha3-MOBL3> 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" Neil, your ABI expertise is required for this patch. 2015-06-15 11:14, Bruce Richardson: > 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