Re: [PATCH] ethdev: expose link status and speed using xstats

From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH] ethdev: expose link status and speed using xstats
Date: Wed, 20 Jan 2016 16:13:34 +0100	[thread overview]
Message-ID: <3240763.FVKPEEMrPZ@xps13> (raw)
In-Reply-To: <CAMFWN9kjhU7njoOEht8beZbxBWBshh44tsFvnD0HnbkVeh2MqQ@mail.gmail.com>

2016-01-20 10:03, Kyle Larose:
> Hi Harry,
> 
> On Wed, Jan 20, 2016 at 9:45 AM, Van Haaren, Harry
> <harry.van.haaren@intel.com> wrote:
> > Hi Kyle,
> 
> >
> > In theory we could create a new API for this, but I think the current xstats API is a good fit for exposing this info, so why create extra APIs? As a client of the DPDK API, I would prefer more statistics in a single API than have to research and implement two or more APIs to retrieve the information to monitor.
> >
> 
> You create new APIs for many reasons: modularity, simplicitly within
> the API, consistency, etc. My main concern with this proposed change
> relates to consistency. Previously, each stat had similar semantics.
> It was a number, representing the amount of times something had
> occurred on a chip. This fact allows you to perform operations like
> addition, subtraction/etc and expect that the result will be
> meaningful for every value in the array.
> 
> For example, suppose I wrote a tool to give the "rate" for each of the
> stats. We could sample these stats periodically, then output the
> difference between the two samples divided by the time between samples
> for each stat. A naive implementation, but quite simple.
> 
> However, if we start adding values like link speed and state, which
> are not really numerical, or not monotonic, you can no longer apply
> the same mathematical operations on them and expect them to be
> meaningful. For example, suppose a link went down. The "rate" for that
> stat would be -1. Does that really make sense? Anyone using this API
> would need to explicitly filter out the non-stats, or risk nonsensical
> output.
> 
> Let's also consider how to interpret the value. When I look at a stat,
> there's usually one of two meanings: it's either a number of packets,
> or it's a number of bytes. We're now adding exceptions to that rule.
> Link state is a boolean. Link speed is a value in mbps. Duplex is
> pretty much an enum.
> 
> We already have the rte_eth_link_get function. Why not let users
> continue to use that? It's well defined, it is simple, and it is
> consistent.

+1

Please also consider this work in progress
about link speed information:
http://dpdk.org/dev/patchwork/patch/7995/