From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id DF5B2C620 for ; Mon, 22 Jun 2015 16:12:28 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 22 Jun 2015 07:12:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,659,1427785200"; d="scan'208";a="751094826" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by orsmga002.jf.intel.com with ESMTP; 22 Jun 2015 07:12:26 -0700 Received: from irsmsx109.ger.corp.intel.com ([169.254.13.200]) by IRSMSX106.ger.corp.intel.com ([169.254.8.121]) with mapi id 14.03.0224.002; Mon, 22 Jun 2015 15:12:25 +0100 From: "Tahhan, Maryam" To: Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH 2/4] ethdev: expose extended error stats Thread-Index: AQHQn7YLyWAV2SOUhEGNw1ygGDqFKJ2wux2AgAfsSUA= Date: Mon, 22 Jun 2015 14:12:25 +0000 Message-ID: <1A27633A6DA49C4A92FCD5D4312DBF536A43EB82@IRSMSX109.ger.corp.intel.com> References: <1433525705-17041-1-git-send-email-maryam.tahhan@intel.com> <1433525705-17041-3-git-send-email-maryam.tahhan@intel.com> <6388992.ny7qEppTdV@xps13> In-Reply-To: <6388992.ny7qEppTdV@xps13> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 2/4] ethdev: expose extended error stats 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, 22 Jun 2015 14:12:29 -0000 > 2015-06-05 18:35, Maryam Tahhan: > > Extend rte_eth_xstats_get to retrieve additional stats from the device > > driver as well the top level extended stats. Add additional drop > > counters to the extended stats. > > > > Signed-off-by: Maryam Tahhan > [..] > Patch 1/4 doesn't compile without patch 2/4. The rebased patches should fix this issue. >=20 > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -129,6 +129,8 @@ static const struct rte_eth_xstats_name_off > rte_stats_strings[] =3D { > > {"rx_crc_errors", offsetof(struct rte_eth_stats, ibadcrc)}, > > {"rx_bad_length_errors", offsetof(struct rte_eth_stats, ibadlen)}, > > {"rx_errors", offsetof(struct rte_eth_stats, ierrors)}, > > + {"rx_mac_err", offsetof(struct rte_eth_stats, imacerr)}, > > + {"rx_phy_err", offsetof(struct rte_eth_stats, iphyerr)}, > > {"alloc_rx_buff_failed", offsetof(struct rte_eth_stats, rx_nombuf)}, > > {"fdir_match", offsetof(struct rte_eth_stats, fdirmatch)}, > > {"fdir_miss", offsetof(struct rte_eth_stats, fdirmiss)}, @@ -136,6 > > +138,8 @@ static const struct rte_eth_xstats_name_off rte_stats_strings= [] > =3D { > > {"rx_flow_control_xon", offsetof(struct rte_eth_stats, > rx_pause_xon)}, > > {"tx_flow_control_xoff", offsetof(struct rte_eth_stats, > tx_pause_xoff)}, > > {"rx_flow_control_xoff", offsetof(struct rte_eth_stats, > > rx_pause_xoff)}, > > + {"tx_drops", offsetof(struct rte_eth_stats, odrop)}, > > + {"rx_drops", offsetof(struct rte_eth_stats, idrop)}, > > }; > [...] > > --- a/lib/librte_ether/rte_ethdev.h > > +++ b/lib/librte_ether/rte_ethdev.h > > @@ -224,6 +224,10 @@ struct rte_eth_stats { > > > > /**< Total number of good bytes received from loopback,VF Only = */ > > uint64_t olbbytes; > > /**< Total number of good bytes transmitted to loopback,VF > > Only */ > > > > + uint64_t imacerr; /**< Total of RX packets with MAC Errors. *= / > > + uint64_t iphyerr; /**< Total of RX packets with PHY Errors. *= / > > + uint64_t idrop; /**< Total number of dropped received packets.= */ > > + uint64_t odrop; /**< Total number of dropped transmitted > > + packets. */ > > }; >=20 > You are extending the generic stats. This is not the idea behind xstats. > The xstats are specific to the driver. I'd followed the example of: http://patchwork.dpdk.org/dev/patchwork/patch/= 85/=20 to added generic extended stats (at least what I thought were generic). I t= hink that dropped packets should fall under struct rte_eth_stats, and should perhaps = be left there, as most NICs/drivers should be able to provide that number. Would th= is be an agreeable solution? I have no other way to expose the total MAC errors and the total PHY errors= without=20 Adding counters into struct ixgbe_hw_stats, but I wasn't sure if this was a= llowable, is it? The only other option is to round up all the errors into ierrors, without h= aving the granularity of what errors fall under. Is the latter option to sum up the v= alues under one umbrella preferred? > Furthermore we should migrate some "not really generic stats" to xstats i= n > order to keep only the really basic and common stats in rte_eth_stats. > By the way, in order to avoid duplicated code when getting generic stats > through xstats API, we need to change the implementation of > rte_eth_xstats_get() to add generic stats automatically, even if the driv= er > provide some xstats.