diff mbox series

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

Message ID 20190312164128.22536-3-nikolay@cumulusnetworks.com
State Accepted
Delegated to: David Ahern
Headers show
Series bond, bridge: add xstats json support | expand

Commit Message

Nikolay Aleksandrov March 12, 2019, 4:41 p.m. UTC 
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(-)

Comments

Stephen Hemminger March 12, 2019, 7:33 p.m. UTC | #1 
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
Nikolay Aleksandrov March 12, 2019, 7:42 p.m. UTC | #2 
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
Stephen Hemminger March 12, 2019, 11:51 p.m. UTC | #3 
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
diff mbox series

Patch

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;