Re: [PATCH v2 iproute2-next 2/5] bridge: colorize output and use JSON print library
On Sat, 14 Jul 2018 18:41:03 -0700
Roopa Prabhu wrote:
> On Tue, Feb 20, 2018 at 11:24 AM, Stephen Hemminger
> wrote:
> > From: Stephen Hemminger
> >
> > 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
> > ---
> > 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
> > #include
> > #include
> > -#include
> > #include
> >
> > +#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 (i
Re: [PATCH v2 iproute2-next 2/5] bridge: colorize output and use JSON print library
On Sat, Jul 14, 2018 at 6:41 PM, Roopa Prabhu wrote:
> On Tue, Feb 20, 2018 at 11:24 AM, Stephen Hemminger
> wrote:
>> From: Stephen Hemminger
>>
>> 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
>> ---
[snip]
>
> 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
> portvlan ids
> hostbond41000
> 1001 PVID Egress Untagged
> 1002
> 1003
> 1004
>
> hostbond31000 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
> portvlan ids
> hostbond4
> 10001001 PVID untagged 100210031004
> hostbond3
> 1000 PVID untagged 1001100210031004
> bridge
> 1 PVID untagged 10001001100210031004
> vxlan0
> 1 PVID untagged 10001001100210031004
>
> $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}]]
Stephen, ping again...
I was trying to fix it ...but its not trivial enough for the time I
have right now.
If this cannot be fixed soon, I request you to please revert the patch
as it has broken the json output completely.
Thanks.
Re: [PATCH v2 iproute2-next 2/5] bridge: colorize output and use JSON print library
On Tue, Feb 20, 2018 at 11:24 AM, Stephen Hemminger
wrote:
> From: Stephen Hemminger
>
> 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
> ---
> 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
> #include
> #include
> -#include
> #include
>
> +#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,
> +
[PATCH v2 iproute2-next 2/5] bridge: colorize output and use JSON print library
From: Stephen Hemminger
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
---
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
#include
#include
-#include
#include
+#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_flag
