From mboxrd@z Thu Jan  1 00:00:00 1970
From: "David Harton (dharton)" <dharton@cisco.com>
Subject: Re: [RFC PATCH v1 2/3] drivers/net/ixgbe: change xstats
 to use integers
Date: Tue, 3 May 2016 13:40:20 +0000
Message-ID: <a568a90901834142a247a187eb0b6a18@XCH-RCD-016.cisco.com>
References: <1460731462-21229-1-git-send-email-remy.horton@intel.com>
 <1460731462-21229-3-git-send-email-remy.horton@intel.com>
 <34cbe07723f64e9f926666164d9d5319@XCH-RCD-016.cisco.com>
 <5728981D.2020003@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
To: Remy Horton <remy.horton@intel.com>, "dev@dpdk.org" <dev@dpdk.org>, "Helin
 Zhang" <helin.zhang@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from rcdn-iport-2.cisco.com (rcdn-iport-2.cisco.com [173.37.86.73])
 by dpdk.org (Postfix) with ESMTP id 8F4572BE6
 for <dev@dpdk.org>; Tue,  3 May 2016 15:40:22 +0200 (CEST)
In-Reply-To: <5728981D.2020003@intel.com>
Content-Language: en-US
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>



> -----Original Message-----
> From: Remy Horton [mailto:remy.horton@intel.com]
> Sent: Tuesday, May 03, 2016 8:23 AM
> To: David Harton (dharton) <dharton@cisco.com>; dev@dpdk.org; Helin Zhang
> <helin.zhang@intel.com>
> Subject: Re: [dpdk-dev] [RFC PATCH v1 2/3] drivers/net/ixgbe: change
> xstats to use integers
>=20
>=20
> On 29/04/2016 14:43, David Harton (dharton) wrote:
> [..]
> >> +		/* RX Priority Stats */
> >> +		for (stat =3D 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
> >> +			for (i =3D 0; i < 8; i++) {
> >
> > 8 seems magical.  Is there a constant somewhere that can be used?
>=20
> Not that I'm aware of. I've made it a #define as a small cleanup.
>=20
>=20
> >> +static int ixgbevf_dev_xstats_names(__rte_unused struct rte_eth_dev
> *dev,
> >> +	struct rte_eth_xstats_name *ptr_names, __rte_unused unsigned limit)
> >> +{
> >> +	unsigned i;
> >> +
> >> +	if (limit < IXGBEVF_NB_XSTATS)
> >
> > Think this check has to be removed since rte_eth_xstats_count() calls
> > with limit =3D=3D 0.
>=20
> Well spotted. It should only error-out if ptr_names is non-NULL.
>=20
> As an aside, I am wondering whether getting stats counts should itself be
> a seperate driver hook.

I personally think it would be cleaner.  Bug, I can also see the argument o=
f keeping
the vector table smaller and would conceded to others that felt strongly ab=
out it.

I do really like having a separate external API to the user.  A user should=
 explicitly
ask for the count.

Cheers,
Dave

>=20
>=20
> ..Remy