All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kyle Larose <eomereadig@gmail.com>
To: "Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH] ethdev: expose link status and speed using xstats
Date: Wed, 20 Jan 2016 10:03:36 -0500	[thread overview]
Message-ID: <CAMFWN9kjhU7njoOEht8beZbxBWBshh44tsFvnD0HnbkVeh2MqQ@mail.gmail.com> (raw)
In-Reply-To: <E923DB57A917B54B9182A2E928D00FA61285F3A6@IRSMSX102.ger.corp.intel.com>

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.

Thanks,

Kyle

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

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-20 14:28 [PATCH] ethdev: expose link status and speed using xstats 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 [this message]
2016-01-20 15:13       ` Thomas Monjalon
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=CAMFWN9kjhU7njoOEht8beZbxBWBshh44tsFvnD0HnbkVeh2MqQ@mail.gmail.com \
    --to=eomereadig@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.