From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 7ED16558D for ; Thu, 28 Apr 2016 16:56:05 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP; 28 Apr 2016 07:56:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,547,1455004800"; d="scan'208";a="93664540" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga004.fm.intel.com with ESMTP; 28 Apr 2016 07:56:04 -0700 Received: from irsmsx106.ger.corp.intel.com ([169.254.8.172]) by IRSMSX152.ger.corp.intel.com ([169.254.6.15]) with mapi id 14.03.0248.002; Thu, 28 Apr 2016 15:56:02 +0100 From: "Tahhan, Maryam" To: "David Harton (dharton)" , "Horton, Remy" , "dev@dpdk.org" CC: "Mcnamara, John" , "Van Haaren, Harry" Thread-Topic: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from xstats Thread-Index: AQHRlyVcBwIlRj9g8UuniMP9sTHe55+S/V6AgAyNCwA= Date: Thu, 28 Apr 2016 14:56:02 +0000 Message-ID: <1A27633A6DA49C4A92FCD5D4312DBF536B1814BF@IRSMSX106.ger.corp.intel.com> References: <1460731462-21229-1-git-send-email-remy.horton@intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjVjYWEzMzEtYTA4OC00NTllLWE0OTYtMTU5YWNkN2UzYmU2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6InZ1ZU1CeWdQSWRtQmlQSFwveWZEQU9UeTZlY1wvc1hiZFJqN3ZSZ090a2Nhcz0ifQ== x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from xstats 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: Thu, 28 Apr 2016 14:56:06 -0000 > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Harton > (dharton) > Sent: Wednesday, April 20, 2016 5:04 PM > To: Horton, Remy ; dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations > from xstats >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Remy Horton > > Sent: Friday, April 15, 2016 10:44 AM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from > > xstats > > > > The current extended ethernet statistics fetching involve doing > > several string operations, which causes performance issues if there > > are lots of statistics and/or network interfaces. This RFC patchset > > changes the API for xstats to use integer identifiers instead of > > strings and implements this new API for the ixgbe driver. Others > drivers to follow. > > > > -- > > > > Since this will involve API & ABI breakage as previously advertised, > > there are several design assumptions that need consideration: > > > > *) id-name & id-value pairs for both lookup and query Permits > > out-of-order and non-contigious returning of names/ids/values, even > > though expected implmentations would in practice return items in > > sorted order by id. Is this sufficent/desirable future proofing? Idea > > is to allow possibility of drivers returning partial statistics. >=20 > I believe forcing drivers to match to a common id-space will become > burdensome. If the stats id-space isn't common then matching strings is > probably just as sufficient as long as drivers don't add/remove stats ad > hoc between the time the device is initialized and removed. I'm not aware of drivers adding/removing the stats ad hoc? The idea is to h= ave a common-id space otherwise it will be a free for all and we won't have= alignment across the drivers. I don't see it being any more burdensome tha= n having a common register naming across the board which is what is there t= oday. The advantage being that you don't have to pull the strings every tim= e. >=20 > > > > *) Bulk name-id mapping lookup only > > At the moment individual lookup is not supported, as this would impose > > extra overheads on drivers. The assumption is that any end user would > > fetch all this data once on startup and then cache the mappings. >=20 > I'm not sure I see the value of looking up a single stat from a user > perspective. I can see where the drivers might say that some stats are > less disruptive/etc but the user doesn't have that knowledge and > wouldn't know how to take advantage. Usually all stats are grabbed > multiple times and the changes noted during debug sessions. >=20 I believe Remy's change doesn't suggest/support individual lookup. It is ju= st a statement that we don't want to burden drivers with individual stats l= ookups. > > > > *) Replacement or additional API > > This patch replaces the current xstats API, but there is no inherant > > reason beyond maintainability why this funtionality could not be in > > addition rather than a replacement. What is consensus on this? >=20 > I came to the conclusion that replacing the existing API isn't necessary > but rather extending it so backwards compatibility could be maintained > during the previous discussions on this topic. However, if we want to go > forward with cleaning up in order to reduce the support drivers provide > I'm all for it. >=20 > I still believe the API we develop should follow an "ethtool stats like" > format as suggested earlier this year: >=20 > extern int rte_eth_xstats_names_get(uint8_t port_id, > struct rte_eth_xstats_name *names, unsigned n); extern int > rte_eth_xstats_values_get(uint8_t port_id, > uint64_t *values, unsigned n); >=20 > Again, these could be provided alongside the existing API or replace it. I'm struggling a bit here. This is really what Remy has posted http://dpdk.= org/dev/patchwork/patch/12094/ or am I missing something obvious? >=20 > I also like the idea you provided of a separate API to obtain the xstats > count rather than deriving the count by calling one of the above > functions with "dummy" values. +1=20 >=20 > Again, I can provide the patches for the changes I've made that align > with this proposed API. I just never got any feedback on it when > requested previously. I believe time is not in our favour on this front. If you have patches can = you post them, otherwise can you please review the patchset that Remy has p= osted? Thanks in advance. BR Maryam