DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [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/

  reply	other threads:[~2016-01-20 15:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-20 14:28 Harry van Haaren
2016-01-20 14:35 ` Kyle Larose
2016-01-20 14:45   ` Van Haaren, Harry
2016-01-20 15:03     ` Kyle Larose
2016-01-20 15:13       ` Thomas Monjalon [this message]
2016-01-20 15:58         ` Van Haaren, Harry
2016-01-20 18:36         ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3240763.FVKPEEMrPZ@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=harry.van.haaren@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).