[PATCH iproute2 0/3] update ifstat for new stats

[PATCH iproute2 0/3] update ifstat for new stats

[Nogah Frankel]

Previously stats were gotten by RTM_GETLINK which return 32 bits based
statistics. It support 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 change ifstat to the new method, add it the ability to
choose an extended type of statistic, and add the extended type of SW
stats for packets that hit cpu.

Nogah Frankel (3):
  ifstat: Change interface to get stats
  ifstat: Add extended statistics to ifstat
  ifstat: Add "sw only" extended statistics to ifstat

 misc/ifstat.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 102 insertions(+), 23 deletions(-)

-- 
2.4.3

[PATCH iproute2 1/3] ifstat: Change interface to get stats

[Nogah Frankel]

ifstat used to get it data from the kernel with RTM_GETLINK.
Change the interface to get this data to RTM_GETSTATS that supports more
stats type beside the default one. It also change the default stats to be
64 bits based.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 misc/ifstat.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/misc/ifstat.c b/misc/ifstat.c
index d551973..25a8fc1 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -35,6 +35,7 @@
 
 #include <SNAPSHOT.h>
 
+#include "utils.h"
 int dump_zeros;
 int reset_history;
 int ignore_history;
@@ -52,15 +53,15 @@ int npatterns;
 char info_source[128];
 int source_mismatch;
 
-#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(__u32))
+#define MAXS (sizeof(struct rtnl_link_stats64)/sizeof(__u64))
 
 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];
+	__u64			ival[MAXS];
 };
 
 static const char *stats[MAXS] = {
@@ -109,32 +110,30 @@ static int match(const char *id)
 static int get_nlmsg(const struct sockaddr_nl *who,
 		     struct nlmsghdr *m, void *arg)
 {
-	struct ifinfomsg *ifi = NLMSG_DATA(m);
-	struct rtattr *tb[IFLA_MAX+1];
+	struct if_stats_msg *ifsm = NLMSG_DATA(m);
+	struct rtattr *tb[IFLA_STATS_MAX+1];
 	int len = m->nlmsg_len;
 	struct ifstat_ent *n;
 	int i;
 
-	if (m->nlmsg_type != RTM_NEWLINK)
+	if (m->nlmsg_type != RTM_NEWSTATS)
 		return 0;
 
-	len -= NLMSG_LENGTH(sizeof(*ifi));
+	len -= NLMSG_LENGTH(sizeof(*ifsm));
 	if (len < 0)
 		return -1;
 
-	if (!(ifi->ifi_flags&IFF_UP))
-		return 0;
-
-	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-	if (tb[IFLA_IFNAME] == NULL || tb[IFLA_STATS] == NULL)
+	parse_rtattr(tb, IFLA_STATS_MAX, IFLA_STATS_RTA(ifsm), len);
+	if (tb[IFLA_STATS_LINK_64] == NULL)
 		return 0;
 
 	n = malloc(sizeof(*n));
 	if (!n)
 		abort();
-	n->ifindex = ifi->ifi_index;
-	n->name = strdup(RTA_DATA(tb[IFLA_IFNAME]));
-	memcpy(&n->ival, RTA_DATA(tb[IFLA_STATS]), sizeof(n->ival));
+
+	n->ifindex = ifsm->ifindex;
+	n->name = strdup(ll_index_to_name(ifsm->ifindex));
+	memcpy(&n->ival, RTA_DATA(tb[IFLA_STATS_LINK_64]), sizeof(n->ival));
 	memset(&n->rate, 0, sizeof(n->rate));
 	for (i = 0; i < MAXS; i++)
 		n->val[i] = n->ival[i];
@@ -147,11 +146,15 @@ static void load_info(void)
 {
 	struct ifstat_ent *db, *n;
 	struct rtnl_handle rth;
+	__u32 filt_mask;
 
 	if (rtnl_open(&rth, 0) < 0)
 		exit(1);
 
-	if (rtnl_wilddump_request(&rth, AF_INET, RTM_GETLINK) < 0) {
+	ll_init_map(&rth);
+	filt_mask = IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_64);
+	if (rtnl_wilddump_stats_req_filter(&rth, AF_UNSPEC, RTM_GETSTATS,
+					   filt_mask) < 0) {
 		perror("Cannot send dump request");
 		exit(1);
 	}
@@ -216,7 +219,7 @@ static void load_raw_table(FILE *fp)
 			*next++ = 0;
 			if (sscanf(p, "%llu", n->val+i) != 1)
 				abort();
-			n->ival[i] = (__u32)n->val[i];
+			n->ival[i] = (__u64)n->val[i];
 			p = next;
 			if (!(next = strchr(p, ' ')))
 				abort();
@@ -546,14 +549,14 @@ static void update_db(int interval)
 				int i;
 
 				for (i = 0; i < MAXS; i++) {
-					if ((long)(h1->ival[i] - n->ival[i]) < 0) {
+					if ((long long)(h1->ival[i] - n->ival[i]) < 0) {
 						memset(n->ival, 0, sizeof(n->ival));
 						break;
 					}
 				}
 				for (i = 0; i < MAXS; i++) {
 					double sample;
-					unsigned long incr = h1->ival[i] - n->ival[i];
+					unsigned long long incr = h1->ival[i] - n->ival[i];
 
 					n->val[i] += incr;
 					n->ival[i] = h1->ival[i];
-- 
2.4.3

[PATCH iproute2 2/3] ifstat: Add extended statistics to ifstat

[Nogah Frankel]

Add extended stats option for ifstat. It supports stats that are in the
nesting level as the "normal" stats or one lower, as long as they are in
the same struct type as the "normal" stats.
Every extension is unaware of data from other extension and is being
presented by itself.
The extension can be called by its name or any shorten of it. If there is
more then 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 | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 81 insertions(+), 7 deletions(-)

diff --git a/misc/ifstat.c b/misc/ifstat.c
index 25a8fc1..90aeeaa 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -49,11 +49,14 @@ int pretty;
 double W;
 char **patterns;
 int npatterns;
+int filter_type;
+int sub_type;
 
 char info_source[128];
 int source_mismatch;
 
 #define MAXS (sizeof(struct rtnl_link_stats64)/sizeof(__u64))
+#define NO_SUB_TYPE 0xffff
 
 struct ifstat_ent {
 	struct ifstat_ent	*next;
@@ -124,7 +127,7 @@ static int get_nlmsg(const struct sockaddr_nl *who,
 		return -1;
 
 	parse_rtattr(tb, IFLA_STATS_MAX, IFLA_STATS_RTA(ifsm), len);
-	if (tb[IFLA_STATS_LINK_64] == NULL)
+	if (tb[filter_type] == NULL)
 		return 0;
 
 	n = malloc(sizeof(*n));
@@ -133,7 +136,17 @@ static int get_nlmsg(const struct sockaddr_nl *who,
 
 	n->ifindex = ifsm->ifindex;
 	n->name = strdup(ll_index_to_name(ifsm->ifindex));
-	memcpy(&n->ival, RTA_DATA(tb[IFLA_STATS_LINK_64]), sizeof(n->ival));
+
+	if (sub_type == NO_SUB_TYPE) {
+		memcpy(&n->ival, RTA_DATA(tb[filter_type]), sizeof(n->ival));
+	} else {
+		struct rtattr *attr;
+
+		attr = parse_rtattr_one_nested(sub_type, tb[filter_type]);
+		if (attr == NULL)
+			return 0;
+		memcpy(&n->ival, RTA_DATA(attr), sizeof(n->ival));
+	}
 	memset(&n->rate, 0, sizeof(n->rate));
 	for (i = 0; i < MAXS; i++)
 		n->val[i] = n->ival[i];
@@ -152,7 +165,7 @@ static void load_info(void)
 		exit(1);
 
 	ll_init_map(&rth);
-	filt_mask = IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_64);
+	filt_mask = IFLA_STATS_FILTER_BIT(filter_type);
 	if (rtnl_wilddump_stats_req_filter(&rth, AF_UNSPEC, RTM_GETSTATS,
 					   filt_mask) < 0) {
 		perror("Cannot send dump request");
@@ -659,6 +672,50 @@ 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 in subset of another, it should be before it in this
+ * list. Therefore the default "" option must always be first.
+ * Name length must be under 64 chars.
+ */
+static const struct extended_stats_options_t extended_stats_options[] = {
+	{"", IFLA_STATS_LINK_64, NO_SUB_TYPE},
+};
+
+static bool get_filter_type(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;
+			strcpy(name, xstat->name);
+			return true;
+		}
+	}
+
+	printf("invalid ifstat extension %s\n", name);
+	xstat_usage();
+	return false;
+}
+
 static void usage(void) __attribute__((noreturn));
 
 static void usage(void)
@@ -676,7 +733,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);
 }
@@ -694,18 +752,22 @@ static const struct option longopts[] = {
 	{ "interval", 1, 0, 't' },
 	{ "version", 0, 0, 'V' },
 	{ "zeros", 0, 0, 'z' },
+	{ "extended", 1, 0, 'x'},
 	{ 0 }
 };
 
+
 int main(int argc, char *argv[])
 {
 	char hist_name[128];
 	struct sockaddr_un sun;
 	FILE *hist_fp = NULL;
+	char stats_type[64];
 	int ch;
 	int fd;
 
-	while ((ch = getopt_long(argc, argv, "hjpvVzrnasd:t:e",
+	memset(stats_type, 0, 128);
+	while ((ch = getopt_long(argc, argv, "hjpvVzrnasd:t:ex:",
 			longopts, NULL)) != EOF) {
 		switch (ch) {
 		case 'z':
@@ -746,6 +808,9 @@ int main(int argc, char *argv[])
 				exit(-1);
 			}
 			break;
+		case 'x':
+			strncpy(stats_type, optarg, 63);
+			break;
 		case 'v':
 		case 'V':
 			printf("ifstat utility, iproute2-ss%s\n", SNAPSHOT);
@@ -760,6 +825,9 @@ int main(int argc, char *argv[])
 	argc -= optind;
 	argv += optind;
 
+	if (!get_filter_type(stats_type))
+		exit(-1);
+
 	sun.sun_family = AF_UNIX;
 	sun.sun_path[0] = 0;
 	sprintf(sun.sun_path+1, "ifstat%d", getuid());
@@ -798,8 +866,14 @@ 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 (strlen(stats_type) == 0)
+			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

Re: [PATCH iproute2 2/3] ifstat: Add extended statistics to ifstat

[Stephen Hemminger]

On Thu, 24 Nov 2016 16:12:39 +0200
Nogah Frankel <nogahf@mellanox.com> wrote:

> Add extended stats option for ifstat. It supports stats that are in the
> nesting level as the "normal" stats or one lower, as long as they are in
> the same struct type as the "normal" stats.
> Every extension is unaware of data from other extension and is being
> presented by itself.
> The extension can be called by its name or any shorten of it. If there is
> more then 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>

Finally clearing up iproute2 patch backlog. This feature looks good,
but does not apply cleanly to current git master branch.

--- misc/ifstat.c
+++ misc/ifstat.c
@@ -733,7 +790,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);
 }

Please rebase your patches and resubmit.

[PATCH iproute2 3/3] ifstat: Add "sw only" extended statistics to ifstat

[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 'software'
(or any shorten of it as 'soft' or simply 's').

For example:
ifstat -x s

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 90aeeaa..7825a3a 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -675,7 +675,8 @@ static int verify_forging(int fd)
 static void xstat_usage(void)
 {
 	fprintf(stderr,
-"Usage: ifstat supported xstats:\n");
+"Usage: ifstat supported xstats:\n"
+"	software	SW stats. Counts only packets that went via the CPU\n");
 
 }
 
@@ -691,6 +692,7 @@ struct extended_stats_options_t {
  */
 static const struct extended_stats_options_t extended_stats_options[] = {
 	{"", IFLA_STATS_LINK_64, NO_SUB_TYPE},
+	{"software",  IFLA_STATS_LINK_OFFLOAD_XSTATS, IFLA_OFFLOAD_XSTATS_CPU_HIT},
 };
 
 static bool get_filter_type(char *name)
-- 
2.4.3

Re: [PATCH iproute2 0/3] update ifstat for new stats

[Roopa Prabhu]

(resending ...failed to send it to the list earlier)

On 11/24/16, 6:12 AM, Nogah Frankel wrote:
> Previously stats were gotten by RTM_GETLINK which return 32 bits based
> statistics. It support 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 change ifstat to the new method, add it the ability to
> choose an extended type of statistic, and add the extended type of SW
> stats for packets that hit cpu.
>
>


(please cc me on the GETSTATS patches)

This looks similar to the one I had submitted here: https://www.spinics.net/lists/netdev/msg375546.html <https://www.spinics.net/lists/netdev/msg375546.html>

There are a few issues with this approach.. (unless they have already been looked at by your patch series).
 This fails new ifstat on older kernels. Moving to 64bit also invalidates existing ifstats history file. 
If you follow the discussion on my patch, there is a way to move to a new history file for 64bit
stats file and still be compatible (ie create a new file for 64 bit stats).

I had started work on fixing these limitations..., but then re-thinking all other new stats in one place
in the context of the new stats api, it is better to extend ip link. This work is also in progress.
here is how we think it should be (also CCing nikolay):

ip link stats /* similar to ip -s link for completeness */
ip link xstats [igmp|lacp]  /* depending on link-type */
ip link afstats [inet|inet6|mpls] /* depending on link-family */
ip link offloadstas [cpu|..]

possible future global non-link stats with 'ip stats [tcp]' and so on.