From: Stephen Hemminger <stephen@networkplumber.org>
To: Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: netdev <netdev@vger.kernel.org>,
Stephen Hemminger <sthemmin@microsoft.com>,
Julien Fortin <julien@cumulusnetworks.com>,
David Ahern <dsahern@gmail.com>
Subject: Re: [PATCH v2 iproute2-next 2/5] bridge: colorize output and use JSON print library
Date: Wed, 29 Aug 2018 08:04:53 -0700 [thread overview]
Message-ID: <20180829080453.0026d52b@xeon-e3> (raw)
In-Reply-To: <CAJieiUi3BZPmaEJP6AK14zUwo20ssTNMz6eUfjFS0ZExizK9ng@mail.gmail.com>
On Sat, 14 Jul 2018 18:41:03 -0700
Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On Tue, Feb 20, 2018 at 11:24 AM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > From: Stephen Hemminger <sthemmin@microsoft.com>
> >
> > Use new functions from json_print to simplify code.
> > Provide standard flag for colorizing output.
> >
> > The shortened -c flag is ambiguous it could mean color or
> > compressvlan; it is now changed to mean color for consistency
> > with other iproute2 commands.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > bridge/br_common.h | 2 +-
> > bridge/bridge.c | 10 +-
> > bridge/fdb.c | 281 +++++++++++++++--------------------------
> > bridge/mdb.c | 362 ++++++++++++++++++++++-------------------------------
> > bridge/vlan.c | 276 +++++++++++++++-------------------------
> > 5 files changed, 363 insertions(+), 568 deletions(-)
> >
> > diff --git a/bridge/br_common.h b/bridge/br_common.h
> > index b25f61e50e05..2f1cb8fd9f3d 100644
> > --- a/bridge/br_common.h
> > +++ b/bridge/br_common.h
> > @@ -6,7 +6,7 @@
> > #define MDB_RTR_RTA(r) \
> > ((struct rtattr *)(((char *)(r)) + RTA_ALIGN(sizeof(__u32))))
> >
> > -extern void print_vlan_info(FILE *fp, struct rtattr *tb, int ifindex);
> > +extern void print_vlan_info(FILE *fp, struct rtattr *tb);
> > extern int print_linkinfo(const struct sockaddr_nl *who,
> > struct nlmsghdr *n,
> > void *arg);
> > diff --git a/bridge/bridge.c b/bridge/bridge.c
> > index 4b112e3b8da9..e5b4c3c2198f 100644
> > --- a/bridge/bridge.c
> > +++ b/bridge/bridge.c
> > @@ -16,12 +16,15 @@
> > #include "utils.h"
> > #include "br_common.h"
> > #include "namespace.h"
> > +#include "color.h"
> >
> > struct rtnl_handle rth = { .fd = -1 };
> > int preferred_family = AF_UNSPEC;
> > int oneline;
> > int show_stats;
> > int show_details;
> > +int show_pretty;
> > +int color;
> > int compress_vlans;
> > int json;
> > int timestamp;
> > @@ -39,7 +42,7 @@ static void usage(void)
> > "where OBJECT := { link | fdb | mdb | vlan | monitor }\n"
> > " OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
> > " -o[neline] | -t[imestamp] | -n[etns] name |\n"
> > -" -c[ompressvlans] -p[retty] -j{son} }\n");
> > +" -c[ompressvlans] -color -p[retty] -j{son} }\n");
> > exit(-1);
> > }
> >
> > @@ -170,6 +173,8 @@ main(int argc, char **argv)
> > NEXT_ARG();
> > if (netns_switch(argv[1]))
> > exit(-1);
> > + } else if (matches(opt, "-color") == 0) {
> > + enable_color();
> > } else if (matches(opt, "-compressvlans") == 0) {
> > ++compress_vlans;
> > } else if (matches(opt, "-force") == 0) {
> > @@ -195,6 +200,9 @@ main(int argc, char **argv)
> >
> > _SL_ = oneline ? "\\" : "\n";
> >
> > + if (json)
> > + check_if_color_enabled();
> > +
> > if (batch_file)
> > return batch(batch_file);
> >
> > diff --git a/bridge/fdb.c b/bridge/fdb.c
> > index 93b5b2e694e3..b4f6e8b3a01b 100644
> > --- a/bridge/fdb.c
> > +++ b/bridge/fdb.c
> > @@ -22,9 +22,9 @@
> > #include <linux/neighbour.h>
> > #include <string.h>
> > #include <limits.h>
> > -#include <json_writer.h>
> > #include <stdbool.h>
> >
> > +#include "json_print.h"
> > #include "libnetlink.h"
> > #include "br_common.h"
> > #include "rt_names.h"
> > @@ -32,8 +32,6 @@
> >
> > static unsigned int filter_index, filter_vlan, filter_state;
> >
> > -json_writer_t *jw_global;
> > -
> > static void usage(void)
> > {
> > fprintf(stderr,
> > @@ -83,13 +81,46 @@ static int state_a2n(unsigned int *s, const char *arg)
> > return 0;
> > }
> >
> > -static void start_json_fdb_flags_array(bool *fdb_flags)
> > +static void fdb_print_flags(FILE *fp, unsigned int flags)
> > +{
> > + open_json_array(PRINT_JSON,
> > + is_json_context() ? "flags" : "");
> > +
> > + if (flags & NTF_SELF)
> > + print_string(PRINT_ANY, NULL, "%s ", "self");
> > +
> > + if (flags & NTF_ROUTER)
> > + print_string(PRINT_ANY, NULL, "%s ", "router");
> > +
> > + if (flags & NTF_EXT_LEARNED)
> > + print_string(PRINT_ANY, NULL, "%s ", "extern_learn");
> > +
> > + if (flags & NTF_OFFLOADED)
> > + print_string(PRINT_ANY, NULL, "%s ", "offload");
> > +
> > + if (flags & NTF_MASTER)
> > + print_string(PRINT_ANY, NULL, "%s ", "master");
> > +
> > + close_json_array(PRINT_JSON, NULL);
> > +}
> > +
> > +static void fdb_print_stats(FILE *fp, const struct nda_cacheinfo *ci)
> > {
> > - if (*fdb_flags)
> > - return;
> > - jsonw_name(jw_global, "flags");
> > - jsonw_start_array(jw_global);
> > - *fdb_flags = true;
> > + static int hz;
> > +
> > + if (!hz)
> > + hz = get_user_hz();
> > +
> > + if (is_json_context()) {
> > + print_uint(PRINT_JSON, "used", NULL,
> > + ci->ndm_used / hz);
> > + print_uint(PRINT_JSON, "updated", NULL,
> > + ci->ndm_updated / hz);
> > + } else {
> > + fprintf(fp, "used %d/%d ", ci->ndm_used / hz,
> > + ci->ndm_updated / hz);
> > +
> > + }
> > }
> >
> > int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> > @@ -99,8 +130,6 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> > int len = n->nlmsg_len;
> > struct rtattr *tb[NDA_MAX+1];
> > __u16 vid = 0;
> > - bool fdb_flags = false;
> > - const char *state_s;
> >
> > if (n->nlmsg_type != RTM_NEWNEIGH && n->nlmsg_type != RTM_DELNEIGH) {
> > fprintf(stderr, "Not RTM_NEWNEIGH: %08x %08x %08x\n",
> > @@ -132,189 +161,98 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> > if (filter_vlan && filter_vlan != vid)
> > return 0;
> >
> > - if (jw_global)
> > - jsonw_start_object(jw_global);
> > -
> > - if (n->nlmsg_type == RTM_DELNEIGH) {
> > - if (jw_global)
> > - jsonw_string_field(jw_global, "opCode", "deleted");
> > - else
> > - fprintf(fp, "Deleted ");
> > - }
> > + open_json_object(NULL);
> > + if (n->nlmsg_type == RTM_DELNEIGH)
> > + print_bool(PRINT_ANY, "deleted", "Deleted ", true);
> >
> > if (tb[NDA_LLADDR]) {
> > + const char *lladdr;
> > SPRINT_BUF(b1);
> > - ll_addr_n2a(RTA_DATA(tb[NDA_LLADDR]),
> > - RTA_PAYLOAD(tb[NDA_LLADDR]),
> > - ll_index_to_type(r->ndm_ifindex),
> > - b1, sizeof(b1));
> > - if (jw_global)
> > - jsonw_string_field(jw_global, "mac", b1);
> > - else
> > - fprintf(fp, "%s ", b1);
> > +
> > + lladdr = ll_addr_n2a(RTA_DATA(tb[NDA_LLADDR]),
> > + RTA_PAYLOAD(tb[NDA_LLADDR]),
> > + ll_index_to_type(r->ndm_ifindex),
> > + b1, sizeof(b1));
> > +
> > + print_color_string(PRINT_ANY, COLOR_MAC,
> > + "mac", "%s ", lladdr);
> > }
> >
> > if (!filter_index && r->ndm_ifindex) {
> > - if (jw_global)
> > - jsonw_string_field(jw_global, "dev",
> > - ll_index_to_name(r->ndm_ifindex));
> > - else
> > - fprintf(fp, "dev %s ",
> > - ll_index_to_name(r->ndm_ifindex));
> > + if (!is_json_context())
> > + fprintf(fp, "dev ");
> > + print_color_string(PRINT_ANY, COLOR_IFNAME,
> > + "ifname", "%s ",
> > + ll_index_to_name(r->ndm_ifindex));
> > }
> >
> > if (tb[NDA_DST]) {
> > int family = AF_INET;
> > - const char *abuf_s;
> > + const char *dst;
> >
> > if (RTA_PAYLOAD(tb[NDA_DST]) == sizeof(struct in6_addr))
> > family = AF_INET6;
> >
> > - abuf_s = format_host(family,
> > - RTA_PAYLOAD(tb[NDA_DST]),
> > - RTA_DATA(tb[NDA_DST]));
> > - if (jw_global)
> > - jsonw_string_field(jw_global, "dst", abuf_s);
> > - else
> > - fprintf(fp, "dst %s ", abuf_s);
> > - }
> > + dst = format_host(family,
> > + RTA_PAYLOAD(tb[NDA_DST]),
> > + RTA_DATA(tb[NDA_DST]));
> >
> > - if (vid) {
> > - if (jw_global)
> > - jsonw_uint_field(jw_global, "vlan", vid);
> > - else
> > - fprintf(fp, "vlan %hu ", vid);
> > + print_color_string(PRINT_ANY,
> > + ifa_family_color(family),
> > + "dst", "%s ", dst);
> > }
> >
> > - if (tb[NDA_PORT]) {
> > - if (jw_global)
> > - jsonw_uint_field(jw_global, "port",
> > - rta_getattr_be16(tb[NDA_PORT]));
> > - else
> > - fprintf(fp, "port %d ",
> > - rta_getattr_be16(tb[NDA_PORT]));
> > - }
> > + if (vid)
> > + print_uint(PRINT_ANY,
> > + "vlan", "vlan %hu ", vid);
> >
> > - if (tb[NDA_VNI]) {
> > - if (jw_global)
> > - jsonw_uint_field(jw_global, "vni",
> > - rta_getattr_u32(tb[NDA_VNI]));
> > - else
> > - fprintf(fp, "vni %d ",
> > - rta_getattr_u32(tb[NDA_VNI]));
> > - }
> > + if (tb[NDA_PORT])
> > + print_uint(PRINT_ANY,
> > + "port", "port %u ",
> > + rta_getattr_be16(tb[NDA_PORT]));
> >
> > - if (tb[NDA_SRC_VNI]) {
> > - if (jw_global)
> > - jsonw_uint_field(jw_global, "src_vni",
> > - rta_getattr_u32(tb[NDA_SRC_VNI]));
> > - else
> > - fprintf(fp, "src_vni %d ",
> > + if (tb[NDA_VNI])
> > + print_uint(PRINT_ANY,
> > + "vni", "vni %u ",
> > + rta_getattr_u32(tb[NDA_VNI]));
> > +
> > + if (tb[NDA_SRC_VNI])
> > + print_uint(PRINT_ANY,
> > + "src_vni", "src_vni %u ",
> > rta_getattr_u32(tb[NDA_SRC_VNI]));
> > - }
> >
> > if (tb[NDA_IFINDEX]) {
> > unsigned int ifindex = rta_getattr_u32(tb[NDA_IFINDEX]);
> >
> > - if (ifindex) {
> > - if (!tb[NDA_LINK_NETNSID]) {
> > - const char *ifname = ll_index_to_name(ifindex);
> > -
> > - if (jw_global)
> > - jsonw_string_field(jw_global, "viaIf",
> > - ifname);
> > - else
> > - fprintf(fp, "via %s ", ifname);
> > - } else {
> > - if (jw_global)
> > - jsonw_uint_field(jw_global, "viaIfIndex",
> > - ifindex);
> > - else
> > - fprintf(fp, "via ifindex %u ", ifindex);
> > - }
> > - }
> > - }
> > -
> > - if (tb[NDA_LINK_NETNSID]) {
> > - if (jw_global)
> > - jsonw_uint_field(jw_global, "linkNetNsId",
> > - rta_getattr_u32(tb[NDA_LINK_NETNSID]));
> > + if (tb[NDA_LINK_NETNSID])
> > + print_uint(PRINT_ANY,
> > + "viaIfIndex", "via ifindex %u ",
> > + ifindex);
> > else
> > - fprintf(fp, "link-netnsid %d ",
> > - rta_getattr_u32(tb[NDA_LINK_NETNSID]));
> > + print_string(PRINT_ANY,
> > + "viaIf", "via %s ",
> > + ll_index_to_name(ifindex));
> > }
> >
> > - if (show_stats && tb[NDA_CACHEINFO]) {
> > - struct nda_cacheinfo *ci = RTA_DATA(tb[NDA_CACHEINFO]);
> > - int hz = get_user_hz();
> > + if (tb[NDA_LINK_NETNSID])
> > + print_uint(PRINT_ANY,
> > + "linkNetNsId", "link-netnsid %d ",
> > + rta_getattr_u32(tb[NDA_LINK_NETNSID]));
> >
> > - if (jw_global) {
> > - jsonw_uint_field(jw_global, "used",
> > - ci->ndm_used/hz);
> > - jsonw_uint_field(jw_global, "updated",
> > - ci->ndm_updated/hz);
> > - } else {
> > - fprintf(fp, "used %d/%d ", ci->ndm_used/hz,
> > - ci->ndm_updated/hz);
> > - }
> > - }
> > + if (show_stats && tb[NDA_CACHEINFO])
> > + fdb_print_stats(fp, RTA_DATA(tb[NDA_CACHEINFO]));
> >
> > - if (jw_global) {
> > - if (r->ndm_flags & NTF_SELF) {
> > - start_json_fdb_flags_array(&fdb_flags);
> > - jsonw_string(jw_global, "self");
> > - }
> > - if (r->ndm_flags & NTF_ROUTER) {
> > - start_json_fdb_flags_array(&fdb_flags);
> > - jsonw_string(jw_global, "router");
> > - }
> > - if (r->ndm_flags & NTF_EXT_LEARNED) {
> > - start_json_fdb_flags_array(&fdb_flags);
> > - jsonw_string(jw_global, "extern_learn");
> > - }
> > - if (r->ndm_flags & NTF_OFFLOADED) {
> > - start_json_fdb_flags_array(&fdb_flags);
> > - jsonw_string(jw_global, "offload");
> > - }
> > - if (r->ndm_flags & NTF_MASTER)
> > - jsonw_string(jw_global, "master");
> > - if (fdb_flags)
> > - jsonw_end_array(jw_global);
> > + fdb_print_flags(fp, r->ndm_flags);
> >
> > - if (tb[NDA_MASTER])
> > - jsonw_string_field(jw_global,
> > - "master",
> > - ll_index_to_name(rta_getattr_u32(tb[NDA_MASTER])));
> >
> > - } else {
> > - if (r->ndm_flags & NTF_SELF)
> > - fprintf(fp, "self ");
> > - if (r->ndm_flags & NTF_ROUTER)
> > - fprintf(fp, "router ");
> > - if (r->ndm_flags & NTF_EXT_LEARNED)
> > - fprintf(fp, "extern_learn ");
> > - if (r->ndm_flags & NTF_OFFLOADED)
> > - fprintf(fp, "offload ");
> > - if (tb[NDA_MASTER]) {
> > - fprintf(fp, "master %s ",
> > - ll_index_to_name(rta_getattr_u32(tb[NDA_MASTER])));
> > - } else if (r->ndm_flags & NTF_MASTER) {
> > - fprintf(fp, "master ");
> > - }
> > - }
> > -
> > - state_s = state_n2a(r->ndm_state);
> > - if (jw_global) {
> > - if (state_s[0])
> > - jsonw_string_field(jw_global, "state", state_s);
> > -
> > - jsonw_end_object(jw_global);
> > - } else {
> > - fprintf(fp, "%s\n", state_s);
> > -
> > - fflush(fp);
> > - }
> > + if (tb[NDA_MASTER])
> > + print_string(PRINT_ANY, "master", "%s ",
> > + ll_index_to_name(rta_getattr_u32(tb[NDA_MASTER])));
> >
> > + print_string(PRINT_ANY, "state", "%s\n",
> > + state_n2a(r->ndm_state));
> > + close_json_object();
> > + fflush(fp);
> > return 0;
> > }
> >
> > @@ -386,26 +324,13 @@ static int fdb_show(int argc, char **argv)
> > exit(1);
> > }
> >
> > - if (json) {
> > - jw_global = jsonw_new(stdout);
> > - if (!jw_global) {
> > - fprintf(stderr, "Error allocation json object\n");
> > - exit(1);
> > - }
> > - if (pretty)
> > - jsonw_pretty(jw_global, 1);
> > -
> > - jsonw_start_array(jw_global);
> > - }
> > -
> > + new_json_obj(json);
> > if (rtnl_dump_filter(&rth, print_fdb, stdout) < 0) {
> > fprintf(stderr, "Dump terminated\n");
> > exit(1);
> > }
> > - if (jw_global) {
> > - jsonw_end_array(jw_global);
> > - jsonw_destroy(&jw_global);
> > - }
> > + delete_json_obj();
> > + fflush(stdout);
> >
> > return 0;
> > }
> > diff --git a/bridge/mdb.c b/bridge/mdb.c
> > index da0282fdc91c..8c08baf570ec 100644
> > --- a/bridge/mdb.c
> > +++ b/bridge/mdb.c
> > @@ -14,12 +14,12 @@
> > #include <linux/if_ether.h>
> > #include <string.h>
> > #include <arpa/inet.h>
> > -#include <json_writer.h>
> >
> > #include "libnetlink.h"
> > #include "br_common.h"
> > #include "rt_names.h"
> > #include "utils.h"
> > +#include "json_print.h"
> >
> > #ifndef MDBA_RTA
> > #define MDBA_RTA(r) \
> > @@ -27,9 +27,6 @@
> > #endif
> >
> > static unsigned int filter_index, filter_vlan;
> > -json_writer_t *jw_global;
> > -static bool print_mdb_entries = true;
> > -static bool print_mdb_router = true;
> >
> > static void usage(void)
> > {
> > @@ -43,162 +40,131 @@ static bool is_temp_mcast_rtr(__u8 type)
> > return type == MDB_RTR_TYPE_TEMP_QUERY || type == MDB_RTR_TYPE_TEMP;
> > }
> >
> > +static const char *format_timer(__u32 ticks)
> > +{
> > + struct timeval tv;
> > + static char tbuf[32];
> > +
> > + __jiffies_to_tv(&tv, ticks);
> > + snprintf(tbuf, sizeof(tbuf), "%4lu.%.2lu",
> > + (unsigned long)tv.tv_sec,
> > + (unsigned long)tv.tv_usec / 10000);
> > +
> > + return tbuf;
> > +}
> > +
> > static void __print_router_port_stats(FILE *f, struct rtattr *pattr)
> > {
> > struct rtattr *tb[MDBA_ROUTER_PATTR_MAX + 1];
> > - struct timeval tv;
> > - __u8 type;
> >
> > parse_rtattr(tb, MDBA_ROUTER_PATTR_MAX, MDB_RTR_RTA(RTA_DATA(pattr)),
> > RTA_PAYLOAD(pattr) - RTA_ALIGN(sizeof(uint32_t)));
> > +
> > if (tb[MDBA_ROUTER_PATTR_TIMER]) {
> > - __jiffies_to_tv(&tv,
> > - rta_getattr_u32(tb[MDBA_ROUTER_PATTR_TIMER]));
> > - if (jw_global) {
> > - char formatted_time[9];
> > -
> > - snprintf(formatted_time, sizeof(formatted_time),
> > - "%4i.%.2i", (int)tv.tv_sec,
> > - (int)tv.tv_usec/10000);
> > - jsonw_string_field(jw_global, "timer", formatted_time);
> > - } else {
> > - fprintf(f, " %4i.%.2i",
> > - (int)tv.tv_sec, (int)tv.tv_usec/10000);
> > - }
> > + __u32 timer = rta_getattr_u32(tb[MDBA_ROUTER_PATTR_TIMER]);
> > +
> > + print_string(PRINT_ANY, "timer", " %s",
> > + format_timer(timer));
> > }
> > +
> > if (tb[MDBA_ROUTER_PATTR_TYPE]) {
> > - type = rta_getattr_u8(tb[MDBA_ROUTER_PATTR_TYPE]);
> > - if (jw_global)
> > - jsonw_string_field(jw_global, "type",
> > - is_temp_mcast_rtr(type) ? "temp" : "permanent");
> > - else
> > - fprintf(f, " %s",
> > - is_temp_mcast_rtr(type) ? "temp" : "permanent");
> > + __u8 type = rta_getattr_u8(tb[MDBA_ROUTER_PATTR_TYPE]);
> > +
> > + print_string(PRINT_ANY, "type", " %s",
> > + is_temp_mcast_rtr(type) ? "temp" : "permanent");
> > }
> > }
> >
> > -static void br_print_router_ports(FILE *f, struct rtattr *attr, __u32 brifidx)
> > +static void br_print_router_ports(FILE *f, struct rtattr *attr,
> > + const char *brifname)
> > {
> > - uint32_t *port_ifindex;
> > + int rem = RTA_PAYLOAD(attr);
> > struct rtattr *i;
> > - int rem;
> >
> > - rem = RTA_PAYLOAD(attr);
> > - if (jw_global) {
> > - jsonw_name(jw_global, ll_index_to_name(brifidx));
> > - jsonw_start_array(jw_global);
> > - for (i = RTA_DATA(attr); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
> > - port_ifindex = RTA_DATA(i);
> > - jsonw_start_object(jw_global);
> > - jsonw_string_field(jw_global,
> > - "port",
> > - ll_index_to_name(*port_ifindex));
> > + if (is_json_context())
> > + open_json_array(PRINT_JSON, brifname);
> > + else if (!show_stats)
> > + fprintf(f, "router ports on %s: ", brifname);
> > +
> > + for (i = RTA_DATA(attr); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
> > + uint32_t *port_ifindex = RTA_DATA(i);
> > + const char *port_ifname = ll_index_to_name(*port_ifindex);
> > +
> > + if (is_json_context()) {
> > + open_json_object(NULL);
> > + print_string(PRINT_JSON, "port", NULL, port_ifname);
> > +
> > if (show_stats)
> > __print_router_port_stats(f, i);
> > - jsonw_end_object(jw_global);
> > - }
> > - jsonw_end_array(jw_global);
> > - } else {
> > - if (!show_stats)
> > - fprintf(f, "router ports on %s: ",
> > - ll_index_to_name(brifidx));
> > - for (i = RTA_DATA(attr); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
> > - port_ifindex = RTA_DATA(i);
> > - if (show_stats) {
> > - fprintf(f, "router ports on %s: %s",
> > - ll_index_to_name(brifidx),
> > - ll_index_to_name(*port_ifindex));
> > - __print_router_port_stats(f, i);
> > - fprintf(f, "\n");
> > - } else{
> > - fprintf(f, "%s ",
> > - ll_index_to_name(*port_ifindex));
> > - }
> > - }
> > - if (!show_stats)
> > + close_json_object();
> > + } else if (show_stats) {
> > + fprintf(f, "router ports on %s: %s",
> > + brifname, port_ifname);
> > +
> > + __print_router_port_stats(f, i);
> > fprintf(f, "\n");
> > + } else {
> > + fprintf(f, "%s ", port_ifname);
> > + }
> > }
> > + close_json_array(PRINT_JSON, NULL);
> > }
> >
> > -static void start_json_mdb_flags_array(bool *mdb_flags)
> > -{
> > - if (*mdb_flags)
> > - return;
> > - jsonw_name(jw_global, "flags");
> > - jsonw_start_array(jw_global);
> > - *mdb_flags = true;
> > -}
> > -
> > -static void print_mdb_entry(FILE *f, int ifindex, struct br_mdb_entry *e,
> > +static void print_mdb_entry(FILE *f, int ifindex, const struct br_mdb_entry *e,
> > struct nlmsghdr *n, struct rtattr **tb)
> > {
> > SPRINT_BUF(abuf);
> > + const char *dev;
> > const void *src;
> > int af;
> > - bool mdb_flags = false;
> >
> > if (filter_vlan && e->vid != filter_vlan)
> > return;
> > +
> > af = e->addr.proto == htons(ETH_P_IP) ? AF_INET : AF_INET6;
> > src = af == AF_INET ? (const void *)&e->addr.u.ip4 :
> > (const void *)&e->addr.u.ip6;
> > - if (jw_global)
> > - jsonw_start_object(jw_global);
> > - if (n->nlmsg_type == RTM_DELMDB) {
> > - if (jw_global)
> > - jsonw_string_field(jw_global, "opCode", "deleted");
> > - else
> > - fprintf(f, "Deleted ");
> > - }
> > - if (jw_global) {
> > - jsonw_string_field(jw_global, "dev", ll_index_to_name(ifindex));
> > - jsonw_string_field(jw_global,
> > - "port",
> > - ll_index_to_name(e->ifindex));
> > - jsonw_string_field(jw_global, "grp", inet_ntop(af, src,
> > - abuf, sizeof(abuf)));
> > - jsonw_string_field(jw_global, "state",
> > - (e->state & MDB_PERMANENT) ? "permanent" : "temp");
> > - if (e->flags & MDB_FLAGS_OFFLOAD) {
> > - start_json_mdb_flags_array(&mdb_flags);
> > - jsonw_string(jw_global, "offload");
> > - }
> > - if (mdb_flags)
> > - jsonw_end_array(jw_global);
> > - } else{
> > - fprintf(f, "dev %s port %s grp %s %s %s",
> > - ll_index_to_name(ifindex),
> > - ll_index_to_name(e->ifindex),
> > - inet_ntop(af, src, abuf, sizeof(abuf)),
> > - (e->state & MDB_PERMANENT) ? "permanent" : "temp",
> > - (e->flags & MDB_FLAGS_OFFLOAD) ? "offload" : "");
> > - }
> > - if (e->vid) {
> > - if (jw_global)
> > - jsonw_uint_field(jw_global, "vid", e->vid);
> > - else
> > - fprintf(f, " vid %hu", e->vid);
> > + dev = ll_index_to_name(ifindex);
> > +
> > + open_json_object(NULL);
> > +
> > + if (n->nlmsg_type == RTM_DELMDB)
> > + print_bool(PRINT_ANY, "deleted", "Deleted ", true);
> > +
> > +
> > + if (is_json_context()) {
> > + print_int(PRINT_JSON, "index", NULL, ifindex);
> > + print_string(PRINT_JSON, "dev", NULL, dev);
> > + } else {
> > + fprintf(f, "%u: ", ifindex);
> > + color_fprintf(f, COLOR_IFNAME, "%s ", dev);
> > }
> > - if (show_stats && tb && tb[MDBA_MDB_EATTR_TIMER]) {
> > - struct timeval tv;
> >
> > - __jiffies_to_tv(&tv, rta_getattr_u32(tb[MDBA_MDB_EATTR_TIMER]));
> > - if (jw_global) {
> > - char formatted_time[9];
> > + print_string(PRINT_ANY, "port", " %s ",
> > + ll_index_to_name(e->ifindex));
> >
> > - snprintf(formatted_time, sizeof(formatted_time),
> > - "%4i.%.2i", (int)tv.tv_sec,
> > - (int)tv.tv_usec/10000);
> > - jsonw_string_field(jw_global, "timer", formatted_time);
> > - } else {
> > - fprintf(f, "%4i.%.2i", (int)tv.tv_sec,
> > - (int)tv.tv_usec/10000);
> > - }
> > + print_color_string(PRINT_ANY, ifa_family_color(af),
> > + "grp", " %s ",
> > + inet_ntop(af, src, abuf, sizeof(abuf)));
> > +
> > + print_string(PRINT_ANY, "state", " %s ",
> > + (e->state & MDB_PERMANENT) ? "permanent" : "temp");
> > +
> > + open_json_array(PRINT_JSON, "flags");
> > + if (e->flags & MDB_FLAGS_OFFLOAD)
> > + print_string(PRINT_ANY, NULL, "%s ", "offload");
> > + close_json_array(PRINT_JSON, NULL);
> > +
> > + if (e->vid)
> > + print_uint(PRINT_ANY, "vid", " vid %u", e->vid);
> > +
> > + if (show_stats && tb && tb[MDBA_MDB_EATTR_TIMER]) {
> > + __u32 timer = rta_getattr_u32(tb[MDBA_MDB_EATTR_TIMER]);
> > +
> > + print_string(PRINT_ANY, "timer", " %s",
> > + format_timer(timer));
> > }
> > - if (jw_global)
> > - jsonw_end_object(jw_global);
> > - else
> > - fprintf(f, "\n");
> > + close_json_object();
> > }
> >
> > static void br_print_mdb_entry(FILE *f, int ifindex, struct rtattr *attr,
> > @@ -218,15 +184,60 @@ static void br_print_mdb_entry(FILE *f, int ifindex, struct rtattr *attr,
> > }
> > }
> >
> > +static void print_mdb_entries(FILE *fp, struct nlmsghdr *n,
> > + int ifindex, struct rtattr *mdb)
> > +{
> > + 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,
> > + int ifindex, struct rtattr *router)
> > +{
> > + 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);
> > + } else {
> > + struct rtattr *i = RTA_DATA(router);
> > + uint32_t *port_ifindex = RTA_DATA(i);
> > +
> > + if (is_json_context()) {
> > + open_json_array(PRINT_JSON, brifname);
> > + open_json_object(NULL);
> > +
> > + print_string(PRINT_JSON, "port", NULL,
> > + ll_index_to_name(*port_ifindex));
> > + close_json_object();
> > + close_json_array(PRINT_JSON, NULL);
> > + } else {
> > + fprintf(fp, "router port dev %s master %s\n",
> > + ll_index_to_name(*port_ifindex),
> > + brifname);
> > + }
> > + }
> > + close_json_array(PRINT_JSON, NULL);
> > +}
> > +
> > int print_mdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> > {
> > FILE *fp = arg;
> > struct br_port_msg *r = NLMSG_DATA(n);
> > int len = n->nlmsg_len;
> > - struct rtattr *tb[MDBA_MAX+1], *i;
> > + struct rtattr *tb[MDBA_MAX+1];
> >
> > - if (n->nlmsg_type != RTM_GETMDB && n->nlmsg_type != RTM_NEWMDB && n->nlmsg_type != RTM_DELMDB) {
> > - fprintf(stderr, "Not RTM_GETMDB, RTM_NEWMDB or RTM_DELMDB: %08x %08x %08x\n",
> > + if (n->nlmsg_type != RTM_GETMDB &&
> > + n->nlmsg_type != RTM_NEWMDB &&
> > + n->nlmsg_type != RTM_DELMDB) {
> > + fprintf(stderr,
> > + "Not RTM_GETMDB, RTM_NEWMDB or RTM_DELMDB: %08x %08x %08x\n",
> > n->nlmsg_len, n->nlmsg_type, n->nlmsg_flags);
> >
> > return 0;
> > @@ -243,50 +254,14 @@ int print_mdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> >
> > parse_rtattr(tb, MDBA_MAX, MDBA_RTA(r), n->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
> >
> > - if (tb[MDBA_MDB] && print_mdb_entries) {
> > - int rem = RTA_PAYLOAD(tb[MDBA_MDB]);
> > + if (n->nlmsg_type == RTM_DELMDB)
> > + print_bool(PRINT_ANY, "deleted", "Deleted ", true);
> >
> > - for (i = RTA_DATA(tb[MDBA_MDB]); RTA_OK(i, rem); i = RTA_NEXT(i, rem))
> > - br_print_mdb_entry(fp, r->ifindex, i, n);
> > - }
> > + if (tb[MDBA_MDB])
> > + print_mdb_entries(fp, n, r->ifindex, tb[MDBA_MDB]);
> >
> > - if (tb[MDBA_ROUTER] && print_mdb_router) {
> > - if (n->nlmsg_type == RTM_GETMDB) {
> > - if (show_details)
> > - br_print_router_ports(fp, tb[MDBA_ROUTER],
> > - r->ifindex);
> > - } else {
> > - uint32_t *port_ifindex;
> > -
> > - i = RTA_DATA(tb[MDBA_ROUTER]);
> > - port_ifindex = RTA_DATA(i);
> > - if (n->nlmsg_type == RTM_DELMDB) {
> > - if (jw_global)
> > - jsonw_string_field(jw_global,
> > - "opCode",
> > - "deleted");
> > - else
> > - fprintf(fp, "Deleted ");
> > - }
> > - if (jw_global) {
> > - jsonw_name(jw_global,
> > - ll_index_to_name(r->ifindex));
> > - jsonw_start_array(jw_global);
> > - jsonw_start_object(jw_global);
> > - jsonw_string_field(jw_global, "port",
> > - ll_index_to_name(*port_ifindex));
> > - jsonw_end_object(jw_global);
> > - jsonw_end_array(jw_global);
> > - } else {
> > - fprintf(fp, "router port dev %s master %s\n",
> > - ll_index_to_name(*port_ifindex),
> > - ll_index_to_name(r->ifindex));
> > - }
> > - }
> > - }
> > -
> > - if (!jw_global)
> > - fflush(fp);
> > + if (tb[MDBA_ROUTER])
> > + print_router_entries(fp, n, r->ifindex, tb[MDBA_ROUTER]);
> >
> > return 0;
> > }
> > @@ -319,62 +294,21 @@ static int mdb_show(int argc, char **argv)
> > }
> > }
> >
> > + new_json_obj(json);
> > +
> > /* get mdb entries*/
> > if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETMDB) < 0) {
> > perror("Cannot send dump request");
> > return -1;
> > }
> >
> > - if (!json) {
> > - /* Normal output */
> > - if (rtnl_dump_filter(&rth, print_mdb, stdout) < 0) {
> > - fprintf(stderr, "Dump terminated\n");
> > - return -1;
> > - }
> > - return 0;
> > - }
> > -
> > - /* Json output */
> > - jw_global = jsonw_new(stdout);
> > - if (!jw_global) {
> > - fprintf(stderr, "Error allocation json object\n");
> > - exit(1);
> > - }
> > -
> > - if (pretty)
> > - jsonw_pretty(jw_global, 1);
> > -
> > - jsonw_start_object(jw_global);
> > - jsonw_name(jw_global, "mdb");
> > - jsonw_start_array(jw_global);
> > -
> > - /* print mdb entries */
> > - print_mdb_entries = true;
> > - print_mdb_router = false;
> > if (rtnl_dump_filter(&rth, print_mdb, stdout) < 0) {
> > fprintf(stderr, "Dump terminated\n");
> > return -1;
> > }
> > - jsonw_end_array(jw_global);
> > -
> > - /* get router ports */
> > - if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETMDB) < 0) {
> > - perror("Cannot send dump request");
> > - return -1;
> > - }
> > - jsonw_name(jw_global, "router");
> > - jsonw_start_object(jw_global);
> >
> > - /* print router ports */
> > - print_mdb_entries = false;
> > - print_mdb_router = true;
> > - if (rtnl_dump_filter(&rth, print_mdb, stdout) < 0) {
> > - fprintf(stderr, "Dump terminated\n");
> > - return -1;
> > - }
> > - jsonw_end_object(jw_global);
> > - jsonw_end_object(jw_global);
> > - jsonw_destroy(&jw_global);
> > + delete_json_obj();
> > + fflush(stdout);
> >
> > return 0;
> > }
> > diff --git a/bridge/vlan.c b/bridge/vlan.c
> > index 7c8b3ad54857..9f4a7a2be55c 100644
> > --- a/bridge/vlan.c
> > +++ b/bridge/vlan.c
> > @@ -8,19 +8,16 @@
> > #include <netinet/in.h>
> > #include <linux/if_bridge.h>
> > #include <linux/if_ether.h>
> > -#include <json_writer.h>
> > #include <string.h>
> >
> > +#include "json_print.h"
> > #include "libnetlink.h"
> > #include "br_common.h"
> > #include "utils.h"
> >
> > static unsigned int filter_index, filter_vlan;
> > -static int last_ifidx = -1;
> > static int show_vlan_tunnel_info = 0;
> >
> > -json_writer_t *jw_global;
> > -
> > static void usage(void)
> > {
> > fprintf(stderr,
> > @@ -257,38 +254,33 @@ static int filter_vlan_check(__u16 vid, __u16 flags)
> >
> > static void print_vlan_port(FILE *fp, int ifi_index)
> > {
> > - if (jw_global) {
> > - jsonw_name(jw_global,
> > - ll_index_to_name(ifi_index));
> > - jsonw_start_array(jw_global);
> > - } else {
> > - fprintf(fp, "%s",
> > - ll_index_to_name(ifi_index));
> > - }
> > + print_string(PRINT_ANY, NULL, "%s",
> > + ll_index_to_name(ifi_index));
> > }
> >
>
> Stephen, this seems to have broken both json and non-json output.
>
> Here is some output before and after the patch (same thing for tunnelshow):
>
> before:
> $bridge vlan show
> port vlan ids
> hostbond4 1000
> 1001 PVID Egress Untagged
> 1002
> 1003
> 1004
>
> hostbond3 1000 PVID Egress Untagged
> 1001
> 1002
> 1003
> 1004
>
> bridge 1 PVID Egress Untagged
> 1000
> 1001
> 1002
> 1003
> 1004
>
> vxlan0 1 PVID Egress Untagged
> 1000
> 1001
> 1002
> 1003
> 1004
>
>
> $ bridge -j -c vlan show
> {
> "hostbond4": [{
> "vlan": 1000
> },{
> "vlan": 1001,
> "flags": ["PVID","Egress Untagged"
> ]
> },{
> "vlan": 1002,
> "vlanEnd": 1004
> }
> ],
> "hostbond3": [{
> "vlan": 1000,
> "flags": ["PVID","Egress Untagged"
> ]
> },{
> "vlan": 1001,
> "vlanEnd": 1004
> }
> ],
> "bridge": [{
> "vlan": 1,
> "flags": ["PVID","Egress Untagged"
> ]
> },{
> "vlan": 1000,
> "vlanEnd": 1004
> }
> ],
> "vxlan0": [{
> "vlan": 1,
> "flags": ["PVID","Egress Untagged"
> ]
> },{
> "vlan": 1000,
> "vlanEnd": 1004
> }
> ]
> }
>
>
> after:
> ====
>
> $bridge vlan show
> port vlan ids
> hostbond4
> 1000 1001 PVID untagged 1002 1003 1004
> hostbond3
> 1000 PVID untagged 1001 1002 1003 1004
> bridge
> 1 PVID untagged 1000 1001 1002 1003 1004
> vxlan0
> 1 PVID untagged 1000 1001 1002 1003 1004
>
> $bridge -j -c vlan show
> ["hostbond4","vlan":[{"vlan":1000},{"vlan":1001,"pvid":null,"untagged":null},{"vlan":1002},{"vlan":1003},{"vlan":1004}],"hostbond3","vlan":[{"vlan":1000,"pvid":null,"untagged":null},{"vlan":1001},{"vlan":1002},{"vlan":1003},{"vlan":1004}],"bridge","vlan":[{"vlan":1,"pvid":null,"untagged":null},{"vlan":1000},{"vlan":1001},{"vlan":1002},{"vlan":1003},{"vlan":1004}],"vxlan0","vlan":[{"vlan":1,"pvid":null,"untagged":null},{"vlan":1000},{"vlan":1001},{"vlan":1002},{"vlan":1003},{"vlan":1004}]]
I can fix it.
next prev parent reply other threads:[~2018-08-29 19:02 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-20 19:24 [PATCH v2 iproute2-next 0/5] bridge: json and color support Stephen Hemminger
2018-02-20 19:24 ` [PATCH v2 iproute2-next 1/5] bridge: implement json pretty print flag Stephen Hemminger
2018-02-20 19:24 ` [PATCH v2 iproute2-next 2/5] bridge: colorize output and use JSON print library Stephen Hemminger
2018-07-15 1:41 ` Roopa Prabhu
2018-08-29 1:17 ` Roopa Prabhu
2018-08-29 15:04 ` Stephen Hemminger [this message]
2018-02-20 19:24 ` [PATCH v2 iproute2-next 3/5] bridge: add json support for link command Stephen Hemminger
2018-02-20 19:24 ` [PATCH v2 iproute2-next 4/5] bridge: update man page for new color and json changes Stephen Hemminger
2018-02-20 19:24 ` [PATCH v2 iproute2-next 5/5] ip: always print interface name in color Stephen Hemminger
2018-02-21 16:48 ` [PATCH v2 iproute2-next 0/5] bridge: json and color support David Ahern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180829080453.0026d52b@xeon-e3 \
--to=stephen@networkplumber.org \
--cc=dsahern@gmail.com \
--cc=julien@cumulusnetworks.com \
--cc=netdev@vger.kernel.org \
--cc=roopa@cumulusnetworks.com \
--cc=sthemmin@microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.