From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from alln-iport-5.cisco.com (alln-iport-5.cisco.com [173.37.142.92]) by dpdk.org (Postfix) with ESMTP id 48791558A for ; Thu, 28 Apr 2016 17:58:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=6184; q=dns/txt; s=iport; t=1461859119; x=1463068719; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=5YjtRnr+yO9dFhvlrc5fc3vkrDolFQOL28r3NJdD4pE=; b=G1jKZJQySFQNZ+Nu2PptF+U+wnJ6NSAY0k5nDz8FmJgDN+dM2URXq4R5 8RyaSLzKc+XfH6h4mCO7JGh6qtHyQK2Lm24mpZCRPo0ofBCM5vsV0Lnsw DUHhg8B1ZcVAsflALW3i06d9wvaKVBqwkMq7dgv8HTfIWO1W32SNOn0no w=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0BcAgDoMSJX/51dJa1egzhTfQa3YYIPA?= =?us-ascii?q?Q2BdiKFbQKBJzgUAQEBAQEBAWUnhEEBAQEEOjIKAwwEAgEIEQQBAR8JBzIUCQg?= =?us-ascii?q?CBAENBQiIIg7DVwEBAQEBAQEBAQEBAQEBAQEBAQEBARWGIYRLihMFjVSKPAGFe?= =?us-ascii?q?4J3hR2Bbk6Df4hdjy8BHgEBQoIFG4FLbAGGaH8BAQE?= X-IronPort-AV: E=Sophos;i="5.24,547,1454976000"; d="scan'208";a="265474059" Received: from rcdn-core-6.cisco.com ([173.37.93.157]) by alln-iport-5.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Apr 2016 15:58:38 +0000 Received: from XCH-ALN-019.cisco.com (xch-aln-019.cisco.com [173.36.7.29]) by rcdn-core-6.cisco.com (8.14.5/8.14.5) with ESMTP id u3SFwcuH028094 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Thu, 28 Apr 2016 15:58:38 GMT Received: from xch-rcd-016.cisco.com (173.37.102.26) by XCH-ALN-019.cisco.com (173.36.7.29) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Thu, 28 Apr 2016 10:58:37 -0500 Received: from xch-rcd-016.cisco.com ([173.37.102.26]) by XCH-RCD-016.cisco.com ([173.37.102.26]) with mapi id 15.00.1104.009; Thu, 28 Apr 2016 10:58:37 -0500 From: "David Harton (dharton)" To: "Tahhan, Maryam" , "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: AQHRlyVTq5+EWbgW3kKb6tsWJ2kqsZ+TB72QgAzZ8wD//7Qu4A== Date: Thu, 28 Apr 2016 15:58:37 +0000 Message-ID: References: <1460731462-21229-1-git-send-email-remy.horton@intel.com> <1A27633A6DA49C4A92FCD5D4312DBF536B1814BF@IRSMSX106.ger.corp.intel.com> In-Reply-To: <1A27633A6DA49C4A92FCD5D4312DBF536B1814BF@IRSMSX106.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.82.253.116] 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 15:58:39 -0000 > -----Original Message----- > From: Tahhan, Maryam [mailto:maryam.tahhan@intel.com] > Sent: Thursday, April 28, 2016 10:56 AM > To: David Harton (dharton) ; Horton, Remy > ; dev@dpdk.org > Cc: Mcnamara, John ; Van Haaren, Harry > > Subject: RE: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from > xstats >=20 > > 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 > > > > > -----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. > > > > 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. >=20 > I'm not aware of drivers adding/removing the stats ad hoc? The idea is to > have 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 than having a common register naming across the board which is > what is there today. The advantage being that you don't have to pull the > strings every time. >=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. > > > > 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 > just a statement that we don't want to burden drivers with individual > stats lookups. >=20 > > > > > > *) 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? > > > > 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. > > > > I still believe the API we develop should follow an "ethtool stats like= " > > format as suggested earlier this year: > > > > 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); > > > > Again, these could be provided alongside the existing API or replace it= . >=20 > 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? Maybe I misread the patch series or missed one but I don't see where=20 stats can be obtained without copying strings? This is the real issue I=20 raised originally. =20 Having the ability to get the names without stats is useful, but, the real gain would be obtaining the stats without the names. >=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. >=20 > +1 >=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. >=20 > I believe time is not in our favour on this front. If you have patches ca= n > you post them, otherwise can you please review the patchset that Remy has > posted? I'm working on it but I have some process I'm navigating. I'm hopeful=20 I'll have the green light within a week if not sooner. I apologize... I'm pushing as hard as I can. If you need to proceed go ahead I=20 completely understand. =20 All I can say is that I have implemented the API above and converted all=20 drivers that supported xstats in v2.2. Any new drivers that have added=20 xstats support since would need to be added. I did not add "get the count" because it wasn't provided in the current API and instead followed the convention but I do believe overtly getting the count it is the better approach. Thanks, Dave