[PATCH iproute2 v5 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output

[PATCH iproute2 v5 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output

[Serhey Popovych]

With this series I want to improve printing of tclass,
flowlabel and encapsulation limit (encap_limit) options.

Everything within this series is open for your comments,
suggestions and criticism.

See individual patch description message for details.

v5
  fix wrong cover letter subject and body

v4
  rebased to current upstream

v1..v3
  no changes. initially started with v3 instead of v1

Thanks,
Serhii

Serhey Popovych (7):
  ip6/tunnel: Fix tclass output
  ip6tnl/tunnel: Do not print obscure flowinfo
  ip6/tunnel: Unify tclass printing
  ip6/tunnel: Unify flowlabel printing
  ip6/tunnel: Unify encap_limit printing
  gre6/tunnel: Output flowlabel after tclass
  ip6tnl/tunnel: Output hoplimit before encapsulation limit

 ip/link_gre6.c   |   54 ++++++++++++++++------------------------------
 ip/link_ip6tnl.c |   63 ++++++++++++++++++------------------------------------
 2 files changed, 40 insertions(+), 77 deletions(-)

-- 
1.7.10.4

[PATCH iproute2 v5 1/7] ip6/tunnel: Fix tclass output

[Serhey Popovych]

In link_gre6.c it seems copy paste error: tclass is 8 bits,
not 20 as flowlabel.

In link_iptnl.c rename "flowinfo_tclass" to "tclass" as it
correct name since flowinfo is implementation internal name
used to label combined within u32 attribute tclass and
flowlabel.

Fixes: 1facc1c61c07 ("ip: link_ip6tnl.c: add json output support")
Fixes: 2e706e12d9b0 ("Merge branch 'master' into net-next")
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |    2 +-
 ip/link_ip6tnl.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7ae4b49..87c313c 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -475,7 +475,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		if (is_json_context()) {
 			SPRINT_BUF(b1);
 
-			snprintf(b1, sizeof(b1), "0x%05x",
+			snprintf(b1, sizeof(b1), "0x%02x",
 				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
 			print_string(PRINT_JSON, "tclass", NULL, b1);
 		} else {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 84205b1..8e84ed0 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -416,7 +416,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			SPRINT_BUF(b1);
 
 			snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
-			print_string(PRINT_JSON, "flowinfo_tclass", NULL, b1);
+			print_string(PRINT_JSON, "tclass", NULL, b1);
 		} else {
 			printf("tclass 0x%02x ", (__u8)(val >> 20));
 		}
-- 
1.7.10.4

[PATCH iproute2 v5 2/7] ip6tnl/tunnel: Do not print obscure flowinfo

[Serhey Popovych]

It is implementation internal and main purpose
of printing it seems debugging.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_ip6tnl.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 8e84ed0..e084975 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -440,16 +440,6 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		}
 	}
 
-	if (is_json_context()) {
-		SPRINT_BUF(flwinfo);
-
-		snprintf(flwinfo, sizeof(flwinfo), "0x%08x", ntohl(flowinfo));
-		print_string(PRINT_JSON, "flowinfo", NULL, flwinfo);
-	} else {
-		printf("(flowinfo 0x%08x) ", ntohl(flowinfo));
-
-	}
-
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_rcv_dscp_copy",
-- 
1.7.10.4

[PATCH iproute2 v5 3/7] ip6/tunnel: Unify tclass printing

[Serhey Popovych]

Use @s2 buffer to store string representation of
tclass and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.

Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.

While there use __u32 for flowinfo in link_gre6.c
and check for IFLA_GRE_FLOWINFO attribute presense.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   16 +++++-----------
 ip/link_ip6tnl.c |   16 +++++-----------
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 87c313c..a02dd4a 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -380,7 +380,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 	unsigned int flags = 0;
-	unsigned int flowinfo = 0;
+	__u32 flowinfo = 0;
 	struct in6_addr in6_addr_any = IN6ADDR_ANY_INIT;
 
 	if (!tb)
@@ -471,17 +471,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			   "ip6_tnl_f_use_orig_tclass",
 			   "tclass inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
 
-			snprintf(b1, sizeof(b1), "0x%02x",
-				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
-			print_string(PRINT_JSON, "tclass", NULL, b1);
-		} else {
-			fprintf(f, "tclass 0x%02x ",
-				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
-		}
+		snprintf(s2, sizeof(s2), "0x%02x", val);
+		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index e084975..05322fd 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -404,22 +404,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			   "hoplimit %u ",
 			   rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
 
-	if (flags & IP6_TNL_F_USE_ORIG_TCLASS)
+	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_tclass",
 			   "tclass inherit ",
 			   true);
-	else if (tb[IFLA_IPTUN_FLOWINFO]) {
-		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS);
+	} else if (tb[IFLA_IPTUN_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
 
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
-			print_string(PRINT_JSON, "tclass", NULL, b1);
-		} else {
-			printf("tclass 0x%02x ", (__u8)(val >> 20));
-		}
+		snprintf(s2, sizeof(s2), "0x%02x", val);
+		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
-- 
1.7.10.4

[PATCH iproute2 v5 4/7] ip6/tunnel: Unify flowlabel printing

[Serhey Popovych]

Use @s2 buffer to store string representation of
flowlabel and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.

Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.

Omit flowlabel from output if no flowinfo attribute
is given and IP6_TNL_F_USE_ORIG_FLOWLABEL isn't set.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   15 ++++-----------
 ip/link_ip6tnl.c |   14 ++++----------
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index a02dd4a..1205946 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -452,18 +452,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			   "ip6_tnl_f_use_orig_flowlabel",
 			   "flowlabel inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%05x",
-				 ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-			print_string(PRINT_JSON, "flowlabel", NULL, b1);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
 
-		} else {
-			fprintf(f, "flowlabel 0x%05x ",
-				ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-		}
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 05322fd..7000056 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -421,17 +421,11 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			   "ip6_tnl_f_use_orig_flowlabel",
 			   "flowlabel inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
+	} else if (tb[IFLA_IPTUN_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
 
-			snprintf(b1, sizeof(b1), "0x%05x",
-				 ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-			print_string(PRINT_JSON, "flowlabel", NULL, b1);
-		} else {
-			printf("flowlabel 0x%05x ",
-			       ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-		}
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
-- 
1.7.10.4

[PATCH iproute2 v5 5/7] ip6/tunnel: Unify encap_limit printing

[Serhey Popovych]

Use %u format specifier to print it in link_gre6.c and
make code more readable.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   11 ++++-------
 ip/link_ip6tnl.c |   12 ++++++------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 1205946..8014207 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -433,18 +433,15 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			print_int(PRINT_JSON, "ttl", NULL, ttl);
 	}
 
-	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_ign_encap_limit",
 			   "encaplimit none ",
 			   true);
-	else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
-		int encap_limit = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
+	} else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
+		__u8 val = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
 
-		print_int(PRINT_ANY,
-			  "encap_limit",
-			  "encaplimit %d ",
-			  encap_limit);
+		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 7000056..379eb33 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -387,16 +387,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
 	}
 
-	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_ign_encap_limit",
 			   "encaplimit none ",
 			   true);
-	else if (tb[IFLA_IPTUN_ENCAP_LIMIT])
-		print_uint(PRINT_ANY,
-			   "encap_limit",
-			   "encaplimit %u ",
-			   rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]));
+	} else if (tb[IFLA_IPTUN_ENCAP_LIMIT]) {
+		__u8 val = rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]);
+
+		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
+	}
 
 	if (tb[IFLA_IPTUN_TTL])
 		print_uint(PRINT_ANY,
-- 
1.7.10.4

[PATCH iproute2 v5 6/7] gre6/tunnel: Output flowlabel after tclass

[Serhey Popovych]

To follow ip6tnl output print flowlabel after tclass
in link_gre6.c.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 8014207..55bd1fb 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -444,18 +444,6 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
 	}
 
-	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
-		print_bool(PRINT_ANY,
-			   "ip6_tnl_f_use_orig_flowlabel",
-			   "flowlabel inherit ",
-			   true);
-	} else if (tb[IFLA_GRE_FLOWINFO]) {
-		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
-
-		snprintf(s2, sizeof(s2), "0x%05x", val);
-		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
-	}
-
 	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_tclass",
@@ -468,6 +456,18 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
+	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
+		print_bool(PRINT_ANY,
+			   "ip6_tnl_f_use_orig_flowlabel",
+			   "flowlabel inherit ",
+			   true);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
+
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
+	}
+
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_rcv_dscp_copy",
-- 
1.7.10.4

[PATCH iproute2 v5 7/7] ip6tnl/tunnel: Output hoplimit before encapsulation limit

[Serhey Popovych]

To follow gre6 output print hoplimit before encapsulation
limit in link_ip6tnl.c.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_ip6tnl.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 379eb33..bbc7878 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -387,6 +387,13 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
 	}
 
+	if (tb[IFLA_IPTUN_TTL]) {
+		print_uint(PRINT_ANY,
+			   "ttl",
+			   "hoplimit %u ",
+			   rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
+	}
+
 	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_ign_encap_limit",
@@ -398,12 +405,6 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
 	}
 
-	if (tb[IFLA_IPTUN_TTL])
-		print_uint(PRINT_ANY,
-			   "ttl",
-			   "hoplimit %u ",
-			   rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
-
 	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_tclass",
-- 
1.7.10.4

Re: [PATCH iproute2 v5 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output

[Stephen Hemminger]

On Wed, 10 Jan 2018 17:53:08 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> With this series I want to improve printing of tclass,
> flowlabel and encapsulation limit (encap_limit) options.
> 
> Everything within this series is open for your comments,
> suggestions and criticism.
> 
> See individual patch description message for details.
> 
> v5
>   fix wrong cover letter subject and body
> 
> v4
>   rebased to current upstream
> 
> v1..v3
>   no changes. initially started with v3 instead of v1
> 
> Thanks,
> Serhii
> 
> Serhey Popovych (7):
>   ip6/tunnel: Fix tclass output
>   ip6tnl/tunnel: Do not print obscure flowinfo
>   ip6/tunnel: Unify tclass printing
>   ip6/tunnel: Unify flowlabel printing
>   ip6/tunnel: Unify encap_limit printing
>   gre6/tunnel: Output flowlabel after tclass
>   ip6tnl/tunnel: Output hoplimit before encapsulation limit
> 
>  ip/link_gre6.c   |   54 ++++++++++++++++------------------------------
>  ip/link_ip6tnl.c |   63 ++++++++++++++++++------------------------------------
>  2 files changed, 40 insertions(+), 77 deletions(-)
> 


Applied, thanks for keeping going.