[dpdk-dev] Future Direction for rte_eth_stats_get()

David Harton (dharton) dharton at cisco.com
Mon Feb 1 17:47:56 CET 2016


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 conventional based definition (note, I believe Matthew Hall made an interesting suggestion to follow a MIB based definition elsewhere).  However, if modifying 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 scalability reasons but still maintain the "ABI flexibility" for which the xstats model was geared towards.

Thanks,
Dave


> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of David Harton (dharton)
> Sent: Friday, January 22, 2016 2:26 PM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>; Thomas Monjalon <thomas.monjalon at 6wind.com>
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] Future Direction for rte_eth_stats_get()
> 
> > > > 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 statistic
> per read.
> 
> 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?
> 
> >
> >
> > > 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.
> 
> 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
> caller is interested in it or not.
> 
> >
> >
> > >  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?
> 
> 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:
> 
> 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;
> }
> 
> 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);
> 
> 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.
> 
> >
> > > 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 )}
> 
> 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.
> 
> >
> >
> > > 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.
> 
> Wasn't trying to optimize image size as much as saying it's a side benefit
> due to string consolidation.
> 
> >
> >
> > > > 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.
> 
> 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*
> 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 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
> application 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
> 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.
> 
> 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.
> 
> Regards,
> Dave



More information about the dev mailing list