From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id BF358C6B6 for ; Mon, 22 Jun 2015 17:35:58 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga101.jf.intel.com with ESMTP; 22 Jun 2015 08:35:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,660,1427785200"; d="scan'208";a="748231205" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by fmsmga002.fm.intel.com with ESMTP; 22 Jun 2015 08:35:54 -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 16:35:51 +0100 From: "Tahhan, Maryam" To: Olivier MATZ , Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH 2/4] ethdev: expose extended error stats Thread-Index: AQHQn7YLyWAV2SOUhEGNw1ygGDqFKJ2wux2AgAfsSUCAAAQTgIAAFbgA Date: Mon, 22 Jun 2015 15:35:51 +0000 Message-ID: <1A27633A6DA49C4A92FCD5D4312DBF536A4404C3@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> <1A27633A6DA49C4A92FCD5D4312DBF536A43EB82@IRSMSX109.ger.corp.intel.com> <558825D3.2090106@6wind.com> In-Reply-To: <558825D3.2090106@6wind.com> 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 15:35:59 -0000 > >>> + packets. */ > >>> }; > >> > >> You are extending the generic stats. This is not the idea behind xstat= s. > >> The xstats are specific to the driver. > > > > I'd followed the example of: > > http://patchwork.dpdk.org/dev/patchwork/patch/85/ > > to added generic extended stats (at least what I thought were > > generic). I think 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 this be an agreeable > solution? > > > > I have no other way to expose the total MAC errors and the total PHY > > errors without Adding counters into struct ixgbe_hw_stats, but I wasn't= sure > if this was allowable, is it? > > > > The only other option is to round up all the errors into ierrors, > > without having the granularity of what errors fall under. Is the > > latter option to sum up the values under one umbrella preferred? >=20 > As Thomas explained, I think we should avoid adding fields in struct > rte_eth_stats. This structure already contains statistics that are specif= ic to > some hardware drivers. We should try to go in the opposite direction and > remove all the fields that do not apply to all nics (physical or virtual)= . For me, it > would be only something like (rx, tx, rx_bytes, tx_bytes, rx_errors, tx_e= rrors). >=20 > The xstats framework allows you to add any driver or hardware specific st= ats > and I think it's the proper place to add them for ixgbe. >=20 Alright that sounds good. > By the way, something could be modified in xstats framework. Today, the > behavior is: > - if ethdev->dev_ops->xstats_get !=3D NULL, call it > - else, dump the generic stats in xstats format As it happens I did do this in: http://dpdk.org/dev/patchwork/patch/5324/=20 but I will reverse the order as what happens now is xstats from the driver are displayed first and will look at always dumping generic stats in xstats= format as you mention below. >=20 > A better behavior could be: > - Always dump the generic stats in xstats format > - and if thdev->dev_ops->xstats_get !=3D NULL, add driver-specific stats >=20 > This would avoid to duplicate code that dumps generic stats in all driver= s. >=20 > So, to summarize what I think should be done for stats/xstats: > 1. modify xstats behavior as described above 2. add the xstats dev_ops in > ixgbe, it will dump the new stats >=20 > Bonus: > 3. identify all the specific stats that could be removed from > rte_eth_stats and start to mark them as deprecated. >=20 Will do. >=20 > Regards, > Olivier Thanks Maryam