From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rcdn-iport-8.cisco.com (rcdn-iport-8.cisco.com [173.37.86.79]) by dpdk.org (Postfix) with ESMTP id 64BF9919B for ; Fri, 22 Jan 2016 20:26:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=6140; q=dns/txt; s=iport; t=1453490780; x=1454700380; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=ZSFeCmLdQ52MClr4CqMLBgvlK9AhQQIhArhsIAj8anQ=; b=E3KPRKyvq9w7lmN1p7y1a0aJ0TSidUSlrwMXtO2x5wrb7keXALJylOas DMQH8GVvqC7IMSQjbhWHl65ZMYbkZIpIYPj9FgPgOXHg0tX2lfZczfcjs Adn/Vgxr/TkCBHJ9AJx+Zg+FYoyqU1qky+ijizABk58tWMsE9G5i7Wj8+ 0=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0BoAgAsgaJW/49dJa1UCoM6Um0BBYQlh?= =?us-ascii?q?CywS4E8AQ2BYiKFbQKBQjgUAQEBAQEBAYEKhEEBAQEDAScTPxACAQg2EDIlAgQ?= =?us-ascii?q?BDQ2ICwgOvl8BAQEBAQEBAQEBAQEBAQEBAQEBF4YyhG2CZ4E6hGUFjSqJTAGFR?= =?us-ascii?q?YJxhRiBZUqHIYUwRI16AR4BAUKBfRmBUGqGJ3wBAQE?= X-IronPort-AV: E=Sophos;i="5.22,332,1449532800"; d="scan'208";a="64304995" Received: from rcdn-core-7.cisco.com ([173.37.93.143]) by rcdn-iport-8.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Jan 2016 19:26:19 +0000 Received: from XCH-RCD-017.cisco.com (xch-rcd-017.cisco.com [173.37.102.27]) by rcdn-core-7.cisco.com (8.14.5/8.14.5) with ESMTP id u0MJQJUP005568 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Fri, 22 Jan 2016 19:26:19 GMT Received: from xch-rcd-016.cisco.com (173.37.102.26) by XCH-RCD-017.cisco.com (173.37.102.27) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Fri, 22 Jan 2016 13:26:18 -0600 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; Fri, 22 Jan 2016 13:26:18 -0600 From: "David Harton (dharton)" To: "Van Haaren, Harry" , Thomas Monjalon Thread-Topic: [dpdk-dev] Future Direction for rte_eth_stats_get() Thread-Index: AdFToifrMKxGpVsMS/qKv8msWpPa2wABEBgwAFcuL6AAA/u1IAACHmaQAAF2QyAADTQZAAABMeyAAAvZZbD//7dUAIAAYwVg Date: Fri, 22 Jan 2016 19:26:18 +0000 Message-ID: References: <1A27633A6DA49C4A92FCD5D4312DBF536B09F7A4@IRSMSX106.ger.corp.intel.com> <1670837.tJHuUIkoht@xps13> <78791945dbf442b291adb72df8ec09f3@XCH-RCD-016.cisco.com> In-Reply-To: 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.234.158] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] Future Direction for rte_eth_stats_get() 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: Fri, 22 Jan 2016 19:26:20 -0000 > > > Do you see a fundamental problem with parsing the strings to > > > retrieve values? > > > > I think parsing strings is expensive CPU wise >=20 > Parsing the strings can be done once at startup, and the index of the > statistics in the array cached. When actually reading statistics, access > of the array of statistics at the previously cached index will return the > same statistic. It is not needed to do strcmp() per statistic per read. How is this order guaranteed through the API? The stat is currently identi= fied by the string not by order returned. What if a driver returns more/le= ss stats based on config that changes after boot? >=20 >=20 > > That's why I was wondering if a binary id could be generated instead. >=20 > I've worked with such a system before, it dynamically registered string- > >int mappings at runtime, and allowed "reverse" mapping them too. Its > workable, and I'm not opposed to it if the conclusion is that it really i= s > necessary, but I'm not sure about that. >=20 >=20 > > The API > > would be very similar to the current xstats API except it would pass > > id/value pairs instead of string/value pairs. This avoids string > > comparisons to figure out while still allowing flexibility between > drivers. >=20 > Apart from a once-off scan at startup, xstats achieves this. Partially true. You may not need to perform strcmp() per read, although I = don't believe the defined API guarantees that this would be safe (see above= ); but, you still have to perform a strcpy() of each stat whether the calle= r is interested in it or not. >=20 >=20 > > It would also 100% guarantee that any strings returned by "const > > char* rte_eth_xstats_str_get(uint32_t stat_id)" are consistent across > > devices. >=20 > Yes - that is a nice feature that xstats (in its current form) doesn't > have. > But what price is there to pay for this? > We need to map an ID for each stat that exists. >=20 > How and where will this mapping happen? Perhaps would you expand a little > on how you see this working? I wasn't thinking dynamic registration as that might be more than necessary= . I can code up a detailed snippet that shares the idea if wanted but I th= ink the following communicates the basic idea: enum rte_eth_stat_e { /* accurate desc #1 */ RTE_ETH_STAT_1,=20 /* accurate desc #2 */ RTE_ETH_STAT_2, ... } struct rte_eth_id_stat { rte_eth_stat_e id; uin64_t value; } int rte_eth_id_stats_num(uint8_t port_id, uint32_t *num_stats); /* returns < 0 on error or the number of stats that could have been read (i= .e. if userd */ int rte_eth_id_stats_get(uint8_t port_id, uint32_t num_stats, rte_eth_id_st= at *id_stats); const char* rte_eth_id_stat_str(rte_eth_stat_e id); This allows a driver to return whatever stats that it supports in a consist= ent manner and also in a performance friendly way. In fact, the driver sid= e would be identical to what they have today but instead of having arrays w= ith "string stat name" they will have the rte_eth_stat_e. >=20 > > I also think there is less chance for error if drivers assign their > > stats to ID versus constructing a string. >=20 > Have a look in how the mapping is done in the xstats implementation for > ixgbe for example: > http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c#n540 >=20 > It's a pretty simple {"string stat name" -> offsetof( stuct hw, > register_var_name )} It's not how they add the strings but rather the format of the string itsel= f that is error prone. =20 IOTW, the "string stat name" is prone to not being formatted correctly or c= onsistently. >=20 >=20 > > I haven't checked my application, but I wonder if the binary size > > would actually be smaller if all the stats strings were centrally > > located versus distributed across binaries (highly dependent on the > linker and optimization level). >=20 > Sounds like optimizing the wrong thing to me. Wasn't trying to optimize image size as much as saying it's a side benefit = due to string consolidation. >=20 >=20 > > > If you like I could code up a minimal sample-app that only pulls > > > statistics, and you can show "interest" in various statistics for > > > printing / monitoring? > > > > Appreciate the offer. I actually understand what's is available. > > And, BTW, I apologize for being late to the game (looks like this was > > discussed last summer) but I'm just now getting looped in and > > following the mailer but I'm just wondering if something that is > performance friendly for the user/application and flexible for the driver= s > is possible. >=20 > We're all looking for the same thing - just different approaches that's > all :) >=20 >=20 > RE: Thomas asking about performance numbers: > I can scrape together some raw tsc data on Monday and post to list, and w= e > can discuss it more then. I can do the same if desired. But, just to make sure we are discussing the= same issue: 1) call rte_eth_xtats_get() This will result in many string copies and depending on the driver *many* c= opies I don't want or care about. 2) "tokenize"/parse/hash the string returned to identify what the stat actu= ally is I'm guessing you are stating that this step could be mitigated at startup. = But, again, I don't think the API provides a guarantee which usually leads= to bugs over time. 3) Copy the value of the stat into the driver agnostic container the applic= ation uses 4) Repeat steps 1-3 for every interface being serviced every 5 or 10 secs Contrast that to my suggestion which has no string copies and a compile tim= e mapping between "stat_id" and "app stat" can be created for step 2. I th= ink the performance differences are obvious even without generating cycle t= imes. I know that dpdk is largely focused on data plane performance and rightly s= o, but, I'm hoping we can keep the control plane in mind as well especially= in the areas around stats or things that happen periodically per port. Regards, Dave