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

[PATCH iproute2 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.

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 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 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 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 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 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 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 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 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output

[Stephen Hemminger]

On Mon,  8 Jan 2018 19:06:31 +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.
> 
> 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(-)
> 


I am confused. this patch series arrived after the v3 version.
Which one is correct?

Also when sending multiple versions of a patch series is it is
best practice to include a revision history in the cover letter like:


v2
  ip6/tunnel  - incorporate review feedback in fou

v3
  ip/gre - fix spelling errors

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

[Serhey Popovych]

[-- Attachment #1.1: Type: text/plain, Size: 1427 bytes --]

Stephen Hemminger wrote:
> On Mon,  8 Jan 2018 19:06:31 +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.
>>
>> 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(-)
>>
> 
> 
> I am confused. this patch series arrived after the v3 version.
> Which one is correct?
> 
> Also when sending multiple versions of a patch series is it is
> best practice to include a revision history in the cover letter like:
> 
> 
> v2
>   ip6/tunnel  - incorporate review feedback in fou
> 
> v3
>   ip/gre - fix spelling errors
> 
> 

Ah, sorry for that inconvenience. Addressed with v4.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]