* bridge mdb json output broken/invalid
@ 2019-04-05 12:59 Nikolay Aleksandrov
2019-04-05 16:52 ` Stephen Hemminger
0 siblings, 1 reply; 4+ messages in thread
From: Nikolay Aleksandrov @ 2019-04-05 12:59 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Roopa Prabhu, David Ahern, network dev
Hi Stephen,
It seems that commit c7c1a1ef51aea ("bridge: colorize output and use JSON print library") broke
bridge mdb's json output. The json after that commit is invalid, more below.
Setup: 2 bridges - virbr0 and br-test
non-json bridge mdb show:
3: virbr0 vnet3 239.10.10.50 temp
3: virbr0 vnet3 225.1.2.3 temp
14: br-test v1 224.224.224.224 permanent
json bridge -d -p -j mdb show before c7c1a1ef51aea:
{
"mdb": [ {
"dev": "virbr0",
"port": "vnet3",
"grp": "239.10.10.50",
"state": "temp"
},{
"dev": "virbr0",
"port": "vnet3",
"grp": "225.1.2.3",
"state": "temp"
},{
"dev": "br-test",
"port": "v1",
"grp": "224.224.224.224",
"state": "permanent"
} ],
"router": {
"virbr0": [ {
"port": "vnet3"
} ],
"br-test": [ {
"port": "v1"
} ]
}
}
json bridge -d -p -j mdb show after c7c1a1ef51aea:
[
"mdb": [ {
"index": 3,
"dev": "virbr0",
"port": "vnet3",
"grp": "239.10.10.50",
"state": "temp",
"flags": [ ]
},{
"index": 3,
"dev": "virbr0",
"port": "vnet3",
"grp": "225.1.2.3",
"state": "temp",
"flags": [ ]
} ],
"router": [
"virbr0": [ {
"port": "vnet3"
} ] ],
"mdb": [ ],
"mdb": [ {
"index": 14,
"dev": "br-test",
"port": "v1",
"grp": "224.224.224.224",
"state": "permanent",
"flags": [ ]
} ],
"router": [
"br-test": [ {
"port": "v1"
} ] ] ]
Obviously this is not a valid object array and even if it was converted, it must not contain
duplicate entries. The "mdb" and "router" array entries must be unique and should contain
all the single entries inside, also the global context shouldn't be array.
Cheers,
Nik
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: bridge mdb json output broken/invalid 2019-04-05 12:59 bridge mdb json output broken/invalid Nikolay Aleksandrov @ 2019-04-05 16:52 ` Stephen Hemminger 2019-04-12 15:30 ` [PATCH iproute2] bridge: mdb: restore valid json output Nikolay Aleksandrov 0 siblings, 1 reply; 4+ messages in thread From: Stephen Hemminger @ 2019-04-05 16:52 UTC (permalink / raw) To: Nikolay Aleksandrov; +Cc: Roopa Prabhu, David Ahern, network dev On Fri, 5 Apr 2019 15:59:03 +0300 Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > Hi Stephen, > It seems that commit c7c1a1ef51aea ("bridge: colorize output and use JSON print library") broke > bridge mdb's json output. The json after that commit is invalid, more below. > > Setup: 2 bridges - virbr0 and br-test > > non-json bridge mdb show: > 3: virbr0 vnet3 239.10.10.50 temp > 3: virbr0 vnet3 225.1.2.3 temp > 14: br-test v1 224.224.224.224 permanent > > json bridge -d -p -j mdb show before c7c1a1ef51aea: > { > "mdb": [ { > "dev": "virbr0", > "port": "vnet3", > "grp": "239.10.10.50", > "state": "temp" > },{ > "dev": "virbr0", > "port": "vnet3", > "grp": "225.1.2.3", > "state": "temp" > },{ > "dev": "br-test", > "port": "v1", > "grp": "224.224.224.224", > "state": "permanent" > } ], > "router": { > "virbr0": [ { > "port": "vnet3" > } ], > "br-test": [ { > "port": "v1" > } ] > } > } > > json bridge -d -p -j mdb show after c7c1a1ef51aea: > [ > "mdb": [ { > "index": 3, > "dev": "virbr0", > "port": "vnet3", > "grp": "239.10.10.50", > "state": "temp", > "flags": [ ] > },{ > "index": 3, > "dev": "virbr0", > "port": "vnet3", > "grp": "225.1.2.3", > "state": "temp", > "flags": [ ] > } ], > "router": [ > "virbr0": [ { > "port": "vnet3" > } ] ], > "mdb": [ ], > "mdb": [ { > "index": 14, > "dev": "br-test", > "port": "v1", > "grp": "224.224.224.224", > "state": "permanent", > "flags": [ ] > } ], > "router": [ > "br-test": [ { > "port": "v1" > } ] ] ] > > > Obviously this is not a valid object array and even if it was converted, it must not contain > duplicate entries. The "mdb" and "router" array entries must be unique and should contain > all the single entries inside, also the global context shouldn't be array. > > Cheers, > Nik Yes this is wrong, sending the list twice. Not sure why that is happening, the original code had some awkward flags to handle this. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH iproute2] bridge: mdb: restore valid json output 2019-04-05 16:52 ` Stephen Hemminger @ 2019-04-12 15:30 ` Nikolay Aleksandrov 2019-04-17 23:35 ` Stephen Hemminger 0 siblings, 1 reply; 4+ messages in thread From: Nikolay Aleksandrov @ 2019-04-12 15:30 UTC (permalink / raw) To: netdev; +Cc: roopa, dsa, stephen, Nikolay Aleksandrov Since the commit below mdb's json output has been invalid and also with changed format. Restore it to a valid json like the previous format. Also takes care of a double "Deleted" print when monitoring for changes. Example bridge -p -d -j mdb show: [ { "mdb": [ { "index": 4, "dev": "virbr0", "port": "vnet2", "grp": "ff02::202", "state": "temp", "flags": [ ] },{ "index": 4, "dev": "virbr0", "port": "vnet2", "grp": "ff02::1:fffb:1939", "state": "temp", "flags": [ ] },{ "index": 6, "dev": "virbr1", "port": "vnet7", "grp": "ff02::202", "state": "temp", "flags": [ ] },{ "index": 6, "dev": "virbr1", "port": "vnet7", "grp": "ff02::1:ffd0:f61f", "state": "temp", "flags": [ ] } ], "router": { "virbr0": [ { "port": "vnet1" },{ "port": "vnet0" } ], "virbr1": [ { "port": "vnet5" } ] } } ] Fixes: c7c1a1ef51ae ("bridge: colorize output and use JSON print library") Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> --- bridge/br_common.h | 2 +- bridge/mdb.c | 80 +++++++++++++++++++++++++++++++++++++++------- bridge/monitor.c | 2 +- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/bridge/br_common.h b/bridge/br_common.h index 23d653df931d..b5798da300e8 100644 --- a/bridge/br_common.h +++ b/bridge/br_common.h @@ -8,8 +8,8 @@ void print_vlan_info(struct rtattr *tb, int ifindex); int print_linkinfo(struct nlmsghdr *n, void *arg); +int print_mdb_mon(struct nlmsghdr *n, void *arg); int print_fdb(struct nlmsghdr *n, void *arg); -int print_mdb(struct nlmsghdr *n, void *arg); int do_fdb(int argc, char **argv); int do_mdb(int argc, char **argv); diff --git a/bridge/mdb.c b/bridge/mdb.c index 855a6a4552c7..59aa17643517 100644 --- a/bridge/mdb.c +++ b/bridge/mdb.c @@ -132,9 +132,6 @@ static void print_mdb_entry(FILE *f, int ifindex, const struct br_mdb_entry *e, open_json_object(NULL); - if (n->nlmsg_type == RTM_DELMDB) - print_bool(PRINT_ANY, "deleted", "Deleted ", true); - print_int(PRINT_ANY, "index", "%u: ", ifindex); print_color_string(PRINT_ANY, COLOR_IFNAME, "dev", "%s ", dev); print_string(PRINT_ANY, "port", " %s ", @@ -189,10 +186,8 @@ static void print_mdb_entries(FILE *fp, struct nlmsghdr *n, int rem = RTA_PAYLOAD(mdb); struct rtattr *i; - open_json_array(PRINT_JSON, "mdb"); for (i = RTA_DATA(mdb); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) br_print_mdb_entry(fp, ifindex, i, n); - close_json_array(PRINT_JSON, NULL); } static void print_router_entries(FILE *fp, struct nlmsghdr *n, @@ -200,7 +195,6 @@ static void print_router_entries(FILE *fp, struct nlmsghdr *n, { const char *brifname = ll_index_to_name(ifindex); - open_json_array(PRINT_JSON, "router"); if (n->nlmsg_type == RTM_GETMDB) { if (show_details) br_print_router_ports(fp, router, brifname); @@ -222,15 +216,12 @@ static void print_router_entries(FILE *fp, struct nlmsghdr *n, port_name, brifname); } } - close_json_array(PRINT_JSON, NULL); } -int print_mdb(struct nlmsghdr *n, void *arg) +static int __parse_mdb_nlmsg(struct nlmsghdr *n, struct rtattr **tb) { - FILE *fp = arg; struct br_port_msg *r = NLMSG_DATA(n); int len = n->nlmsg_len; - struct rtattr *tb[MDBA_MAX+1]; if (n->nlmsg_type != RTM_GETMDB && n->nlmsg_type != RTM_NEWMDB && @@ -253,6 +244,54 @@ int print_mdb(struct nlmsghdr *n, void *arg) parse_rtattr(tb, MDBA_MAX, MDBA_RTA(r), n->nlmsg_len - NLMSG_LENGTH(sizeof(*r))); + return 1; +} + +static int print_mdbs(struct nlmsghdr *n, void *arg) +{ + struct br_port_msg *r = NLMSG_DATA(n); + struct rtattr *tb[MDBA_MAX+1]; + FILE *fp = arg; + int ret; + + ret = __parse_mdb_nlmsg(n, tb); + if (ret != 1) + return ret; + + if (tb[MDBA_MDB]) + print_mdb_entries(fp, n, r->ifindex, tb[MDBA_MDB]); + + return 0; +} + +static int print_rtrs(struct nlmsghdr *n, void *arg) +{ + struct br_port_msg *r = NLMSG_DATA(n); + struct rtattr *tb[MDBA_MAX+1]; + FILE *fp = arg; + int ret; + + ret = __parse_mdb_nlmsg(n, tb); + if (ret != 1) + return ret; + + if (tb[MDBA_ROUTER]) + print_router_entries(fp, n, r->ifindex, tb[MDBA_ROUTER]); + + return 0; +} + +int print_mdb_mon(struct nlmsghdr *n, void *arg) +{ + struct br_port_msg *r = NLMSG_DATA(n); + struct rtattr *tb[MDBA_MAX+1]; + FILE *fp = arg; + int ret; + + ret = __parse_mdb_nlmsg(n, tb); + if (ret != 1) + return ret; + if (n->nlmsg_type == RTM_DELMDB) print_bool(PRINT_ANY, "deleted", "Deleted ", true); @@ -291,18 +330,35 @@ static int mdb_show(int argc, char **argv) } new_json_obj(json); + open_json_object(NULL); - /* get mdb entries*/ + /* get mdb entries */ if (rtnl_mdbdump_req(&rth, PF_BRIDGE) < 0) { perror("Cannot send dump request"); return -1; } - if (rtnl_dump_filter(&rth, print_mdb, stdout) < 0) { + open_json_array(PRINT_JSON, "mdb"); + if (rtnl_dump_filter(&rth, print_mdbs, stdout) < 0) { fprintf(stderr, "Dump terminated\n"); return -1; } + close_json_array(PRINT_JSON, NULL); + + /* get router ports */ + if (rtnl_mdbdump_req(&rth, PF_BRIDGE) < 0) { + perror("Cannot send dump request"); + return -1; + } + open_json_object("router"); + if (rtnl_dump_filter(&rth, print_rtrs, stdout) < 0) { + fprintf(stderr, "Dump terminated\n"); + return -1; + } + close_json_object(); + + close_json_object(); delete_json_obj(); fflush(stdout); diff --git a/bridge/monitor.c b/bridge/monitor.c index 708a1bd2ccb0..08439a60288a 100644 --- a/bridge/monitor.c +++ b/bridge/monitor.c @@ -61,7 +61,7 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl, case RTM_DELMDB: if (prefix_banner) fprintf(fp, "[MDB]"); - return print_mdb(n, arg); + return print_mdb_mon(n, arg); case NLMSG_TSTAMP: print_nlmsg_timestamp(fp, n); -- 2.20.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2] bridge: mdb: restore valid json output 2019-04-12 15:30 ` [PATCH iproute2] bridge: mdb: restore valid json output Nikolay Aleksandrov @ 2019-04-17 23:35 ` Stephen Hemminger 0 siblings, 0 replies; 4+ messages in thread From: Stephen Hemminger @ 2019-04-17 23:35 UTC (permalink / raw) To: Nikolay Aleksandrov; +Cc: netdev, roopa, dsa On Fri, 12 Apr 2019 18:30:55 +0300 Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > Since the commit below mdb's json output has been invalid and also with > changed format. Restore it to a valid json like the previous format. > Also takes care of a double "Deleted" print when monitoring for changes. > > Example bridge -p -d -j mdb show: > [ { > "mdb": [ { > "index": 4, > "dev": "virbr0", > "port": "vnet2", > "grp": "ff02::202", > "state": "temp", > "flags": [ ] > },{ > "index": 4, > "dev": "virbr0", > "port": "vnet2", > "grp": "ff02::1:fffb:1939", > "state": "temp", > "flags": [ ] > },{ > "index": 6, > "dev": "virbr1", > "port": "vnet7", > "grp": "ff02::202", > "state": "temp", > "flags": [ ] > },{ > "index": 6, > "dev": "virbr1", > "port": "vnet7", > "grp": "ff02::1:ffd0:f61f", > "state": "temp", > "flags": [ ] > } ], > "router": { > "virbr0": [ { > "port": "vnet1" > },{ > "port": "vnet0" > } ], > "virbr1": [ { > "port": "vnet5" > } ] > } > } ] > > Fixes: c7c1a1ef51ae ("bridge: colorize output and use JSON print library") > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Applied, thanks for fixing up my mess from using JSON here. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-17 23:35 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-04-05 12:59 bridge mdb json output broken/invalid Nikolay Aleksandrov 2019-04-05 16:52 ` Stephen Hemminger 2019-04-12 15:30 ` [PATCH iproute2] bridge: mdb: restore valid json output Nikolay Aleksandrov 2019-04-17 23:35 ` Stephen Hemminger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).