[PATCH iproute2-next 0/3] bond, bridge: add xstats json support

[PATCH iproute2-next 0/3] bond, bridge: add xstats json support

[Nikolay Aleksandrov]

Hi,
This set adds json output support to the xstats API (patch 01) and then
adds json support to the bridge xstats output (patch 02) and adds xstats
output support (both plain text and json) for the bonding (patch 03).
It doesn't change the bridge's plain text output, but it fixes an
inconsistency that could happen if new bridge xstats attributes were
added (print the interface name once for each group of xstats attrs).

Thanks,
 Nik


Nikolay Aleksandrov (3):
  ip: xstats: add json output support
  ip: bridge: add xstats json support
  ip: bond: add xstats support

 ip/ip_common.h         |   3 +
 ip/iplink_bond.c       | 167 +++++++++++++++++++++++++++++++++++++++-
 ip/iplink_bond_slave.c |   2 +
 ip/iplink_bridge.c     | 169 +++++++++++++++++++++++++----------------
 ip/iplink_xstats.c     |   3 +
 5 files changed, 276 insertions(+), 68 deletions(-)

-- 
2.17.2

[PATCH iproute2-next 1/3] ip: xstats: add json output support

[Nikolay Aleksandrov]

This adds only initial object support if json argument is specified.
Later patches convert the current xstats users to json.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 ip/iplink_xstats.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ip/iplink_xstats.c b/ip/iplink_xstats.c
index 908d9228369f..c64e6885678c 100644
--- a/ip/iplink_xstats.c
+++ b/ip/iplink_xstats.c
@@ -70,10 +70,13 @@ int iplink_ifla_xstats(int argc, char **argv)
 		return -1;
 	}
 
+	new_json_obj(json);
 	if (rtnl_dump_filter(&rth, lu->print_ifla_xstats, stdout) < 0) {
+		delete_json_obj();
 		fprintf(stderr, "Dump terminated\n");
 		return -1;
 	}
+	delete_json_obj();
 
 	return 0;
 }
-- 
2.17.2

[PATCH iproute2-next 2/3] ip: bridge: add xstats json support

[Nikolay Aleksandrov]

Add json support for bridge's xstats output.
The plain text output format should remain the same.
Note that this patch pulls the interface out of the attribute
loop, this was an oversight when the set was upstreamed. This does not
change the output format, but fixes it when new xstats attributes show
up.

Example:
$ ip -p -j link xstats type bridge
  [ {
        "ifname": "br0",
        "multicast": {
            "igmp_queries": {
                "rx_v1": 0,
                "rx_v2": 32,
                "rx_v3": 0,
                "tx_v1": 0,
                "tx_v2": 0,
                "tx_v3": 0
            },
            "igmp_reports": {
                "rx_v1": 0,
                "rx_v2": 32,
                "rx_v3": 0,
                "tx_v1": 0,
                "tx_v2": 0,
                "tx_v3": 0
            },
            "igmp_leaves": {
                "rx": 0,
                "tx": 0
            },
            "igmp_parse_errors": 0,
            "mld_queries": {
                "rx_v1": 33,
                "rx_v2": 0,
                "tx_v1": 0,
                "tx_v2": 0
            },
            "mld_reports": {
                "rx_v1": 66,
                "rx_v2": 2,
                "tx_v1": 0,
                "tx_v2": 0
            },
            "mld_leaves": {
                "rx": 0,
                "tx": 0
            },
            "mld_parse_errors": 0
        }
    } ]

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 ip/iplink_bridge.c | 169 ++++++++++++++++++++++++++++-----------------
 1 file changed, 104 insertions(+), 65 deletions(-)

diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index fbf8a79bfbbb..e9b77fdfe377 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -670,7 +670,7 @@ static void bridge_print_xstats_help(struct link_util *lu, FILE *f)
 	fprintf(f, "Usage: ... %s [ igmp ] [ dev DEVICE ]\n", lu->id);
 }
 
-static void bridge_print_stats_attr(FILE *f, struct rtattr *attr, int ifindex)
+static void bridge_print_stats_attr(struct rtattr *attr, int ifindex)
 {
 	struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1];
 	struct br_mcast_stats *mstats;
@@ -685,76 +685,116 @@ static void bridge_print_stats_attr(FILE *f, struct rtattr *attr, int ifindex)
 
 	list = brtb[LINK_XSTATS_TYPE_BRIDGE];
 	rem = RTA_PAYLOAD(list);
+	open_json_object(NULL);
+	ifname = ll_index_to_name(ifindex);
+	print_string(PRINT_ANY, "ifname", "%-16s\n", ifname);
 	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
 		if (xstats_print_attr && i->rta_type != xstats_print_attr)
 			continue;
 		switch (i->rta_type) {
 		case BRIDGE_XSTATS_MCAST:
 			mstats = RTA_DATA(i);
-			ifname = ll_index_to_name(ifindex);
-			fprintf(f, "%-16s\n", ifname);
-			fprintf(f, "%-16s    IGMP queries:\n", "");
-			fprintf(f, "%-16s      RX: v1 %llu v2 %llu v3 %llu\n",
-				"",
-				mstats->igmp_v1queries[BR_MCAST_DIR_RX],
-				mstats->igmp_v2queries[BR_MCAST_DIR_RX],
-				mstats->igmp_v3queries[BR_MCAST_DIR_RX]);
-			fprintf(f, "%-16s      TX: v1 %llu v2 %llu v3 %llu\n",
-				"",
-				mstats->igmp_v1queries[BR_MCAST_DIR_TX],
-				mstats->igmp_v2queries[BR_MCAST_DIR_TX],
-				mstats->igmp_v3queries[BR_MCAST_DIR_TX]);
-
-			fprintf(f, "%-16s    IGMP reports:\n", "");
-			fprintf(f, "%-16s      RX: v1 %llu v2 %llu v3 %llu\n",
-				"",
-				mstats->igmp_v1reports[BR_MCAST_DIR_RX],
-				mstats->igmp_v2reports[BR_MCAST_DIR_RX],
-				mstats->igmp_v3reports[BR_MCAST_DIR_RX]);
-			fprintf(f, "%-16s      TX: v1 %llu v2 %llu v3 %llu\n",
-				"",
-				mstats->igmp_v1reports[BR_MCAST_DIR_TX],
-				mstats->igmp_v2reports[BR_MCAST_DIR_TX],
-				mstats->igmp_v3reports[BR_MCAST_DIR_TX]);
-
-			fprintf(f, "%-16s    IGMP leaves: RX: %llu TX: %llu\n",
-				"",
-				mstats->igmp_leaves[BR_MCAST_DIR_RX],
-				mstats->igmp_leaves[BR_MCAST_DIR_TX]);
-
-			fprintf(f, "%-16s    IGMP parse errors: %llu\n",
-				"", mstats->igmp_parse_errors);
-
-			fprintf(f, "%-16s    MLD queries:\n", "");
-			fprintf(f, "%-16s      RX: v1 %llu v2 %llu\n",
-				"",
-				mstats->mld_v1queries[BR_MCAST_DIR_RX],
-				mstats->mld_v2queries[BR_MCAST_DIR_RX]);
-			fprintf(f, "%-16s      TX: v1 %llu v2 %llu\n",
-				"",
-				mstats->mld_v1queries[BR_MCAST_DIR_TX],
-				mstats->mld_v2queries[BR_MCAST_DIR_TX]);
-
-			fprintf(f, "%-16s    MLD reports:\n", "");
-			fprintf(f, "%-16s      RX: v1 %llu v2 %llu\n",
-				"",
-				mstats->mld_v1reports[BR_MCAST_DIR_RX],
-				mstats->mld_v2reports[BR_MCAST_DIR_RX]);
-			fprintf(f, "%-16s      TX: v1 %llu v2 %llu\n",
-				"",
-				mstats->mld_v1reports[BR_MCAST_DIR_TX],
-				mstats->mld_v2reports[BR_MCAST_DIR_TX]);
-
-			fprintf(f, "%-16s    MLD leaves: RX: %llu TX: %llu\n",
-				"",
-				mstats->mld_leaves[BR_MCAST_DIR_RX],
-				mstats->mld_leaves[BR_MCAST_DIR_TX]);
-
-			fprintf(f, "%-16s    MLD parse errors: %llu\n",
-				"", mstats->mld_parse_errors);
+			open_json_object("multicast");
+			open_json_object("igmp_queries");
+			print_string(PRINT_FP, NULL,
+				     "%-16s    IGMP queries:\n", "");
+			print_string(PRINT_FP, NULL, "%-16s      ", "");
+			print_u64(PRINT_ANY, "rx_v1", "RX: v1 %llu ",
+				  mstats->igmp_v1queries[BR_MCAST_DIR_RX]);
+			print_u64(PRINT_ANY, "rx_v2", "v2 %llu ",
+				  mstats->igmp_v2queries[BR_MCAST_DIR_RX]);
+			print_u64(PRINT_ANY, "rx_v3", "v3 %llu\n",
+				  mstats->igmp_v3queries[BR_MCAST_DIR_RX]);
+			print_string(PRINT_FP, NULL, "%-16s      ", "");
+			print_u64(PRINT_ANY, "tx_v1", "TX: v1 %llu ",
+				  mstats->igmp_v1queries[BR_MCAST_DIR_TX]);
+			print_u64(PRINT_ANY, "tx_v2", "v2 %llu ",
+				  mstats->igmp_v2queries[BR_MCAST_DIR_TX]);
+			print_u64(PRINT_ANY, "tx_v3", "v3 %llu\n",
+				  mstats->igmp_v3queries[BR_MCAST_DIR_TX]);
+			close_json_object();
+
+			open_json_object("igmp_reports");
+			print_string(PRINT_FP, NULL,
+				     "%-16s    IGMP reports:\n", "");
+			print_string(PRINT_FP, NULL, "%-16s      ", "");
+			print_u64(PRINT_ANY, "rx_v1", "RX: v1 %llu ",
+				  mstats->igmp_v1reports[BR_MCAST_DIR_RX]);
+			print_u64(PRINT_ANY, "rx_v2", "v2 %llu ",
+				  mstats->igmp_v2reports[BR_MCAST_DIR_RX]);
+			print_u64(PRINT_ANY, "rx_v3", "v3 %llu\n",
+				  mstats->igmp_v3reports[BR_MCAST_DIR_RX]);
+			print_string(PRINT_FP, NULL, "%-16s      ", "");
+			print_u64(PRINT_ANY, "tx_v1", "TX: v1 %llu ",
+				  mstats->igmp_v1reports[BR_MCAST_DIR_TX]);
+			print_u64(PRINT_ANY, "tx_v2", "v2 %llu",
+				  mstats->igmp_v2reports[BR_MCAST_DIR_TX]);
+			print_u64(PRINT_ANY, "tx_v3", "v3 %llu\n",
+				  mstats->igmp_v3reports[BR_MCAST_DIR_TX]);
+			close_json_object();
+
+			open_json_object("igmp_leaves");
+			print_string(PRINT_FP, NULL,
+				     "%-16s    IGMP leaves: ", "");
+			print_u64(PRINT_ANY, "rx", "RX: %llu ",
+				  mstats->igmp_leaves[BR_MCAST_DIR_RX]);
+			print_u64(PRINT_ANY, "tx", "TX: %llu\n",
+				  mstats->igmp_leaves[BR_MCAST_DIR_TX]);
+			close_json_object();
+
+			print_string(PRINT_FP, NULL,
+				     "%-16s    IGMP parse errors: ", "");
+			print_u64(PRINT_ANY, "igmp_parse_errors", "%llu\n",
+				  mstats->igmp_parse_errors);
+
+			open_json_object("mld_queries");
+			print_string(PRINT_FP, NULL,
+				     "%-16s    MLD queries:\n", "");
+			print_string(PRINT_FP, NULL, "%-16s      ", "");
+			print_u64(PRINT_ANY, "rx_v1", "RX: v1 %llu ",
+				  mstats->mld_v1queries[BR_MCAST_DIR_RX]);
+			print_u64(PRINT_ANY, "rx_v2", "v2 %llu\n",
+				  mstats->mld_v2queries[BR_MCAST_DIR_RX]);
+			print_string(PRINT_FP, NULL, "%-16s      ", "");
+			print_u64(PRINT_ANY, "tx_v1", "TX: v1 %llu ",
+				  mstats->mld_v1queries[BR_MCAST_DIR_TX]);
+			print_u64(PRINT_ANY, "tx_v2", "v2 %llu\n",
+				  mstats->mld_v2queries[BR_MCAST_DIR_TX]);
+			close_json_object();
+
+			open_json_object("mld_reports");
+			print_string(PRINT_FP, NULL,
+				     "%-16s    MLD reports:\n", "");
+			print_string(PRINT_FP, NULL, "%-16s      ", "");
+			print_u64(PRINT_ANY, "rx_v1", "RX: v1 %llu ",
+				  mstats->mld_v1reports[BR_MCAST_DIR_RX]);
+			print_u64(PRINT_ANY, "rx_v2", "v2 %llu\n",
+				  mstats->mld_v2reports[BR_MCAST_DIR_RX]);
+			print_string(PRINT_FP, NULL, "%-16s      ", "");
+			print_u64(PRINT_ANY, "tx_v1", "TX: v1 %llu ",
+				  mstats->mld_v1reports[BR_MCAST_DIR_TX]);
+			print_u64(PRINT_ANY, "tx_v2", "v2 %llu\n",
+				  mstats->mld_v2reports[BR_MCAST_DIR_TX]);
+			close_json_object();
+
+			open_json_object("mld_leaves");
+			print_string(PRINT_FP, NULL,
+				     "%-16s    MLD leaves: ", "");
+			print_u64(PRINT_ANY, "rx", "RX: %llu ",
+				  mstats->mld_leaves[BR_MCAST_DIR_RX]);
+			print_u64(PRINT_ANY, "tx", "TX: %llu\n",
+				  mstats->mld_leaves[BR_MCAST_DIR_TX]);
+			close_json_object();
+
+			print_string(PRINT_FP, NULL,
+				     "%-16s    MLD parse errors: ", "");
+			print_u64(PRINT_ANY, "mld_parse_errors", "%llu\n",
+				  mstats->mld_parse_errors);
+			close_json_object();
 			break;
 		}
 	}
+	close_json_object();
 }
 
 int bridge_print_xstats(struct nlmsghdr *n, void *arg)
@@ -762,7 +802,6 @@ int bridge_print_xstats(struct nlmsghdr *n, void *arg)
 	struct if_stats_msg *ifsm = NLMSG_DATA(n);
 	struct rtattr *tb[IFLA_STATS_MAX+1];
 	int len = n->nlmsg_len;
-	FILE *fp = arg;
 
 	len -= NLMSG_LENGTH(sizeof(*ifsm));
 	if (len < 0) {
@@ -774,11 +813,11 @@ int bridge_print_xstats(struct nlmsghdr *n, void *arg)
 
 	parse_rtattr(tb, IFLA_STATS_MAX, IFLA_STATS_RTA(ifsm), len);
 	if (tb[IFLA_STATS_LINK_XSTATS])
-		bridge_print_stats_attr(fp, tb[IFLA_STATS_LINK_XSTATS],
+		bridge_print_stats_attr(tb[IFLA_STATS_LINK_XSTATS],
 					ifsm->ifindex);
 
 	if (tb[IFLA_STATS_LINK_XSTATS_SLAVE])
-		bridge_print_stats_attr(fp, tb[IFLA_STATS_LINK_XSTATS_SLAVE],
+		bridge_print_stats_attr(tb[IFLA_STATS_LINK_XSTATS_SLAVE],
 					ifsm->ifindex);
 
 	return 0;
-- 
2.17.2

[PATCH iproute2-next 3/3] ip: bond: add xstats support

[Nikolay Aleksandrov]

Add bond and bond_slave xstats support with optional json output.
Example:
- Plain text:
$ ip link xstats type bond 802.3ad
 bond0
                    LACPDU Rx 2017
                    LACPDU Tx 2038
                    LACPDU Unknown type Rx 0
                    LACPDU Illegal Rx 0
                    Marker Rx 0
                    Marker Tx 0
                    Marker response Rx 0
                    Marker response Tx 0
                    Marker unknown type Rx 0

- JSON:
$ ip -j -p link xstats type bond 802.3ad
  [ {
        "ifname": "bond0",
        "802.3ad": {
            "lacpdu_rx": 219,
            "lacpdu_tx": 241,
            "lacpdu_unknown_rx": 0,
            "lacpdu_illegal_rx": 0,
            "marker_rx": 0,
            "marker_tx": 0,
            "marker_response_rx": 0,
            "marker_response_tx": 0,
            "marker_unknown_rx": 0
        }
    } ]

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 ip/ip_common.h         |   3 +
 ip/iplink_bond.c       | 167 ++++++++++++++++++++++++++++++++++++++++-
 ip/iplink_bond_slave.c |   2 +
 3 files changed, 169 insertions(+), 3 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index d67575c63c24..b4aa34a70c92 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -130,6 +130,9 @@ void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len);
 int bridge_parse_xstats(struct link_util *lu, int argc, char **argv);
 int bridge_print_xstats(struct nlmsghdr *n, void *arg);
 
+int bond_parse_xstats(struct link_util *lu, int argc, char **argv);
+int bond_print_xstats(struct nlmsghdr *n, void *arg);
+
 /* iproute_lwtunnel.c */
 int lwt_parse_encap(struct rtattr *rta, size_t len, int *argcp, char ***argvp);
 void lwt_print_encap(FILE *fp, struct rtattr *encap_type, struct rtattr *encap);
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index f906e7f1b323..c60f0e8ad0a0 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -13,16 +13,18 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <linux/if_link.h>
-#include <linux/if_ether.h>
-#include <net/if.h>
+#include <linux/if_bonding.h>
 
 #include "rt_names.h"
 #include "utils.h"
 #include "ip_common.h"
+#include "json_print.h"
 
 #define BOND_MAX_ARP_TARGETS    16
 
+static unsigned int xstats_print_attr;
+static int filter_index;
+
 static const char *mode_tbl[] = {
 	"balance-rr",
 	"active-backup",
@@ -649,10 +651,169 @@ static void bond_print_help(struct link_util *lu, int argc, char **argv,
 	print_explain(f);
 }
 
+static void bond_print_xstats_help(struct link_util *lu, FILE *f)
+{
+	fprintf(f, "Usage: ... %s [ 802.3ad ] [ dev DEVICE ]\n", lu->id);
+}
+
+static void bond_print_3ad_stats(struct rtattr *lacpattr)
+{
+	struct rtattr *lacptb[BOND_3AD_STAT_MAX+1];
+	__u64 val;
+
+	parse_rtattr(lacptb, BOND_3AD_STAT_MAX, RTA_DATA(lacpattr),
+		     RTA_PAYLOAD(lacpattr));
+	open_json_object("802.3ad");
+	if (lacptb[BOND_3AD_STAT_LACPDU_RX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		print_u64(PRINT_ANY, "lacpdu_rx", "LACPDU Rx %llu\n",
+			  rta_getattr_u64(lacptb[BOND_3AD_STAT_LACPDU_RX]));
+	}
+	if (lacptb[BOND_3AD_STAT_LACPDU_TX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		print_u64(PRINT_ANY, "lacpdu_tx", "LACPDU Tx %llu\n",
+			  rta_getattr_u64(lacptb[BOND_3AD_STAT_LACPDU_TX]));
+	}
+	if (lacptb[BOND_3AD_STAT_LACPDU_UNKNOWN_RX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		val = rta_getattr_u64(lacptb[BOND_3AD_STAT_LACPDU_UNKNOWN_RX]);
+		print_u64(PRINT_ANY,
+			  "lacpdu_unknown_rx",
+			  "LACPDU Unknown type Rx %llu\n",
+			  val);
+	}
+	if (lacptb[BOND_3AD_STAT_LACPDU_ILLEGAL_RX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		val = rta_getattr_u64(lacptb[BOND_3AD_STAT_LACPDU_ILLEGAL_RX]);
+		print_u64(PRINT_ANY,
+			  "lacpdu_illegal_rx",
+			  "LACPDU Illegal Rx %llu\n",
+			  val);
+	}
+	if (lacptb[BOND_3AD_STAT_MARKER_RX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		print_u64(PRINT_ANY, "marker_rx", "Marker Rx %llu\n",
+			  rta_getattr_u64(lacptb[BOND_3AD_STAT_MARKER_RX]));
+	}
+	if (lacptb[BOND_3AD_STAT_MARKER_TX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		print_u64(PRINT_ANY, "marker_tx", "Marker Tx %llu\n",
+			  rta_getattr_u64(lacptb[BOND_3AD_STAT_MARKER_TX]));
+	}
+	if (lacptb[BOND_3AD_STAT_MARKER_RESP_RX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		val = rta_getattr_u64(lacptb[BOND_3AD_STAT_MARKER_RESP_RX]);
+		print_u64(PRINT_ANY,
+			  "marker_response_rx",
+			  "Marker response Rx %llu\n",
+			  val);
+	}
+	if (lacptb[BOND_3AD_STAT_MARKER_RESP_TX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		val = rta_getattr_u64(lacptb[BOND_3AD_STAT_MARKER_RESP_TX]);
+		print_u64(PRINT_ANY,
+			  "marker_response_tx",
+			  "Marker response Tx %llu\n",
+			  val);
+	}
+	if (lacptb[BOND_3AD_STAT_MARKER_UNKNOWN_RX]) {
+		print_string(PRINT_FP, NULL, "%-16s    ", "");
+		val = rta_getattr_u64(lacptb[BOND_3AD_STAT_MARKER_UNKNOWN_RX]);
+		print_u64(PRINT_ANY,
+			  "marker_unknown_rx",
+			  "Marker unknown type Rx %llu\n",
+			  val);
+	}
+	close_json_object();
+}
+
+static void bond_print_stats_attr(struct rtattr *attr, int ifindex)
+{
+	struct rtattr *bondtb[LINK_XSTATS_TYPE_MAX+1];
+	struct rtattr *i, *list;
+	const char *ifname = "";
+	int rem;
+
+	parse_rtattr(bondtb, LINK_XSTATS_TYPE_MAX+1, RTA_DATA(attr),
+	RTA_PAYLOAD(attr));
+	if (!bondtb[LINK_XSTATS_TYPE_BOND])
+		return;
+
+	list = bondtb[LINK_XSTATS_TYPE_BOND];
+	rem = RTA_PAYLOAD(list);
+	open_json_object(NULL);
+	ifname = ll_index_to_name(ifindex);
+	print_string(PRINT_ANY, "ifname", "%-16s\n", ifname);
+	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
+		if (xstats_print_attr && i->rta_type != xstats_print_attr)
+			continue;
+
+		switch (i->rta_type) {
+		case BOND_XSTATS_3AD:
+			bond_print_3ad_stats(i);
+			break;
+		}
+		break;
+	}
+	close_json_object();
+}
+
+int bond_print_xstats(struct nlmsghdr *n, void *arg)
+{
+	struct if_stats_msg *ifsm = NLMSG_DATA(n);
+	struct rtattr *tb[IFLA_STATS_MAX+1];
+	int len = n->nlmsg_len;
+
+	len -= NLMSG_LENGTH(sizeof(*ifsm));
+	if (len < 0) {
+		fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
+		return -1;
+	}
+	if (filter_index && filter_index != ifsm->ifindex)
+		return 0;
+
+	parse_rtattr(tb, IFLA_STATS_MAX, IFLA_STATS_RTA(ifsm), len);
+	if (tb[IFLA_STATS_LINK_XSTATS])
+		bond_print_stats_attr(tb[IFLA_STATS_LINK_XSTATS],
+				      ifsm->ifindex);
+
+	if (tb[IFLA_STATS_LINK_XSTATS_SLAVE])
+		bond_print_stats_attr(tb[IFLA_STATS_LINK_XSTATS_SLAVE],
+				      ifsm->ifindex);
+
+	return 0;
+}
+
+int bond_parse_xstats(struct link_util *lu, int argc, char **argv)
+{
+	while (argc > 0) {
+		if (strcmp(*argv, "lacp") == 0 ||
+		    strcmp(*argv, "802.3ad") == 0) {
+			xstats_print_attr = BOND_XSTATS_3AD;
+		} else if (strcmp(*argv, "dev") == 0) {
+			NEXT_ARG();
+			filter_index = ll_name_to_index(*argv);
+			if (!filter_index)
+				return nodev(*argv);
+		} else if (strcmp(*argv, "help") == 0) {
+			bond_print_xstats_help(lu, stdout);
+			exit(0);
+		} else {
+			invarg("unknown attribute", *argv);
+		}
+		argc--; argv++;
+	}
+
+	return 0;
+}
+
+
 struct link_util bond_link_util = {
 	.id		= "bond",
 	.maxattr	= IFLA_BOND_MAX,
 	.parse_opt	= bond_parse_opt,
 	.print_opt	= bond_print_opt,
 	.print_help	= bond_print_help,
+	.parse_ifla_xstats = bond_parse_xstats,
+	.print_ifla_xstats = bond_print_xstats,
 };
diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c
index 67219c67241b..4eaf72b86f59 100644
--- a/ip/iplink_bond_slave.c
+++ b/ip/iplink_bond_slave.c
@@ -156,4 +156,6 @@ struct link_util bond_slave_link_util = {
 	.print_opt	= bond_slave_print_opt,
 	.parse_opt	= bond_slave_parse_opt,
 	.print_help	= bond_slave_print_help,
+	.parse_ifla_xstats = bond_parse_xstats,
+	.print_ifla_xstats = bond_print_xstats,
 };
-- 
2.17.2

Re: [PATCH iproute2-next 2/3] ip: bridge: add xstats json support

[Stephen Hemminger]

On Tue, 12 Mar 2019 18:41:27 +0200
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:

I like it.

> +			print_u64(PRINT_ANY, "rx_v1", "RX: v1 %llu ",
> +				  mstats->igmp_v1reports[BR_MCAST_DIR_RX]);
> +			print_u64(PRINT_ANY, "rx_v2", "v2 %llu ",
> +				  mstats->igmp_v2reports[BR_MCAST_DIR_RX]);
> +			print_u64(PRINT_ANY, "rx_v3", "v3 %llu\n",
> +				  mstats->igmp_v3reports[BR_MCAST_DIR_RX]);
> +			print_string(PRINT_FP, NULL, "%-16s      ", "");
> +			print_u64(PRINT_ANY, "tx_v1", "TX: v1 %llu ",
> +				  mstats->igmp_v1reports[BR_MCAST_DIR_TX]);
> +			print_u64(PRINT_ANY, "tx_v2", "v2 %llu",
> +				  mstats->igmp_v2reports[BR_MCAST_DIR_TX]);
> +			print_u64(PRINT_ANY, "tx_v3", "v3 %llu\n",
> +				  mstats->igmp_v3reports[BR_MCAST_DIR_TX]);

Maybe the IGMP reports should be shown in JSON as as an array?
Or use "rx_igmpv1"  the tag could be more descriptive

Re: [PATCH iproute2-next 2/3] ip: bridge: add xstats json support

[nikolay]

On 12 March 2019 21:33:48 EET, Stephen Hemminger <stephen@networkplumber.org> wrote:
>On Tue, 12 Mar 2019 18:41:27 +0200
>Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>
>I like it.
>
>> +			print_u64(PRINT_ANY, "rx_v1", "RX: v1 %llu ",
>> +				  mstats->igmp_v1reports[BR_MCAST_DIR_RX]);
>> +			print_u64(PRINT_ANY, "rx_v2", "v2 %llu ",
>> +				  mstats->igmp_v2reports[BR_MCAST_DIR_RX]);
>> +			print_u64(PRINT_ANY, "rx_v3", "v3 %llu\n",
>> +				  mstats->igmp_v3reports[BR_MCAST_DIR_RX]);
>> +			print_string(PRINT_FP, NULL, "%-16s      ", "");
>> +			print_u64(PRINT_ANY, "tx_v1", "TX: v1 %llu ",
>> +				  mstats->igmp_v1reports[BR_MCAST_DIR_TX]);
>> +			print_u64(PRINT_ANY, "tx_v2", "v2 %llu",
>> +				  mstats->igmp_v2reports[BR_MCAST_DIR_TX]);
>> +			print_u64(PRINT_ANY, "tx_v3", "v3 %llu\n",
>> +				  mstats->igmp_v3reports[BR_MCAST_DIR_TX]);
>
>Maybe the IGMP reports should be shown in JSON as as an array?
>Or use "rx_igmpv1"  the tag could be more descriptive

Please note that these are inside of an object called "igmp_reports", 
if I change them it will become:
"igmp_reports" : {"rx_igmpv1" :0} 
I think adding igmp inside is redundant. 

Thanks, 
   Nik

Re: [PATCH iproute2-next 2/3] ip: bridge: add xstats json support

[Stephen Hemminger]

On Tue, 12 Mar 2019 21:42:29 +0200
nikolay@cumulusnetworks.com wrote:

> On 12 March 2019 21:33:48 EET, Stephen Hemminger <stephen@networkplumber.org> wrote:
> >On Tue, 12 Mar 2019 18:41:27 +0200
> >Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> >
> >I like it.
> >  
> >> +			print_u64(PRINT_ANY, "rx_v1", "RX: v1 %llu ",
> >> +				  mstats->igmp_v1reports[BR_MCAST_DIR_RX]);
> >> +			print_u64(PRINT_ANY, "rx_v2", "v2 %llu ",
> >> +				  mstats->igmp_v2reports[BR_MCAST_DIR_RX]);
> >> +			print_u64(PRINT_ANY, "rx_v3", "v3 %llu\n",
> >> +				  mstats->igmp_v3reports[BR_MCAST_DIR_RX]);
> >> +			print_string(PRINT_FP, NULL, "%-16s      ", "");
> >> +			print_u64(PRINT_ANY, "tx_v1", "TX: v1 %llu ",
> >> +				  mstats->igmp_v1reports[BR_MCAST_DIR_TX]);
> >> +			print_u64(PRINT_ANY, "tx_v2", "v2 %llu",
> >> +				  mstats->igmp_v2reports[BR_MCAST_DIR_TX]);
> >> +			print_u64(PRINT_ANY, "tx_v3", "v3 %llu\n",
> >> +				  mstats->igmp_v3reports[BR_MCAST_DIR_TX]);  
> >
> >Maybe the IGMP reports should be shown in JSON as as an array?
> >Or use "rx_igmpv1"  the tag could be more descriptive  
> 
> Please note that these are inside of an object called "igmp_reports", 
> if I change them it will become:
> "igmp_reports" : {"rx_igmpv1" :0} 
> I think adding igmp inside is redundant. 
> 
> Thanks, 
>    Nik
> 

ok, that makes sense now

Re: [PATCH iproute2-next 0/3] bond, bridge: add xstats json support

[David Ahern]

On 3/12/19 10:41 AM, Nikolay Aleksandrov wrote:
> Hi,
> This set adds json output support to the xstats API (patch 01) and then
> adds json support to the bridge xstats output (patch 02) and adds xstats
> output support (both plain text and json) for the bonding (patch 03).
> It doesn't change the bridge's plain text output, but it fixes an
> inconsistency that could happen if new bridge xstats attributes were
> added (print the interface name once for each group of xstats attrs).
> 

applied to iproute2-next. Thanks