From: Remy Horton <remy.horton@intel.com>
To: "David Harton (dharton)" <dharton@cisco.com>,
"Tahhan, Maryam" <maryam.tahhan@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "Mcnamara, John" <john.mcnamara@intel.com>,
"Van Haaren, Harry" <harry.van.haaren@intel.com>
Subject: Re: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from xstats
Date: Fri, 29 Apr 2016 11:21:46 +0100 [thread overview]
Message-ID: <572335BA.7000202@intel.com> (raw)
In-Reply-To: <a642c8fd7b3f451fb0f9df1d8436b3e7@XCH-RCD-016.cisco.com>
Morning,
On 28/04/2016 16:58, David Harton (dharton) wrote:
[..]
>>>> *) id-name & id-value pairs for both lookup and query Permits
>>>> out-of-order and non-contigious returning of names/ids/values, even
>>>> though expected implmentations would in practice return items in
>>>> sorted order by id. Is this sufficent/desirable future proofing?
>>>> Idea is to allow possibility of drivers returning partial statistics.
>>>
>>> I believe forcing drivers to match to a common id-space will become
>>> burdensome. If the stats id-space isn't common then matching strings
>>> is probably just as sufficient as long as drivers don't add/remove
>>> stats ad hoc between the time the device is initialized and removed.
>>
>> I'm not aware of drivers adding/removing the stats ad hoc? The idea is to
>> have a common-id space otherwise it will be a free for all and we won't
>> have alignment across the drivers. I don't see it being any more
>> burdensome than having a common register naming across the board which is
>> what is there today. The advantage being that you don't have to pull the
>> strings every time.
Returning both stats (id,value) and names (id,string) as pairs would
allow (amoung other things) common ids but actually having common ids is
not an intended goal of mine. I think the whole idea of common ids was
implicity vetoed when the idea of ENUMs was thrown out.
I opted for both stats and lookup to be provided as pairs because when
it comes to APIs, I have a slight preference for having that bit of
extra generality. Not sure its really worth it, so might change stats to
just use id-indexed integer arrays (ethtool-like basically) rather than
a typedef that includes the numeric id.
>>>> *) Bulk name-id mapping lookup only
[..]
>>> I'm not sure I see the value of looking up a single stat from a user
>>> perspective. I can see where the drivers might say that some stats
>>> are less disruptive/etc but the user doesn't have that knowledge and
>>> wouldn't know how to take advantage. Usually all stats are grabbed
>>> multiple times and the changes noted during debug sessions.
>>>
>>
>> I believe Remy's change doesn't suggest/support individual lookup. It is
>> just a statement that we don't want to burden drivers with individual
>> stats lookups.
Correct.
>>>> *) Replacement or additional API
[..]
>>> However, if
>>> we want to go forward with cleaning up in order to reduce the support
>>> drivers provide I'm all for it.
Whether we want to do such a cleanup is my open question.
> Maybe I misread the patch series or missed one but I don't see where
> stats can be obtained without copying strings? This is the real issue I
> raised originally.
See http://dpdk.org/dev/patchwork/patch/12096/ where xstats[].key is
used to lookup string from ptr_names[].name - I didn't delete the name
field from rte_eth_xstats because doing so would cause a compile error
with the drivers I've not yet converted.
> I did not add "get the count" because it wasn't provided in the current API
> and instead followed the convention but I do believe overtly getting the
> count it is the better approach.
I didn't either for the same reason, but if the API is going to be
broken, I think it should be added.
..Remy
next prev parent reply other threads:[~2016-04-29 10:22 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-15 14:44 Remy Horton
2016-04-15 14:44 ` [dpdk-dev] [RFC PATCH v1 1/3] rte: change xstats to use integer keys Remy Horton
2016-04-29 13:17 ` David Harton (dharton)
2016-04-15 14:44 ` [dpdk-dev] [RFC PATCH v1 2/3] drivers/net/ixgbe: change xstats to use integers Remy Horton
2016-04-29 13:43 ` David Harton (dharton)
2016-05-03 12:22 ` Remy Horton
2016-05-03 13:40 ` David Harton (dharton)
2016-04-15 14:44 ` [dpdk-dev] [RFC PATCH v1 3/3] examples/ethtool: add xstats display command Remy Horton
2016-04-20 16:03 ` [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from xstats David Harton (dharton)
2016-04-20 16:49 ` Mcnamara, John
2016-04-22 15:04 ` David Harton (dharton)
2016-04-28 14:56 ` Tahhan, Maryam
2016-04-28 15:58 ` David Harton (dharton)
2016-04-29 10:21 ` Remy Horton [this message]
2016-04-29 12:15 ` David Harton (dharton)
2016-04-29 12:52 ` David Harton (dharton)
2016-05-06 11:11 ` [dpdk-dev] [RFC PATCH v2 " Remy Horton
2016-05-06 11:11 ` [dpdk-dev] [RFC PATCH v2 1/3] rte: change xstats to use integer keys Remy Horton
2016-05-09 13:59 ` David Harton (dharton)
2016-05-10 8:58 ` Remy Horton
2016-05-12 16:17 ` Thomas Monjalon
2016-05-16 10:47 ` Tahhan, Maryam
2016-05-18 8:31 ` Tahhan, Maryam
2016-05-18 8:45 ` Remy Horton
2016-05-06 11:11 ` [dpdk-dev] [RFC PATCH v2 2/3] drivers/net/ixgbe: change xstats to use integer id Remy Horton
2016-05-09 14:06 ` David Harton (dharton)
2016-05-18 8:41 ` Tahhan, Maryam
2016-05-06 11:11 ` [dpdk-dev] [RFC PATCH v2 3/3] examples/ethtool: add xstats display command Remy Horton
2016-05-09 14:08 ` David Harton (dharton)
2016-05-18 8:42 ` Tahhan, Maryam
2016-05-16 10:42 ` [dpdk-dev] [RFC PATCH v2 0/3] Remove string operations from xstats Tahhan, Maryam
2016-05-18 10:12 ` Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 00/10] " Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 01/10] rte: change xstats to use integer ids Remy Horton
2016-06-08 9:37 ` Thomas Monjalon
2016-06-08 11:16 ` Remy Horton
2016-06-08 12:22 ` Thomas Monjalon
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 02/10] drivers/net/ixgbe: " Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 03/10] drivers/net/e1000: " Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 04/10] drivers/net/fm10k: " Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 05/10] drivers/net/i40e: " Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 06/10] drivers/net/virtio: " Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 07/10] app/test-pmd: " Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 08/10] app/proc_info: " Remy Horton
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 09/10] remove name field from struct rte_eth_xstats Remy Horton
2016-06-08 12:23 ` Thomas Monjalon
2016-05-30 10:48 ` [dpdk-dev] [PATCH v3 10/10] doc: update xstats documentation Remy Horton
2016-06-09 8:48 ` Mcnamara, John
2016-06-06 12:45 ` [dpdk-dev] [PATCH v3 00/10] Remove string operations from xstats David Harton (dharton)
2016-06-13 15:51 ` [dpdk-dev] [PATCH v4 0/8] " Remy Horton
2016-06-13 15:51 ` [dpdk-dev] [PATCH v4 1/8] rte: change xstats to use integer ids Remy Horton
2016-06-15 9:19 ` Thomas Monjalon
2016-06-13 15:51 ` [dpdk-dev] [PATCH v4 2/8] drivers/net/ixgbe: " Remy Horton
2016-06-13 15:51 ` [dpdk-dev] [PATCH v4 3/8] drivers/net/e1000: " Remy Horton
2016-06-13 15:51 ` [dpdk-dev] [PATCH v4 4/8] drivers/net/fm10k: " Remy Horton
2016-06-13 15:51 ` [dpdk-dev] [PATCH v4 5/8] drivers/net/i40e: " Remy Horton
2016-06-13 15:51 ` [dpdk-dev] [PATCH v4 6/8] drivers/net/virtio: " Remy Horton
2016-06-13 15:52 ` [dpdk-dev] [PATCH v4 7/8] rte: change xstats usage to new API Remy Horton
2016-06-15 9:13 ` Thomas Monjalon
2016-06-13 15:52 ` [dpdk-dev] [PATCH v4 8/8] doc: update xstats documentation Remy Horton
2016-06-14 14:06 ` Mcnamara, John
2016-06-15 15:25 ` [dpdk-dev] [PATCH v5 0/7] Remove string operations from xstats Remy Horton
2016-06-15 15:25 ` [dpdk-dev] [PATCH v5 1/7] rte: change xstats to use integer ids Remy Horton
2016-06-15 15:25 ` [dpdk-dev] [PATCH v5 2/7] drivers/net/ixgbe: " Remy Horton
2016-06-15 15:25 ` [dpdk-dev] [PATCH v5 3/7] drivers/net/e1000: " Remy Horton
2016-06-15 15:25 ` [dpdk-dev] [PATCH v5 4/7] drivers/net/fm10k: " Remy Horton
2016-06-15 15:25 ` [dpdk-dev] [PATCH v5 5/7] drivers/net/i40e: " Remy Horton
2016-06-15 15:25 ` [dpdk-dev] [PATCH v5 6/7] drivers/net/virtio: " Remy Horton
2016-06-20 10:43 ` Yuanhan Liu
2016-06-15 15:25 ` [dpdk-dev] [PATCH v5 7/7] rte: change xstats usage to new API Remy Horton
2016-06-16 16:02 ` [dpdk-dev] [PATCH v5 0/7] Remove string operations from xstats Thomas Monjalon
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=572335BA.7000202@intel.com \
--to=remy.horton@intel.com \
--cc=dev@dpdk.org \
--cc=dharton@cisco.com \
--cc=harry.van.haaren@intel.com \
--cc=john.mcnamara@intel.com \
--cc=maryam.tahhan@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).