From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from alln-iport-8.cisco.com (alln-iport-8.cisco.com [173.37.142.95]) by dpdk.org (Postfix) with ESMTP id A77E395C8 for ; Mon, 1 Feb 2016 17:47:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=7333; q=dns/txt; s=iport; t=1454345278; x=1455554878; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=4uoBBUTJLD+uKToMpjvgvm1eYY2un1YFL5hE9/E6vY4=; b=YYbWpf/H832U4wY2uI5zCZ2bSvQRzEvN25Fh3NSoMqJrphaV61vSdP9D 6QfVW2Vrogxm8gGZ7hmNkJVLBmQht0Jv6weDdwPKZlR4KlJL1FsEi42A6 03zFi2R94mdxtqZh7vArLegW/U2e4MqvTKi8YQAf2bKUt8TBmVeE4nQXe k=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0CTDABki69W/5xdJa1UCoM6Um0GiFKwI?= =?us-ascii?q?YE8AQ2BZCKFbQKBOzgUAQEBAQEBAYEKhEEBAQEEJxM/DAQCAQgRBAEBHwkHMhQ?= =?us-ascii?q?JCAIEAQ0FCIgTDr0JAQEBAQEBAQEBAQEBAQEBAQEBAQEBFYYPhDeCVIE0hGQFj?= =?us-ascii?q?SWJSgGFRoJshRGBYkqHHYUuRI15AR4BAUKCARqBUWoBiAB8AQEB?= X-IronPort-AV: E=Sophos;i="5.22,381,1449532800"; d="scan'208";a="233557016" Received: from rcdn-core-5.cisco.com ([173.37.93.156]) by alln-iport-8.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Feb 2016 16:47:57 +0000 Received: from XCH-RCD-017.cisco.com (xch-rcd-017.cisco.com [173.37.102.27]) by rcdn-core-5.cisco.com (8.14.5/8.14.5) with ESMTP id u11GlvPw006585 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Mon, 1 Feb 2016 16:47:57 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; Mon, 1 Feb 2016 10:47:56 -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; Mon, 1 Feb 2016 10:47:56 -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//EX76A= Date: Mon, 1 Feb 2016 16:47:56 +0000 Message-ID: <3b3f956b6f7c490181b3db81b2c8d99a@XCH-RCD-016.cisco.com> 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.215.48] 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: Mon, 01 Feb 2016 16:47:59 -0000 Hi folks, I didn't see any follow up to this response. My original concern was rte_eth_stats_get() moving away from a more convent= ional based definition (note, I believe Matthew Hall made an interesting su= ggestion to follow a MIB based definition elsewhere). However, if modifyin= g that API is not desired then I'd really like to have some feedback about = extending the current xstats model. Again, it is desired not to have to copy and/or parse strings for scalabili= ty reasons but still maintain the "ABI flexibility" for which the xstats mo= del was geared towards. Thanks, Dave > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Harton (dharto= n) > Sent: Friday, January 22, 2016 2:26 PM > To: Van Haaren, Harry ; Thomas Monjalon > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] Future Direction for rte_eth_stats_get() >=20 > > > > Do you see a fundamental problem with parsing the strings to > > > > retrieve values? > > > > > > I think parsing strings is expensive CPU wise > > > > 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 statisti= c > per read. >=20 > How is this order guaranteed through the API? The stat is currently > identified by the string not by order returned. What if a driver returns > more/less stats based on config that changes after boot? >=20 > > > > > > > That's why I was wondering if a binary id could be generated instead. > > > > 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 is necessary, but I'm not sure about that. > > > > > > > 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. > > > > Apart from a once-off scan at startup, xstats achieves this. >=20 > 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 th= e > caller is interested in it or not. >=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. > > > > 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. > > > > How and where will this mapping happen? Perhaps would you expand a > > little on how you see this working? >=20 > 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 think the following communicates the basic idea: >=20 > enum rte_eth_stat_e { > /* accurate desc #1 */ > RTE_ETH_STAT_1, > /* accurate desc #2 */ > RTE_ETH_STAT_2, > ... > } > struct rte_eth_id_stat { > rte_eth_stat_e id; > uin64_t value; > } >=20 > 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_stat *id_stats); const char* > rte_eth_id_stat_str(rte_eth_stat_e id); >=20 > This allows a driver to return whatever stats that it supports in a > consistent manner and also in a performance friendly way. In fact, the > driver side would be identical to what they have today but instead of > having arrays with "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. > > > > 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 > > > > It's a pretty simple {"string stat name" -> offsetof( stuct hw, > > register_var_name )} >=20 > It's not how they add the strings but rather the format of the string > itself that is error prone. > IOTW, the "string stat name" is prone to not being formatted correctly or > consistently. >=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). > > > > Sounds like optimizing the wrong thing to me. >=20 > Wasn't trying to optimize image size as much as saying it's a side benefi= t > due to string consolidation. >=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 > > drivers is possible. > > > > We're all looking for the same thing - just different approaches > > that's all :) > > > > > > RE: Thomas asking about performance numbers: > > I can scrape together some raw tsc data on Monday and post to list, > > and we can discuss it more then. >=20 > I can do the same if desired. But, just to make sure we are discussing > the same issue: >=20 > 1) call rte_eth_xtats_get() > This will result in many string copies and depending on the driver *many* > copies I don't want or care about. > 2) "tokenize"/parse/hash the string returned to identify what the stat > actually is I'm guessing you are stating that this step could be mitigate= d > 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 > application uses > 4) Repeat steps 1-3 for every interface being serviced every 5 or 10 secs >=20 > Contrast that to my suggestion which has no string copies and a compile > time mapping between "stat_id" and "app stat" can be created for step 2. > I think the performance differences are obvious even without generating > cycle times. >=20 > I know that dpdk is largely focused on data plane performance and rightly > so, 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. >=20 > Regards, > Dave