From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 15 Jun 2015 03:14:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,618,1427785200"; d="scan'208";a="743575371" Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.22]) by fmsmga002.fm.intel.com with SMTP; 15 Jun 2015 03:14:19 -0700 Received: by (sSMTP sendmail emulation); Mon, 15 Jun 2015 11:14:18 +0025 Date: Mon, 15 Jun 2015 11:14:18 +0100 From: Bruce Richardson To: "Roger B. Melton" 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 Content-Disposition: inline In-Reply-To: <557B17C8.3090300@cisco.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 4/4] ethdev: check support for rx_queue_count and descriptor_done fns X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jun 2015 10:14:23 -0000 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