From: roopa <roopa@cumulusnetworks•com>
To: Thomas Graf <tgraf@suug•ch>
Cc: netdev@vger•kernel.org, jhs@mojatatu•com, davem@davemloft•net
Subject: Re: [PATCH net-next v2 1/2] rtnetlink: add new RTM_GETSTATS message to dump link stats
Date: Sun, 10 Apr 2016 11:28:29 -0700 [thread overview]
Message-ID: <570A9B4D.80104@cumulusnetworks.com> (raw)
In-Reply-To: <20160410081650.GB22873@pox.localdomain>
On 4/10/16, 1:16 AM, Thomas Graf wrote:
> On 04/08/16 at 11:38pm, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks•com>
>>
>> This patch adds a new RTM_GETSTATS message to query link stats via netlink
>> from the kernel. RTM_NEWLINK also dumps stats today, but RTM_NEWLINK
>> returns a lot more than just stats and is expensive in some cases when
>> frequent polling for stats from userspace is a common operation.
>>
>> RTM_GETSTATS is an attempt to provide a light weight netlink message
>> to explicity query only link stats from the kernel on an interface.
>> The idea is to also keep it extensible so that new kinds of stats can be
>> added to it in the future.
>>
>> This patch adds the following attribute for NETDEV stats:
>> struct nla_policy ifla_stats_policy[IFLA_STATS_MAX + 1] = {
>> [IFLA_STATS_LINK64] = { .len = sizeof(struct rtnl_link_stats64) },
>> };
>>
>> This patch also allows for af family stats (an example af stats for IPV6
>> is available with the second patch in the series).
>>
>> Like any other rtnetlink message, RTM_GETSTATS can be used to get stats of
>> a single interface or all interfaces with NLM_F_DUMP.
> Awesome stuff Roopa.
>
> This currently ties everything to a net_device with a selector to
> include certain bits of that net_device. How about we take it half a
> step further and allow for non net_device stats such as IP, TCP,
> routing or ipsec stats to be retrieved as well?
yes, absolutely. and that is also the goal.
> A simple array of nested attributes replacing IFLA_STATS_* would
> allow for that, e.g.
>
> 1. {.type = ST_IPSTATS, value = { ...} }
>
> 2. {.type = ST_LINK, .value = {
> {.type = ST_LINK_NAME, .value = "eth0"},
> {.type = ST_LINK_Q, .value = 10}
> }}
>
> 3. ...
One thing though, Its unclear to me if we absolutely need the additional nest.
Every stats netlink msg has an ifindex in the header (if_stats_msg) if the scope
of the stats is a netdev. If the msg does not have an ifindex in the if_stats_msg,
it represents a global stat. ie Cant a dump, include other stats netlink msgs after
all the netdev msgs are done when the filter has global stat filters ?.
same will apply to RTM_GETSTATS (without NLM_F_DUMP).
Since the msg may potentially have more nest levels
in the IFLA_EXT_STATS categories, just trying to see if i can avoid adding another
top-level nest. We can sure add it if there is no other way to include global
stats in the same dump.
> So for your initial patch, you'd simply add a new top level attribute
> which ties all the net_device specific statistics to a top level
> attribute and we can add the non net_device specific stats later on.
> We can't do that later on though without breaking ABI so that would
> have to go in with the first iteration.
agreed
>
> We can preserve all the existing attribute formats, we simply have
> to introduce new attribute types which don't overlap and document
> which statistic identifier maps to which existing attribute id.
sure,
thanks Thomas.
next prev parent reply other threads:[~2016-04-10 18:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-09 6:38 [PATCH net-next v2 1/2] rtnetlink: add new RTM_GETSTATS message to dump link stats Roopa Prabhu
2016-04-09 14:30 ` Jamal Hadi Salim
2016-04-09 18:00 ` roopa
2016-04-10 13:48 ` Jamal Hadi Salim
2016-04-10 19:17 ` roopa
2016-04-10 8:16 ` Thomas Graf
2016-04-10 18:28 ` roopa [this message]
2016-04-12 3:53 ` roopa
2016-04-12 13:21 ` Thomas Graf
2016-04-13 12:11 ` Jamal Hadi Salim
2016-04-14 6:36 ` roopa
2016-04-14 4:19 ` David Miller
2016-04-14 6:35 ` roopa
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=570A9B4D.80104@cumulusnetworks.com \
--to=roopa@cumulusnetworks$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=jhs@mojatatu$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=tgraf@suug$(echo .)ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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