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 4C3E9567E for ; Wed, 27 May 2015 00:30:10 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 26 May 2015 15:30:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,502,1427785200"; d="scan'208";a="715986549" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga001.fm.intel.com with ESMTP; 26 May 2015 15:30:07 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.59]) by IRSMSX152.ger.corp.intel.com ([169.254.6.148]) with mapi id 14.03.0224.002; Tue, 26 May 2015 23:30:07 +0100 From: "Dumitrescu, Cristian" To: Stephen Hemminger Thread-Topic: [dpdk-dev] [PATCH v3] pipeline: add statistics for librte_pipeline ports and tables Thread-Index: AQHQl8RXd7p4Z3r9hEW+IUJr9z/taZ2Ovb8Q///9JACAABER0A== Date: Tue, 26 May 2015 22:30:06 +0000 Message-ID: <3EB4FA525960D640B5BDFFD6A3D891263236CAC8@IRSMSX108.ger.corp.intel.com> References: <1432647558-9062-1-git-send-email-maciejx.t.gajdzica@intel.com> <20150526075719.24bed9cf@urahara> <3EB4FA525960D640B5BDFFD6A3D891263236CA70@IRSMSX108.ger.corp.intel.com> <20150526144742.04debea8@urahara> In-Reply-To: <20150526144742.04debea8@urahara> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] 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 v3] pipeline: add statistics for librte_pipeline ports and tables 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: Tue, 26 May 2015 22:30:11 -0000 > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Tuesday, May 26, 2015 10:48 PM > To: Dumitrescu, Cristian > Cc: Gajdzica, MaciejX T; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] pipeline: add statistics for librte_pi= peline > ports and tables >=20 > On Tue, 26 May 2015 21:35:22 +0000 > "Dumitrescu, Cristian" wrote: >=20 > > > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen > > > Hemminger > > > Sent: Tuesday, May 26, 2015 3:57 PM > > > To: Gajdzica, MaciejX T > > > Cc: dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v3] pipeline: add statistics for > librte_pipeline > > > ports and tables > > > > > > On Tue, 26 May 2015 15:39:18 +0200 > > > Maciej Gajdzica wrote: > > > > > > > +#if RTE_LOG_LEVEL =3D=3D RTE_LOG_DEBUG > > > > +#define RTE_PIPELINE_STATS_ADD(counter, val) \ > > > > + ({ (counter) +=3D (val); }) > > > > + > > > > +#define RTE_PIPELINE_STATS_ADD_M(counter, mask) \ > > > > + ({ (counter) +=3D __builtin_popcountll(mask); }) > > > > +#else > > > > +#define RTE_PIPELINE_STATS_ADD(counter, val) > > > > +#define RTE_PIPELINE_STATS_ADD_M(counter, mask) > > > > +#endif > > > > > > This is worse idea than the previous one. > > > I want statistics done on a per lcore basis, and always available > > > because real users on production system want statistics (but they > > > don't want log spam). > > > > Stephen, > > > > Thomas and myself converged towards this solution, Thomas asked if > anybody objects, you did not (http://www.dpdk.org/ml/archives/dev/2015- > May/018099.html) . You say this idea is bad, but what exactly is your > objection? Do you have an alternative proposal? >=20 > Yes. Always keep statistics. >=20 > We use functions like this internally. >=20 > struct xxx_stats { > uint64_t mib[XXX_MIB_MAX]; > }; > extern struct xxx_stats xxx_stats[RTE_MAX_LCORE]; >=20 > #define XXXSTAT_INC(type) xxxstats[rte_lcore_id()].mibs[type]++ >=20 >=20 This can only be applied if stats are always enabled. I know this is your p= referred choice, but other people have the requirement to be able to have t= he stats collection configurable, and we should also accommodate the needs = of those people. Your code snippet does not provide a solution for this pro= blem. > > You already mentioned in the previous thread you would like to have per > lcore statistics. I was kindly asking you to describe your idea, but you = did not > do this yet (http://www.dpdk.org/ml/archives/dev/2015-May/017956.html ). > Can you please describe it? Each port instance has its own statistics cou= nters. > Each lcore can run one or more pipeline instances, therefore each lcore > typically runs several port instances of the same or different type (each= port > instance with its own statistics), so how is "per lcore stats" requiremen= t > applicable here? >=20 > I thought you were familiar with technique of having per-cpu structures a= nd > variables > widely used in Linux kernel to prevent cache thrashing. Although you call= it > false sharing, > it isn't false., it happens when same counter ping-pongs between multiple > threads for > no added benefit. The "per lcore stats" is applicable to stats structures that are accessed b= y more than one lcore. In our case, each port instance is handled by a sing= le lcore, so it is never the case that two lcores would access the stats of= the same port instance. So, we can safely say that port stats are "per lco= re" already. >=20 >=20 > > You also reiterate that you would like to have the stats always enabled= . You > can definitely do this, it is one of the available choices, but why not a= lso > accommodate the users that want to pick the opposite choice? Why force > apps to spend cycles on stats if the app either does not want these count= ers > (library counters not relevant for that app, maybe the app is only intere= sted > in maintaining some other stats that it implements itself) or do not want > them anymore (maybe they only needed them during debug phase), etc? > Jay asked this question, and I did my best in my reply to describe our > motivation (http://www.dpdk.org/ml/archives/dev/2015-May/017992.html). > Maybe you missed that post, it would be good to get your reply on this on= e > too. >=20 > I want to see DPDK get out of the config madness. > This is real code, not an Intel benchmark special. You seem to make this assumption that our real reason for having the stats = collection configurable is to get good benchmark results by disabling the s= tats. This cannot be a real reason, as a complete benchmark reports always = specifies if stats were enabled or not. Easy to check. By stating this, you implicitly acknowledge that stats collection consumes = CPU cycles that might be significant for the overall app performance, right= ? So why waste CPU cycles in the case when the app does not need those coun= ters? The message I am trying to get across to you is that there are multiple _re= al_ reasons why an app would not want to collect the stats counters of libr= ary X, considering that stats counters do not come for free, but at the exp= ense of CPU cycles: 1. For app A, the stats counters of library X might be totally irrelevant. = Maybe the app does not care how many packets got through/got dropped by por= t Y of pipeline instance Z. Maybe the app has some other stats that it care= s about, e.g. number of online users, mean connection time, etc that are co= llected by the app, not the library, therefore counters collected by librar= y X are meaningless and they should not be collected; 2. For app B, the library stats are only relevant during debugging phase; o= nce debugging completed, stats are no longer required. Why do you want to have the library forcing the app to collect some counter= s that the app might not want to collect? Let's not force people to do what= we think is right, our job here is to provide options for them to pick.