* [PATCH iproute2 v5 0/4] update ifstat for new stats
@ 2017-01-26 12:44 Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 1/4] ifstat: Includes reorder Nogah Frankel
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Nogah Frankel @ 2017-01-26 12:44 UTC (permalink / raw)
To: netdev
Cc: stephen, roopa, roszenrami, jbenc, sergei.shtylyov, jiri, eladr,
idosch, yotamg, ogerlitz, Nogah Frankel
Previously stats were gotten by RTM_GETLINK which returns 32 bits based
statistics. It supports only one type of stats.
Lately, a new method to get stats was added - RTM_GETSTATS. It supports
ability to choose stats type. The basic stats were changed from 32 bits
based to 64 bits based.
This patchset adds ifstat the ability to get extended stats (xstats) by
this method. Its adds xstat of cpu_hits - for packets that hit cpu.
---
v4->v5:
- remove patch 3/4 that added 64 based "normal" stats
- add a new patch 4/4 that updates ifstat man page
v3->v4:
- patch 2/4:
- change xstat name read to avoid redundant copy.
- delete extra line
- patch 4/4:
- change xstat name.
v2->v3:
- patch 1/4:
- add a new patch to reorder includes in misc/ifstat.c
- patch 2/4: (previously 1/3)
- fix typos.
- change error print to use fprintf.
v1->v2:
- change from using RTM_GETSTATS always to using it only for extended
stats.
- Add 64bits extended stats type.
Nogah Frankel (4):
ifstat: Includes reorder
ifstat: Add extended statistics to ifstat
ifstat: Add "sw only" extended statistics to ifstat
ifstat: Add xstat to ifstat man page
man/man8/Makefile | 3 +-
man/man8/ifstat.8 | 12 +++-
misc/ifstat.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++------
3 files changed, 163 insertions(+), 20 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH iproute2 v5 1/4] ifstat: Includes reorder
2017-01-26 12:44 [PATCH iproute2 v5 0/4] update ifstat for new stats Nogah Frankel
@ 2017-01-26 12:44 ` Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat Nogah Frankel
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Nogah Frankel @ 2017-01-26 12:44 UTC (permalink / raw)
To: netdev
Cc: stephen, roopa, roszenrami, jbenc, sergei.shtylyov, jiri, eladr,
idosch, yotamg, ogerlitz, Nogah Frankel
Reorder the includes in misc/ifstat.c to match convention.
Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
misc/ifstat.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/misc/ifstat.c b/misc/ifstat.c
index 92d67b0..5bcbcc8 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -28,12 +28,12 @@
#include <math.h>
#include <getopt.h>
-#include <libnetlink.h>
-#include <json_writer.h>
#include <linux/if.h>
#include <linux/if_link.h>
-#include <SNAPSHOT.h>
+#include "libnetlink.h"
+#include "json_writer.h"
+#include "SNAPSHOT.h"
int dump_zeros;
int reset_history;
--
2.4.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
2017-01-26 12:44 [PATCH iproute2 v5 0/4] update ifstat for new stats Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 1/4] ifstat: Includes reorder Nogah Frankel
@ 2017-01-26 12:44 ` Nogah Frankel
2017-01-30 4:34 ` Stephen Hemminger
2017-02-03 18:07 ` Stephen Hemminger
2017-01-26 12:44 ` [PATCH iproute2 v5 3/4] ifstat: Add "sw only" " Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 4/4] ifstat: Add xstat to ifstat man page Nogah Frankel
3 siblings, 2 replies; 10+ messages in thread
From: Nogah Frankel @ 2017-01-26 12:44 UTC (permalink / raw)
To: netdev
Cc: stephen, roopa, roszenrami, jbenc, sergei.shtylyov, jiri, eladr,
idosch, yotamg, ogerlitz, Nogah Frankel
Extended stats are part of the RTM_GETSTATS method. This patch adds them
to ifstat.
While extended stats can come in many forms, we support only the
rtnl_link_stats64 struct for them (which is the 64 bits version of struct
rtnl_link_stats).
We support stats in the main nesting level, or one lower.
The extension can be called by its name or any shorten of it. If there is
more than one matched, the first one will be picked.
To get the extended stats the flag -x <stats type> is used.
Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
misc/ifstat.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 145 insertions(+), 15 deletions(-)
diff --git a/misc/ifstat.c b/misc/ifstat.c
index 5bcbcc8..9467119 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -34,6 +34,7 @@
#include "libnetlink.h"
#include "json_writer.h"
#include "SNAPSHOT.h"
+#include "utils.h"
int dump_zeros;
int reset_history;
@@ -48,17 +49,21 @@ int pretty;
double W;
char **patterns;
int npatterns;
+bool is_extended;
+int filter_type;
+int sub_type;
char info_source[128];
int source_mismatch;
#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(__u32))
+#define NO_SUB_TYPE 0xffff
struct ifstat_ent {
struct ifstat_ent *next;
char *name;
int ifindex;
- unsigned long long val[MAXS];
+ __u64 val[MAXS];
double rate[MAXS];
__u32 ival[MAXS];
};
@@ -106,6 +111,48 @@ static int match(const char *id)
return 0;
}
+static int get_nlmsg_extended(const struct sockaddr_nl *who,
+ struct nlmsghdr *m, void *arg)
+{
+ struct if_stats_msg *ifsm = NLMSG_DATA(m);
+ struct rtattr *tb[IFLA_STATS_MAX+1];
+ int len = m->nlmsg_len;
+ struct ifstat_ent *n;
+
+ if (m->nlmsg_type != RTM_NEWSTATS)
+ return 0;
+
+ len -= NLMSG_LENGTH(sizeof(*ifsm));
+ if (len < 0)
+ return -1;
+
+ parse_rtattr(tb, IFLA_STATS_MAX, IFLA_STATS_RTA(ifsm), len);
+ if (tb[filter_type] == NULL)
+ return 0;
+
+ n = malloc(sizeof(*n));
+ if (!n)
+ abort();
+
+ n->ifindex = ifsm->ifindex;
+ n->name = strdup(ll_index_to_name(ifsm->ifindex));
+
+ if (sub_type == NO_SUB_TYPE) {
+ memcpy(&n->val, RTA_DATA(tb[filter_type]), sizeof(n->val));
+ } else {
+ struct rtattr *attr;
+
+ attr = parse_rtattr_one_nested(sub_type, tb[filter_type]);
+ if (attr == NULL)
+ return 0;
+ memcpy(&n->val, RTA_DATA(attr), sizeof(n->val));
+ }
+ memset(&n->rate, 0, sizeof(n->rate));
+ n->next = kern_db;
+ kern_db = n;
+ return 0;
+}
+
static int get_nlmsg(const struct sockaddr_nl *who,
struct nlmsghdr *m, void *arg)
{
@@ -147,18 +194,34 @@ static void load_info(void)
{
struct ifstat_ent *db, *n;
struct rtnl_handle rth;
+ __u32 filter_mask;
if (rtnl_open(&rth, 0) < 0)
exit(1);
- if (rtnl_wilddump_request(&rth, AF_INET, RTM_GETLINK) < 0) {
- perror("Cannot send dump request");
- exit(1);
- }
+ if (is_extended) {
+ ll_init_map(&rth);
+ filter_mask = IFLA_STATS_FILTER_BIT(filter_type);
+ if (rtnl_wilddump_stats_req_filter(&rth, AF_UNSPEC, RTM_GETSTATS,
+ filter_mask) < 0) {
+ perror("Cannot send dump request");
+ exit(1);
+ }
- if (rtnl_dump_filter(&rth, get_nlmsg, NULL) < 0) {
- fprintf(stderr, "Dump terminated\n");
- exit(1);
+ if (rtnl_dump_filter(&rth, get_nlmsg_extended, NULL) < 0) {
+ fprintf(stderr, "Dump terminated\n");
+ exit(1);
+ }
+ } else {
+ if (rtnl_wilddump_request(&rth, AF_INET, RTM_GETLINK) < 0) {
+ perror("Cannot send dump request");
+ exit(1);
+ }
+
+ if (rtnl_dump_filter(&rth, get_nlmsg, NULL) < 0) {
+ fprintf(stderr, "Dump terminated\n");
+ exit(1);
+ }
}
rtnl_close(&rth);
@@ -553,10 +616,17 @@ static void update_db(int interval)
}
for (i = 0; i < MAXS; i++) {
double sample;
- unsigned long incr = h1->ival[i] - n->ival[i];
+ __u64 incr;
+
+ if (is_extended) {
+ incr = h1->val[i] - n->val[i];
+ n->val[i] = h1->val[i];
+ } else {
+ incr = (__u32) (h1->ival[i] - n->ival[i]);
+ n->val[i] += incr;
+ n->ival[i] = h1->ival[i];
+ }
- n->val[i] += incr;
- n->ival[i] = h1->ival[i];
sample = (double)(incr*1000)/interval;
if (interval >= scan_interval) {
n->rate[i] += W*(sample-n->rate[i]);
@@ -656,6 +726,47 @@ static int verify_forging(int fd)
return -1;
}
+static void xstat_usage(void)
+{
+ fprintf(stderr,
+"Usage: ifstat supported xstats:\n");
+}
+
+struct extended_stats_options_t {
+ char *name;
+ int id;
+ int sub_type;
+};
+
+/* Note: if one xstat name is subset of another, it should be before it in this
+ * list.
+ * Name length must be under 64 chars.
+ */
+static const struct extended_stats_options_t extended_stats_options[] = {
+};
+
+static const char *get_filter_type(const char *name)
+{
+ int name_len;
+ int i;
+
+ name_len = strlen(name);
+ for (i = 0; i < ARRAY_SIZE(extended_stats_options); i++) {
+ const struct extended_stats_options_t *xstat;
+
+ xstat = &extended_stats_options[i];
+ if (strncmp(name, xstat->name, name_len) == 0) {
+ filter_type = xstat->id;
+ sub_type = xstat->sub_type;
+ return xstat->name;
+ }
+ }
+
+ fprintf(stderr, "invalid ifstat extension %s\n", name);
+ xstat_usage();
+ return NULL;
+}
+
static void usage(void) __attribute__((noreturn));
static void usage(void)
@@ -673,7 +784,8 @@ static void usage(void)
" -s, --noupdate don't update history\n"
" -t, --interval=SECS report average over the last SECS\n"
" -V, --version output version information\n"
-" -z, --zeros show entries with zero activity\n");
+" -z, --zeros show entries with zero activity\n"
+" -x, --extended=TYPE show extended stats of TYPE\n");
exit(-1);
}
@@ -691,6 +803,7 @@ static const struct option longopts[] = {
{ "interval", 1, 0, 't' },
{ "version", 0, 0, 'V' },
{ "zeros", 0, 0, 'z' },
+ { "extended", 1, 0, 'x'},
{ 0 }
};
@@ -699,10 +812,12 @@ int main(int argc, char *argv[])
char hist_name[128];
struct sockaddr_un sun;
FILE *hist_fp = NULL;
+ const char *stats_type = NULL;
int ch;
int fd;
- while ((ch = getopt_long(argc, argv, "hjpvVzrnasd:t:e",
+ is_extended = false;
+ while ((ch = getopt_long(argc, argv, "hjpvVzrnasd:t:ex:",
longopts, NULL)) != EOF) {
switch (ch) {
case 'z':
@@ -743,6 +858,10 @@ int main(int argc, char *argv[])
exit(-1);
}
break;
+ case 'x':
+ stats_type = optarg;
+ is_extended = true;
+ break;
case 'v':
case 'V':
printf("ifstat utility, iproute2-ss%s\n", SNAPSHOT);
@@ -757,6 +876,12 @@ int main(int argc, char *argv[])
argc -= optind;
argv += optind;
+ if (stats_type) {
+ stats_type = get_filter_type(stats_type);
+ if (!stats_type)
+ exit(-1);
+ }
+
sun.sun_family = AF_UNIX;
sun.sun_path[0] = 0;
sprintf(sun.sun_path+1, "ifstat%d", getuid());
@@ -795,8 +920,13 @@ int main(int argc, char *argv[])
snprintf(hist_name, sizeof(hist_name),
"%s", getenv("IFSTAT_HISTORY"));
else
- snprintf(hist_name, sizeof(hist_name),
- "%s/.ifstat.u%d", P_tmpdir, getuid());
+ if (!stats_type)
+ snprintf(hist_name, sizeof(hist_name),
+ "%s/.ifstat.u%d", P_tmpdir, getuid());
+ else
+ snprintf(hist_name, sizeof(hist_name),
+ "%s/.%s_ifstat.u%d", P_tmpdir, stats_type,
+ getuid());
if (reset_history)
unlink(hist_name);
--
2.4.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH iproute2 v5 3/4] ifstat: Add "sw only" extended statistics to ifstat
2017-01-26 12:44 [PATCH iproute2 v5 0/4] update ifstat for new stats Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 1/4] ifstat: Includes reorder Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat Nogah Frankel
@ 2017-01-26 12:44 ` Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 4/4] ifstat: Add xstat to ifstat man page Nogah Frankel
3 siblings, 0 replies; 10+ messages in thread
From: Nogah Frankel @ 2017-01-26 12:44 UTC (permalink / raw)
To: netdev
Cc: stephen, roopa, roszenrami, jbenc, sergei.shtylyov, jiri, eladr,
idosch, yotamg, ogerlitz, Nogah Frankel
Add support for extended statistics of SW only type, for counting only the
packets that went via the cpu. (useful for systems with forward
offloading). It reads it from filter type IFLA_STATS_LINK_OFFLOAD_XSTATS
and sub type IFLA_OFFLOAD_XSTATS_CPU_HIT.
It is under the name 'cpu_hits'
(or any shorten of it as 'cpu' or simply 'c')
For example:
ifstat -x c
Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
misc/ifstat.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/misc/ifstat.c b/misc/ifstat.c
index 9467119..a853ee6 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -729,7 +729,8 @@ static int verify_forging(int fd)
static void xstat_usage(void)
{
fprintf(stderr,
-"Usage: ifstat supported xstats:\n");
+"Usage: ifstat supported xstats:\n"
+" cpu_hits Counts only packets that went via the CPU.\n");
}
struct extended_stats_options_t {
@@ -743,6 +744,7 @@ struct extended_stats_options_t {
* Name length must be under 64 chars.
*/
static const struct extended_stats_options_t extended_stats_options[] = {
+ {"cpu_hits", IFLA_STATS_LINK_OFFLOAD_XSTATS, IFLA_OFFLOAD_XSTATS_CPU_HIT},
};
static const char *get_filter_type(const char *name)
--
2.4.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH iproute2 v5 4/4] ifstat: Add xstat to ifstat man page
2017-01-26 12:44 [PATCH iproute2 v5 0/4] update ifstat for new stats Nogah Frankel
` (2 preceding siblings ...)
2017-01-26 12:44 ` [PATCH iproute2 v5 3/4] ifstat: Add "sw only" " Nogah Frankel
@ 2017-01-26 12:44 ` Nogah Frankel
3 siblings, 0 replies; 10+ messages in thread
From: Nogah Frankel @ 2017-01-26 12:44 UTC (permalink / raw)
To: netdev
Cc: stephen, roopa, roszenrami, jbenc, sergei.shtylyov, jiri, eladr,
idosch, yotamg, ogerlitz, Nogah Frankel
Add documentation about the extended statistics to the ifstat man page.
Add ifstat man age to the man8 Makefile
Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
man/man8/Makefile | 3 ++-
man/man8/ifstat.8 | 12 +++++++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/man/man8/Makefile b/man/man8/Makefile
index 77d347c..bc2fc81 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -18,7 +18,8 @@ MAN8PAGES = $(TARGETS) ip.8 arpd.8 lnstat.8 routel.8 rtacct.8 rtmon.8 rtpr.8 ss.
tc-connmark.8 tc-csum.8 tc-mirred.8 tc-nat.8 tc-pedit.8 tc-police.8 \
tc-simple.8 tc-skbedit.8 tc-vlan.8 tc-xt.8 tc-ife.8 \
tc-tunnel_key.8 \
- devlink.8 devlink-dev.8 devlink-monitor.8 devlink-port.8 devlink-sb.8
+ devlink.8 devlink-dev.8 devlink-monitor.8 devlink-port.8 devlink-sb.8 \
+ ifstat.8
all: $(TARGETS)
diff --git a/man/man8/ifstat.8 b/man/man8/ifstat.8
index e49d868..3ba0088 100644
--- a/man/man8/ifstat.8
+++ b/man/man8/ifstat.8
@@ -14,7 +14,8 @@ ifstat \- handy utility to read network interface statistics
The utility keeps records of the previous data displayed in history files and
by default only shows difference between the last and the current call.
Location of the history files defaults to /tmp/.ifstat.u$UID but may be
-overridden with the IFSTAT_HISTORY environment variable.
+overridden with the IFSTAT_HISTORY environment variable. Similarly, the default
+location for xstat (extended stats) is /tmp/.<xstat name>_ifstat.u$UID.
.SH OPTIONS
.TP
.B \-h, \-\-help
@@ -46,6 +47,15 @@ Report average over the last SECS seconds.
.TP
.B \-z, \-\-zeros
Show entries with zero activity.
+.TP
+.B \-x, \-\-extended=TYPE
+Show extended stats of TYPE. Supported types are:
+
+.in +8
+.B cpu_hits
+- Counts only packets that went via the CPU.
+.in -8
+
.SH ENVIRONMENT
.TP
.B IFSTAT_HISTORY
--
2.4.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
2017-01-26 12:44 ` [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat Nogah Frankel
@ 2017-01-30 4:34 ` Stephen Hemminger
2017-02-02 8:53 ` Nogah Frankel
2017-02-03 18:07 ` Stephen Hemminger
1 sibling, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2017-01-30 4:34 UTC (permalink / raw)
To: Nogah Frankel
Cc: netdev, roopa, roszenrami, jbenc, sergei.shtylyov, jiri, eladr,
idosch, yotamg, ogerlitz
On Thu, 26 Jan 2017 14:44:39 +0200
Nogah Frankel <nogahf@mellanox.com> wrote:
> Extended stats are part of the RTM_GETSTATS method. This patch adds them
> to ifstat.
> While extended stats can come in many forms, we support only the
> rtnl_link_stats64 struct for them (which is the 64 bits version of struct
> rtnl_link_stats).
> We support stats in the main nesting level, or one lower.
> The extension can be called by its name or any shorten of it. If there is
> more than one matched, the first one will be picked.
>
> To get the extended stats the flag -x <stats type> is used.
It would be better if this command used 64 bit statistics transparently
like other tools in iproute2. Transparency is always better user experience.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
2017-01-30 4:34 ` Stephen Hemminger
@ 2017-02-02 8:53 ` Nogah Frankel
0 siblings, 0 replies; 10+ messages in thread
From: Nogah Frankel @ 2017-02-02 8:53 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, roopa, roszenrami, jbenc, sergei.shtylyov, Jiri Pirko,
Elad Raz, Ido Schimmel, Yotam Gigi, Or Gerlitz
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, January 30, 2017 6:34 AM
> To: Nogah Frankel <nogahf@mellanox.com>
> Cc: netdev@vger.kernel.org; roopa@cumulusnetworks.com; roszenrami@gmail.com;
> jbenc@redhat.com; sergei.shtylyov@cogentembedded.com; Jiri Pirko
> <jiri@mellanox.com>; Elad Raz <eladr@mellanox.com>; Ido Schimmel
> <idosch@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>; Or Gerlitz
> <ogerlitz@mellanox.com>
> Subject: Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
>
> On Thu, 26 Jan 2017 14:44:39 +0200
> Nogah Frankel <nogahf@mellanox.com> wrote:
>
> > Extended stats are part of the RTM_GETSTATS method. This patch adds them
> > to ifstat.
> > While extended stats can come in many forms, we support only the
> > rtnl_link_stats64 struct for them (which is the 64 bits version of struct
> > rtnl_link_stats).
> > We support stats in the main nesting level, or one lower.
> > The extension can be called by its name or any shorten of it. If there is
> > more than one matched, the first one will be picked.
> >
> > To get the extended stats the flag -x <stats type> is used.
>
> It would be better if this command used 64 bit statistics transparently
> like other tools in iproute2. Transparency is always better user experience.
From user point of view, all the statistics in ifstat are 64 bits and are saved that way.
We currently get the stats in 32 bits structs and translate them to 64 bits. It keeps track
of int overflows (in the 32 bits).
The problem with it is that if between one ifstat call to another passes too much time,
one might lose one of the overflows.
But to change it is not a trivial matter, and I believe it is not related to the xstat feature I
want to add.
I don't think I understood what is the change you want.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
2017-01-26 12:44 ` [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat Nogah Frankel
2017-01-30 4:34 ` Stephen Hemminger
@ 2017-02-03 18:07 ` Stephen Hemminger
2017-02-07 12:59 ` Nogah Frankel
1 sibling, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2017-02-03 18:07 UTC (permalink / raw)
To: Nogah Frankel
Cc: netdev, roopa, roszenrami, jbenc, sergei.shtylyov, jiri, eladr,
idosch, yotamg, ogerlitz
On Thu, 26 Jan 2017 14:44:39 +0200
Nogah Frankel <nogahf@mellanox.com> wrote:
> Extended stats are part of the RTM_GETSTATS method. This patch adds them
> to ifstat.
> While extended stats can come in many forms, we support only the
> rtnl_link_stats64 struct for them (which is the 64 bits version of struct
> rtnl_link_stats).
> We support stats in the main nesting level, or one lower.
> The extension can be called by its name or any shorten of it. If there is
> more than one matched, the first one will be picked.
>
> To get the extended stats the flag -x <stats type> is used.
>
> Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Sorry I was confused because RTM_GETSTATS contains multiple statistics.
Your patch is about getting LINK_XSTATS and after looking in more detail, you are
correct this should be an option. Although it would make sense to show this as addition
to the basic statistics. And when I tested it no output happens which seems confusing.
$ ./misc/ifstat -p -x cpu_hits
#kernel
Interface RX Pkts/Rate TX Pkts/Rate RX Data/Rate TX Data/Rate
RX Errs/Drop TX Errs/Drop RX Over/Rate TX Coll/Rate
What I was intending in earlier discussion was using IFLA_STATS_LINK_64 which would
allow supporting 64 bit statistics on 32 bit platforms.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
2017-02-03 18:07 ` Stephen Hemminger
@ 2017-02-07 12:59 ` Nogah Frankel
2017-02-07 16:48 ` Stephen Hemminger
0 siblings, 1 reply; 10+ messages in thread
From: Nogah Frankel @ 2017-02-07 12:59 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, roopa, roszenrami, jbenc, sergei.shtylyov, Jiri Pirko,
Elad Raz, Ido Schimmel, Yotam Gigi, Or Gerlitz
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, February 03, 2017 8:07 PM
> To: Nogah Frankel <nogahf@mellanox.com>
> Cc: netdev@vger.kernel.org; roopa@cumulusnetworks.com; roszenrami@gmail.com;
> jbenc@redhat.com; sergei.shtylyov@cogentembedded.com; Jiri Pirko
> <jiri@mellanox.com>; Elad Raz <eladr@mellanox.com>; Ido Schimmel
> <idosch@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>; Or Gerlitz
> <ogerlitz@mellanox.com>
> Subject: Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
>
> On Thu, 26 Jan 2017 14:44:39 +0200
> Nogah Frankel <nogahf@mellanox.com> wrote:
>
> > Extended stats are part of the RTM_GETSTATS method. This patch adds them
> > to ifstat.
> > While extended stats can come in many forms, we support only the
> > rtnl_link_stats64 struct for them (which is the 64 bits version of struct
> > rtnl_link_stats).
> > We support stats in the main nesting level, or one lower.
> > The extension can be called by its name or any shorten of it. If there is
> > more than one matched, the first one will be picked.
> >
> > To get the extended stats the flag -x <stats type> is used.
> >
> > Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
> > Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>
> Sorry I was confused because RTM_GETSTATS contains multiple statistics.
> Your patch is about getting LINK_XSTATS and after looking in more detail, you are
> correct this should be an option. Although it would make sense to show this as addition
> to the basic statistics. And when I tested it no output happens which seems confusing.
>
> $ ./misc/ifstat -p -x cpu_hits
> #kernel
> Interface RX Pkts/Rate TX Pkts/Rate RX Data/Rate TX Data/Rate
> RX Errs/Drop TX Errs/Drop RX Over/Rate TX Coll/Rate
>
Not all devices support this xstat.
Do you prefer another print in this case?
About printing both the xstat and the default stats together, it may be problematic since
ifstat is about diffs. I think it is better that one ifstat call for a specific stats, won't change
the other stats data. (And since we are talking about diffs, reading data meaning changing
it).
> What I was intending in earlier discussion was using IFLA_STATS_LINK_64 which would
> allow supporting 64 bit statistics on 32 bit platforms.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
2017-02-07 12:59 ` Nogah Frankel
@ 2017-02-07 16:48 ` Stephen Hemminger
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2017-02-07 16:48 UTC (permalink / raw)
To: Nogah Frankel
Cc: netdev, roopa, roszenrami, jbenc, sergei.shtylyov, Jiri Pirko,
Elad Raz, Ido Schimmel, Yotam Gigi, Or Gerlitz
On Tue, 7 Feb 2017 12:59:11 +0000
Nogah Frankel <nogahf@mellanox.com> wrote:
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Friday, February 03, 2017 8:07 PM
> > To: Nogah Frankel <nogahf@mellanox.com>
> > Cc: netdev@vger.kernel.org; roopa@cumulusnetworks.com; roszenrami@gmail.com;
> > jbenc@redhat.com; sergei.shtylyov@cogentembedded.com; Jiri Pirko
> > <jiri@mellanox.com>; Elad Raz <eladr@mellanox.com>; Ido Schimmel
> > <idosch@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>; Or Gerlitz
> > <ogerlitz@mellanox.com>
> > Subject: Re: [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat
> >
> > On Thu, 26 Jan 2017 14:44:39 +0200
> > Nogah Frankel <nogahf@mellanox.com> wrote:
> >
> > > Extended stats are part of the RTM_GETSTATS method. This patch adds them
> > > to ifstat.
> > > While extended stats can come in many forms, we support only the
> > > rtnl_link_stats64 struct for them (which is the 64 bits version of struct
> > > rtnl_link_stats).
> > > We support stats in the main nesting level, or one lower.
> > > The extension can be called by its name or any shorten of it. If there is
> > > more than one matched, the first one will be picked.
> > >
> > > To get the extended stats the flag -x <stats type> is used.
> > >
> > > Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
> > > Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> >
> > Sorry I was confused because RTM_GETSTATS contains multiple statistics.
> > Your patch is about getting LINK_XSTATS and after looking in more detail, you are
> > correct this should be an option. Although it would make sense to show this as addition
> > to the basic statistics. And when I tested it no output happens which seems confusing.
> >
> > $ ./misc/ifstat -p -x cpu_hits
> > #kernel
> > Interface RX Pkts/Rate TX Pkts/Rate RX Data/Rate TX Data/Rate
> > RX Errs/Drop TX Errs/Drop RX Over/Rate TX Coll/Rate
> >
>
> Not all devices support this xstat.
> Do you prefer another print in this case?
>
> About printing both the xstat and the default stats together, it may be problematic since
> ifstat is about diffs. I think it is better that one ifstat call for a specific stats, won't change
> the other stats data. (And since we are talking about diffs, reading data meaning changing
> it).
Maybe an error would be a better experience for user.
I merged the current version but try and think about how to make it work better in general.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-07 16:48 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-26 12:44 [PATCH iproute2 v5 0/4] update ifstat for new stats Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 1/4] ifstat: Includes reorder Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 2/4] ifstat: Add extended statistics to ifstat Nogah Frankel
2017-01-30 4:34 ` Stephen Hemminger
2017-02-02 8:53 ` Nogah Frankel
2017-02-03 18:07 ` Stephen Hemminger
2017-02-07 12:59 ` Nogah Frankel
2017-02-07 16:48 ` Stephen Hemminger
2017-01-26 12:44 ` [PATCH iproute2 v5 3/4] ifstat: Add "sw only" " Nogah Frankel
2017-01-26 12:44 ` [PATCH iproute2 v5 4/4] ifstat: Add xstat to ifstat man page Nogah Frankel
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.