All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS
@ 2022-04-22  8:30 Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 01/11] libnetlink: Add filtering to rtnl_statsdump_req_filter() Petr Machata
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

A new rtnetlink message, RTM_SETSTATS, has been added recently in kernel
commit ca0a53dcec94 ("Merge branch 'net-hw-counters-for-soft-devices'").

At the same time, RTM_GETSTATS has been around for a while. The users of
this API are spread in a couple different places: "ip link xstats" reads
stats from the IFLA_STATS_LINK_XSTATS and _XSTATS_SLAVE subgroups, "ip
link afstats" then reads IFLA_STATS_AF_SPEC.

Finally, to read IFLA_STATS_LINK_OFFLOAD_XSTATS, one would use ifstats.
This does not seem to be a good fit for IFLA_OFFLOAD_XSTATS_HW_S_INFO in
particular.

The obvious place to expose all these offload stats suites would be
under a new link subcommand "ip link offload_xstats", or similar, which
would then have syntax for both showing stats and setting them.

However, this looks like a good opportunity to introduce a new top-level
command, "ip stats", that would be the go-to place to access anything
backed by RTM_GETSTATS and RTM_SETSTATS.

This patchset therefore does the following:

- It adds the new "stats" infrastructure

- It adds specifically the ability to toggle and show the suites that
  were recently added to Linux, IFLA_OFFLOAD_XSTATS_HW_S_INFO and
  IFLA_OFFLOAD_XSTATS_L3_STATS.

- It adds support to dump IFLA_OFFLOAD_XSTATS_CPU_HIT, which was not
  available under "ip" at all.

- Does all this in a way that is easy to extend for new stats suites.

The patchset proceeds as follows:

- Patches #1 and #2 lay some groundwork and tweak existing code.

- Patch #3 adds the shell of the new "ip stats" command.

- Patch #4 adds "ip stats set" and the ability to toggle l3_stats in
  particular.

- Patch #5 adds "ip stats show", but no actual stats suites.

- Patches #6-#9 add support for showing individual stats suites:
  respectively, IFLA_STATS_LINK_64, IFLA_OFFLOAD_XSTATS_CPU_HIT,
  IFLA_OFFLOAD_XSTATS_HW_S_INFO and IFLA_OFFLOAD_XSTATS_L3_STATS.

- Patch #10 adds support for monitoring stats events to "ip monitor".

- Patch #11 adds man page verbiage for the above.

The plan is to contribute support for afstats and xstats in a follow-up
patch set.

Petr Machata (11):
  libnetlink: Add filtering to rtnl_statsdump_req_filter()
  ip: Publish functions for stats formatting
  ip: Add a new family of commands, "stats"
  ipstats: Add a "set" command
  ipstats: Add a shell of "show" command
  ipstats: Add a group "link"
  ipstats: Add a group "offload", subgroup "cpu_hit"
  ipstats: Add offload subgroup "hw_stats_info"
  ipstats: Add offload subgroup "l3_stats"
  ipmonitor: Add monitoring support for stats events
  man: Add man pages for the "stats" functions

 bridge/vlan.c         |    6 +-
 include/libnetlink.h  |   11 +-
 ip/Makefile           |    3 +-
 ip/ip.c               |    1 +
 ip/ip_common.h        |   31 +
 ip/ipaddress.c        |   33 +-
 ip/iplink.c           |    3 +-
 ip/iplink_xstats.c    |    3 +-
 ip/ipmonitor.c        |   16 +-
 ip/ipstats.c          | 1241 +++++++++++++++++++++++++++++++++++++++++
 lib/libnetlink.c      |   19 +-
 man/man8/ip-monitor.8 |    2 +-
 man/man8/ip-stats.8   |  160 ++++++
 man/man8/ip.8         |    7 +-
 misc/ifstat.c         |    2 +-
 15 files changed, 1512 insertions(+), 26 deletions(-)
 create mode 100644 ip/ipstats.c
 create mode 100644 man/man8/ip-stats.8

-- 
2.31.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 01/11] libnetlink: Add filtering to rtnl_statsdump_req_filter()
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 02/11] ip: Publish functions for stats formatting Petr Machata
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

A number of functions in the rtnl_*_req family accept a caller-provided
callback to set up arbitrary filtering. rtnl_statsdump_req_filter()
currently only allows setting a field in the IFSM header, not custom
attributes. So far these were not necessary, but with introduction of more
detailed filtering settings, the callback becomes necessary.

To that end, add a filter_fn and filter_data arguments to the function.
Unlike the other filters, this one is typed to expect an IFSM pointer, to
permit tweaking the header itself as well.

Pass NULLs in the existing callers.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 bridge/vlan.c        |  6 ++++--
 include/libnetlink.h | 11 ++++++++++-
 ip/iplink.c          |  3 ++-
 ip/iplink_xstats.c   |  3 ++-
 lib/libnetlink.c     | 19 ++++++++++++++-----
 misc/ifstat.c        |  2 +-
 6 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 8300f353..390a2037 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -1179,7 +1179,8 @@ static int vlan_show(int argc, char **argv, int subject)
 		__u32 filt_mask;
 
 		filt_mask = IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_XSTATS);
-		if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask) < 0) {
+		if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask,
+					      NULL, NULL) < 0) {
 			perror("Cannot send dump request");
 			exit(1);
 		}
@@ -1194,7 +1195,8 @@ static int vlan_show(int argc, char **argv, int subject)
 		}
 
 		filt_mask = IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_XSTATS_SLAVE);
-		if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask) < 0) {
+		if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask,
+					      NULL, NULL) < 0) {
 			perror("Cannot send slave dump request");
 			exit(1);
 		}
diff --git a/include/libnetlink.h b/include/libnetlink.h
index 9e4cc101..372c3706 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -37,6 +37,12 @@ struct nlmsg_chain {
 	struct nlmsg_list *tail;
 };
 
+struct ipstats_req {
+	struct nlmsghdr nlh;
+	struct if_stats_msg ifsm;
+	char buf[128];
+};
+
 extern int rcvbuf;
 
 int rtnl_open(struct rtnl_handle *rth, unsigned int subscriptions)
@@ -88,7 +94,10 @@ int rtnl_fdb_linkdump_req_filter_fn(struct rtnl_handle *rth,
 int rtnl_nsiddump_req_filter_fn(struct rtnl_handle *rth, int family,
 				req_filter_fn_t filter_fn)
 	__attribute__((warn_unused_result));
-int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
+int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask,
+			      int (*filter_fn)(struct ipstats_req *req,
+					       void *data),
+			      void *filter_data)
 	__attribute__((warn_unused_result));
 int rtnl_dump_request(struct rtnl_handle *rth, int type, void *req,
 			     int len)
diff --git a/ip/iplink.c b/ip/iplink.c
index dc76a12b..23eb6c6e 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1644,7 +1644,8 @@ static int iplink_afstats(int argc, char **argv)
 		}
 	}
 
-	if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask) < 0) {
+	if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask,
+				      NULL, NULL) < 0) {
 		perror("Cannont send dump request");
 		return 1;
 	}
diff --git a/ip/iplink_xstats.c b/ip/iplink_xstats.c
index c64e6885..1d180b0b 100644
--- a/ip/iplink_xstats.c
+++ b/ip/iplink_xstats.c
@@ -65,7 +65,8 @@ int iplink_ifla_xstats(int argc, char **argv)
 	else
 		filt_mask = IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_XSTATS);
 
-	if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask) < 0) {
+	if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC, filt_mask,
+				      NULL, NULL) < 0) {
 		perror("Cannont send dump request");
 		return -1;
 	}
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 6d1b1187..4d33e4dd 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -619,12 +619,13 @@ int rtnl_fdb_linkdump_req_filter_fn(struct rtnl_handle *rth,
 	return send(rth->fd, &req, sizeof(req), 0);
 }
 
-int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
+int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam,
+			      __u32 filt_mask,
+			      int (*filter_fn)(struct ipstats_req *req,
+					       void *data),
+			      void *filter_data)
 {
-	struct {
-		struct nlmsghdr nlh;
-		struct if_stats_msg ifsm;
-	} req;
+	struct ipstats_req req;
 
 	memset(&req, 0, sizeof(req));
 	req.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct if_stats_msg));
@@ -635,6 +636,14 @@ int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
 	req.ifsm.family = fam;
 	req.ifsm.filter_mask = filt_mask;
 
+	if (filter_fn) {
+		int err;
+
+		err = filter_fn(&req, filter_data);
+		if (err)
+			return err;
+	}
+
 	return send(rth->fd, &req, sizeof(req), 0);
 }
 
diff --git a/misc/ifstat.c b/misc/ifstat.c
index d4a33429..291f288b 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -202,7 +202,7 @@ static void load_info(void)
 		ll_init_map(&rth);
 		filter_mask = IFLA_STATS_FILTER_BIT(filter_type);
 		if (rtnl_statsdump_req_filter(&rth, AF_UNSPEC,
-					      filter_mask) < 0) {
+					      filter_mask, NULL, NULL) < 0) {
 			perror("Cannot send dump request");
 			exit(1);
 		}
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 02/11] ip: Publish functions for stats formatting
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 01/11] libnetlink: Add filtering to rtnl_statsdump_req_filter() Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 03/11] ip: Add a new family of commands, "stats" Petr Machata
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Formatting struct rtnl_link_stats64 will be useful outside of iplink.c as
well. Extract from __print_link_stats() a new function, print_stats64(),
make it non-static and publish in the header file.

Additionally, publish the helper size_columns(), which will be useful for
formatting the new struct rtnl_hw_stats64.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/ip_common.h |  3 +++
 ip/ipaddress.c | 33 ++++++++++++++++++++++-----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index ea04c8ff..51a7edc7 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -171,4 +171,7 @@ void print_rta_ifidx(FILE *fp, __u32 ifidx, const char *prefix);
 void __print_rta_gateway(FILE *fp, unsigned char family, const char *gateway);
 void print_rta_gateway(FILE *fp, unsigned char family,
 		       const struct rtattr *rta);
+void size_columns(unsigned int cols[], unsigned int n, ...);
+void print_stats64(FILE *fp, struct rtnl_link_stats64 *s,
+		   const struct rtattr *carrier_changes, const char *what);
 #endif /* _IP_COMMON_H_ */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index a80996ef..17341d28 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -546,7 +546,7 @@ static void print_vfinfo(FILE *fp, struct ifinfomsg *ifi, struct rtattr *vfinfo)
 		print_vf_stats64(fp, vf[IFLA_VF_STATS]);
 }
 
-static void size_columns(unsigned int cols[], unsigned int n, ...)
+void size_columns(unsigned int cols[], unsigned int n, ...)
 {
 	unsigned int i, len;
 	uint64_t val, powi;
@@ -680,10 +680,10 @@ static void print_vf_stats64(FILE *fp, struct rtattr *vfstats)
 	}
 }
 
-static void __print_link_stats(FILE *fp, struct rtattr *tb[])
+void print_stats64(FILE *fp, struct rtnl_link_stats64 *s,
+		   const struct rtattr *carrier_changes,
+		   const char *what)
 {
-	const struct rtattr *carrier_changes = tb[IFLA_CARRIER_CHANGES];
-	struct rtnl_link_stats64 _s, *s = &_s;
 	unsigned int cols[] = {
 		strlen("*X errors:"),
 		strlen("packets"),
@@ -693,14 +693,10 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[])
 		strlen("overrun"),
 		strlen("compressed"),
 	};
-	int ret;
-
-	ret = get_rtnl_link_stats_rta(s, tb);
-	if (ret < 0)
-		return;
 
 	if (is_json_context()) {
-		open_json_object((ret == sizeof(*s)) ? "stats64" : "stats");
+		if (what)
+			open_json_object(what);
 
 		/* RX stats */
 		open_json_object("rx");
@@ -771,7 +767,8 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[])
 		}
 
 		close_json_object();
-		close_json_object();
+		if (what)
+			close_json_object();
 	} else {
 		size_columns(cols, ARRAY_SIZE(cols),
 			     s->rx_bytes, s->rx_packets, s->rx_errors,
@@ -870,6 +867,20 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[])
 	}
 }
 
+static void __print_link_stats(FILE *fp, struct rtattr *tb[])
+{
+	const struct rtattr *carrier_changes = tb[IFLA_CARRIER_CHANGES];
+	struct rtnl_link_stats64 _s, *s = &_s;
+	int ret;
+
+	ret = get_rtnl_link_stats_rta(s, tb);
+	if (ret < 0)
+		return;
+
+	print_stats64(fp, s, carrier_changes,
+		      (ret == sizeof(*s)) ? "stats64" : "stats");
+}
+
 static void print_link_stats(FILE *fp, struct nlmsghdr *n)
 {
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 03/11] ip: Add a new family of commands, "stats"
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 01/11] libnetlink: Add filtering to rtnl_statsdump_req_filter() Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 02/11] ip: Publish functions for stats formatting Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 04/11] ipstats: Add a "set" command Petr Machata
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Add a core of a new frontend tool for interfacing with the RTM_*STATS
family of messages. The following patches will add subcommands for showing
and setting individual statistics suites.

Note that in this patch, "ip stats" is made to be an invalid command line.
This will be changed in later patches to default to "show" when that is
introduced.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/Makefile    |  3 ++-
 ip/ip.c        |  1 +
 ip/ip_common.h |  1 +
 ip/ipstats.c   | 31 +++++++++++++++++++++++++++++++
 4 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 ip/ipstats.c

diff --git a/ip/Makefile b/ip/Makefile
index e06a7c84..6c2e0720 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -12,7 +12,8 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
     iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o ipila.o \
     ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o \
     ipnexthop.o ipmptcp.o iplink_bareudp.o iplink_wwan.o ipioam6.o \
-    iplink_amt.o iplink_batadv.o iplink_gtp.o iplink_virt_wifi.o
+    iplink_amt.o iplink_batadv.o iplink_gtp.o iplink_virt_wifi.o \
+    ipstats.o
 
 RTMONOBJ=rtmon.o
 
diff --git a/ip/ip.c b/ip/ip.c
index c784f819..82282bab 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -123,6 +123,7 @@ static const struct cmd {
 	{ "mptcp",	do_mptcp },
 	{ "ioam",	do_ioam6 },
 	{ "help",	do_help },
+	{ "stats",	do_ipstats },
 	{ 0 }
 };
 
diff --git a/ip/ip_common.h b/ip/ip_common.h
index 51a7edc7..53866d7a 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -90,6 +90,7 @@ int do_seg6(int argc, char **argv);
 int do_ipnh(int argc, char **argv);
 int do_mptcp(int argc, char **argv);
 int do_ioam6(int argc, char **argv);
+int do_ipstats(int argc, char **argv);
 
 int iplink_get(char *name, __u32 filt_mask);
 int iplink_ifla_xstats(int argc, char **argv);
diff --git a/ip/ipstats.c b/ip/ipstats.c
new file mode 100644
index 00000000..099e18a2
--- /dev/null
+++ b/ip/ipstats.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include "utils.h"
+#include "ip_common.h"
+
+static int do_help(void)
+{
+	fprintf(stderr,
+		"Usage: ip stats help\n"
+		);
+
+	return 0;
+}
+
+int do_ipstats(int argc, char **argv)
+{
+	int rc;
+
+	if (argc == 0) {
+		do_help();
+		rc = -1;
+	} else if (strcmp(*argv, "help") == 0) {
+		do_help();
+		rc = 0;
+	} else {
+		fprintf(stderr, "Command \"%s\" is unknown, try \"ip stats help\".\n",
+			*argv);
+		rc = -1;
+	}
+
+	return rc;
+}
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 04/11] ipstats: Add a "set" command
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (2 preceding siblings ...)
  2022-04-22  8:30 ` [PATCH iproute2-next 03/11] ip: Add a new family of commands, "stats" Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 05/11] ipstats: Add a shell of "show" command Petr Machata
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Add a command to allow toggling HW stats. An example usage:

 # ip stats set dev swp1 l3_stats on

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/ipstats.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/ip/ipstats.c b/ip/ipstats.c
index 099e18a2..1f5b3f77 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -1,4 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
+#include <errno.h>
+
 #include "utils.h"
 #include "ip_common.h"
 
@@ -6,11 +8,85 @@ static int do_help(void)
 {
 	fprintf(stderr,
 		"Usage: ip stats help\n"
+		"       ip stats set dev DEV l3_stats { on | off }\n"
 		);
 
 	return 0;
 }
 
+static int ipstats_set_do(int ifindex, int at, bool enable)
+{
+	struct ipstats_req req = {
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct if_stats_msg)),
+		.nlh.nlmsg_flags = NLM_F_REQUEST,
+		.nlh.nlmsg_type = RTM_SETSTATS,
+		.ifsm.family = PF_UNSPEC,
+		.ifsm.ifindex = ifindex,
+	};
+
+	addattr8(&req.nlh, sizeof(req), at, enable);
+
+	if (rtnl_talk(&rth, &req.nlh, NULL) < 0)
+		return -2;
+	return 0;
+}
+
+static int ipstats_set(int argc, char **argv)
+{
+	const char *dev = NULL;
+	bool enable = false;
+	int ifindex;
+	int at = 0;
+
+	while (argc > 0) {
+		if (strcmp(*argv, "dev") == 0) {
+			NEXT_ARG();
+			if (dev)
+				duparg2("dev", *argv);
+			if (check_ifname(*argv))
+				invarg("\"dev\" not a valid ifname", *argv);
+			dev = *argv;
+		} else if (strcmp(*argv, "l3_stats") == 0) {
+			int err;
+
+			NEXT_ARG();
+			if (at) {
+				fprintf(stderr, "A statistics suite to toggle was already given.\n");
+				return -EINVAL;
+			}
+			at = IFLA_STATS_SET_OFFLOAD_XSTATS_L3_STATS;
+			enable = parse_on_off("l3_stats", *argv, &err);
+			if (err)
+				return err;
+		} else if (strcmp(*argv, "help") == 0) {
+			do_help();
+			return 0;
+		} else {
+			fprintf(stderr, "What is \"%s\"?\n", *argv);
+			do_help();
+			return -EINVAL;
+		}
+
+		NEXT_ARG_FWD();
+	}
+
+	if (!dev) {
+		fprintf(stderr, "Not enough information: \"dev\" argument is required.\n");
+		exit(-1);
+	}
+
+	if (!at) {
+		fprintf(stderr, "Not enough information: stat type to toggle is required.\n");
+		exit(-1);
+	}
+
+	ifindex = ll_name_to_index(dev);
+	if (!ifindex)
+		return nodev(dev);
+
+	return ipstats_set_do(ifindex, at, enable);
+}
+
 int do_ipstats(int argc, char **argv)
 {
 	int rc;
@@ -21,6 +97,8 @@ int do_ipstats(int argc, char **argv)
 	} else if (strcmp(*argv, "help") == 0) {
 		do_help();
 		rc = 0;
+	} else if (strcmp(*argv, "set") == 0) {
+		rc = ipstats_set(argc-1, argv+1);
 	} else {
 		fprintf(stderr, "Command \"%s\" is unknown, try \"ip stats help\".\n",
 			*argv);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 05/11] ipstats: Add a shell of "show" command
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (3 preceding siblings ...)
  2022-04-22  8:30 ` [PATCH iproute2-next 04/11] ipstats: Add a "set" command Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 06/11] ipstats: Add a group "link" Petr Machata
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Add the scaffolding necessary for adding individual stats suites to show.

Expose some ipstats artifacts in ip_common.h. These will be used to support
"xstats" in a follow-up patchset.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/ip_common.h |  26 +++
 ip/ipstats.c   | 614 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 638 insertions(+), 2 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index 53866d7a..8b0a6426 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -158,6 +158,32 @@ void xdp_dump(FILE *fp, struct rtattr *tb, bool link, bool details);
 __u32 ipvrf_get_table(const char *name);
 int name_is_vrf(const char *name);
 
+/* ipstats.c */
+enum ipstats_stat_desc_kind {
+	IPSTATS_STAT_DESC_KIND_LEAF,
+	IPSTATS_STAT_DESC_KIND_GROUP,
+};
+
+struct ipstats_stat_dump_filters;
+struct ipstats_stat_show_attrs;
+
+struct ipstats_stat_desc {
+	const char *name;
+	enum ipstats_stat_desc_kind kind;
+	union {
+		struct {
+			const struct ipstats_stat_desc **subs;
+			size_t nsubs;
+		};
+		struct {
+			void (*pack)(struct ipstats_stat_dump_filters *filters,
+				     const struct ipstats_stat_desc *desc);
+			int (*show)(struct ipstats_stat_show_attrs *attrs,
+				    const struct ipstats_stat_desc *desc);
+		};
+	};
+};
+
 #ifndef	INFINITY_LIFE_TIME
 #define     INFINITY_LIFE_TIME      0xFFFFFFFFU
 #endif
diff --git a/ip/ipstats.c b/ip/ipstats.c
index 1f5b3f77..79f7b1ff 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -1,19 +1,624 @@
 // SPDX-License-Identifier: GPL-2.0+
+#include <assert.h>
 #include <errno.h>
 
 #include "utils.h"
 #include "ip_common.h"
 
+struct ipstats_stat_dump_filters {
+	/* mask[0] filters outer attributes. Then individual nests have their
+	 * filtering mask at the index of the nested attribute.
+	 */
+	__u32 mask[IFLA_STATS_MAX + 1];
+};
+
+struct ipstats_stat_show_attrs {
+	struct if_stats_msg *ifsm;
+	int len;
+
+	/* tbs[0] contains top-level attribute table. Then individual nests have
+	 * their attribute tables at the index of the nested attribute.
+	 */
+	struct rtattr **tbs[IFLA_STATS_MAX + 1];
+};
+
+static const char *const ipstats_levels[] = {
+	"group",
+	"subgroup",
+};
+
+enum {
+	IPSTATS_LEVELS_COUNT = ARRAY_SIZE(ipstats_levels),
+};
+
+struct ipstats_sel {
+	const char *sel[IPSTATS_LEVELS_COUNT];
+};
+
+struct ipstats_stat_enabled_one {
+	const struct ipstats_stat_desc *desc;
+	struct ipstats_sel sel;
+};
+
+struct ipstats_stat_enabled {
+	struct ipstats_stat_enabled_one *enabled;
+	size_t nenabled;
+};
+
+static const unsigned int ipstats_stat_ifla_max[] = {
+	[0] = IFLA_STATS_MAX,
+	[IFLA_STATS_LINK_XSTATS] = LINK_XSTATS_TYPE_MAX,
+	[IFLA_STATS_LINK_XSTATS_SLAVE] = LINK_XSTATS_TYPE_MAX,
+	[IFLA_STATS_LINK_OFFLOAD_XSTATS] = IFLA_OFFLOAD_XSTATS_MAX,
+	[IFLA_STATS_AF_SPEC] = AF_MAX - 1,
+};
+
+static_assert(ARRAY_SIZE(ipstats_stat_ifla_max) == IFLA_STATS_MAX + 1,
+	      "An IFLA_STATS attribute is missing from the ifla_max table");
+
+static int
+ipstats_stat_show_attrs_alloc_tb(struct ipstats_stat_show_attrs *attrs,
+				 unsigned int group)
+{
+	unsigned int ifla_max;
+	int err;
+
+	assert(group < ARRAY_SIZE(ipstats_stat_ifla_max));
+	assert(group < ARRAY_SIZE(attrs->tbs));
+	ifla_max = ipstats_stat_ifla_max[group];
+	assert(ifla_max != 0);
+
+	if (attrs->tbs[group])
+		return 0;
+
+	attrs->tbs[group] = calloc(ifla_max + 1, sizeof(*attrs->tbs[group]));
+	if (attrs->tbs[group] == NULL)
+		return -ENOMEM;
+
+	if (group == 0)
+		err = parse_rtattr(attrs->tbs[group], ifla_max,
+				   IFLA_STATS_RTA(attrs->ifsm), attrs->len);
+	else
+		err = parse_rtattr_nested(attrs->tbs[group], ifla_max,
+					  attrs->tbs[0][group]);
+
+	if (err != 0) {
+		free(attrs->tbs[group]);
+		attrs->tbs[group] = NULL;
+	}
+	return err;
+}
+
+static void
+ipstats_stat_show_attrs_free(struct ipstats_stat_show_attrs *attrs)
+{
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(attrs->tbs); i++)
+		free(attrs->tbs[i]);
+}
+
+static const struct ipstats_stat_desc *ipstats_stat_desc_toplev_subs[] = {
+};
+
+static const struct ipstats_stat_desc ipstats_stat_desc_toplev_group = {
+	.name = "top-level",
+	.kind = IPSTATS_STAT_DESC_KIND_GROUP,
+	.subs = ipstats_stat_desc_toplev_subs,
+	.nsubs = ARRAY_SIZE(ipstats_stat_desc_toplev_subs),
+};
+
+static void ipstats_show_group(const struct ipstats_sel *sel)
+{
+	int i;
+
+	for (i = 0; i < IPSTATS_LEVELS_COUNT; i++) {
+		if (sel->sel[i] == NULL)
+			break;
+		print_string(PRINT_JSON, ipstats_levels[i], NULL, sel->sel[i]);
+		print_string(PRINT_FP, NULL, " %s ", ipstats_levels[i]);
+		print_string(PRINT_FP, NULL, "%s", sel->sel[i]);
+	}
+}
+
+static int
+ipstats_process_ifsm(struct nlmsghdr *answer,
+		     struct ipstats_stat_enabled *enabled)
+{
+	struct ipstats_stat_show_attrs show_attrs = {};
+	const char *dev;
+	int err = 0;
+	int i;
+
+	show_attrs.ifsm = NLMSG_DATA(answer);
+	show_attrs.len = (answer->nlmsg_len -
+			  NLMSG_LENGTH(sizeof(*show_attrs.ifsm)));
+	if (show_attrs.len < 0) {
+		fprintf(stderr, "BUG: wrong nlmsg len %d\n", show_attrs.len);
+		return -EINVAL;
+	}
+
+	err = ipstats_stat_show_attrs_alloc_tb(&show_attrs, 0);
+	if (err != 0) {
+		fprintf(stderr, "Error parsing netlink answer: %s\n",
+			strerror(err));
+		return err;
+	}
+
+	dev = ll_index_to_name(show_attrs.ifsm->ifindex);
+
+	for (i = 0; i < enabled->nenabled; i++) {
+		const struct ipstats_stat_desc *desc = enabled->enabled[i].desc;
+
+		open_json_object(NULL);
+		print_int(PRINT_ANY, "ifindex", "%d:",
+			  show_attrs.ifsm->ifindex);
+		print_color_string(PRINT_ANY, COLOR_IFNAME,
+				   "ifname", " %s:", dev);
+		ipstats_show_group(&enabled->enabled[i].sel);
+		err = desc->show(&show_attrs, desc);
+		if (err != 0)
+			goto out;
+		close_json_object();
+		print_nl();
+	}
+
+out:
+	ipstats_stat_show_attrs_free(&show_attrs);
+	return err;
+}
+
+static bool
+ipstats_req_should_filter_at(struct ipstats_stat_dump_filters *filters, int at)
+{
+	return filters->mask[at] != 0 &&
+	       filters->mask[at] != (1 << ipstats_stat_ifla_max[at]) - 1;
+}
+
+static int
+ipstats_req_add_filters(struct ipstats_req *req, void *data)
+{
+	struct ipstats_stat_dump_filters dump_filters = {};
+	struct ipstats_stat_enabled *enabled = data;
+	bool get_filters = false;
+	int i;
+
+	for (i = 0; i < enabled->nenabled; i++)
+		enabled->enabled[i].desc->pack(&dump_filters,
+					       enabled->enabled[i].desc);
+
+	for (i = 1; i < ARRAY_SIZE(dump_filters.mask); i++) {
+		if (ipstats_req_should_filter_at(&dump_filters, i)) {
+			get_filters = true;
+			break;
+		}
+	}
+
+	req->ifsm.filter_mask = dump_filters.mask[0];
+	if (get_filters) {
+		struct rtattr *nest;
+
+		nest = addattr_nest(&req->nlh, sizeof(*req),
+				    IFLA_STATS_GET_FILTERS | NLA_F_NESTED);
+
+		for (i = 1; i < ARRAY_SIZE(dump_filters.mask); i++) {
+			if (ipstats_req_should_filter_at(&dump_filters, i))
+				addattr32(&req->nlh, sizeof(*req), i,
+					  dump_filters.mask[i]);
+		}
+
+		addattr_nest_end(&req->nlh, nest);
+	}
+
+	return 0;
+}
+
+static int
+ipstats_show_one(int ifindex, struct ipstats_stat_enabled *enabled)
+{
+	struct ipstats_req req = {
+		.nlh.nlmsg_flags = NLM_F_REQUEST,
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct if_stats_msg)),
+		.nlh.nlmsg_type = RTM_GETSTATS,
+		.ifsm.family = PF_UNSPEC,
+		.ifsm.ifindex = ifindex,
+	};
+	struct nlmsghdr *answer;
+	int err = 0;
+
+	ipstats_req_add_filters(&req, enabled);
+	if (rtnl_talk(&rth, &req.nlh, &answer) < 0)
+		return -2;
+	err = ipstats_process_ifsm(answer, enabled);
+	free(answer);
+
+	return err;
+}
+
+static int ipstats_dump_one(struct nlmsghdr *n, void *arg)
+{
+	struct ipstats_stat_enabled *enabled = arg;
+	int rc;
+
+	rc = ipstats_process_ifsm(n, enabled);
+	if (rc)
+		return rc;
+
+	print_nl();
+	return 0;
+}
+
+static int ipstats_dump(struct ipstats_stat_enabled *enabled)
+{
+	int rc = 0;
+
+	if (rtnl_statsdump_req_filter(&rth, PF_UNSPEC, 0,
+				      ipstats_req_add_filters,
+				      enabled) < 0) {
+		perror("Cannot send dump request");
+		return -2;
+	}
+
+	if (rtnl_dump_filter(&rth, ipstats_dump_one, enabled) < 0) {
+		fprintf(stderr, "Dump terminated\n");
+		rc = -2;
+	}
+
+	fflush(stdout);
+	return rc;
+}
+
+static int
+ipstats_show_do(int ifindex, struct ipstats_stat_enabled *enabled)
+{
+	int rc;
+
+	new_json_obj(json);
+	if (ifindex)
+		rc = ipstats_show_one(ifindex, enabled);
+	else
+		rc = ipstats_dump(enabled);
+	delete_json_obj();
+
+	return rc;
+}
+
+static int ipstats_add_enabled(struct ipstats_stat_enabled_one ens[],
+			       size_t nens,
+			       struct ipstats_stat_enabled *enabled)
+{
+	struct ipstats_stat_enabled_one *new_en;
+
+	new_en = realloc(enabled->enabled,
+			 sizeof(*new_en) * (enabled->nenabled + nens));
+	if (new_en == NULL)
+		return -ENOMEM;
+
+	enabled->enabled = new_en;
+	while (nens-- > 0)
+		enabled->enabled[enabled->nenabled++] = *ens++;
+	return 0;
+}
+
+static void ipstats_select_push(struct ipstats_sel *sel, const char *name)
+{
+	int i;
+
+	for (i = 0; i < IPSTATS_LEVELS_COUNT; i++)
+		if (sel->sel[i] == NULL) {
+			sel->sel[i] = name;
+			return;
+		}
+
+	assert(false);
+}
+
+static int
+ipstats_enable_recursively(const struct ipstats_stat_desc *desc,
+			   struct ipstats_stat_enabled *enabled,
+			   const struct ipstats_sel *sel)
+{
+	bool found = false;
+	size_t i;
+	int err;
+
+	if (desc->kind == IPSTATS_STAT_DESC_KIND_LEAF) {
+		struct ipstats_stat_enabled_one en[] = {{
+			.desc = desc,
+			.sel = *sel,
+		}};
+
+		return ipstats_add_enabled(en, ARRAY_SIZE(en), enabled);
+	}
+
+	for (i = 0; i < desc->nsubs; i++) {
+		struct ipstats_sel subsel = *sel;
+
+		ipstats_select_push(&subsel, desc->subs[i]->name);
+		err = ipstats_enable_recursively(desc->subs[i], enabled,
+						 &subsel);
+		if (err == -ENOENT)
+			continue;
+		if (err != 0)
+			return err;
+		found = true;
+	}
+
+	return found ? 0 : -ENOENT;
+}
+
+static int ipstats_comp_enabled(const void *a, const void *b)
+{
+	const struct ipstats_stat_enabled_one *en_a = a;
+	const struct ipstats_stat_enabled_one *en_b = b;
+
+	if (en_a->desc < en_b->desc)
+		return -1;
+	if (en_a->desc > en_b->desc)
+		return 1;
+
+	return 0;
+}
+
+static void ipstats_enabled_free(struct ipstats_stat_enabled *enabled)
+{
+	free(enabled->enabled);
+}
+
+static const struct ipstats_stat_desc *
+ipstats_stat_desc_find(const struct ipstats_stat_desc *desc,
+		       const char *name)
+{
+	size_t i;
+
+	assert(desc->kind == IPSTATS_STAT_DESC_KIND_GROUP);
+	for (i = 0; i < desc->nsubs; i++) {
+		const struct ipstats_stat_desc *sub = desc->subs[i];
+
+		if (strcmp(sub->name, name) == 0)
+			return sub;
+	}
+
+	return NULL;
+}
+
+static const struct ipstats_stat_desc *
+ipstats_enable_find_stat_desc(struct ipstats_sel *sel)
+{
+	const struct ipstats_stat_desc *toplev = &ipstats_stat_desc_toplev_group;
+	const struct ipstats_stat_desc *desc = toplev;
+	int i;
+
+	for (i = 0; i < IPSTATS_LEVELS_COUNT; i++) {
+		const struct ipstats_stat_desc *next_desc;
+
+		if (sel->sel[i] == NULL)
+			break;
+		if (desc->kind == IPSTATS_STAT_DESC_KIND_LEAF) {
+			fprintf(stderr, "Error: %s %s requested inside leaf %s %s\n",
+				ipstats_levels[i], sel->sel[i],
+				ipstats_levels[i - 1], desc->name);
+			return NULL;
+		}
+
+		next_desc = ipstats_stat_desc_find(desc, sel->sel[i]);
+		if (next_desc == NULL) {
+			fprintf(stderr, "Error: no %s named %s found inside %s\n",
+				ipstats_levels[i], sel->sel[i], desc->name);
+			return NULL;
+		}
+
+		desc = next_desc;
+	}
+
+	return desc;
+}
+
+static int ipstats_enable(struct ipstats_sel *sel,
+			  struct ipstats_stat_enabled *enabled)
+{
+	struct ipstats_stat_enabled new_enabled = {};
+	const struct ipstats_stat_desc *desc;
+	size_t i, j;
+	int err = 0;
+
+	desc = ipstats_enable_find_stat_desc(sel);
+	if (desc == NULL)
+		return -EINVAL;
+
+	err = ipstats_enable_recursively(desc, &new_enabled, sel);
+	if (err != 0)
+		return err;
+
+	err = ipstats_add_enabled(new_enabled.enabled, new_enabled.nenabled,
+				  enabled);
+	if (err != 0)
+		goto out;
+
+	qsort(enabled->enabled, enabled->nenabled, sizeof(*enabled->enabled),
+	      ipstats_comp_enabled);
+
+	for (i = 1, j = 1; i < enabled->nenabled; i++) {
+		if (enabled->enabled[i].desc != enabled->enabled[j - 1].desc)
+			enabled->enabled[j++] = enabled->enabled[i];
+	}
+	enabled->nenabled = j;
+
+out:
+	ipstats_enabled_free(&new_enabled);
+	return err;
+}
+
+static int ipstats_enable_check(struct ipstats_sel *sel,
+				struct ipstats_stat_enabled *enabled)
+{
+	int err;
+	int i;
+
+	err = ipstats_enable(sel, enabled);
+	if (err == -ENOENT) {
+		fprintf(stderr, "The request for");
+		for (i = 0; i < IPSTATS_LEVELS_COUNT; i++)
+			if (sel->sel[i] != NULL)
+				fprintf(stderr, " %s %s",
+					ipstats_levels[i], sel->sel[i]);
+			else
+				break;
+		fprintf(stderr, " did not match any known stats.\n");
+	}
+
+	return err;
+}
+
 static int do_help(void)
 {
+	const struct ipstats_stat_desc *toplev = &ipstats_stat_desc_toplev_group;
+	int i;
+
 	fprintf(stderr,
 		"Usage: ip stats help\n"
+		"       ip stats show [ dev DEV ] [ group GROUP [ subgroup SUBGROUP ] ... ] ...\n"
 		"       ip stats set dev DEV l3_stats { on | off }\n"
 		);
 
+	for (i = 0; i < toplev->nsubs; i++) {
+		const struct ipstats_stat_desc *desc = toplev->subs[i];
+
+		if (i == 0)
+			fprintf(stderr, "GROUP := { %s", desc->name);
+		else
+			fprintf(stderr, " | %s", desc->name);
+	}
+	if (i > 0)
+		fprintf(stderr, " }\n");
+
+	for (i = 0; i < toplev->nsubs; i++) {
+		const struct ipstats_stat_desc *desc = toplev->subs[i];
+		bool opened = false;
+		size_t j;
+
+		if (desc->kind != IPSTATS_STAT_DESC_KIND_GROUP)
+			continue;
+
+		for (j = 0; j < desc->nsubs; j++) {
+			if (j == 0)
+				fprintf(stderr, "%s SUBGROUP := {", desc->name);
+			else
+				fprintf(stderr, " |");
+			fprintf(stderr, " %s", desc->subs[j]->name);
+			opened = true;
+
+			if (desc->subs[j]->kind != IPSTATS_STAT_DESC_KIND_GROUP)
+				continue;
+		}
+		if (opened)
+			fprintf(stderr, " }\n");
+	}
+
 	return 0;
 }
 
+static int ipstats_select(struct ipstats_sel *old_sel,
+			  const char *new_sel, int level,
+			  struct ipstats_stat_enabled *enabled)
+{
+	int err;
+	int i;
+
+	for (i = 0; i < level; i++) {
+		if (old_sel->sel[i] == NULL) {
+			fprintf(stderr, "Error: %s %s requested without selecting a %s first\n",
+				ipstats_levels[level], new_sel,
+				ipstats_levels[i]);
+			return -EINVAL;
+		}
+	}
+
+	for (i = level; i < IPSTATS_LEVELS_COUNT; i++) {
+		if (old_sel->sel[i] != NULL) {
+			err = ipstats_enable_check(old_sel, enabled);
+			if (err)
+				return err;
+			break;
+		}
+	}
+
+	old_sel->sel[level] = new_sel;
+	for (i = level + 1; i < IPSTATS_LEVELS_COUNT; i++)
+		old_sel->sel[i] = NULL;
+
+	return 0;
+}
+
+static int ipstats_show(int argc, char **argv)
+{
+	struct ipstats_stat_enabled enabled = {};
+	struct ipstats_sel sel = {};
+	const char *dev = NULL;
+	int ifindex;
+	int err;
+	int i;
+
+	while (argc > 0) {
+		if (strcmp(*argv, "dev") == 0) {
+			NEXT_ARG();
+			if (dev != NULL)
+				duparg2("dev", *argv);
+			if (check_ifname(*argv))
+				invarg("\"dev\" not a valid ifname", *argv);
+			dev = *argv;
+		} else if (strcmp(*argv, "help") == 0) {
+			do_help();
+			return 0;
+		} else {
+			bool found_level = false;
+
+			for (i = 0; i < ARRAY_SIZE(ipstats_levels); i++) {
+				if (strcmp(*argv, ipstats_levels[i]) == 0) {
+					NEXT_ARG();
+					err = ipstats_select(&sel, *argv, i,
+							     &enabled);
+					if (err)
+						goto err;
+
+					found_level = true;
+				}
+			}
+
+			if (!found_level) {
+				fprintf(stderr, "What is \"%s\"?\n", *argv);
+				do_help();
+				err = -EINVAL;
+				goto err;
+			}
+		}
+
+		NEXT_ARG_FWD();
+	}
+
+	/* Push whatever was given. */
+	err = ipstats_enable_check(&sel, &enabled);
+	if (err)
+		goto err;
+
+	if (dev) {
+		ifindex = ll_name_to_index(dev);
+		if (!ifindex) {
+			err = nodev(dev);
+			goto err;
+		}
+	} else {
+		ifindex = 0;
+	}
+
+
+	err = ipstats_show_do(ifindex, &enabled);
+
+err:
+	ipstats_enabled_free(&enabled);
+	return err;
+}
+
 static int ipstats_set_do(int ifindex, int at, bool enable)
 {
 	struct ipstats_req req = {
@@ -92,11 +697,16 @@ int do_ipstats(int argc, char **argv)
 	int rc;
 
 	if (argc == 0) {
-		do_help();
-		rc = -1;
+		rc = ipstats_show(0, NULL);
 	} else if (strcmp(*argv, "help") == 0) {
 		do_help();
 		rc = 0;
+	} else if (strcmp(*argv, "show") == 0) {
+		/* Invoking "stats show" implies one -s. Passing -d adds one
+		 * more -s.
+		 */
+		show_stats += show_details + 1;
+		rc = ipstats_show(argc-1, argv+1);
 	} else if (strcmp(*argv, "set") == 0) {
 		rc = ipstats_set(argc-1, argv+1);
 	} else {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 06/11] ipstats: Add a group "link"
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (4 preceding siblings ...)
  2022-04-22  8:30 ` [PATCH iproute2-next 05/11] ipstats: Add a shell of "show" command Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-05-01 14:52   ` Ido Schimmel
  2022-04-22  8:30 ` [PATCH iproute2-next 07/11] ipstats: Add a group "offload", subgroup "cpu_hit" Petr Machata
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Add the "link" top-level group for showing IFLA_STATS_LINK_64 statistics.
For example:

 # ip stats show dev swp1 group link
 4178: swp1: group link
     RX:  bytes packets errors dropped  missed   mcast
          47048     354      0       0       0      64
     TX:  bytes packets errors dropped carrier collsns
          47474     355      0       0       0       0

 # ip -j stats show dev swp1 group link | jq
 [
   {
     "ifindex": 4178,
     "ifname": "swp1",
     "group": "link",
     "stats64": {
       "rx": {
         "bytes": 47048,
         "packets": 354,
         "errors": 0,
         "dropped": 0,
         "over_errors": 0,
         "multicast": 64
       },
       "tx": {
         "bytes": 47474,
         "packets": 355,
         "errors": 0,
         "dropped": 0,
         "carrier_errors": 0,
         "collisions": 0
       }
     }
   }
 ]

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/ipstats.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/ip/ipstats.c b/ip/ipstats.c
index 79f7b1ff..e4f97ddd 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -12,6 +12,15 @@ struct ipstats_stat_dump_filters {
 	__u32 mask[IFLA_STATS_MAX + 1];
 };
 
+static void
+ipstats_stat_desc_enable_bit(struct ipstats_stat_dump_filters *filters,
+			     unsigned int group, unsigned int subgroup)
+{
+	filters->mask[0] |= IFLA_STATS_FILTER_BIT(group);
+	if (subgroup)
+		filters->mask[group] |= IFLA_STATS_FILTER_BIT(subgroup);
+}
+
 struct ipstats_stat_show_attrs {
 	struct if_stats_msg *ifsm;
 	int len;
@@ -89,6 +98,29 @@ ipstats_stat_show_attrs_alloc_tb(struct ipstats_stat_show_attrs *attrs,
 	return err;
 }
 
+static const struct rtattr *
+ipstats_stat_show_get_attr(struct ipstats_stat_show_attrs *attrs,
+			   int group, int subgroup, int *err)
+{
+	int tmp_err;
+
+	if (err == NULL)
+		err = &tmp_err;
+
+	*err = 0;
+	if (subgroup == 0)
+		return attrs->tbs[0][group];
+
+	if (attrs->tbs[0][group] == NULL)
+		return NULL;
+
+	*err = ipstats_stat_show_attrs_alloc_tb(attrs, group);
+	if (*err != 0)
+		return NULL;
+
+	return attrs->tbs[group][subgroup];
+}
+
 static void
 ipstats_stat_show_attrs_free(struct ipstats_stat_show_attrs *attrs)
 {
@@ -98,7 +130,65 @@ ipstats_stat_show_attrs_free(struct ipstats_stat_show_attrs *attrs)
 		free(attrs->tbs[i]);
 }
 
+#define IPSTATS_RTA_PAYLOAD(TYPE, AT)					\
+	({								\
+		const struct rtattr *__at = (AT);			\
+		TYPE *__ret = NULL;					\
+									\
+		if (__at != NULL &&					\
+		    __at->rta_len - RTA_LENGTH(0) >= sizeof(TYPE))	\
+			__ret = RTA_DATA(__at);				\
+		__ret;							\
+	})
+
+static int ipstats_show_64(struct ipstats_stat_show_attrs *attrs,
+			   unsigned int group, unsigned int subgroup)
+{
+	struct rtnl_link_stats64 *stats;
+	const struct rtattr *at;
+	int err;
+
+	at = ipstats_stat_show_get_attr(attrs, group, subgroup, &err);
+	if (at == NULL)
+		return err;
+
+	stats = IPSTATS_RTA_PAYLOAD(struct rtnl_link_stats64, at);
+	if (stats == NULL) {
+		fprintf(stderr, "Error: attribute payload too short");
+		return -EINVAL;
+	}
+
+	open_json_object("stats64");
+	print_stats64(stdout, stats, NULL, NULL);
+	close_json_object();
+	return 0;
+}
+
+static void
+ipstats_stat_desc_pack_link(struct ipstats_stat_dump_filters *filters,
+			    const struct ipstats_stat_desc *desc)
+{
+	ipstats_stat_desc_enable_bit(filters,
+				     IFLA_STATS_LINK_64, 0);
+}
+
+static int
+ipstats_stat_desc_show_link(struct ipstats_stat_show_attrs *attrs,
+			    const struct ipstats_stat_desc *desc)
+{
+	print_nl();
+	return ipstats_show_64(attrs, IFLA_STATS_LINK_64, 0);
+}
+
+static const struct ipstats_stat_desc ipstats_stat_desc_toplev_link = {
+	.name = "link",
+	.kind = IPSTATS_STAT_DESC_KIND_LEAF,
+	.pack = &ipstats_stat_desc_pack_link,
+	.show = &ipstats_stat_desc_show_link,
+};
+
 static const struct ipstats_stat_desc *ipstats_stat_desc_toplev_subs[] = {
+	&ipstats_stat_desc_toplev_link,
 };
 
 static const struct ipstats_stat_desc ipstats_stat_desc_toplev_group = {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 07/11] ipstats: Add a group "offload", subgroup "cpu_hit"
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (5 preceding siblings ...)
  2022-04-22  8:30 ` [PATCH iproute2-next 06/11] ipstats: Add a group "link" Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 08/11] ipstats: Add offload subgroup "hw_stats_info" Petr Machata
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Add a new group, "offload", for showing counters from the
IFLA_STATS_LINK_OFFLOAD_XSTATS nest, and a subgroup "cpu_hit" for the
IFLA_OFFLOAD_XSTATS_CPU_HIT stats suite.

For example:

 # ip stats show dev swp1 group offload subgroup cpu_hit
 4178: swp1: group offload subgroup cpu_hit
     RX:  bytes packets errors dropped  missed   mcast
          45522     353      0       0       0       0
     TX:  bytes packets errors dropped carrier collsns
          46054     355      0       0       0       0

 # ip -j stats show dev swp1 group offload subgroup cpu_hit | jq
 [
   {
     "ifindex": 4178,
     "ifname": "swp1",
     "group": "offload",
     "subgroup": "cpu_hit",
     "stats64": {
       "rx": {
         "bytes": 45522,
         "packets": 353,
         "errors": 0,
         "dropped": 0,
         "over_errors": 0,
         "multicast": 0
       },
       "tx": {
         "bytes": 46054,
         "packets": 355,
         "errors": 0,
         "dropped": 0,
         "carrier_errors": 0,
         "collisions": 0
       }
     }
   }
 ]

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/ipstats.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/ip/ipstats.c b/ip/ipstats.c
index e4f97ddd..283fba4e 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -164,6 +164,42 @@ static int ipstats_show_64(struct ipstats_stat_show_attrs *attrs,
 	return 0;
 }
 
+static void
+ipstats_stat_desc_pack_cpu_hit(struct ipstats_stat_dump_filters *filters,
+			       const struct ipstats_stat_desc *desc)
+{
+	ipstats_stat_desc_enable_bit(filters,
+				     IFLA_STATS_LINK_OFFLOAD_XSTATS,
+				     IFLA_OFFLOAD_XSTATS_CPU_HIT);
+}
+
+static int ipstats_stat_desc_show_cpu_hit(struct ipstats_stat_show_attrs *attrs,
+					  const struct ipstats_stat_desc *desc)
+{
+	print_nl();
+	return ipstats_show_64(attrs,
+			       IFLA_STATS_LINK_OFFLOAD_XSTATS,
+			       IFLA_OFFLOAD_XSTATS_CPU_HIT);
+}
+
+static const struct ipstats_stat_desc ipstats_stat_desc_offload_cpu_hit = {
+	.name = "cpu_hit",
+	.kind = IPSTATS_STAT_DESC_KIND_LEAF,
+	.pack = &ipstats_stat_desc_pack_cpu_hit,
+	.show = &ipstats_stat_desc_show_cpu_hit,
+};
+
+static const struct ipstats_stat_desc *ipstats_stat_desc_offload_subs[] = {
+	&ipstats_stat_desc_offload_cpu_hit,
+};
+
+static const struct ipstats_stat_desc ipstats_stat_desc_offload_group = {
+	.name = "offload",
+	.kind = IPSTATS_STAT_DESC_KIND_GROUP,
+	.subs = ipstats_stat_desc_offload_subs,
+	.nsubs = ARRAY_SIZE(ipstats_stat_desc_offload_subs),
+};
+
 static void
 ipstats_stat_desc_pack_link(struct ipstats_stat_dump_filters *filters,
 			    const struct ipstats_stat_desc *desc)
@@ -189,6 +225,7 @@ static const struct ipstats_stat_desc ipstats_stat_desc_toplev_link = {
 
 static const struct ipstats_stat_desc *ipstats_stat_desc_toplev_subs[] = {
 	&ipstats_stat_desc_toplev_link,
+	&ipstats_stat_desc_offload_group,
 };
 
 static const struct ipstats_stat_desc ipstats_stat_desc_toplev_group = {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 08/11] ipstats: Add offload subgroup "hw_stats_info"
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (6 preceding siblings ...)
  2022-04-22  8:30 ` [PATCH iproute2-next 07/11] ipstats: Add a group "offload", subgroup "cpu_hit" Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 09/11] ipstats: Add offload subgroup "l3_stats" Petr Machata
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Add into the group "offload" a subgroup "hw_stats_info" for showing
information about HW statistics counters.

For example:

 # ip stats show dev swp1 group offload subgroup hw_stats_info
 4178: swp1: group offload subgroup hw_stats_info
     l3_stats on used off
 # ip -j stats show dev swp1 group offload subgroup hw_stats_info | jq
 [
   {
     "ifindex": 4178,
     "ifname": "swp1",
     "group": "offload",
     "subgroup": "hw_stats_info",
     "info": {
       "l3_stats": {
         "request": true,
         "used": false
       }
     }
   }
 ]

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/ipstats.c | 217 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 217 insertions(+)

diff --git a/ip/ipstats.c b/ip/ipstats.c
index 283fba4e..4cdd5122 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -164,6 +164,197 @@ static int ipstats_show_64(struct ipstats_stat_show_attrs *attrs,
 	return 0;
 }
 
+enum ipstats_maybe_on_off {
+	IPSTATS_MOO_OFF = -1,
+	IPSTATS_MOO_INVALID,
+	IPSTATS_MOO_ON,
+};
+
+static bool ipstats_moo_to_bool(enum ipstats_maybe_on_off moo)
+{
+	assert(moo != IPSTATS_MOO_INVALID);
+	return moo + 1;
+}
+
+static int ipstats_print_moo(enum output_type t, const char *key,
+			     const char *fmt, enum ipstats_maybe_on_off moo)
+{
+	if (!moo)
+		return 0;
+	return print_on_off(t, key, fmt, ipstats_moo_to_bool(moo));
+}
+
+struct ipstats_hw_s_info_one {
+	enum ipstats_maybe_on_off request;
+	enum ipstats_maybe_on_off used;
+};
+
+enum ipstats_hw_s_info_idx {
+	IPSTATS_HW_S_INFO_IDX_L3_STATS,
+	IPSTATS_HW_S_INFO_IDX_COUNT
+};
+
+static const char *const ipstats_hw_s_info_name[] = {
+	"l3_stats",
+};
+
+static_assert(ARRAY_SIZE(ipstats_hw_s_info_name) ==
+	      IPSTATS_HW_S_INFO_IDX_COUNT,
+	      "mismatch: enum ipstats_hw_s_info_idx x ipstats_hw_s_info_name");
+
+struct ipstats_hw_s_info {
+	/* Indexed by enum ipstats_hw_s_info_idx. */
+	struct ipstats_hw_s_info_one *infos[IPSTATS_HW_S_INFO_IDX_COUNT];
+};
+
+static enum ipstats_maybe_on_off ipstats_dissect_01(int value, const char *what)
+{
+	switch (value) {
+	case 0:
+		return IPSTATS_MOO_OFF;
+	case 1:
+		return IPSTATS_MOO_ON;
+	default:
+		fprintf(stderr, "Invalid value for %s: expected 0 or 1, got %d.\n",
+			what, value);
+		return IPSTATS_MOO_INVALID;
+	}
+}
+
+static int ipstats_dissect_hw_s_info_one(const struct rtattr *at,
+					 struct ipstats_hw_s_info_one *p_hwsio,
+					 const char *what)
+{
+	int attr_id_request = IFLA_OFFLOAD_XSTATS_HW_S_INFO_REQUEST;
+	struct rtattr *tb[IFLA_OFFLOAD_XSTATS_HW_S_INFO_MAX + 1];
+	int attr_id_used = IFLA_OFFLOAD_XSTATS_HW_S_INFO_USED;
+	struct ipstats_hw_s_info_one hwsio = {};
+	int err;
+	int v;
+
+	err = parse_rtattr_nested(tb, IFLA_OFFLOAD_XSTATS_HW_S_INFO_MAX, at);
+	if (err)
+		return err;
+
+	if (tb[attr_id_request]) {
+		v = rta_getattr_u8(tb[attr_id_request]);
+		hwsio.request = ipstats_dissect_01(v, "request");
+
+		/* This has to be present & valid. */
+		if (!hwsio.request)
+			return -EINVAL;
+	}
+
+	if (tb[attr_id_used]) {
+		v = rta_getattr_u8(tb[attr_id_used]);
+		hwsio.used = ipstats_dissect_01(v, "used");
+	}
+
+	*p_hwsio = hwsio;
+	return 0;
+}
+
+static int ipstats_dissect_hw_s_info(const struct rtattr *at,
+				     struct ipstats_hw_s_info *hwsi)
+{
+	struct rtattr *tb[IFLA_OFFLOAD_XSTATS_MAX + 1];
+	int attr_id_l3 = IFLA_OFFLOAD_XSTATS_L3_STATS;
+	struct ipstats_hw_s_info_one *hwsio = NULL;
+	int err;
+
+	err = parse_rtattr_nested(tb, IFLA_OFFLOAD_XSTATS_MAX, at);
+	if (err)
+		return err;
+
+	*hwsi = (struct ipstats_hw_s_info){};
+
+	if (tb[attr_id_l3]) {
+		hwsio = malloc(sizeof(*hwsio));
+		if (!hwsio) {
+			err = -ENOMEM;
+			goto out;
+		}
+
+		err = ipstats_dissect_hw_s_info_one(tb[attr_id_l3], hwsio, "l3");
+		if (err)
+			goto out;
+
+		hwsi->infos[IPSTATS_HW_S_INFO_IDX_L3_STATS] = hwsio;
+		hwsio = NULL;
+	}
+
+	return 0;
+
+out:
+	free(hwsio);
+	return err;
+}
+
+static void ipstats_fini_hw_s_info(struct ipstats_hw_s_info *hwsi)
+{
+	int i;
+
+	for (i = 0; i < IPSTATS_HW_S_INFO_IDX_COUNT; i++)
+		free(hwsi->infos[i]);
+}
+
+static void
+__ipstats_show_hw_s_info_one(const struct ipstats_hw_s_info_one *hwsio)
+{
+	if (hwsio == NULL)
+		return;
+
+	ipstats_print_moo(PRINT_ANY, "request", " %s", hwsio->request);
+	ipstats_print_moo(PRINT_ANY, "used", " used %s", hwsio->used);
+}
+
+static void
+ipstats_show_hw_s_info_one(const struct ipstats_hw_s_info *hwsi,
+			   enum ipstats_hw_s_info_idx idx)
+{
+	const struct ipstats_hw_s_info_one *hwsio = hwsi->infos[idx];
+	const char *name = ipstats_hw_s_info_name[idx];
+
+	if (hwsio == NULL)
+		return;
+
+	print_string(PRINT_FP, NULL, "    %s", name);
+	open_json_object(name);
+	__ipstats_show_hw_s_info_one(hwsio);
+	close_json_object();
+}
+
+static int __ipstats_show_hw_s_info(const struct rtattr *at)
+{
+	struct ipstats_hw_s_info hwsi = {};
+	int err;
+
+	err = ipstats_dissect_hw_s_info(at, &hwsi);
+	if (err)
+		return err;
+
+	open_json_object("info");
+	ipstats_show_hw_s_info_one(&hwsi, IPSTATS_HW_S_INFO_IDX_L3_STATS);
+	close_json_object();
+
+	ipstats_fini_hw_s_info(&hwsi);
+	return 0;
+}
+
+static int ipstats_show_hw_s_info(struct ipstats_stat_show_attrs *attrs,
+				  unsigned int group, unsigned int subgroup)
+{
+	const struct rtattr *at;
+	int err;
+
+	at = ipstats_stat_show_get_attr(attrs, group, subgroup, &err);
+	if (at == NULL)
+		return err;
+
+	print_nl();
+	return __ipstats_show_hw_s_info(at);
+}
+
 static void
 ipstats_stat_desc_pack_cpu_hit(struct ipstats_stat_dump_filters *filters,
 			       const struct ipstats_stat_desc *desc)
@@ -189,8 +380,34 @@ static const struct ipstats_stat_desc ipstats_stat_desc_offload_cpu_hit = {
 	.show = &ipstats_stat_desc_show_cpu_hit,
 };
 
+static void
+ipstats_stat_desc_pack_hw_stats_info(struct ipstats_stat_dump_filters *filters,
+				     const struct ipstats_stat_desc *desc)
+{
+	ipstats_stat_desc_enable_bit(filters,
+				     IFLA_STATS_LINK_OFFLOAD_XSTATS,
+				     IFLA_OFFLOAD_XSTATS_HW_S_INFO);
+}
+
+static int
+ipstats_stat_desc_show_hw_stats_info(struct ipstats_stat_show_attrs *attrs,
+				     const struct ipstats_stat_desc *desc)
+{
+	return ipstats_show_hw_s_info(attrs,
+				      IFLA_STATS_LINK_OFFLOAD_XSTATS,
+				      IFLA_OFFLOAD_XSTATS_HW_S_INFO);
+}
+
+static const struct ipstats_stat_desc ipstats_stat_desc_offload_hw_s_info = {
+	.name = "hw_stats_info",
+	.kind = IPSTATS_STAT_DESC_KIND_LEAF,
+	.pack = &ipstats_stat_desc_pack_hw_stats_info,
+	.show = &ipstats_stat_desc_show_hw_stats_info,
+};
+
 static const struct ipstats_stat_desc *ipstats_stat_desc_offload_subs[] = {
 	&ipstats_stat_desc_offload_cpu_hit,
+	&ipstats_stat_desc_offload_hw_s_info,
 };
 
 static const struct ipstats_stat_desc ipstats_stat_desc_offload_group = {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 09/11] ipstats: Add offload subgroup "l3_stats"
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (7 preceding siblings ...)
  2022-04-22  8:30 ` [PATCH iproute2-next 08/11] ipstats: Add offload subgroup "hw_stats_info" Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:30 ` [PATCH iproute2-next 10/11] ipmonitor: Add monitoring support for stats events Petr Machata
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Add into the group "offload" a subgroup "l3_stats" for showing
L3 statistics.

For example:

 # ip stats show dev swp2.200 group offload subgroup l3_stats
 4212: swp2.200: group offload subgroup l3_stats on used on
     RX: bytes packets errors dropped   mcast
          1920      21      1       0       0
     TX: bytes packets errors dropped
           756       9      0       0

 # ip -j stats show dev swp2.200 group offload subgroup l3_stats | jq
 [
   {
     "ifindex": 4212,
     "ifname": "swp2.200",
     "group": "offload",
     "subgroup": "l3_stats",
     "info": {
       "request": true,
       "used": true
     },
     "stats64": {
       "rx": {
         "bytes": 1920,
         "packets": 21,
         "errors": 1,
         "dropped": 0,
         "multicast": 0
       },
       "tx": {
         "bytes": 756,
         "packets": 9,
         "errors": 0,
         "dropped": 0
       }
     }
   }
 ]

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/ipstats.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/ip/ipstats.c b/ip/ipstats.c
index 4cdd5122..29ca0731 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -164,6 +164,82 @@ static int ipstats_show_64(struct ipstats_stat_show_attrs *attrs,
 	return 0;
 }
 
+static void print_hw_stats64(FILE *fp, struct rtnl_hw_stats64 *s)
+{
+	unsigned int cols[] = {
+		strlen("*X: bytes"),
+		strlen("packets"),
+		strlen("errors"),
+		strlen("dropped"),
+		strlen("overrun"),
+	};
+
+	if (is_json_context()) {
+		/* RX stats */
+		open_json_object("rx");
+		print_u64(PRINT_JSON, "bytes", NULL, s->rx_bytes);
+		print_u64(PRINT_JSON, "packets", NULL, s->rx_packets);
+		print_u64(PRINT_JSON, "errors", NULL, s->rx_errors);
+		print_u64(PRINT_JSON, "dropped", NULL, s->rx_dropped);
+		print_u64(PRINT_JSON, "multicast", NULL, s->multicast);
+		close_json_object();
+
+		/* TX stats */
+		open_json_object("tx");
+		print_u64(PRINT_JSON, "bytes", NULL, s->tx_bytes);
+		print_u64(PRINT_JSON, "packets", NULL, s->tx_packets);
+		print_u64(PRINT_JSON, "errors", NULL, s->tx_errors);
+		print_u64(PRINT_JSON, "dropped", NULL, s->tx_dropped);
+		close_json_object();
+	} else {
+		size_columns(cols, ARRAY_SIZE(cols),
+			     s->rx_bytes, s->rx_packets, s->rx_errors,
+			     s->rx_dropped, s->multicast);
+		size_columns(cols, ARRAY_SIZE(cols),
+			     s->tx_bytes, s->tx_packets, s->tx_errors,
+			     s->tx_dropped, 0);
+
+		/* RX stats */
+		fprintf(fp, "    RX: %*s %*s %*s %*s %*s%s",
+			cols[0] - 4, "bytes", cols[1], "packets",
+			cols[2], "errors", cols[3], "dropped",
+			cols[4], "mcast", _SL_);
+
+		fprintf(fp, "    ");
+		print_num(fp, cols[0], s->rx_bytes);
+		print_num(fp, cols[1], s->rx_packets);
+		print_num(fp, cols[2], s->rx_errors);
+		print_num(fp, cols[3], s->rx_dropped);
+		print_num(fp, cols[4], s->multicast);
+		fprintf(fp, "%s", _SL_);
+
+		/* TX stats */
+		fprintf(fp, "    TX: %*s %*s %*s %*s%s",
+			cols[0] - 4, "bytes", cols[1], "packets",
+			cols[2], "errors", cols[3], "dropped", _SL_);
+
+		fprintf(fp, "    ");
+		print_num(fp, cols[0], s->tx_bytes);
+		print_num(fp, cols[1], s->tx_packets);
+		print_num(fp, cols[2], s->tx_errors);
+		print_num(fp, cols[3], s->tx_dropped);
+	}
+}
+
+static int ipstats_show_hw64(const struct rtattr *at)
+{
+	struct rtnl_hw_stats64 *stats;
+
+	stats = IPSTATS_RTA_PAYLOAD(struct rtnl_hw_stats64, at);
+	if (stats == NULL) {
+		fprintf(stderr, "Error: attribute payload too short");
+		return -EINVAL;
+	}
+
+	print_hw_stats64(stdout, stats);
+	return 0;
+}
+
 enum ipstats_maybe_on_off {
 	IPSTATS_MOO_OFF = -1,
 	IPSTATS_MOO_INVALID,
@@ -355,6 +431,57 @@ static int ipstats_show_hw_s_info(struct ipstats_stat_show_attrs *attrs,
 	return __ipstats_show_hw_s_info(at);
 }
 
+static int __ipstats_show_hw_stats(const struct rtattr *at_hwsi,
+				   const struct rtattr *at_stats,
+				   enum ipstats_hw_s_info_idx idx)
+{
+	int err = 0;
+
+	if (at_hwsi != NULL) {
+		struct ipstats_hw_s_info hwsi = {};
+
+		err = ipstats_dissect_hw_s_info(at_hwsi, &hwsi);
+		if (err)
+			return err;
+
+		open_json_object("info");
+		__ipstats_show_hw_s_info_one(hwsi.infos[idx]);
+		close_json_object();
+
+		ipstats_fini_hw_s_info(&hwsi);
+	}
+
+	if (at_stats != NULL) {
+		print_nl();
+		open_json_object("stats64");
+		err = ipstats_show_hw64(at_stats);
+		close_json_object();
+	}
+
+	return err;
+}
+
+static int ipstats_show_hw_stats(struct ipstats_stat_show_attrs *attrs,
+				 unsigned int group,
+				 unsigned int hw_s_info,
+				 unsigned int hw_stats,
+				 enum ipstats_hw_s_info_idx idx)
+{
+	const struct rtattr *at_stats;
+	const struct rtattr *at_hwsi;
+	int err = 0;
+
+	at_hwsi = ipstats_stat_show_get_attr(attrs, group, hw_s_info, &err);
+	if (at_hwsi == NULL)
+		return err;
+
+	at_stats = ipstats_stat_show_get_attr(attrs, group, hw_stats, &err);
+	if (at_stats == NULL && err != 0)
+		return err;
+
+	return __ipstats_show_hw_stats(at_hwsi, at_stats, idx);
+}
+
 static void
 ipstats_stat_desc_pack_cpu_hit(struct ipstats_stat_dump_filters *filters,
 			       const struct ipstats_stat_desc *desc)
@@ -405,9 +532,40 @@ static const struct ipstats_stat_desc ipstats_stat_desc_offload_hw_s_info = {
 	.show = &ipstats_stat_desc_show_hw_stats_info,
 };
 
+static void
+ipstats_stat_desc_pack_l3_stats(struct ipstats_stat_dump_filters *filters,
+				const struct ipstats_stat_desc *desc)
+{
+	ipstats_stat_desc_enable_bit(filters,
+				     IFLA_STATS_LINK_OFFLOAD_XSTATS,
+				     IFLA_OFFLOAD_XSTATS_L3_STATS);
+	ipstats_stat_desc_enable_bit(filters,
+				     IFLA_STATS_LINK_OFFLOAD_XSTATS,
+				     IFLA_OFFLOAD_XSTATS_HW_S_INFO);
+}
+
+static int
+ipstats_stat_desc_show_l3_stats(struct ipstats_stat_show_attrs *attrs,
+				const struct ipstats_stat_desc *desc)
+{
+	return ipstats_show_hw_stats(attrs,
+				     IFLA_STATS_LINK_OFFLOAD_XSTATS,
+				     IFLA_OFFLOAD_XSTATS_HW_S_INFO,
+				     IFLA_OFFLOAD_XSTATS_L3_STATS,
+				     IPSTATS_HW_S_INFO_IDX_L3_STATS);
+}
+
+static const struct ipstats_stat_desc ipstats_stat_desc_offload_l3_stats = {
+	.name = "l3_stats",
+	.kind = IPSTATS_STAT_DESC_KIND_LEAF,
+	.pack = &ipstats_stat_desc_pack_l3_stats,
+	.show = &ipstats_stat_desc_show_l3_stats,
+};
+
 static const struct ipstats_stat_desc *ipstats_stat_desc_offload_subs[] = {
 	&ipstats_stat_desc_offload_cpu_hit,
 	&ipstats_stat_desc_offload_hw_s_info,
+	&ipstats_stat_desc_offload_l3_stats,
 };
 
 static const struct ipstats_stat_desc ipstats_stat_desc_offload_group = {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 10/11] ipmonitor: Add monitoring support for stats events
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (8 preceding siblings ...)
  2022-04-22  8:30 ` [PATCH iproute2-next 09/11] ipstats: Add offload subgroup "l3_stats" Petr Machata
@ 2022-04-22  8:30 ` Petr Machata
  2022-04-22  8:31 ` [PATCH iproute2-next 11/11] man: Add man pages for the "stats" functions Petr Machata
  2022-04-28  2:20 ` [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS patchwork-bot+netdevbpf
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:30 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Toggles and offloads of HW statistics cause emission of and RTM_NEWSTATS
event. Add support to "ip monitor" for these events.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/ip_common.h |  1 +
 ip/ipmonitor.c | 16 +++++++++++++++-
 ip/ipstats.c   | 20 ++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index 8b0a6426..9eeeb387 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -57,6 +57,7 @@ int print_nexthop_bucket(struct nlmsghdr *n, void *arg);
 void netns_map_init(void);
 void netns_nsid_socket_init(void);
 int print_nsid(struct nlmsghdr *n, void *arg);
+int ipstats_print(struct nlmsghdr *n, void *arg);
 char *get_name_from_nsid(int nsid);
 int get_netnsid_from_name(const char *name);
 int set_netnsid_from_name(const char *name, int nsid);
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 0badda4e..a4326d2a 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -34,7 +34,7 @@ static void usage(void)
 		"Usage: ip monitor [ all | OBJECTS ] [ FILE ] [ label ] [ all-nsid ]\n"
 		"                  [ dev DEVICE ]\n"
 		"OBJECTS :=  address | link | mroute | neigh | netconf |\n"
-		"            nexthop | nsid | prefix | route | rule\n"
+		"            nexthop | nsid | prefix | route | rule | stats\n"
 		"FILE := file FILENAME\n");
 	exit(-1);
 }
@@ -158,6 +158,11 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
 		print_nsid(n, arg);
 		return 0;
 
+	case RTM_NEWSTATS:
+		print_headers(fp, "[STATS]", ctrl);
+		ipstats_print(n, arg);
+		return 0;
+
 	case NLMSG_ERROR:
 	case NLMSG_NOOP:
 	case NLMSG_DONE:
@@ -185,6 +190,7 @@ int do_ipmonitor(int argc, char **argv)
 	int lprefix = 0;
 	int lneigh = 0;
 	int lnetconf = 0;
+	int lstats = 0;
 	int lrule = 0;
 	int lnsid = 0;
 	int ifindex = 0;
@@ -253,6 +259,9 @@ int do_ipmonitor(int argc, char **argv)
 		} else if (matches(*argv, "nexthop") == 0) {
 			lnexthop = 1;
 			groups = 0;
+		} else if (strcmp(*argv, "stats") == 0) {
+			lstats = 1;
+			groups = 0;
 		} else if (strcmp(*argv, "all") == 0) {
 			prefix_banner = 1;
 		} else if (matches(*argv, "all-nsid") == 0) {
@@ -349,6 +358,11 @@ int do_ipmonitor(int argc, char **argv)
 		exit(1);
 	}
 
+	if (lstats && rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0) {
+		fprintf(stderr, "Failed to add stats group to list\n");
+		exit(1);
+	}
+
 	if (listen_all_nsid && rtnl_listen_all_nsid(&rth) < 0)
 		exit(1);
 
diff --git a/ip/ipstats.c b/ip/ipstats.c
index 29ca0731..39ddca01 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -1219,3 +1219,23 @@ int do_ipstats(int argc, char **argv)
 
 	return rc;
 }
+
+int ipstats_print(struct nlmsghdr *n, void *arg)
+{
+	struct ipstats_stat_enabled_one one = {
+		.desc = &ipstats_stat_desc_offload_hw_s_info,
+	};
+	struct ipstats_stat_enabled enabled = {
+		.enabled = &one,
+		.nenabled = 1,
+	};
+	FILE *fp = arg;
+	int rc;
+
+	rc = ipstats_process_ifsm(n, &enabled);
+	if (rc)
+		return rc;
+
+	fflush(fp);
+	return 0;
+}
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH iproute2-next 11/11] man: Add man pages for the "stats" functions
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (9 preceding siblings ...)
  2022-04-22  8:30 ` [PATCH iproute2-next 10/11] ipmonitor: Add monitoring support for stats events Petr Machata
@ 2022-04-22  8:31 ` Petr Machata
  2022-04-28  2:20 ` [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS patchwork-bot+netdevbpf
  11 siblings, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-04-22  8:31 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel, Petr Machata

Add a man page for the new "stats" command.
Also mention the new "stats" group in ip-monitor.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 man/man8/ip-monitor.8 |   2 +-
 man/man8/ip-stats.8   | 160 ++++++++++++++++++++++++++++++++++++++++++
 man/man8/ip.8         |   7 +-
 3 files changed, 167 insertions(+), 2 deletions(-)
 create mode 100644 man/man8/ip-stats.8

diff --git a/man/man8/ip-monitor.8 b/man/man8/ip-monitor.8
index f886d31b..ec033c69 100644
--- a/man/man8/ip-monitor.8
+++ b/man/man8/ip-monitor.8
@@ -55,7 +55,7 @@ command is the first in the command line and then the object list follows:
 is the list of object types that we want to monitor.
 It may contain
 .BR link ", " address ", " route ", " mroute ", " prefix ", "
-.BR neigh ", " netconf ", "  rule ", " nsid " and " nexthop "."
+.BR neigh ", " netconf ", "  rule ", " stats ", " nsid " and " nexthop "."
 If no
 .B file
 argument is given,
diff --git a/man/man8/ip-stats.8 b/man/man8/ip-stats.8
new file mode 100644
index 00000000..7eaaf122
--- /dev/null
+++ b/man/man8/ip-stats.8
@@ -0,0 +1,160 @@
+.TH IP\-STATS 8 "16 Mar 2022" "iproute2" "Linux"
+.SH NAME
+ip-stats \- manage and show interface statistics
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+.ti -8
+.B ip
+.B stats
+.RI  " { " COMMAND " | "
+.BR help " }"
+.sp
+
+.ti -8
+.BR "ip stats show"
+.RB "[ " dev
+.IR DEV " ] "
+.RB "[ " group
+.IR GROUP " [ "
+.BI subgroup " SUBGROUP"
+.R " ] ... ] ..."
+
+.ti -8
+.BR "ip stats set"
+.BI dev " DEV"
+.BR l3_stats " { "
+.BR on " | " off " }"
+
+.SH DESCRIPTION
+
+.TP
+.B ip stats set
+is used for toggling whether a certain HW statistics suite is collected on
+a given netdevice. The following statistics suites are supported:
+
+.in 21
+
+.ti 14
+.B l3_stats
+L3 stats reflect traffic that takes place in a HW device on an object that
+corresponds to the given software netdevice.
+
+.TP
+.B ip stats show
+is used for showing stats on a given netdevice, or dumping statistics
+across all netdevices. By default, all stats are requested. It is possible
+to filter which stats are requested by using the
+.B group
+and
+.B subgroup
+keywords.
+
+It is possible to specify several groups, or several subgroups for one
+group. When no subgroups are given for a group, all the subgroups are
+requested.
+
+The following groups are recognized:
+.in 21
+
+.ti 14
+.B group link
+- Link statistics. The same suite that "ip -s link show" shows.
+
+.ti 14
+.B group offload
+- A group that contains a number of HW-oriented statistics. See below for
+individual subgroups within this group.
+
+.TQ
+.BR "group offload " subgroups:
+.in 21
+
+.ti 14
+.B subgroup cpu_hit
+- The
+.B cpu_hit
+statistics suite is useful on hardware netdevices. The
+.B link
+statistics on these devices reflect both the hardware- and
+software-datapath traffic. The
+.B cpu_hit
+statistics then only reflect software-datapath traffic.
+
+.ti 14
+.B subgroup hw_stats_info
+- This suite does not include traffic statistics, but rather communicates
+the state of other statistics. Through this subgroup, it is possible to
+discover whether a given statistic was enabled, and when it was, whether
+any device driver actually configured its device to collect these
+statistics. For example,
+.B l3_stats
+was enabled in the following case, but no driver has installed it:
+
+# ip stats show dev swp1 group offload subgroup hw_stats_info
+.br
+56: swp1: group offload subgroup hw_stats_info
+.br
+    l3_stats on used off
+
+After an L3 address is added to the netdevice, the counter will be
+installed:
+
+# ip addr add dev swp1 192.0.2.1/28
+.br
+# ip stats show dev swp1 group offload subgroup hw_stats_info
+.br
+56: swp1: group offload subgroup hw_stats_info
+.br
+    l3_stats on used on
+
+.ti 14
+.B subgroup l3_stats
+- These statistics reflect L3 traffic that takes place in HW on an object
+that corresponds to the netdevice. Note that this suite is disabled by
+default and needs to be first enabled through
+.B ip stats set\fR.
+
+For example:
+
+# ip stats show dev swp2.200 group offload subgroup l3_stats
+.br
+112: swp2.200: group offload subgroup l3_stats on used on
+.br
+    RX:  bytes packets errors dropped   mcast
+.br
+          8900      72      2       0       3
+.br
+    TX:  bytes packets errors dropped
+.br
+          7176      58      0       0
+
+Note how the l3_stats_info for the selected group is also part of the dump.
+
+.SH EXAMPLES
+.PP
+# ip stats set dev swp1 l3_stats on
+.RS
+Enables collection of L3 HW statistics on swp1.
+.RE
+
+.PP
+# ip stats show group offload
+.RS
+Shows all offload statistics on all netdevices.
+.RE
+
+.PP
+# ip stats show dev swp1 group link
+.RS
+Shows link statistics on the given netdevice.
+.RE
+
+.SH SEE ALSO
+.br
+.BR ip (8),
+.BR ip-link (8),
+
+.SH AUTHOR
+Manpage by Petr Machata.
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 2a4848b7..f6adbc77 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -22,7 +22,7 @@ ip \- show / manipulate routing, network devices, interfaces and tunnels
 .BR link " | " address " | " addrlabel " | " route " | " rule " | " neigh " | "\
  ntable " | " tunnel " | " tuntap " | " maddress " | "  mroute " | " mrule " | "\
  monitor " | " xfrm " | " netns " | "  l2tp " | "  tcp_metrics " | " token " | "\
- macsec " | " vrf " | " mptcp " | " ioam " }"
+ macsec " | " vrf " | " mptcp " | " ioam " | " stats " }"
 .sp
 
 .ti -8
@@ -302,6 +302,10 @@ readability.
 .B rule
 - rule in routing policy database.
 
+.TP
+.B stats
+- manage and show interface statistics.
+
 .TP
 .B tcp_metrics/tcpmetrics
 - manage TCP Metrics
@@ -419,6 +423,7 @@ was written by Alexey N. Kuznetsov and added in Linux 2.2.
 .BR ip-ntable (8),
 .BR ip-route (8),
 .BR ip-rule (8),
+.BR ip-stats (8)
 .BR ip-tcp_metrics (8),
 .BR ip-token (8),
 .BR ip-tunnel (8),
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS
  2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
                   ` (10 preceding siblings ...)
  2022-04-22  8:31 ` [PATCH iproute2-next 11/11] man: Add man pages for the "stats" functions Petr Machata
@ 2022-04-28  2:20 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 17+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-04-28  2:20 UTC (permalink / raw)
  To: Petr Machata; +Cc: netdev, dsahern, stephen, idosch

Hello:

This series was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Fri, 22 Apr 2022 10:30:49 +0200 you wrote:
> A new rtnetlink message, RTM_SETSTATS, has been added recently in kernel
> commit ca0a53dcec94 ("Merge branch 'net-hw-counters-for-soft-devices'").
> 
> At the same time, RTM_GETSTATS has been around for a while. The users of
> this API are spread in a couple different places: "ip link xstats" reads
> stats from the IFLA_STATS_LINK_XSTATS and _XSTATS_SLAVE subgroups, "ip
> link afstats" then reads IFLA_STATS_AF_SPEC.
> 
> [...]

Here is the summary with links:
  - [iproute2-next,01/11] libnetlink: Add filtering to rtnl_statsdump_req_filter()
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=a463d6b19107
  - [iproute2-next,02/11] ip: Publish functions for stats formatting
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=5520cf1603ba
  - [iproute2-next,03/11] ip: Add a new family of commands, "stats"
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=54d82b0699a0
  - [iproute2-next,04/11] ipstats: Add a "set" command
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=82f6444f83c7
  - [iproute2-next,05/11] ipstats: Add a shell of "show" command
    (no matching commit)
  - [iproute2-next,06/11] ipstats: Add a group "link"
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=0517a2fd66ae
  - [iproute2-next,07/11] ipstats: Add a group "offload", subgroup "cpu_hit"
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=af5e7955273e
  - [iproute2-next,08/11] ipstats: Add offload subgroup "hw_stats_info"
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=179030fa6bc7
  - [iproute2-next,09/11] ipstats: Add offload subgroup "l3_stats"
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=0f1fd40cc9e8
  - [iproute2-next,10/11] ipmonitor: Add monitoring support for stats events
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=a05a27c07cbf
  - [iproute2-next,11/11] man: Add man pages for the "stats" functions
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=b28eb051b321

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH iproute2-next 06/11] ipstats: Add a group "link"
  2022-04-22  8:30 ` [PATCH iproute2-next 06/11] ipstats: Add a group "link" Petr Machata
@ 2022-05-01 14:52   ` Ido Schimmel
  2022-05-02  4:56     ` David Ahern
  2022-05-02  9:37     ` Petr Machata
  0 siblings, 2 replies; 17+ messages in thread
From: Ido Schimmel @ 2022-05-01 14:52 UTC (permalink / raw)
  To: Petr Machata; +Cc: netdev, David Ahern, Stephen Hemminger, Ido Schimmel

On Fri, Apr 22, 2022 at 10:30:55AM +0200, Petr Machata wrote:
> +#define IPSTATS_RTA_PAYLOAD(TYPE, AT)					\
> +	({								\
> +		const struct rtattr *__at = (AT);			\
> +		TYPE *__ret = NULL;					\
> +									\
> +		if (__at != NULL &&					\
> +		    __at->rta_len - RTA_LENGTH(0) >= sizeof(TYPE))	\
> +			__ret = RTA_DATA(__at);				\
> +		__ret;							\
> +	})
> +
> +static int ipstats_show_64(struct ipstats_stat_show_attrs *attrs,
> +			   unsigned int group, unsigned int subgroup)
> +{
> +	struct rtnl_link_stats64 *stats;
> +	const struct rtattr *at;
> +	int err;
> +
> +	at = ipstats_stat_show_get_attr(attrs, group, subgroup, &err);
> +	if (at == NULL)
> +		return err;
> +
> +	stats = IPSTATS_RTA_PAYLOAD(struct rtnl_link_stats64, at);
> +	if (stats == NULL) {
> +		fprintf(stderr, "Error: attribute payload too short");

When I tested this on 5.15 / 5.16 everything was fine, but now I get:

$ ip stats show dev lo group link
1: lo: group link
Error: attribute payload too short

Payload on 5.16:

recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[{nlmsg_len=224, nlmsg_type=RTM_NEWSTATS, nlmsg_flags=0, nlmsg_seq=1651407379, nlmsg_pid=330213}, {family=AF_UNSPEC, ifindex=if_nametoindex("lo"), filter_mask=1<<IFLA_STATS_UNSPEC}, [{nla_len=196, nla_type=IFLA_STATS_LINK_64}, {rx_packets=321113, tx_packets=321113, rx_bytes=322735996, tx_bytes=322735996, rx_errors=0, tx_errors=0, rx_dropped=0, tx_dropped=0, multicast=0, collisions=0, rx_length_errors=0, rx_over_errors=0, rx_crc_errors=0, rx_frame_errors=0, rx_fifo_errors=0, rx_missed_errors=0, tx_aborted_errors=0, tx_carrier_errors=0, tx_fifo_errors=0, tx_heartbeat_errors=0, tx_window_errors=0, rx_compressed=0, tx_compressed=0, rx_nohandler=0}]], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 224
1: lo: group link
Error: attribute payload too short+++ exited with 22 +++

Payload on net-next:

recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[{nlmsg_len=232, nlmsg_type=RTM_NEWSTATS, nlmsg_flags=0, nlmsg_seq=1651407411, nlmsg_pid=198}, {family=AF_UNSPEC, ifindex=if_nametoindex("lo"), filter_mask=1<<IFLA_STATS_UNSPEC}, [{nla_len=204, nla_type=IFLA_STATS_LINK_64}, {rx_packets=0, tx_packets=0, rx_bytes=0, tx_bytes=0, rx_errors=0, tx_errors=0, rx_dropped=0, tx_dropped=0, multicast=0, collisions=0, rx_length_errors=0, rx_over_errors=0, rx_crc_errors=0, rx_frame_errors=0, rx_fifo_errors=0, rx_missed_errors=0, tx_aborted_errors=0, tx_carrier_errors=0, tx_fifo_errors=0, tx_heartbeat_errors=0, tx_window_errors=0, rx_compressed=0, tx_compressed=0, rx_nohandler=0}]], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 232
1: lo: group link
    RX:  bytes packets errors dropped  missed   mcast           
             0       0      0       0       0       0 
    TX:  bytes packets errors dropped carrier collsns           
             0       0      0       0       0       0 
+++ exited with 0 +++

Note the difference in size of IFLA_STATS_LINK_64 which carries struct
rtnl_link_stats64: 196 bytes vs. 204 bytes

The 8 byte difference is most likely from the addition of
rx_otherhost_dropped at the end:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=794c24e9921f32ded4422833a990ccf11dc3c00e

I guess it worked for me because I didn't have this member in my copy of
the uAPI file, but it's now in iproute2-next:

https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=bba95837524d09ee2f0efdf6350b83a985f4b2f8

I was under the impression that such a size increase in a uAPI struct is
forbidden, which is why we usually avoid passing structs over netlink.

> +		return -EINVAL;
> +	}
> +
> +	open_json_object("stats64");
> +	print_stats64(stdout, stats, NULL, NULL);
> +	close_json_object();
> +	return 0;
> +}

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH iproute2-next 06/11] ipstats: Add a group "link"
  2022-05-01 14:52   ` Ido Schimmel
@ 2022-05-02  4:56     ` David Ahern
  2022-05-02  6:19       ` Ido Schimmel
  2022-05-02  9:37     ` Petr Machata
  1 sibling, 1 reply; 17+ messages in thread
From: David Ahern @ 2022-05-02  4:56 UTC (permalink / raw)
  To: Ido Schimmel, Petr Machata; +Cc: netdev, Stephen Hemminger, Ido Schimmel

On 5/1/22 8:52 AM, Ido Schimmel wrote:
> When I tested this on 5.15 / 5.16 everything was fine, but now I get:
> 
> $ ip stats show dev lo group link
> 1: lo: group link
> Error: attribute payload too short
> 

...

> 
> Note the difference in size of IFLA_STATS_LINK_64 which carries struct
> rtnl_link_stats64: 196 bytes vs. 204 bytes
> 
> The 8 byte difference is most likely from the addition of
> rx_otherhost_dropped at the end:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=794c24e9921f32ded4422833a990ccf11dc3c00e
> 
> I guess it worked for me because I didn't have this member in my copy of
> the uAPI file, but it's now in iproute2-next:
> 
> https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=bba95837524d09ee2f0efdf6350b83a985f4b2f8
> 
> I was under the impression that such a size increase in a uAPI struct is
> forbidden, which is why we usually avoid passing structs over netlink.

extending structs at the end is typically allowed. The ABI breakage is
when an entry is added in the middle.


> 
>> +		return -EINVAL;
>> +	}
>> +
>> +	open_json_object("stats64");
>> +	print_stats64(stdout, stats, NULL, NULL);
>> +	close_json_object();
>> +	return 0;
>> +}


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH iproute2-next 06/11] ipstats: Add a group "link"
  2022-05-02  4:56     ` David Ahern
@ 2022-05-02  6:19       ` Ido Schimmel
  0 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2022-05-02  6:19 UTC (permalink / raw)
  To: David Ahern; +Cc: Petr Machata, netdev, Stephen Hemminger, Ido Schimmel

On Sun, May 01, 2022 at 09:56:16PM -0700, David Ahern wrote:
> On 5/1/22 8:52 AM, Ido Schimmel wrote:
> > When I tested this on 5.15 / 5.16 everything was fine, but now I get:
> > 
> > $ ip stats show dev lo group link
> > 1: lo: group link
> > Error: attribute payload too short
> > 
> 
> ...
> 
> > 
> > Note the difference in size of IFLA_STATS_LINK_64 which carries struct
> > rtnl_link_stats64: 196 bytes vs. 204 bytes
> > 
> > The 8 byte difference is most likely from the addition of
> > rx_otherhost_dropped at the end:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=794c24e9921f32ded4422833a990ccf11dc3c00e
> > 
> > I guess it worked for me because I didn't have this member in my copy of
> > the uAPI file, but it's now in iproute2-next:
> > 
> > https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=bba95837524d09ee2f0efdf6350b83a985f4b2f8
> > 
> > I was under the impression that such a size increase in a uAPI struct is
> > forbidden, which is why we usually avoid passing structs over netlink.
> 
> extending structs at the end is typically allowed.

In this case, the iproute2 check needs to be modified as new iproute2
should work with old kernels.

> The ABI breakage is when an entry is added in the middle.

Sure, that's well understood.

Thanks!

> 
> 
> > 
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	open_json_object("stats64");
> >> +	print_stats64(stdout, stats, NULL, NULL);
> >> +	close_json_object();
> >> +	return 0;
> >> +}
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH iproute2-next 06/11] ipstats: Add a group "link"
  2022-05-01 14:52   ` Ido Schimmel
  2022-05-02  4:56     ` David Ahern
@ 2022-05-02  9:37     ` Petr Machata
  1 sibling, 0 replies; 17+ messages in thread
From: Petr Machata @ 2022-05-02  9:37 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: Petr Machata, netdev, David Ahern, Stephen Hemminger, Ido Schimmel


Ido Schimmel <idosch@idosch.org> writes:

> When I tested this on 5.15 / 5.16 everything was fine, but now I get:
>
> $ ip stats show dev lo group link
> 1: lo: group link
> Error: attribute payload too short
>
> Payload on 5.16:
>
> recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[{nlmsg_len=224, nlmsg_type=RTM_NEWSTATS, nlmsg_flags=0, nlmsg_seq=1651407379, nlmsg_pid=330213}, {family=AF_UNSPEC, ifindex=if_nametoindex("lo"), filter_mask=1<<IFLA_STATS_UNSPEC}, [{nla_len=196, nla_type=IFLA_STATS_LINK_64}, {rx_packets=321113, tx_packets=321113, rx_bytes=322735996, tx_bytes=322735996, rx_errors=0, tx_errors=0, rx_dropped=0, tx_dropped=0, multicast=0, collisions=0, rx_length_errors=0, rx_over_errors=0, rx_crc_errors=0, rx_frame_errors=0, rx_fifo_errors=0, rx_missed_errors=0, tx_aborted_errors=0, tx_carrier_errors=0, tx_fifo_errors=0, tx_heartbeat_errors=0, tx_window_errors=0, rx_compressed=0, tx_compressed=0, rx_nohandler=0}]], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 224
> 1: lo: group link
> Error: attribute payload too short+++ exited with 22 +++
>
> Payload on net-next:
>
> recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[{nlmsg_len=232, nlmsg_type=RTM_NEWSTATS, nlmsg_flags=0, nlmsg_seq=1651407411, nlmsg_pid=198}, {family=AF_UNSPEC, ifindex=if_nametoindex("lo"), filter_mask=1<<IFLA_STATS_UNSPEC}, [{nla_len=204, nla_type=IFLA_STATS_LINK_64}, {rx_packets=0, tx_packets=0, rx_bytes=0, tx_bytes=0, rx_errors=0, tx_errors=0, rx_dropped=0, tx_dropped=0, multicast=0, collisions=0, rx_length_errors=0, rx_over_errors=0, rx_crc_errors=0, rx_frame_errors=0, rx_fifo_errors=0, rx_missed_errors=0, tx_aborted_errors=0, tx_carrier_errors=0, tx_fifo_errors=0, tx_heartbeat_errors=0, tx_window_errors=0, rx_compressed=0, tx_compressed=0, rx_nohandler=0}]], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 232
> 1: lo: group link
>     RX:  bytes packets errors dropped  missed   mcast           
>              0       0      0       0       0       0 
>     TX:  bytes packets errors dropped carrier collsns           
>              0       0      0       0       0       0 
> +++ exited with 0 +++
>
> Note the difference in size of IFLA_STATS_LINK_64 which carries struct
> rtnl_link_stats64: 196 bytes vs. 204 bytes
>
> The 8 byte difference is most likely from the addition of
> rx_otherhost_dropped at the end:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=794c24e9921f32ded4422833a990ccf11dc3c00e
>
> I guess it worked for me because I didn't have this member in my copy of
> the uAPI file, but it's now in iproute2-next:
>
> https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=bba95837524d09ee2f0efdf6350b83a985f4b2f8

Thanks, I'll send a fix.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-05-02  9:41 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22  8:30 [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 01/11] libnetlink: Add filtering to rtnl_statsdump_req_filter() Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 02/11] ip: Publish functions for stats formatting Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 03/11] ip: Add a new family of commands, "stats" Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 04/11] ipstats: Add a "set" command Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 05/11] ipstats: Add a shell of "show" command Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 06/11] ipstats: Add a group "link" Petr Machata
2022-05-01 14:52   ` Ido Schimmel
2022-05-02  4:56     ` David Ahern
2022-05-02  6:19       ` Ido Schimmel
2022-05-02  9:37     ` Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 07/11] ipstats: Add a group "offload", subgroup "cpu_hit" Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 08/11] ipstats: Add offload subgroup "hw_stats_info" Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 09/11] ipstats: Add offload subgroup "l3_stats" Petr Machata
2022-04-22  8:30 ` [PATCH iproute2-next 10/11] ipmonitor: Add monitoring support for stats events Petr Machata
2022-04-22  8:31 ` [PATCH iproute2-next 11/11] man: Add man pages for the "stats" functions Petr Machata
2022-04-28  2:20 ` [PATCH iproute2-next 00/11] ip stats: A new front-end for RTM_GETSTATS / RTM_SETSTATS patchwork-bot+netdevbpf

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.