[PATCH iproute2 v4 0/2] vxlan: option printing

[PATCH iproute2 v4 0/2] vxlan: option printing

[Stephen Hemminger]

This patchset makes printing of vxlan details more consistent.
It also adds extra verbose output. The boolean options
are now brinted after all the non-boolean options.

Before:
$ ip -d link show dev vxlan0
4: vxlan0: <BROADCAST,MULTICAST> mtu 1450 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether e6:a4:54:b2:34:85 brd ff:ff:ff:ff:ff:ff promiscuity 0  allmulti 0 minmtu 68 maxmtu 65535 
    vxlan id 42 group 239.1.1.1 dev enp2s0 srcport 0 0 dstport 4789 ttl auto ageing 300 udpcsum noudp6zerocsumtx noudp6zerocsumrx addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 64000 gso_max_segs 64 tso_max_size 64000 tso_max_segs 64 gro_max_size 65536 

After:
$ ip -d link show dev vxlan0
4: vxlan0: <BROADCAST,MULTICAST> mtu 1450 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether e6:a4:54:b2:34:85 brd ff:ff:ff:ff:ff:ff promiscuity 0  allmulti 0 minmtu 68 maxmtu 65535 
    vxlan id 42 group 239.1.1.1 dev enp2s0 srcport 0 0 dstport 4789 ttl auto ageing 300 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 64000 gso_max_segs 64 tso_max_size 64000 tso_max_segs 64 gro_max_size 65536

To get all settings, use multiple detail flags
$ ip -d -d link show dev vxlan0
4: vxlan0: <BROADCAST,MULTICAST> mtu 1450 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether e6:a4:54:b2:34:85 brd ff:ff:ff:ff:ff:ff promiscuity 0  allmulti 0 minmtu 68 maxmtu 65535 
    vxlan id 42 group 239.1.1.1 dev enp2s0 srcport 0 0 dstport 4789 ttl auto ageing 300 nometadata learning noproxy norsc nol2miss nol3miss udp_csum noudp_zero_csum6_tx noudp_zero_csum6_rx noremcsum_tx noremcsum_rx addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 64000 gso_max_segs 64 tso_max_size 64000 tso_max_segs 64 gro_max_size 65536 

Stephen Hemminger (2):
  vxlan: use print_nll for gbp and gpe
  vxlan: make option printing more consistent

 include/json_print.h |   9 ++++
 ip/iplink_vxlan.c    | 110 +++++++++++++------------------------------
 lib/json_print.c     |  19 ++++++++
 3 files changed, 60 insertions(+), 78 deletions(-)

-- 
2.39.2

[PATCH iproute2 v4 1/2] vxlan: use print_nll for gbp and gpe

[Stephen Hemminger]

The Gbp and Gpe are presence, not booleans so use print_null()
for them

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/iplink_vxlan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index c7e0e1c47606..cb6745c74507 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -650,9 +650,9 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_bool(PRINT_ANY, "remcsum_rx", "remcsumrx ", true);
 
 	if (tb[IFLA_VXLAN_GBP])
-		print_bool(PRINT_ANY, "gbp", "gbp ", true);
+		print_null(PRINT_ANY, "gbp", "gbp ", NULL);
 	if (tb[IFLA_VXLAN_GPE])
-		print_bool(PRINT_ANY, "gpe", "gpe ", true);
+		print_null(PRINT_ANY, "gpe", "gpe ", NULL);
 }
 
 static void vxlan_print_help(struct link_util *lu, int argc, char **argv,
-- 
2.39.2

[PATCH iproute2 v4 2/2] vxlan: make option printing more consistent

[Stephen Hemminger]

Add new helper function print_bool_opt() which prints
with no prefix and use it for vxlan options.

If the option matches the expected default value,
it is not printed if in non JSON mode unless the details
setting is repeated.

Use a table for the vxlan options. This will change
the order of the printing of options.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 include/json_print.h |   9 ++++
 ip/iplink_vxlan.c    | 110 +++++++++++++------------------------------
 lib/json_print.c     |  19 ++++++++
 3 files changed, 60 insertions(+), 78 deletions(-)

diff --git a/include/json_print.h b/include/json_print.h
index 91b34571ceb0..49d3cc14789c 100644
--- a/include/json_print.h
+++ b/include/json_print.h
@@ -101,6 +101,15 @@ static inline int print_rate(bool use_iec, enum output_type t,
 	return print_color_rate(use_iec, t, COLOR_NONE, key, fmt, rate);
 }
 
+int print_color_bool_opt(enum output_type type, enum color_attr color,
+			 const char *key, bool value, bool show);
+
+static inline int print_bool_opt(enum output_type type,
+				 const char *key, bool value, bool show)
+{
+	return print_color_bool_opt(type, COLOR_NONE, key, value, show);
+}
+
 /* A backdoor to the size formatter. Please use print_size() instead. */
 char *sprint_size(__u32 sz, char *buf);
 
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index cb6745c74507..e77c3aa2e3a5 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -19,6 +19,25 @@
 
 #define VXLAN_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0)
 
+static const struct vxlan_bool_opt {
+	const char *key;
+	int type;
+	bool default_value;
+} vxlan_opts[] = {
+	{ "metadata",	IFLA_VXLAN_COLLECT_METADATA,	false },
+	{ "vnifilter",	IFLA_VXLAN_VNIFILTER,		false },
+	{ "learning", 	IFLA_VXLAN_LEARNING,		true },
+	{ "proxy",	IFLA_VXLAN_PROXY,		false },
+	{ "rsc",	IFLA_VXLAN_RSC,			false },
+	{ "l2miss",	IFLA_VXLAN_L2MISS,		false },
+	{ "l3miss",	IFLA_VXLAN_L3MISS,		false },
+	{ "udp_csum",	IFLA_VXLAN_UDP_CSUM,		true },
+	{ "udp_zero_csum6_tx", IFLA_VXLAN_UDP_ZERO_CSUM6_TX, false },
+	{ "udp_zero_csum6_rx", IFLA_VXLAN_UDP_ZERO_CSUM6_RX, false },
+	{ "remcsum_tx", IFLA_VXLAN_REMCSUM_TX,		false },
+	{ "remcsum_rx", IFLA_VXLAN_REMCSUM_RX,		false },
+};
+
 static void print_explain(FILE *f)
 {
 	fprintf(f,
@@ -420,6 +439,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 
 static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
+	unsigned int i;
 	__u8 ttl = 0;
 	__u8 tos = 0;
 	__u32 maxaddr;
@@ -427,16 +447,6 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (!tb)
 		return;
 
-	if (tb[IFLA_VXLAN_COLLECT_METADATA] &&
-	    rta_getattr_u8(tb[IFLA_VXLAN_COLLECT_METADATA])) {
-		print_bool(PRINT_ANY, "external", "external ", true);
-	}
-
-	if (tb[IFLA_VXLAN_VNIFILTER] &&
-	    rta_getattr_u8(tb[IFLA_VXLAN_VNIFILTER])) {
-		print_bool(PRINT_ANY, "vnifilter", "vnifilter", true);
-	}
-
 	if (tb[IFLA_VXLAN_ID] &&
 	    RTA_PAYLOAD(tb[IFLA_VXLAN_ID]) >= sizeof(__u32)) {
 		print_uint(PRINT_ANY, "id", "id %u ", rta_getattr_u32(tb[IFLA_VXLAN_ID]));
@@ -529,26 +539,6 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			   "dstport %u ",
 			   rta_getattr_be16(tb[IFLA_VXLAN_PORT]));
 
-	if (tb[IFLA_VXLAN_LEARNING]) {
-		__u8 learning = rta_getattr_u8(tb[IFLA_VXLAN_LEARNING]);
-
-		print_bool(PRINT_JSON, "learning", NULL, learning);
-		if (!learning)
-			print_bool(PRINT_FP, NULL, "nolearning ", true);
-	}
-
-	if (tb[IFLA_VXLAN_PROXY] && rta_getattr_u8(tb[IFLA_VXLAN_PROXY]))
-		print_bool(PRINT_ANY, "proxy", "proxy ", true);
-
-	if (tb[IFLA_VXLAN_RSC] && rta_getattr_u8(tb[IFLA_VXLAN_RSC]))
-		print_bool(PRINT_ANY, "rsc", "rsc ", true);
-
-	if (tb[IFLA_VXLAN_L2MISS] && rta_getattr_u8(tb[IFLA_VXLAN_L2MISS]))
-		print_bool(PRINT_ANY, "l2miss", "l2miss ", true);
-
-	if (tb[IFLA_VXLAN_L3MISS] && rta_getattr_u8(tb[IFLA_VXLAN_L3MISS]))
-		print_bool(PRINT_ANY, "l3miss", "l3miss ", true);
-
 	if (tb[IFLA_VXLAN_TOS])
 		tos = rta_getattr_u8(tb[IFLA_VXLAN_TOS]);
 	if (tos) {
@@ -601,58 +591,22 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	    ((maxaddr = rta_getattr_u32(tb[IFLA_VXLAN_LIMIT])) != 0))
 		print_uint(PRINT_ANY, "limit", "maxaddr %u ", maxaddr);
 
-	if (tb[IFLA_VXLAN_UDP_CSUM]) {
-		__u8 udp_csum = rta_getattr_u8(tb[IFLA_VXLAN_UDP_CSUM]);
-
-		if (is_json_context()) {
-			print_bool(PRINT_ANY, "udp_csum", NULL, udp_csum);
-		} else {
-			if (!udp_csum)
-				fputs("no", f);
-			fputs("udpcsum ", f);
-		}
-	}
-
-	if (tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
-		__u8 csum6 = rta_getattr_u8(tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]);
-
-		if (is_json_context()) {
-			print_bool(PRINT_ANY,
-				   "udp_zero_csum6_tx", NULL, csum6);
-		} else {
-			if (!csum6)
-				fputs("no", f);
-			fputs("udp6zerocsumtx ", f);
-		}
-	}
-
-	if (tb[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) {
-		__u8 csum6 = rta_getattr_u8(tb[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]);
-
-		if (is_json_context()) {
-			print_bool(PRINT_ANY,
-				   "udp_zero_csum6_rx",
-				   NULL,
-				   csum6);
-		} else {
-			if (!csum6)
-				fputs("no", f);
-			fputs("udp6zerocsumrx ", f);
-		}
-	}
-
-	if (tb[IFLA_VXLAN_REMCSUM_TX] &&
-	    rta_getattr_u8(tb[IFLA_VXLAN_REMCSUM_TX]))
-		print_bool(PRINT_ANY, "remcsum_tx", "remcsumtx ", true);
-
-	if (tb[IFLA_VXLAN_REMCSUM_RX] &&
-	    rta_getattr_u8(tb[IFLA_VXLAN_REMCSUM_RX]))
-		print_bool(PRINT_ANY, "remcsum_rx", "remcsumrx ", true);
-
 	if (tb[IFLA_VXLAN_GBP])
 		print_null(PRINT_ANY, "gbp", "gbp ", NULL);
 	if (tb[IFLA_VXLAN_GPE])
 		print_null(PRINT_ANY, "gpe", "gpe ", NULL);
+
+	for (i = 0; i < ARRAY_SIZE(vxlan_opts); i++) {
+		const struct vxlan_bool_opt *opt = &vxlan_opts[i];
+		__u8 val;
+
+		if (!tb[opt->type])
+			continue;
+		val = rta_getattr_u8(tb[opt->type]);
+
+		print_bool_opt(PRINT_ANY, opt->key, val,
+			       val != opt->default_value || show_details > 1);
+	}
 }
 
 static void vxlan_print_help(struct link_util *lu, int argc, char **argv,
diff --git a/lib/json_print.c b/lib/json_print.c
index d7ee76b10de8..602de027ca27 100644
--- a/lib/json_print.c
+++ b/lib/json_print.c
@@ -215,6 +215,25 @@ int print_color_bool(enum output_type type,
 				  value ? "true" : "false");
 }
 
+/* In JSON mode, acts like print_color_bool.
+ * Otherwise, will print key with prefix of "no" if false.
+ * The show flag is used to suppres printing in non-JSON mode
+ */
+int print_color_bool_opt(enum output_type type,
+			 enum color_attr color,
+			 const char *key,
+			 bool value, bool show)
+{
+	int ret = 0;
+
+	if (_IS_JSON_CONTEXT(type))
+		jsonw_bool_field(_jw, key, value);
+	else if (_IS_FP_CONTEXT(type) && show)
+		ret = color_fprintf(stdout, color, "%s%s ",
+				    value ? "" : "no", key);
+	return ret;
+}
+
 int print_color_on_off(enum output_type type,
 		       enum color_attr color,
 		       const char *key,
-- 
2.39.2

Re: [PATCH iproute2 v4 2/2] vxlan: make option printing more consistent

[Andrea Claudi]

On Fri, May 26, 2023 at 10:41:41AM -0700, Stephen Hemminger wrote:
> Add new helper function print_bool_opt() which prints
> with no prefix and use it for vxlan options.
> 
> If the option matches the expected default value,
> it is not printed if in non JSON mode unless the details
> setting is repeated.
> 
> Use a table for the vxlan options. This will change
> the order of the printing of options.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  include/json_print.h |   9 ++++
>  ip/iplink_vxlan.c    | 110 +++++++++++++------------------------------
>  lib/json_print.c     |  19 ++++++++
>  3 files changed, 60 insertions(+), 78 deletions(-)
> 
> diff --git a/include/json_print.h b/include/json_print.h
> index 91b34571ceb0..49d3cc14789c 100644
> --- a/include/json_print.h
> +++ b/include/json_print.h
> @@ -101,6 +101,15 @@ static inline int print_rate(bool use_iec, enum output_type t,
>  	return print_color_rate(use_iec, t, COLOR_NONE, key, fmt, rate);
>  }
>  
> +int print_color_bool_opt(enum output_type type, enum color_attr color,
> +			 const char *key, bool value, bool show);
> +
> +static inline int print_bool_opt(enum output_type type,
> +				 const char *key, bool value, bool show)
> +{
> +	return print_color_bool_opt(type, COLOR_NONE, key, value, show);
> +}
> +
>  /* A backdoor to the size formatter. Please use print_size() instead. */
>  char *sprint_size(__u32 sz, char *buf);
>  
> diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> index cb6745c74507..e77c3aa2e3a5 100644
> --- a/ip/iplink_vxlan.c
> +++ b/ip/iplink_vxlan.c
> @@ -19,6 +19,25 @@
>  
>  #define VXLAN_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0)
>  
> +static const struct vxlan_bool_opt {
> +	const char *key;
> +	int type;
> +	bool default_value;
> +} vxlan_opts[] = {
> +	{ "metadata",	IFLA_VXLAN_COLLECT_METADATA,	false },

Here you are changing the output from "external" to "metadata", while
continuing to use "external" to toggle the option. This may surprise a
user which checks the output for this option after enabling it.

Moreover, looking at the man page for ip link, this option is used to
specify "whether an external control plane (e.g. ip route encap) or the
internal FDB should be used", so maybe "external" is more close to the
true meaning of this option.

I would suggest either to continue using "external" or to switch to
"metadata" also for option toggling (and update the manpage, too).

All the rest looks good to me.

> +	{ "vnifilter",	IFLA_VXLAN_VNIFILTER,		false },
> +	{ "learning", 	IFLA_VXLAN_LEARNING,		true },
> +	{ "proxy",	IFLA_VXLAN_PROXY,		false },
> +	{ "rsc",	IFLA_VXLAN_RSC,			false },
> +	{ "l2miss",	IFLA_VXLAN_L2MISS,		false },
> +	{ "l3miss",	IFLA_VXLAN_L3MISS,		false },
> +	{ "udp_csum",	IFLA_VXLAN_UDP_CSUM,		true },
> +	{ "udp_zero_csum6_tx", IFLA_VXLAN_UDP_ZERO_CSUM6_TX, false },
> +	{ "udp_zero_csum6_rx", IFLA_VXLAN_UDP_ZERO_CSUM6_RX, false },
> +	{ "remcsum_tx", IFLA_VXLAN_REMCSUM_TX,		false },
> +	{ "remcsum_rx", IFLA_VXLAN_REMCSUM_RX,		false },
> +};
> +
>  static void print_explain(FILE *f)
>  {
>  	fprintf(f,
> @@ -420,6 +439,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
>  
>  static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>  {
> +	unsigned int i;
>  	__u8 ttl = 0;
>  	__u8 tos = 0;
>  	__u32 maxaddr;
> @@ -427,16 +447,6 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>  	if (!tb)
>  		return;
>  
> -	if (tb[IFLA_VXLAN_COLLECT_METADATA] &&
> -	    rta_getattr_u8(tb[IFLA_VXLAN_COLLECT_METADATA])) {
> -		print_bool(PRINT_ANY, "external", "external ", true);
> -	}
> -
> -	if (tb[IFLA_VXLAN_VNIFILTER] &&
> -	    rta_getattr_u8(tb[IFLA_VXLAN_VNIFILTER])) {
> -		print_bool(PRINT_ANY, "vnifilter", "vnifilter", true);
> -	}
> -
>  	if (tb[IFLA_VXLAN_ID] &&
>  	    RTA_PAYLOAD(tb[IFLA_VXLAN_ID]) >= sizeof(__u32)) {
>  		print_uint(PRINT_ANY, "id", "id %u ", rta_getattr_u32(tb[IFLA_VXLAN_ID]));
> @@ -529,26 +539,6 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>  			   "dstport %u ",
>  			   rta_getattr_be16(tb[IFLA_VXLAN_PORT]));
>  
> -	if (tb[IFLA_VXLAN_LEARNING]) {
> -		__u8 learning = rta_getattr_u8(tb[IFLA_VXLAN_LEARNING]);
> -
> -		print_bool(PRINT_JSON, "learning", NULL, learning);
> -		if (!learning)
> -			print_bool(PRINT_FP, NULL, "nolearning ", true);
> -	}
> -
> -	if (tb[IFLA_VXLAN_PROXY] && rta_getattr_u8(tb[IFLA_VXLAN_PROXY]))
> -		print_bool(PRINT_ANY, "proxy", "proxy ", true);
> -
> -	if (tb[IFLA_VXLAN_RSC] && rta_getattr_u8(tb[IFLA_VXLAN_RSC]))
> -		print_bool(PRINT_ANY, "rsc", "rsc ", true);
> -
> -	if (tb[IFLA_VXLAN_L2MISS] && rta_getattr_u8(tb[IFLA_VXLAN_L2MISS]))
> -		print_bool(PRINT_ANY, "l2miss", "l2miss ", true);
> -
> -	if (tb[IFLA_VXLAN_L3MISS] && rta_getattr_u8(tb[IFLA_VXLAN_L3MISS]))
> -		print_bool(PRINT_ANY, "l3miss", "l3miss ", true);
> -
>  	if (tb[IFLA_VXLAN_TOS])
>  		tos = rta_getattr_u8(tb[IFLA_VXLAN_TOS]);
>  	if (tos) {
> @@ -601,58 +591,22 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>  	    ((maxaddr = rta_getattr_u32(tb[IFLA_VXLAN_LIMIT])) != 0))
>  		print_uint(PRINT_ANY, "limit", "maxaddr %u ", maxaddr);
>  
> -	if (tb[IFLA_VXLAN_UDP_CSUM]) {
> -		__u8 udp_csum = rta_getattr_u8(tb[IFLA_VXLAN_UDP_CSUM]);
> -
> -		if (is_json_context()) {
> -			print_bool(PRINT_ANY, "udp_csum", NULL, udp_csum);
> -		} else {
> -			if (!udp_csum)
> -				fputs("no", f);
> -			fputs("udpcsum ", f);
> -		}
> -	}
> -
> -	if (tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
> -		__u8 csum6 = rta_getattr_u8(tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]);
> -
> -		if (is_json_context()) {
> -			print_bool(PRINT_ANY,
> -				   "udp_zero_csum6_tx", NULL, csum6);
> -		} else {
> -			if (!csum6)
> -				fputs("no", f);
> -			fputs("udp6zerocsumtx ", f);
> -		}
> -	}
> -
> -	if (tb[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) {
> -		__u8 csum6 = rta_getattr_u8(tb[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]);
> -
> -		if (is_json_context()) {
> -			print_bool(PRINT_ANY,
> -				   "udp_zero_csum6_rx",
> -				   NULL,
> -				   csum6);
> -		} else {
> -			if (!csum6)
> -				fputs("no", f);
> -			fputs("udp6zerocsumrx ", f);
> -		}
> -	}
> -
> -	if (tb[IFLA_VXLAN_REMCSUM_TX] &&
> -	    rta_getattr_u8(tb[IFLA_VXLAN_REMCSUM_TX]))
> -		print_bool(PRINT_ANY, "remcsum_tx", "remcsumtx ", true);
> -
> -	if (tb[IFLA_VXLAN_REMCSUM_RX] &&
> -	    rta_getattr_u8(tb[IFLA_VXLAN_REMCSUM_RX]))
> -		print_bool(PRINT_ANY, "remcsum_rx", "remcsumrx ", true);
> -
>  	if (tb[IFLA_VXLAN_GBP])
>  		print_null(PRINT_ANY, "gbp", "gbp ", NULL);
>  	if (tb[IFLA_VXLAN_GPE])
>  		print_null(PRINT_ANY, "gpe", "gpe ", NULL);
> +
> +	for (i = 0; i < ARRAY_SIZE(vxlan_opts); i++) {
> +		const struct vxlan_bool_opt *opt = &vxlan_opts[i];
> +		__u8 val;
> +
> +		if (!tb[opt->type])
> +			continue;
> +		val = rta_getattr_u8(tb[opt->type]);
> +
> +		print_bool_opt(PRINT_ANY, opt->key, val,
> +			       val != opt->default_value || show_details > 1);
> +	}
>  }
>  
>  static void vxlan_print_help(struct link_util *lu, int argc, char **argv,
> diff --git a/lib/json_print.c b/lib/json_print.c
> index d7ee76b10de8..602de027ca27 100644
> --- a/lib/json_print.c
> +++ b/lib/json_print.c
> @@ -215,6 +215,25 @@ int print_color_bool(enum output_type type,
>  				  value ? "true" : "false");
>  }
>  
> +/* In JSON mode, acts like print_color_bool.
> + * Otherwise, will print key with prefix of "no" if false.
> + * The show flag is used to suppres printing in non-JSON mode
> + */
> +int print_color_bool_opt(enum output_type type,
> +			 enum color_attr color,
> +			 const char *key,
> +			 bool value, bool show)
> +{
> +	int ret = 0;
> +
> +	if (_IS_JSON_CONTEXT(type))
> +		jsonw_bool_field(_jw, key, value);
> +	else if (_IS_FP_CONTEXT(type) && show)
> +		ret = color_fprintf(stdout, color, "%s%s ",
> +				    value ? "" : "no", key);
> +	return ret;
> +}
> +
>  int print_color_on_off(enum output_type type,
>  		       enum color_attr color,
>  		       const char *key,
> -- 
> 2.39.2
> 
> 

Re: [PATCH iproute2 v4 2/2] vxlan: make option printing more consistent

[Stephen Hemminger]

On Fri, 26 May 2023 21:42:17 +0200
Andrea Claudi <aclaudi@redhat.com> wrote:

> On Fri, May 26, 2023 at 10:41:41AM -0700, Stephen Hemminger wrote:
> > Add new helper function print_bool_opt() which prints
> > with no prefix and use it for vxlan options.
> > 
> > If the option matches the expected default value,
> > it is not printed if in non JSON mode unless the details
> > setting is repeated.
> > 
> > Use a table for the vxlan options. This will change
> > the order of the printing of options.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  include/json_print.h |   9 ++++
> >  ip/iplink_vxlan.c    | 110 +++++++++++++------------------------------
> >  lib/json_print.c     |  19 ++++++++
> >  3 files changed, 60 insertions(+), 78 deletions(-)
> > 
> > diff --git a/include/json_print.h b/include/json_print.h
> > index 91b34571ceb0..49d3cc14789c 100644
> > --- a/include/json_print.h
> > +++ b/include/json_print.h
> > @@ -101,6 +101,15 @@ static inline int print_rate(bool use_iec, enum output_type t,
> >  	return print_color_rate(use_iec, t, COLOR_NONE, key, fmt, rate);
> >  }
> >  
> > +int print_color_bool_opt(enum output_type type, enum color_attr color,
> > +			 const char *key, bool value, bool show);
> > +
> > +static inline int print_bool_opt(enum output_type type,
> > +				 const char *key, bool value, bool show)
> > +{
> > +	return print_color_bool_opt(type, COLOR_NONE, key, value, show);
> > +}
> > +
> >  /* A backdoor to the size formatter. Please use print_size() instead. */
> >  char *sprint_size(__u32 sz, char *buf);
> >  
> > diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> > index cb6745c74507..e77c3aa2e3a5 100644
> > --- a/ip/iplink_vxlan.c
> > +++ b/ip/iplink_vxlan.c
> > @@ -19,6 +19,25 @@
> >  
> >  #define VXLAN_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0)
> >  
> > +static const struct vxlan_bool_opt {
> > +	const char *key;
> > +	int type;
> > +	bool default_value;
> > +} vxlan_opts[] = {
> > +	{ "metadata",	IFLA_VXLAN_COLLECT_METADATA,	false },  
> 
> Here you are changing the output from "external" to "metadata", while
> continuing to use "external" to toggle the option. This may surprise a
> user which checks the output for this option after enabling it.
> 
> Moreover, looking at the man page for ip link, this option is used to
> specify "whether an external control plane (e.g. ip route encap) or the
> internal FDB should be used", so maybe "external" is more close to the
> true meaning of this option.
> 
> I would suggest either to continue using "external" or to switch to
> "metadata" also for option toggling (and update the manpage, too).
> 
> All the rest looks good to me.
>

Thanks will fix that.

Re: [PATCH iproute2 v4 0/2] vxlan: option printing

[patchwork-bot+netdevbpf]

Hello:

This series was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Fri, 26 May 2023 10:41:39 -0700 you wrote:
> This patchset makes printing of vxlan details more consistent.
> It also adds extra verbose output. The boolean options
> are now brinted after all the non-boolean options.
> 
> Before:
> $ ip -d link show dev vxlan0
> 4: vxlan0: <BROADCAST,MULTICAST> mtu 1450 qdisc noop state DOWN mode DEFAULT group default qlen 1000
>     link/ether e6:a4:54:b2:34:85 brd ff:ff:ff:ff:ff:ff promiscuity 0  allmulti 0 minmtu 68 maxmtu 65535
>     vxlan id 42 group 239.1.1.1 dev enp2s0 srcport 0 0 dstport 4789 ttl auto ageing 300 udpcsum noudp6zerocsumtx noudp6zerocsumrx addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 64000 gso_max_segs 64 tso_max_size 64000 tso_max_segs 64 gro_max_size 65536
> 
> [...]

Here is the summary with links:
  - [iproute2,v4,1/2] vxlan: use print_nll for gbp and gpe
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=a183eba71bc2
  - [iproute2,v4,2/2] vxlan: make option printing more consistent
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html