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: Sun, 26 Jul 2015 22:44:54 +0200 Message-ID: <4059529.Cd3avY46BJ@xps13> References: <1434108496-1993-1-git-send-email-bruce.richardson@intel.com> <20150615101418.GA3872@bricha3-MOBL3> <2329102.ivJhQypGCc@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org To: nhorman@tuxdriver.com, Bruce Richardson Return-path: Received: from mail-wi0-f182.google.com (mail-wi0-f182.google.com [209.85.212.182]) by dpdk.org (Postfix) with ESMTP id 1A88CC45A for ; Sun, 26 Jul 2015 22:46:19 +0200 (CEST) Received: by wicgb10 with SMTP id gb10so86984851wic.1 for ; Sun, 26 Jul 2015 13:46:18 -0700 (PDT) In-Reply-To: <2329102.ivJhQypGCc@xps13> 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, Bruce, Can we move forward? 2015-07-06 17:11, Thomas Monjalon: > 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 > >