From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v3 01/10] rte: change xstats to use integer ids Date: Wed, 08 Jun 2016 14:22:17 +0200 Message-ID: <2855986.LtnRHLyA0p@xps13> References: <1464605292-4599-1-git-send-email-remy.horton@intel.com> <2437213.nzTWdctYcm@xps13> <5757FE95.3090403@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org To: Remy Horton Return-path: Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id C86D19AD6 for ; Wed, 8 Jun 2016 14:22:19 +0200 (CEST) Received: by mail-wm0-f48.google.com with SMTP id n184so179082351wmn.1 for ; Wed, 08 Jun 2016 05:22:19 -0700 (PDT) In-Reply-To: <5757FE95.3090403@intel.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" 2016-06-08 12:16, Remy Horton: > 'noon, > > On 08/06/2016 10:37, Thomas Monjalon wrote: > > 2016-05-30 11:48, Remy Horton: > >> struct rte_eth_xstats { > >> + /* FIXME: Remove name[] once remaining drivers converted */ > >> char name[RTE_ETH_XSTATS_NAME_SIZE]; > > > > What is the plan? This field must be deprecated with an attribute. > > We cannot have 2 different APIs depending of the driver. > > This is where it gets logistically tricky.. > > Since there's an API/ABI breakage notice in place on this, my own > preference would be to have the entire patchset quashed into a single > patch. Problem is that rte/app changes (patches 1 & 7-9) are normally > applied via master whereas driver changes (patches 2-6) go in via > dpdk-next-net - it is not clear to me how patches should be submitted > for this case.. Misunderstanding here. Patches are fine and will be integrated in the main tree because they are not only some drivers changes. I was talking about the old API with name in rte_eth_xstats. I have not seen the patch 9 which removes it. > >> +int rte_eth_xstats_count(uint8_t port_id); > > > > This function is useless because we can have the count with > > rte_eth_xstats_get(p, NULL, 0) > > By the way it would be more consistent to have the same behaviour > > in rte_eth_xstats_names(). > > Feedback I got with earlier patches was that a seperate count function > was preferable to overloading the fetch function using *data==NULL - is > the use of the latter specifically preferred? I prefer the fetch/NULL style to get a count. It also handles nicely the fetch error because of a too small buffer.