All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing
@ 2018-01-18 14:04 Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

Continue improvements to tunnel modules. Final goal
is to make merge IP and IPv6 variants and make that
transition as transparent as possible.

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

See individual patch description message for details.

v2
  1) Fix checkpatch issues: no assignment in condtional
     anymore.

  2) Better code abstraction: introduce tnl_print_encap()
     helper and get rid of duplicated code for printing
     tunnel encapsulation options.

     Patch with subject
     "ip/tunnel: Abstract tunnel encapsulation options printing"
     replaces two previous in v1 series
     "ip/tunnel: Use print_string() and simplify encap option printing"
     and
     "ip/tunnel: Minor cleanups in print routines"

  3) Include patch with subject
     "tunnel: Return constant string without copying it"
     in the series: it is related to tunneling code too.

Thanks,
Serhii

Serhey Popovych (9):
  iplink: Use ll_index_to_name() instead of if_indextoname()
  ip/tunnel: Correct and unify ttl/hoplimit printing
  ip/tunnel: Simplify and unify tos printing
  ip/tunnel: Use print_0xhex() instead of print_string()
  ip/tunnel: Abstract tunnel encapsulation options printing
  gre/tunnel: Print erspan_index using print_uint()
  vti/tunnel: Unify ikey/okey printing
  vti6/tunnel: Unify and simplify link type help functions
  tunnel: Return constant string without copying it

 bridge/fdb.c          |    5 +-
 bridge/link.c         |   19 +++----
 ip/ip6tunnel.c        |    5 +-
 ip/iplink_bond.c      |   38 ++++++--------
 ip/iplink_geneve.c    |   38 ++++++--------
 ip/iplink_vxlan.c     |   49 ++++++++----------
 ip/iproute_lwtunnel.c |   11 ++---
 ip/iptunnel.c         |    2 +-
 ip/link_gre.c         |  132 +++++++++++--------------------------------------
 ip/link_gre6.c        |  103 +++++++++-----------------------------
 ip/link_ip6tnl.c      |  105 ++++++++++-----------------------------
 ip/link_iptnl.c       |  128 +++++++++++------------------------------------
 ip/link_vti.c         |   42 ++++++++--------
 ip/link_vti6.c        |   64 +++++++++++++++---------
 ip/tunnel.c           |  116 ++++++++++++++++++++++++++++++++++++-------
 ip/tunnel.h           |    5 ++
 16 files changed, 340 insertions(+), 522 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname()
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 2/9] ip/tunnel: Correct and unify ttl/hoplimit printing Serhey Popovych
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

There are two reasons for switching to cached variant:

  1) ll_index_to_name() may return result from cache,
     eliminating expensive ioctl() to the kernel.

     Note that most of the code already switched from plain
     if_indextoname() to ll_index_to_name() to cached variant
     in print path because in most cases cache populated.

  2) It always return name in the form "if%d", even if
     entry is not in cache and ioctl() fails. This drops
     "link_index" from JSON output.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 bridge/fdb.c          |    5 ++---
 bridge/link.c         |   19 +++++++------------
 ip/iplink_bond.c      |   38 ++++++++++++++------------------------
 ip/iplink_vxlan.c     |   15 ++++++---------
 ip/iproute_lwtunnel.c |    7 ++-----
 ip/link_gre.c         |   11 +++++------
 ip/link_gre6.c        |   11 +++++------
 ip/link_ip6tnl.c      |   11 +++++------
 ip/link_iptnl.c       |   11 +++++------
 ip/link_vti.c         |   14 ++++++--------
 ip/link_vti6.c        |   13 ++++++-------
 11 files changed, 63 insertions(+), 92 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index 376713b..4d55fb0 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -219,10 +219,9 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		unsigned int ifindex = rta_getattr_u32(tb[NDA_IFINDEX]);
 
 		if (ifindex) {
-			char ifname[IF_NAMESIZE];
+			if (!tb[NDA_LINK_NETNSID]) {
+				const char *ifname = ll_index_to_name(ifindex);
 
-			if (!tb[NDA_LINK_NETNSID] &&
-			    if_indextoname(ifindex, ifname)) {
 				if (jw_global)
 					jsonw_string_field(jw_global, "viaIf",
 							   ifname);
diff --git a/bridge/link.c b/bridge/link.c
index e2371d0..870ebe0 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -26,8 +26,6 @@ static const char *port_states[] = {
 	[BR_STATE_BLOCKING] = "blocking",
 };
 
-extern char *if_indextoname(unsigned int __ifindex, char *__ifname);
-
 static void print_link_flags(FILE *fp, unsigned int flags)
 {
 	fprintf(fp, "<");
@@ -104,7 +102,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	int len = n->nlmsg_len;
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct rtattr *tb[IFLA_MAX+1];
-	char b1[IFNAMSIZ];
 
 	len -= NLMSG_LENGTH(sizeof(*ifi));
 	if (len < 0) {
@@ -135,14 +132,10 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
 
 	if (tb[IFLA_LINK]) {
-		SPRINT_BUF(b1);
 		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
 
-		if (iflink == 0)
-			fprintf(fp, "@NONE: ");
-		else
-			fprintf(fp, "@%s: ",
-				if_indextoname(iflink, b1));
+		fprintf(fp, "@%s: ",
+			iflink ? ll_index_to_name(iflink) : "NONE");
 	} else
 		fprintf(fp, ": ");
 
@@ -151,9 +144,11 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	if (tb[IFLA_MTU])
 		fprintf(fp, "mtu %u ", rta_getattr_u32(tb[IFLA_MTU]));
 
-	if (tb[IFLA_MASTER])
-		fprintf(fp, "master %s ",
-			if_indextoname(rta_getattr_u32(tb[IFLA_MASTER]), b1));
+	if (tb[IFLA_MASTER]) {
+		int master = rta_getattr_u32(tb[IFLA_MASTER]);
+
+		fprintf(fp, "master %s ", ll_index_to_name(master));
+	}
 
 	if (tb[IFLA_PROTINFO]) {
 		if (tb[IFLA_PROTINFO]->rta_type & NLA_F_NESTED) {
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index 2b5cf4f..f01fd8d 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -369,8 +369,6 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 
 static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	unsigned int ifindex;
-
 	if (!tb)
 		return;
 
@@ -380,21 +378,16 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_string(PRINT_ANY, "mode", "mode %s ", mode);
 	}
 
-	if (tb[IFLA_BOND_ACTIVE_SLAVE] &&
-	    (ifindex = rta_getattr_u32(tb[IFLA_BOND_ACTIVE_SLAVE]))) {
-		char buf[IFNAMSIZ];
-		const char *n = if_indextoname(ifindex, buf);
+	if (tb[IFLA_BOND_ACTIVE_SLAVE]) {
+		unsigned int ifindex =
+			rta_getattr_u32(tb[IFLA_BOND_ACTIVE_SLAVE]);
 
-		if (n)
+		if (ifindex) {
 			print_string(PRINT_ANY,
 				     "active_slave",
 				     "active_slave %s ",
-				     n);
-		else
-			print_uint(PRINT_ANY,
-				   "active_slave_index",
-				   "active_slave %u ",
-				   ifindex);
+				     ll_index_to_name(ifindex));
+		}
 	}
 
 	if (tb[IFLA_BOND_MIIMON])
@@ -479,18 +472,15 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			     arp_all_targets);
 	}
 
-	if (tb[IFLA_BOND_PRIMARY] &&
-	    (ifindex = rta_getattr_u32(tb[IFLA_BOND_PRIMARY]))) {
-		char buf[IFNAMSIZ];
-		const char *n = if_indextoname(ifindex, buf);
+	if (tb[IFLA_BOND_PRIMARY]) {
+		unsigned int ifindex = rta_getattr_u32(tb[IFLA_BOND_PRIMARY]);
 
-		if (n)
-			print_string(PRINT_ANY, "primary", "primary %s ", n);
-		else
-			print_uint(PRINT_ANY,
-				   "primary_index",
-				   "primary %u ",
-				   ifindex);
+		if (ifindex) {
+			print_string(PRINT_ANY,
+				     "primary",
+				     "primary %s ",
+				     ll_index_to_name(ifindex));
+		}
 	}
 
 	if (tb[IFLA_BOND_PRIMARY_RESELECT]) {
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 661eaa7..ad7ef1c 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -394,10 +394,8 @@ 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[])
 {
 	__u32 vni;
-	unsigned int link;
 	__u8 tos;
 	__u32 maxaddr;
-	char s2[64];
 
 	if (!tb)
 		return;
@@ -467,14 +465,13 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 						 &addr));
 	}
 
-	if (tb[IFLA_VXLAN_LINK] &&
-	    (link = rta_getattr_u32(tb[IFLA_VXLAN_LINK]))) {
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_VXLAN_LINK]) {
+		unsigned int link = rta_getattr_u32(tb[IFLA_VXLAN_LINK]);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		if (link) {
+			print_string(PRINT_ANY, "link", "dev %s ",
+				     ll_index_to_name(link));
+		}
 	}
 
 	if (tb[IFLA_VXLAN_PORT_RANGE]) {
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 740da7c..a8d7171 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -196,7 +196,6 @@ static int read_action_type(const char *name)
 static void print_encap_seg6local(FILE *fp, struct rtattr *encap)
 {
 	struct rtattr *tb[SEG6_LOCAL_MAX + 1];
-	char ifbuf[IFNAMSIZ];
 	int action;
 
 	parse_rtattr_nested(tb, SEG6_LOCAL_MAX, encap);
@@ -229,15 +228,13 @@ static void print_encap_seg6local(FILE *fp, struct rtattr *encap)
 	if (tb[SEG6_LOCAL_IIF]) {
 		int iif = rta_getattr_u32(tb[SEG6_LOCAL_IIF]);
 
-		fprintf(fp, "iif %s ",
-			if_indextoname(iif, ifbuf) ?: "<unknown>");
+		fprintf(fp, "iif %s ", ll_index_to_name(iif));
 	}
 
 	if (tb[SEG6_LOCAL_OIF]) {
 		int oif = rta_getattr_u32(tb[SEG6_LOCAL_OIF]);
 
-		fprintf(fp, "oif %s ",
-			if_indextoname(oif, ifbuf) ?: "<unknown>");
+		fprintf(fp, "oif %s ", ll_index_to_name(oif));
 	}
 }
 
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 3c0b6d6..7463d7c 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -380,14 +380,13 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_GRE_LINK] && rta_getattr_u32(tb[IFLA_GRE_LINK])) {
+	if (tb[IFLA_GRE_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_GRE_LINK]);
-		const char *n = if_indextoname(link, s2);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		if (link) {
+			print_string(PRINT_ANY, "link", "dev %s ",
+				     ll_index_to_name(link));
+		}
 	}
 
 	if (tb[IFLA_GRE_TTL]) {
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 55bd1fb..7d38f47 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -414,14 +414,13 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_GRE_LINK] && rta_getattr_u32(tb[IFLA_GRE_LINK])) {
+	if (tb[IFLA_GRE_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_GRE_LINK]);
-		const char *n = if_indextoname(link, s2);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		if (link) {
+			print_string(PRINT_ANY, "link", "dev %s ",
+				     ll_index_to_name(link));
+		}
 	}
 
 	if (tb[IFLA_GRE_TTL]) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index bbc7878..b22e7bc 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -377,14 +377,13 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			     rt_addr_n2a_rta(AF_INET6, tb[IFLA_IPTUN_LOCAL]));
 	}
 
-	if (tb[IFLA_IPTUN_LINK] && rta_getattr_u32(tb[IFLA_IPTUN_LINK])) {
+	if (tb[IFLA_IPTUN_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]);
-		const char *n = if_indextoname(link, s2);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		if (link) {
+			print_string(PRINT_ANY, "link", "dev %s ",
+				     ll_index_to_name(link));
+		}
 	}
 
 	if (tb[IFLA_IPTUN_TTL]) {
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 24a0f0c..35ef5c0 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -407,14 +407,13 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_IPTUN_LINK] && rta_getattr_u32(tb[IFLA_IPTUN_LINK])) {
+	if (tb[IFLA_IPTUN_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]);
-		const char *n = if_indextoname(link, s2);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_int(PRINT_ANY, "link_index", "dev %u ", link);
+		if (link) {
+			print_string(PRINT_ANY, "link", "dev %s ",
+				     ll_index_to_name(link));
+		}
 	}
 
 	if (tb[IFLA_IPTUN_TTL]) {
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 2b0fab2..554cfcc 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -168,7 +168,6 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	const char *local = "any";
 	const char *remote = "any";
 	__u32 key;
-	unsigned int link;
 	char s2[IFNAMSIZ];
 
 	if (!tb)
@@ -192,14 +191,13 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_VTI_LINK] &&
-	    (link = rta_getattr_u32(tb[IFLA_VTI_LINK]))) {
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_VTI_LINK]) {
+		unsigned int link = rta_getattr_u32(tb[IFLA_VTI_LINK]);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		if (link) {
+			print_string(PRINT_ANY, "link", "dev %s ",
+				     ll_index_to_name(link));
+		}
 	}
 
 	if (tb[IFLA_VTI_IKEY] &&
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 74c246d..a63f49f 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -168,7 +168,6 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	const char *remote = "any";
 	struct in6_addr saddr;
 	struct in6_addr daddr;
-	unsigned int link;
 	char s2[64];
 
 	if (!tb)
@@ -190,13 +189,13 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_VTI_LINK] && (link = rta_getattr_u32(tb[IFLA_VTI_LINK]))) {
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_VTI_LINK]) {
+		unsigned int link = rta_getattr_u32(tb[IFLA_VTI_LINK]);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		if (link) {
+			print_string(PRINT_ANY, "link", "dev %s ",
+				     ll_index_to_name(link));
+		}
 	}
 
 	if (tb[IFLA_VTI_IKEY]) {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 2/9] ip/tunnel: Correct and unify ttl/hoplimit printing
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 3/9] ip/tunnel: Simplify and unify tos printing Serhey Popovych
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

Both ttl/hoplimit is from 1 to 255. Zero has special meaning:
use encapsulated packet value. In ip-link(8) -d output this
looks like "ttl/hoplimit inherit". In JSON we have "int" type
for ttl and therefore values from 0 (inherit) to 255.

To do the best in handling ttl/hoplimit we need to accept
both cases: missing attribute in netlink dump and zero value
for "inherit"ed case. Last one is broken since JSON output
introduction for gre/iptnl versions and was never true for
gre6/ip6tnl.

For all tunnels, except ip6tnl change JSON type from "int" to
"uint" to reflect true nature of the ttl.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/ip6tunnel.c        |    5 ++++-
 ip/iplink_geneve.c    |   13 +++++++------
 ip/iplink_vxlan.c     |   15 +++++++--------
 ip/iproute_lwtunnel.c |    4 ++--
 ip/iptunnel.c         |    2 +-
 ip/link_gre.c         |   15 ++++++---------
 ip/link_gre6.c        |   15 +++++++--------
 ip/link_ip6tnl.c      |   13 +++++++------
 ip/link_iptnl.c       |   15 ++++++---------
 9 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index b8db49c..783e28a 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -92,7 +92,10 @@ static void print_tunnel(struct ip6_tnl_parm2 *p)
 	else
 		printf(" encaplimit %u", p->encap_limit);
 
-	printf(" hoplimit %u", p->hop_limit);
+	if (p->hop_limit)
+		printf(" hoplimit %u", p->hop_limit);
+	else
+		printf(" hoplimit inherit");
 
 	if (p->flags & IP6_TNL_F_USE_ORIG_TCLASS)
 		printf(" tclass inherit");
diff --git a/ip/iplink_geneve.c b/ip/iplink_geneve.c
index f0f1d1c..5a0afd4 100644
--- a/ip/iplink_geneve.c
+++ b/ip/iplink_geneve.c
@@ -227,6 +227,7 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
 static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	__u32 vni;
+	__u8 ttl = 0;
 	__u8 tos;
 
 	if (!tb)
@@ -262,12 +263,12 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		}
 	}
 
-	if (tb[IFLA_GENEVE_TTL]) {
-		__u8 ttl = rta_getattr_u8(tb[IFLA_GENEVE_TTL]);
-
-		if (ttl)
-			print_int(PRINT_ANY, "ttl", "ttl %d ", ttl);
-	}
+	if (tb[IFLA_GENEVE_TTL])
+		ttl = rta_getattr_u8(tb[IFLA_GENEVE_TTL]);
+	if (is_json_context() || ttl)
+		print_uint(PRINT_ANY, "ttl", "ttl %u ", ttl);
+	else
+		print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
 
 	if (tb[IFLA_GENEVE_TOS] &&
 	    (tos = rta_getattr_u8(tb[IFLA_GENEVE_TOS]))) {
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index ad7ef1c..b9c335d 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -394,6 +394,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[])
 {
 	__u32 vni;
+	__u8 ttl = 0;
 	__u8 tos;
 	__u32 maxaddr;
 
@@ -526,14 +527,12 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		}
 	}
 
-	if (tb[IFLA_VXLAN_TTL]) {
-		__u8 ttl = rta_getattr_u8(tb[IFLA_VXLAN_TTL]);
-
-		if (ttl)
-			print_int(PRINT_ANY, "ttl", "ttl %d ", ttl);
-		else
-			print_int(PRINT_JSON, "ttl", NULL, ttl);
-	}
+	if (tb[IFLA_VXLAN_TTL])
+		ttl = rta_getattr_u8(tb[IFLA_VXLAN_TTL]);
+	if (is_json_context() || ttl)
+		print_uint(PRINT_ANY, "ttl", "ttl %u ", ttl);
+	else
+		print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
 
 	if (tb[IFLA_VXLAN_LABEL]) {
 		__u32 label = rta_getattr_u32(tb[IFLA_VXLAN_LABEL]);
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index a8d7171..a1d36ba 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -271,7 +271,7 @@ static void print_encap_ip(FILE *fp, struct rtattr *encap)
 			rt_addr_n2a_rta(AF_INET, tb[LWTUNNEL_IP_DST]));
 
 	if (tb[LWTUNNEL_IP_TTL])
-		fprintf(fp, "ttl %d ", rta_getattr_u8(tb[LWTUNNEL_IP_TTL]));
+		fprintf(fp, "ttl %u ", rta_getattr_u8(tb[LWTUNNEL_IP_TTL]));
 
 	if (tb[LWTUNNEL_IP_TOS])
 		fprintf(fp, "tos %d ", rta_getattr_u8(tb[LWTUNNEL_IP_TOS]));
@@ -326,7 +326,7 @@ static void print_encap_ip6(FILE *fp, struct rtattr *encap)
 			rt_addr_n2a_rta(AF_INET6, tb[LWTUNNEL_IP6_DST]));
 
 	if (tb[LWTUNNEL_IP6_HOPLIMIT])
-		fprintf(fp, "hoplimit %d ",
+		fprintf(fp, "hoplimit %u ",
 			rta_getattr_u8(tb[LWTUNNEL_IP6_HOPLIMIT]));
 
 	if (tb[LWTUNNEL_IP6_TC])
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index ce610f8..0aa3b33 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -326,7 +326,7 @@ static void print_tunnel(struct ip_tunnel_parm *p)
 	}
 
 	if (p->iph.ttl)
-		printf(" ttl %d", p->iph.ttl);
+		printf(" ttl %u", p->iph.ttl);
 	else
 		printf(" ttl inherit");
 
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 7463d7c..1cf7f4e 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -361,6 +361,7 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 	const char *remote = "any";
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
+	__u8 ttl = 0;
 
 	if (tb[IFLA_GRE_REMOTE]) {
 		unsigned int addr = rta_getattr_u32(tb[IFLA_GRE_REMOTE]);
@@ -389,16 +390,12 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 		}
 	}
 
-	if (tb[IFLA_GRE_TTL]) {
-		__u8 ttl = rta_getattr_u8(tb[IFLA_GRE_TTL]);
-
-		if (ttl)
-			print_int(PRINT_ANY, "ttl", "ttl %d ", ttl);
-		else
-			print_int(PRINT_JSON, "ttl", NULL, ttl);
-	} else {
+	if (tb[IFLA_GRE_TTL])
+		ttl = rta_getattr_u8(tb[IFLA_GRE_TTL]);
+	if (is_json_context() || ttl)
+		print_uint(PRINT_ANY, "ttl", "ttl %u ", ttl);
+	else
 		print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
-	}
 
 	if (tb[IFLA_GRE_TOS] && rta_getattr_u8(tb[IFLA_GRE_TOS])) {
 		int tos = rta_getattr_u8(tb[IFLA_GRE_TOS]);
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7d38f47..37bc544 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -382,6 +382,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	unsigned int flags = 0;
 	__u32 flowinfo = 0;
 	struct in6_addr in6_addr_any = IN6ADDR_ANY_INIT;
+	__u8 ttl = 0;
 
 	if (!tb)
 		return;
@@ -423,14 +424,12 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		}
 	}
 
-	if (tb[IFLA_GRE_TTL]) {
-		__u8 ttl = rta_getattr_u8(tb[IFLA_GRE_TTL]);
-
-		if (ttl)
-			print_int(PRINT_ANY, "ttl", "hoplimit %d ", ttl);
-		else
-			print_int(PRINT_JSON, "ttl", NULL, ttl);
-	}
+	if (tb[IFLA_GRE_TTL])
+		ttl = rta_getattr_u8(tb[IFLA_GRE_TTL]);
+	if (is_json_context() || ttl)
+		print_uint(PRINT_ANY, "ttl", "hoplimit %u ", ttl);
+	else
+		print_string(PRINT_FP, NULL, "hoplimit %s ", "inherit");
 
 	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index b22e7bc..605bc61 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -336,6 +336,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 	char s2[64];
 	int flags = 0;
 	__u32 flowinfo = 0;
+	__u8 ttl = 0;
 
 	if (!tb)
 		return;
@@ -386,12 +387,12 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		}
 	}
 
-	if (tb[IFLA_IPTUN_TTL]) {
-		print_uint(PRINT_ANY,
-			   "ttl",
-			   "hoplimit %u ",
-			   rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
-	}
+	if (tb[IFLA_IPTUN_TTL])
+		ttl = rta_getattr_u8(tb[IFLA_IPTUN_TTL]);
+	if (is_json_context() || ttl)
+		print_uint(PRINT_ANY, "ttl", "hoplimit %u ", ttl);
+	else
+		print_string(PRINT_FP, NULL, "hoplimit %s ", "inherit");
 
 	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 35ef5c0..93e1a1b 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -365,6 +365,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 	const char *local = "any";
 	const char *remote = "any";
 	__u16 prefixlen, type;
+	__u8 ttl = 0;
 
 	if (!tb)
 		return;
@@ -416,16 +417,12 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		}
 	}
 
-	if (tb[IFLA_IPTUN_TTL]) {
-		__u8 ttl = rta_getattr_u8(tb[IFLA_IPTUN_TTL]);
-
-		if (ttl)
-			print_int(PRINT_ANY, "ttl", "ttl %d ", ttl);
-		else
-			print_int(PRINT_JSON, "ttl", NULL, ttl);
-	} else {
+	if (tb[IFLA_IPTUN_TTL])
+		ttl = rta_getattr_u8(tb[IFLA_IPTUN_TTL]);
+	if (is_json_context() || ttl)
+		print_uint(PRINT_ANY, "ttl", "ttl %u ", ttl);
+	else
 		print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
-	}
 
 	if (tb[IFLA_IPTUN_TOS]) {
 		int tos = rta_getattr_u8(tb[IFLA_IPTUN_TOS]);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 3/9] ip/tunnel: Simplify and unify tos printing
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 2/9] ip/tunnel: Correct and unify ttl/hoplimit printing Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 4/9] ip/tunnel: Use print_0xhex() instead of print_string() Serhey Popovych
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

For ip tunnels tos can be 0 when not configured, 1 when
inherited from encapsulated packet and rest specifying
diffserv (rfc2474) or tos (rfc1349) bits. It is stored
in packet tos/diffserv field and returned in tos
netlink attribute to userspace.

Simplify and unify tos printing by using print_0xhex()
and print_string() instead of fprintf() to output values.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/iplink_geneve.c |   23 ++++++++---------------
 ip/iplink_vxlan.c  |   19 ++++++++-----------
 ip/link_gre.c      |   23 ++++++++---------------
 ip/link_iptnl.c    |   22 ++++++++--------------
 4 files changed, 32 insertions(+), 55 deletions(-)

diff --git a/ip/iplink_geneve.c b/ip/iplink_geneve.c
index 5a0afd4..2d0a041 100644
--- a/ip/iplink_geneve.c
+++ b/ip/iplink_geneve.c
@@ -228,7 +228,7 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	__u32 vni;
 	__u8 ttl = 0;
-	__u8 tos;
+	__u8 tos = 0;
 
 	if (!tb)
 		return;
@@ -270,20 +270,13 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	else
 		print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
 
-	if (tb[IFLA_GENEVE_TOS] &&
-	    (tos = rta_getattr_u8(tb[IFLA_GENEVE_TOS]))) {
-		if (is_json_context()) {
-			print_0xhex(PRINT_JSON, "tos", "%#x", tos);
-		} else {
-			if (tos == 1) {
-				print_string(PRINT_FP,
-					     "tos",
-					     "tos %s ",
-					     "inherit");
-			} else {
-				fprintf(f, "tos %#x ", tos);
-			}
-		}
+	if (tb[IFLA_GENEVE_TOS])
+		tos = rta_getattr_u8(tb[IFLA_GENEVE_TOS]);
+	if (tos) {
+		if (is_json_context() || tos != 1)
+			print_0xhex(PRINT_ANY, "tos", "tos 0x%x ", tos);
+		else
+			print_string(PRINT_FP, NULL, "tos %s ", "inherit");
 	}
 
 	if (tb[IFLA_GENEVE_LABEL]) {
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index b9c335d..88b5662 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -395,7 +395,7 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	__u32 vni;
 	__u8 ttl = 0;
-	__u8 tos;
+	__u8 tos = 0;
 	__u32 maxaddr;
 
 	if (!tb)
@@ -515,16 +515,13 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	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 (is_json_context()) {
-			print_0xhex(PRINT_JSON, "tos", "%#x", tos);
-		} else {
-			if (tos == 1)
-				fprintf(f, "tos %s ", "inherit");
-			else
-				fprintf(f, "tos %#x ", tos);
-		}
+	if (tb[IFLA_VXLAN_TOS])
+		tos = rta_getattr_u8(tb[IFLA_VXLAN_TOS]);
+	if (tos) {
+		if (is_json_context() || tos != 1)
+			print_0xhex(PRINT_ANY, "tos", "tos 0x%x ", tos);
+		else
+			print_string(PRINT_FP, NULL, "tos %s ", "inherit");
 	}
 
 	if (tb[IFLA_VXLAN_TTL])
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 1cf7f4e..490d7db 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -362,6 +362,7 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 	__u8 ttl = 0;
+	__u8 tos = 0;
 
 	if (tb[IFLA_GRE_REMOTE]) {
 		unsigned int addr = rta_getattr_u32(tb[IFLA_GRE_REMOTE]);
@@ -397,21 +398,13 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 	else
 		print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
 
-	if (tb[IFLA_GRE_TOS] && rta_getattr_u8(tb[IFLA_GRE_TOS])) {
-		int tos = rta_getattr_u8(tb[IFLA_GRE_TOS]);
-
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%x", tos);
-			print_string(PRINT_JSON, "tos", NULL, b1);
-		} else {
-			fputs("tos ", f);
-			if (tos == 1)
-				fputs("inherit ", f);
-			else
-				fprintf(f, "0x%x ", tos);
-		}
+	if (tb[IFLA_GRE_TOS])
+		tos = rta_getattr_u8(tb[IFLA_GRE_TOS]);
+	if (tos) {
+		if (is_json_context() || tos != 1)
+			print_0xhex(PRINT_ANY, "tos", "tos 0x%x ", tos);
+		else
+			print_string(PRINT_FP, NULL, "tos %s ", "inherit");
 	}
 
 	if (tb[IFLA_GRE_PMTUDISC]) {
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 93e1a1b..7fda580 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -366,6 +366,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 	const char *remote = "any";
 	__u16 prefixlen, type;
 	__u8 ttl = 0;
+	__u8 tos = 0;
 
 	if (!tb)
 		return;
@@ -424,20 +425,13 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 	else
 		print_string(PRINT_FP, NULL, "ttl %s ", "inherit");
 
-	if (tb[IFLA_IPTUN_TOS]) {
-		int tos = rta_getattr_u8(tb[IFLA_IPTUN_TOS]);
-
-		if (tos) {
-			if (is_json_context()) {
-				print_0xhex(PRINT_JSON, "tos", "%#x", tos);
-			} else {
-				fputs("tos ", f);
-				if (tos == 1)
-					fputs("inherit ", f);
-				else
-					fprintf(f, "0x%x ", tos);
-			}
-		}
+	if (tb[IFLA_IPTUN_TOS])
+		tos = rta_getattr_u8(tb[IFLA_IPTUN_TOS]);
+	if (tos) {
+		if (is_json_context() || tos != 1)
+			print_0xhex(PRINT_ANY, "tos", "tos 0x%x ", tos);
+		else
+			print_string(PRINT_FP, NULL, "tos %s ", "inherit");
 	}
 
 	if (tb[IFLA_IPTUN_PMTUDISC] && rta_getattr_u8(tb[IFLA_IPTUN_PMTUDISC]))
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 4/9] ip/tunnel: Use print_0xhex() instead of print_string()
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (2 preceding siblings ...)
  2018-01-18 14:04 ` [PATCH iproute2 v2 3/9] ip/tunnel: Simplify and unify tos printing Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 5/9] ip/tunnel: Abstract tunnel encapsulation options printing Serhey Popovych
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

No need for custom SPRINT_BUF() and snprintf() 0x%x
value to this buffer: we can use print_0xhex() instead
of print_string().

In link_iptnl.c use s2 instead of s1 buffer and remove
s1.

While there adjust fwmark option print order in iptnl
and ip6tnl to get it match each other.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre.c    |    5 ++---
 ip/link_gre6.c   |    9 ++++-----
 ip/link_ip6tnl.c |   18 ++++++++----------
 ip/link_iptnl.c  |   22 ++++++++++------------
 ip/link_vti.c    |    6 ++----
 ip/link_vti6.c   |    5 ++---
 6 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index 490d7db..2462828 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -443,9 +443,8 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 		__u32 fwmark = rta_getattr_u32(tb[IFLA_GRE_FWMARK]);
 
 		if (fwmark) {
-			snprintf(s2, sizeof(s2), "0x%x", fwmark);
-
-			print_string(PRINT_ANY, "fwmark", "fwmark %s ", s2);
+			print_0xhex(PRINT_ANY,
+				    "fwmark", "fwmark 0x%x ", fwmark);
 		}
 	}
 }
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 37bc544..7dcd70d 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -497,18 +497,17 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (oflags & GRE_CSUM)
 		print_bool(PRINT_ANY, "ocsum", "ocsum ", true);
 
-	if (flags & IP6_TNL_F_USE_ORIG_FWMARK)
+	if (flags & IP6_TNL_F_USE_ORIG_FWMARK) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_fwmark",
 			   "fwmark inherit ",
 			   true);
-	else if (tb[IFLA_GRE_FWMARK]) {
+	} else if (tb[IFLA_GRE_FWMARK]) {
 		__u32 fwmark = rta_getattr_u32(tb[IFLA_GRE_FWMARK]);
 
 		if (fwmark) {
-			snprintf(s2, sizeof(s2), "0x%x", fwmark);
-
-			print_string(PRINT_ANY, "fwmark", "fwmark %s ", s2);
+			print_0xhex(PRINT_ANY,
+				    "fwmark", "fwmark 0x%x ", fwmark);
 		}
 	}
 
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 605bc61..00bf00a 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -438,6 +438,12 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 	if (flags & IP6_TNL_F_MIP6_DEV)
 		print_bool(PRINT_ANY, "ip6_tnl_f_mip6_dev", "mip6 ", true);
 
+	if (flags & IP6_TNL_F_ALLOW_LOCAL_REMOTE)
+		print_bool(PRINT_ANY,
+			   "ip6_tnl_f_allow_local_remote",
+			   "allow-localremote ",
+			   true);
+
 	if (flags & IP6_TNL_F_USE_ORIG_FWMARK) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_fwmark",
@@ -447,19 +453,11 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		__u32 fwmark = rta_getattr_u32(tb[IFLA_IPTUN_FWMARK]);
 
 		if (fwmark) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%x", fwmark);
-			print_string(PRINT_ANY, "fwmark", "fwmark %s ", b1);
+			print_0xhex(PRINT_ANY,
+				    "fwmark", "fwmark 0x%x ", fwmark);
 		}
 	}
 
-	if (flags & IP6_TNL_F_ALLOW_LOCAL_REMOTE)
-		print_bool(PRINT_ANY,
-			   "ip6_tnl_f_allow_local_remote",
-			   "allow-localremote ",
-			   true);
-
 	if (tb[IFLA_IPTUN_ENCAP_TYPE] &&
 	    rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_TYPE]) != TUNNEL_ENCAP_NONE) {
 		__u16 type = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_TYPE]);
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 7fda580..00a8366 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -360,7 +360,6 @@ get_failed:
 
 static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	char s1[1024];
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
@@ -455,7 +454,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 
 		const char *prefix = inet_ntop(AF_INET6,
 					       RTA_DATA(tb[IFLA_IPTUN_6RD_PREFIX]),
-					       s1, sizeof(s1));
+					       s2, sizeof(s2));
 
 		if (is_json_context()) {
 			print_string(PRINT_JSON, "prefix", NULL, prefix);
@@ -482,6 +481,15 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		}
 	}
 
+	if (tb[IFLA_IPTUN_FWMARK]) {
+		__u32 fwmark = rta_getattr_u32(tb[IFLA_IPTUN_FWMARK]);
+
+		if (fwmark) {
+			print_0xhex(PRINT_ANY,
+				    "fwmark", "fwmark 0x%x ", fwmark);
+		}
+	}
+
 	if (tb[IFLA_IPTUN_ENCAP_TYPE] &&
 	    (type = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_TYPE])) != TUNNEL_ENCAP_NONE) {
 		__u16 flags = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_FLAGS]);
@@ -545,16 +553,6 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 				fputs("noencap-remcsum ", f);
 		}
 	}
-
-	if (tb[IFLA_IPTUN_FWMARK]) {
-		__u32 fwmark = rta_getattr_u32(tb[IFLA_IPTUN_FWMARK]);
-
-		if (fwmark) {
-			snprintf(s2, sizeof(s2), "0x%x", fwmark);
-
-			print_string(PRINT_ANY, "fwmark", "fwmark %s ", s2);
-		}
-	}
 }
 
 static void iptunnel_print_help(struct link_util *lu, int argc, char **argv,
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 554cfcc..baaaf2f 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -213,10 +213,8 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		__u32 fwmark = rta_getattr_u32(tb[IFLA_VTI_FWMARK]);
 
 		if (fwmark) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%x", fwmark);
-			print_string(PRINT_ANY, "fwmark", "fwmark %s ", s2);
+			print_0xhex(PRINT_ANY,
+				    "fwmark", "fwmark 0x%x ", fwmark);
 		}
 	}
 }
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index a63f49f..952dbb0 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -212,9 +212,8 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		__u32 fwmark = rta_getattr_u32(tb[IFLA_VTI_FWMARK]);
 
 		if (fwmark) {
-			snprintf(s2, sizeof(s2), "0x%x", fwmark);
-
-			print_string(PRINT_ANY, "fwmark", "fwmark %s ", s2);
+			print_0xhex(PRINT_ANY,
+				    "fwmark", "fwmark 0x%x ", fwmark);
 		}
 	}
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 5/9] ip/tunnel: Abstract tunnel encapsulation options printing
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (3 preceding siblings ...)
  2018-01-18 14:04 ` [PATCH iproute2 v2 4/9] ip/tunnel: Use print_0xhex() instead of print_string() Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 6/9] gre/tunnel: Print erspan_index using print_uint() Serhey Popovych
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

Get rid of code duplications and consolidate encapsulation
options printing in single function - tnl_print_encap().

Introduce and use tnl_encap_str() to format encapsulation
option string according to tempate and given values to avoid
code duplication and simplify it.

Use print_string() instead of fputs() and fprintf() to
print encapsulation for !is_json_context().

Print "unknown" parameter for "encap" type in PRINT_FP
context using "%s " format specifier and benefit from
complite time string merge.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre.c    |   75 +++-----------------------------------------
 ip/link_gre6.c   |   64 +++-----------------------------------
 ip/link_ip6tnl.c |   63 +++----------------------------------
 ip/link_iptnl.c  |   70 ++++-------------------------------------
 ip/tunnel.c      |   91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ip/tunnel.h      |    5 +++
 6 files changed, 117 insertions(+), 251 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index 2462828..d403d24 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -468,76 +468,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		fprintf(f, "erspan_index %u ", erspan_idx);
 	}
 
-	if (tb[IFLA_GRE_ENCAP_TYPE] &&
-	    rta_getattr_u16(tb[IFLA_GRE_ENCAP_TYPE]) != TUNNEL_ENCAP_NONE) {
-		__u16 type = rta_getattr_u16(tb[IFLA_GRE_ENCAP_TYPE]);
-		__u16 flags = rta_getattr_u16(tb[IFLA_GRE_ENCAP_FLAGS]);
-		__u16 sport = rta_getattr_u16(tb[IFLA_GRE_ENCAP_SPORT]);
-		__u16 dport = rta_getattr_u16(tb[IFLA_GRE_ENCAP_DPORT]);
-
-
-		open_json_object("encap");
-		print_string(PRINT_FP, NULL, "encap ", NULL);
-
-		switch (type) {
-		case TUNNEL_ENCAP_FOU:
-			print_string(PRINT_ANY, "type", "%s ", "fou");
-			break;
-		case TUNNEL_ENCAP_GUE:
-			print_string(PRINT_ANY, "type", "%s ", "gue");
-			break;
-		default:
-			print_null(PRINT_ANY, "type", "%s ", "unknown");
-			break;
-		}
-
-		if (is_json_context()) {
-			print_uint(PRINT_JSON,
-				   "sport",
-				   NULL,
-				   sport ? ntohs(sport) : 0);
-			print_uint(PRINT_JSON, "dport", NULL, ntohs(dport));
-
-			print_bool(PRINT_JSON,
-				   "csum",
-				   NULL,
-				   flags & TUNNEL_ENCAP_FLAG_CSUM);
-
-			print_bool(PRINT_JSON,
-				   "csum6",
-				   NULL,
-				   flags & TUNNEL_ENCAP_FLAG_CSUM6);
-
-			print_bool(PRINT_JSON,
-				   "remcsum",
-				   NULL,
-				   flags & TUNNEL_ENCAP_FLAG_REMCSUM);
-
-			close_json_object();
-		} else {
-			if (sport == 0)
-				fputs("encap-sport auto ", f);
-			else
-				fprintf(f, "encap-sport %u", ntohs(sport));
-
-			fprintf(f, "encap-dport %u ", ntohs(dport));
-
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM)
-				fputs("encap-csum ", f);
-			else
-				fputs("noencap-csum ", f);
-
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM6)
-				fputs("encap-csum6 ", f);
-			else
-				fputs("noencap-csum6 ", f);
-
-			if (flags & TUNNEL_ENCAP_FLAG_REMCSUM)
-				fputs("encap-remcsum ", f);
-			else
-				fputs("noencap-remcsum ", f);
-		}
-	}
+	tnl_print_encap(tb,
+			IFLA_GRE_ENCAP_TYPE,
+			IFLA_GRE_ENCAP_FLAGS,
+			IFLA_GRE_ENCAP_SPORT,
+			IFLA_GRE_ENCAP_DPORT);
 }
 
 static void gre_print_help(struct link_util *lu, int argc, char **argv,
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7dcd70d..a159b54 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -516,65 +516,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		fprintf(f, "erspan_index %u ", erspan_idx);
 	}
 
-	if (tb[IFLA_GRE_ENCAP_TYPE] &&
-	    rta_getattr_u16(tb[IFLA_GRE_ENCAP_TYPE]) != TUNNEL_ENCAP_NONE) {
-		__u16 type = rta_getattr_u16(tb[IFLA_GRE_ENCAP_TYPE]);
-		__u16 flags = rta_getattr_u16(tb[IFLA_GRE_ENCAP_FLAGS]);
-		__u16 sport = rta_getattr_u16(tb[IFLA_GRE_ENCAP_SPORT]);
-		__u16 dport = rta_getattr_u16(tb[IFLA_GRE_ENCAP_DPORT]);
-
-		open_json_object("encap");
-
-		print_string(PRINT_FP, NULL, "encap ", NULL);
-		switch (type) {
-		case TUNNEL_ENCAP_FOU:
-			print_string(PRINT_ANY, "type", "%s ", "fou");
-			break;
-		case TUNNEL_ENCAP_GUE:
-			print_string(PRINT_ANY, "type", "%s ", "gue");
-			break;
-		default:
-			print_null(PRINT_ANY, "type", "unknown ", NULL);
-			break;
-		}
-
-		if (is_json_context()) {
-			print_uint(PRINT_JSON,
-				   "sport",
-				   NULL,
-				   sport ? ntohs(sport) : 0);
-			print_uint(PRINT_JSON, "dport", NULL, ntohs(dport));
-			print_bool(PRINT_JSON, "csum", NULL,
-					   flags & TUNNEL_ENCAP_FLAG_CSUM);
-			print_bool(PRINT_JSON, "csum6", NULL,
-					   flags & TUNNEL_ENCAP_FLAG_CSUM6);
-			print_bool(PRINT_JSON, "remcsum", NULL,
-					   flags & TUNNEL_ENCAP_FLAG_REMCSUM);
-			close_json_object();
-		} else {
-			if (sport == 0)
-				fputs("encap-sport auto ", f);
-			else
-				fprintf(f, "encap-sport %u", ntohs(sport));
-
-			fprintf(f, "encap-dport %u ", ntohs(dport));
-
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM)
-				fputs("encap-csum ", f);
-			else
-				fputs("noencap-csum ", f);
-
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM6)
-				fputs("encap-csum6 ", f);
-			else
-				fputs("noencap-csum6 ", f);
-
-			if (flags & TUNNEL_ENCAP_FLAG_REMCSUM)
-				fputs("encap-remcsum ", f);
-			else
-				fputs("noencap-remcsum ", f);
-		}
-	}
+	tnl_print_encap(tb,
+			IFLA_GRE_ENCAP_TYPE,
+			IFLA_GRE_ENCAP_FLAGS,
+			IFLA_GRE_ENCAP_SPORT,
+			IFLA_GRE_ENCAP_DPORT);
 }
 
 static void gre_print_help(struct link_util *lu, int argc, char **argv,
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 00bf00a..8f5c9bd 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -458,64 +458,11 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		}
 	}
 
-	if (tb[IFLA_IPTUN_ENCAP_TYPE] &&
-	    rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_TYPE]) != TUNNEL_ENCAP_NONE) {
-		__u16 type = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_TYPE]);
-		__u16 flags = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_FLAGS]);
-		__u16 sport = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_SPORT]);
-		__u16 dport = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_DPORT]);
-
-		open_json_object("encap");
-		print_string(PRINT_FP, NULL, "encap ", NULL);
-		switch (type) {
-		case TUNNEL_ENCAP_FOU:
-			print_string(PRINT_ANY, "type", "%s ", "fou");
-			break;
-		case TUNNEL_ENCAP_GUE:
-			print_string(PRINT_ANY, "type", "%s ", "gue");
-			break;
-		default:
-			print_null(PRINT_ANY, "type", "unknown ", NULL);
-			break;
-		}
-
-		if (is_json_context()) {
-			print_uint(PRINT_JSON,
-				   "sport",
-				   NULL,
-				   sport ? ntohs(sport) : 0);
-			print_uint(PRINT_JSON, "dport", NULL, ntohs(dport));
-			print_bool(PRINT_JSON, "csum", NULL,
-				   flags & TUNNEL_ENCAP_FLAG_CSUM);
-			print_bool(PRINT_JSON, "csum6", NULL,
-				   flags & TUNNEL_ENCAP_FLAG_CSUM6);
-			print_bool(PRINT_JSON, "remcsum", NULL,
-				   flags & TUNNEL_ENCAP_FLAG_REMCSUM);
-			close_json_object();
-		} else {
-			if (sport == 0)
-				fputs("encap-sport auto ", f);
-			else
-				fprintf(f, "encap-sport %u", ntohs(sport));
-
-			fprintf(f, "encap-dport %u ", ntohs(dport));
-
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM)
-				fputs("encap-csum ", f);
-			else
-				fputs("noencap-csum ", f);
-
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM6)
-				fputs("encap-csum6 ", f);
-			else
-				fputs("noencap-csum6 ", f);
-
-			if (flags & TUNNEL_ENCAP_FLAG_REMCSUM)
-				fputs("encap-remcsum ", f);
-			else
-				fputs("noencap-remcsum ", f);
-		}
-	}
+	tnl_print_encap(tb,
+			IFLA_IPTUN_ENCAP_TYPE,
+			IFLA_IPTUN_ENCAP_FLAGS,
+			IFLA_IPTUN_ENCAP_SPORT,
+			IFLA_IPTUN_ENCAP_DPORT);
 }
 
 static void ip6tunnel_print_help(struct link_util *lu, int argc, char **argv,
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 00a8366..ce3855c 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -363,7 +363,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
-	__u16 prefixlen, type;
+	__u16 prefixlen;
 	__u8 ttl = 0;
 	__u8 tos = 0;
 
@@ -490,69 +490,11 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		}
 	}
 
-	if (tb[IFLA_IPTUN_ENCAP_TYPE] &&
-	    (type = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_TYPE])) != TUNNEL_ENCAP_NONE) {
-		__u16 flags = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_FLAGS]);
-		__u16 sport = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_SPORT]);
-		__u16 dport = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_DPORT]);
-
-		open_json_object("encap");
-		print_string(PRINT_FP, NULL, "encap ", NULL);
-		switch (type) {
-		case TUNNEL_ENCAP_FOU:
-			print_string(PRINT_ANY, "type", "%s ", "fou");
-			break;
-		case TUNNEL_ENCAP_GUE:
-			print_string(PRINT_ANY, "type", "%s ", "gue");
-			break;
-		default:
-			print_null(PRINT_ANY, "type", "unknown ", NULL);
-			break;
-		}
-
-		if (is_json_context()) {
-			print_uint(PRINT_JSON,
-				   "sport",
-				   NULL,
-				   sport ? ntohs(sport) : 0);
-			print_uint(PRINT_JSON, "dport", NULL, ntohs(dport));
-			print_bool(PRINT_JSON,
-				   "csum",
-				   NULL,
-				   flags & TUNNEL_ENCAP_FLAG_CSUM);
-			print_bool(PRINT_JSON,
-				   "csum6",
-				   NULL,
-				   flags & TUNNEL_ENCAP_FLAG_CSUM6);
-			print_bool(PRINT_JSON,
-				   "remcsum",
-				   NULL,
-				   flags & TUNNEL_ENCAP_FLAG_REMCSUM);
-			close_json_object();
-		} else {
-			if (sport == 0)
-				fputs("encap-sport auto ", f);
-			else
-				fprintf(f, "encap-sport %u", ntohs(sport));
-
-			fprintf(f, "encap-dport %u ", ntohs(dport));
-
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM)
-				fputs("encap-csum ", f);
-			else
-				fputs("noencap-csum ", f);
-
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM6)
-				fputs("encap-csum6 ", f);
-			else
-				fputs("noencap-csum6 ", f);
-
-			if (flags & TUNNEL_ENCAP_FLAG_REMCSUM)
-				fputs("encap-remcsum ", f);
-			else
-				fputs("noencap-remcsum ", f);
-		}
-	}
+	tnl_print_encap(tb,
+			IFLA_IPTUN_ENCAP_TYPE,
+			IFLA_IPTUN_ENCAP_FLAGS,
+			IFLA_IPTUN_ENCAP_SPORT,
+			IFLA_IPTUN_ENCAP_DPORT);
 }
 
 static void iptunnel_print_help(struct link_util *lu, int argc, char **argv,
diff --git a/ip/tunnel.c b/ip/tunnel.c
index f860103..42ae70e 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -36,6 +36,7 @@
 
 #include "utils.h"
 #include "tunnel.h"
+#include "json_print.h"
 
 const char *tnl_strproto(__u8 proto)
 {
@@ -200,6 +201,96 @@ __be32 tnl_parse_key(const char *name, const char *key)
 	return htonl(uval);
 }
 
+static const char *tnl_encap_str(const char *name, int enabled, int port)
+{
+	static const char ne[][sizeof("no")] = {
+		[0] = "no",
+		[1] = "",
+	};
+	static char buf[32];
+	char b1[16];
+	const char *val;
+
+	if (!port) {
+		val = "auto";
+	} else if (port < 0) {
+		val = "";
+	} else {
+		snprintf(b1, sizeof(b1), "%u", port - 1);
+		val = b1;
+	}
+
+	snprintf(buf, sizeof(buf), "%sencap-%s %s", ne[!!enabled], name, val);
+	return buf;
+}
+
+void tnl_print_encap(struct rtattr *tb[],
+		     int encap_type, int encap_flags,
+		     int encap_sport, int encap_dport)
+{
+	__u16 type, flags, sport, dport;
+
+	if (!tb[encap_type])
+		return;
+
+	type = rta_getattr_u16(tb[encap_type]);
+	if (type == TUNNEL_ENCAP_NONE)
+		return;
+
+	flags = rta_getattr_u16(tb[encap_flags]);
+	sport = rta_getattr_u16(tb[encap_sport]);
+	dport = rta_getattr_u16(tb[encap_dport]);
+
+	open_json_object("encap");
+	print_string(PRINT_FP, NULL, "encap ", NULL);
+
+	switch (type) {
+	case TUNNEL_ENCAP_FOU:
+		print_string(PRINT_ANY, "type", "%s ", "fou");
+		break;
+	case TUNNEL_ENCAP_GUE:
+		print_string(PRINT_ANY, "type", "%s ", "gue");
+		break;
+	default:
+		print_null(PRINT_ANY, "type", "%s ", "unknown");
+		break;
+	}
+
+	if (is_json_context()) {
+		print_uint(PRINT_JSON, "sport", NULL, ntohs(sport));
+		print_uint(PRINT_JSON, "dport", NULL, ntohs(dport));
+		print_bool(PRINT_JSON, "csum", NULL,
+			   flags & TUNNEL_ENCAP_FLAG_CSUM);
+		print_bool(PRINT_JSON, "csum6", NULL,
+			   flags & TUNNEL_ENCAP_FLAG_CSUM6);
+		print_bool(PRINT_JSON, "remcsum", NULL,
+			   flags & TUNNEL_ENCAP_FLAG_REMCSUM);
+		close_json_object();
+	} else {
+		int t;
+
+		t = sport ? ntohs(sport) + 1 : 0;
+		print_string(PRINT_FP, NULL, "%s",
+			     tnl_encap_str("sport", 1, t));
+
+		t = ntohs(dport) + 1;
+		print_string(PRINT_FP, NULL, "%s",
+			     tnl_encap_str("dport", 1, t));
+
+		t = flags & TUNNEL_ENCAP_FLAG_CSUM;
+		print_string(PRINT_FP, NULL, "%s",
+			     tnl_encap_str("csum", t, -1));
+
+		t = flags & TUNNEL_ENCAP_FLAG_CSUM6;
+		print_string(PRINT_FP, NULL, "%s",
+			     tnl_encap_str("csum6", t, -1));
+
+		t = flags & TUNNEL_ENCAP_FLAG_REMCSUM;
+		print_string(PRINT_FP, NULL, "%s",
+			     tnl_encap_str("remcsum", t, -1));
+	}
+}
+
 /* tnl_print_stats - print tunnel statistics
  *
  * @buf - tunnel interface's line in /proc/net/dev,
diff --git a/ip/tunnel.h b/ip/tunnel.h
index 9a03c0d..a5c537c 100644
--- a/ip/tunnel.h
+++ b/ip/tunnel.h
@@ -23,6 +23,8 @@
 
 #include <linux/types.h>
 
+struct rtattr;
+
 const char *tnl_strproto(__u8 proto);
 
 int tnl_get_ioctl(const char *basedev, void *p);
@@ -32,6 +34,9 @@ int tnl_prl_ioctl(int cmd, const char *name, void *p);
 int tnl_6rd_ioctl(int cmd, const char *name, void *p);
 int tnl_ioctl_get_6rd(const char *name, void *p);
 __be32 tnl_parse_key(const char *name, const char *key);
+void tnl_print_encap(struct rtattr *tb[],
+		     int encap_type, int encap_flags,
+		     int encap_sport, int encap_dport);
 void tnl_print_stats(const char *buf);
 
 #endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 6/9] gre/tunnel: Print erspan_index using print_uint()
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (4 preceding siblings ...)
  2018-01-18 14:04 ` [PATCH iproute2 v2 5/9] ip/tunnel: Abstract tunnel encapsulation options printing Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 7/9] vti/tunnel: Unify ikey/okey printing Serhey Popovych
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

One is missing in JSON output because fprintf()
is used instead of print_uint().

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre.c  |    3 ++-
 ip/link_gre6.c |    4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index d403d24..6254e88 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -465,7 +465,8 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (tb[IFLA_GRE_ERSPAN_INDEX]) {
 		__u32 erspan_idx = rta_getattr_u32(tb[IFLA_GRE_ERSPAN_INDEX]);
 
-		fprintf(f, "erspan_index %u ", erspan_idx);
+		print_uint(PRINT_ANY,
+			   "erspan_index", "erspan_index %u ", erspan_idx);
 	}
 
 	tnl_print_encap(tb,
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index a159b54..29ca3d1 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -513,7 +513,9 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_GRE_ERSPAN_INDEX]) {
 		__u32 erspan_idx = rta_getattr_u32(tb[IFLA_GRE_ERSPAN_INDEX]);
-		fprintf(f, "erspan_index %u ", erspan_idx);
+
+		print_uint(PRINT_ANY,
+			   "erspan_index", "erspan_index %u ", erspan_idx);
 	}
 
 	tnl_print_encap(tb,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 7/9] vti/tunnel: Unify ikey/okey printing
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (5 preceding siblings ...)
  2018-01-18 14:04 ` [PATCH iproute2 v2 6/9] gre/tunnel: Print erspan_index using print_uint() Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 8/9] vti6/tunnel: Unify and simplify link type help functions Serhey Popovych
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

For vti6 tunnel we print [io]key in dotted-quad notation
(ipv4 address) while in vti we do that in hex format.

For vti tunnel we print [io]key only if value is not
zero while for vti6 we miss such check.

Unify vti and vti6 tunnel [io]key output.

While here enlarge s2 buffer to the same size as in rest
of tunnel support code (64 bytes) and check return from
inet_ntop().

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_vti.c  |   22 ++++++++++++++--------
 ip/link_vti6.c |   14 ++++++++++----
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/ip/link_vti.c b/ip/link_vti.c
index baaaf2f..1439e53 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -167,8 +167,7 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	const char *local = "any";
 	const char *remote = "any";
-	__u32 key;
-	char s2[IFNAMSIZ];
+	char s2[64];
 
 	if (!tb)
 		return;
@@ -200,14 +199,21 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		}
 	}
 
-	if (tb[IFLA_VTI_IKEY] &&
-	    (key = rta_getattr_u32(tb[IFLA_VTI_IKEY])))
-		print_0xhex(PRINT_ANY, "ikey", "ikey %#x ", ntohl(key));
+	if (tb[IFLA_VTI_IKEY]) {
+		struct rtattr *rta = tb[IFLA_VTI_IKEY];
+		__u32 key = rta_getattr_u32(rta);
 
+		if (key && inet_ntop(AF_INET, RTA_DATA(rta), s2, sizeof(s2)))
+			print_string(PRINT_ANY, "ikey", "ikey %s ", s2);
+	}
+
+	if (tb[IFLA_VTI_OKEY]) {
+		struct rtattr *rta = tb[IFLA_VTI_OKEY];
+		__u32 key = rta_getattr_u32(rta);
 
-	if (tb[IFLA_VTI_OKEY] &&
-	    (key = rta_getattr_u32(tb[IFLA_VTI_OKEY])))
-		print_0xhex(PRINT_ANY, "okey", "okey %#x ", ntohl(key));
+		if (key && inet_ntop(AF_INET, RTA_DATA(rta), s2, sizeof(s2)))
+			print_string(PRINT_ANY, "okey", "okey %s ", s2);
+	}
 
 	if (tb[IFLA_VTI_FWMARK]) {
 		__u32 fwmark = rta_getattr_u32(tb[IFLA_VTI_FWMARK]);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 952dbb0..6b61e3d 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -199,13 +199,19 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	}
 
 	if (tb[IFLA_VTI_IKEY]) {
-		inet_ntop(AF_INET, RTA_DATA(tb[IFLA_VTI_IKEY]), s2, sizeof(s2));
-		print_string(PRINT_ANY, "ikey", "ikey %s ", s2);
+		struct rtattr *rta = tb[IFLA_VTI_IKEY];
+		__u32 key = rta_getattr_u32(rta);
+
+		if (key && inet_ntop(AF_INET, RTA_DATA(rta), s2, sizeof(s2)))
+			print_string(PRINT_ANY, "ikey", "ikey %s ", s2);
 	}
 
 	if (tb[IFLA_VTI_OKEY]) {
-		inet_ntop(AF_INET, RTA_DATA(tb[IFLA_VTI_OKEY]), s2, sizeof(s2));
-		print_string(PRINT_ANY, "okey", "okey %s ", s2);
+		struct rtattr *rta = tb[IFLA_VTI_OKEY];
+		__u32 key = rta_getattr_u32(rta);
+
+		if (key && inet_ntop(AF_INET, RTA_DATA(rta), s2, sizeof(s2)))
+			print_string(PRINT_ANY, "okey", "okey %s ", s2);
 	}
 
 	if (tb[IFLA_VTI_FWMARK]) {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 8/9] vti6/tunnel: Unify and simplify link type help functions
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (6 preceding siblings ...)
  2018-01-18 14:04 ` [PATCH iproute2 v2 7/9] vti/tunnel: Unify ikey/okey printing Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-18 14:04 ` [PATCH iproute2 v2 9/9] tunnel: Return constant string without copying it Serhey Popovych
  2018-01-19  0:35 ` [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Stephen Hemminger
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

Both of these two changes are missing for link_vti6.c:

  commit 8b47135474cd ("ip: link: Unify link type help functions a bit")
  commit 561e650eff67 ("ip link: Shortify printing the usage of link type")

Replay them on link_vti6.c to bring link type help functions
inline with other tunneling code.

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

diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 6b61e3d..2a86d59 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -24,20 +24,25 @@
 #include "ip_common.h"
 #include "tunnel.h"
 
+static void print_usage(FILE *f)
+{
+	fprintf(f,
+		"Usage: ... vti6 [ remote ADDR ]\n"
+		"                [ local ADDR ]\n"
+		"                [ [i|o]key KEY ]\n"
+		"                [ dev PHYS_DEV ]\n"
+		"                [ fwmark MARK ]\n"
+		"\n"
+		"Where: ADDR := { IPV6_ADDRESS }\n"
+		"       KEY  := { DOTTED_QUAD | NUMBER }\n"
+		"       MARK := { 0x0..0xffffffff }\n"
+	);
+}
 
 static void usage(void) __attribute__((noreturn));
 static void usage(void)
 {
-	fprintf(stderr, "Usage: ip link { add | set | change | replace | del } NAME\n");
-	fprintf(stderr, "          type { vti6 } [ remote ADDR ] [ local ADDR ]\n");
-	fprintf(stderr, "          [ [i|o]key KEY ]\n");
-	fprintf(stderr, "          [ dev PHYS_DEV ]\n");
-	fprintf(stderr, "          [ fwmark MARK ]\n");
-	fprintf(stderr, "\n");
-	fprintf(stderr, "Where: NAME := STRING\n");
-	fprintf(stderr, "       ADDR := { IPV6_ADDRESS }\n");
-	fprintf(stderr, "       KEY  := { DOTTED_QUAD | NUMBER }\n");
-	fprintf(stderr, "       MARK := { 0x0..0xffffffff }\n");
+	print_usage(stderr);
 	exit(-1);
 }
 
@@ -224,9 +229,16 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	}
 }
 
+static void vti6_print_help(struct link_util *lu, int argc, char **argv,
+	FILE *f)
+{
+	print_usage(f);
+}
+
 struct link_util vti6_link_util = {
 	.id = "vti6",
 	.maxattr = IFLA_VTI_MAX,
 	.parse_opt = vti6_parse_opt,
 	.print_opt = vti6_print_opt,
+	.print_help = vti6_print_help,
 };
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH iproute2 v2 9/9] tunnel: Return constant string without copying it
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (7 preceding siblings ...)
  2018-01-18 14:04 ` [PATCH iproute2 v2 8/9] vti6/tunnel: Unify and simplify link type help functions Serhey Popovych
@ 2018-01-18 14:04 ` Serhey Popovych
  2018-01-19  0:35 ` [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Stephen Hemminger
  9 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-18 14:04 UTC (permalink / raw)
  To: netdev

We return constant string from tnl_strproto(), no need
to copy it to temporary buffer and then return such
buffer as const: return constant string instead.

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

diff --git a/ip/tunnel.c b/ip/tunnel.c
index 42ae70e..0414804 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -40,33 +40,22 @@
 
 const char *tnl_strproto(__u8 proto)
 {
-	static char buf[16];
-
 	switch (proto) {
 	case IPPROTO_IPIP:
-		strcpy(buf, "ip");
-		break;
+		return "ip";
 	case IPPROTO_GRE:
-		strcpy(buf, "gre");
-		break;
+		return "gre";
 	case IPPROTO_IPV6:
-		strcpy(buf, "ipv6");
-		break;
+		return "ipv6";
 	case IPPROTO_ESP:
-		strcpy(buf, "esp");
-		break;
+		return "esp";
 	case IPPROTO_MPLS:
-		strcpy(buf, "mpls");
-		break;
+		return "mpls";
 	case 0:
-		strcpy(buf, "any");
-		break;
+		return "any";
 	default:
-		strcpy(buf, "unknown");
-		break;
+		return "unknown";
 	}
-
-	return buf;
 }
 
 int tnl_get_ioctl(const char *basedev, void *p)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing
  2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (8 preceding siblings ...)
  2018-01-18 14:04 ` [PATCH iproute2 v2 9/9] tunnel: Return constant string without copying it Serhey Popovych
@ 2018-01-19  0:35 ` Stephen Hemminger
  9 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2018-01-19  0:35 UTC (permalink / raw)
  To: Serhey Popovych; +Cc: netdev

On Thu, 18 Jan 2018 16:04:27 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> Continue improvements to tunnel modules. Final goal
> is to make merge IP and IPv6 variants and make that
> transition as transparent as possible.
> 
> Everything within this series is open for your comments,
> suggestions and criticism.
> 
> See individual patch description message for details.
> 
> v2
>   1) Fix checkpatch issues: no assignment in condtional
>      anymore.
> 
>   2) Better code abstraction: introduce tnl_print_encap()
>      helper and get rid of duplicated code for printing
>      tunnel encapsulation options.
> 
>      Patch with subject
>      "ip/tunnel: Abstract tunnel encapsulation options printing"
>      replaces two previous in v1 series
>      "ip/tunnel: Use print_string() and simplify encap option printing"
>      and
>      "ip/tunnel: Minor cleanups in print routines"
> 
>   3) Include patch with subject
>      "tunnel: Return constant string without copying it"
>      in the series: it is related to tunneling code too.
> 
> Thanks,
> Serhii
> 
> Serhey Popovych (9):
>   iplink: Use ll_index_to_name() instead of if_indextoname()
>   ip/tunnel: Correct and unify ttl/hoplimit printing
>   ip/tunnel: Simplify and unify tos printing
>   ip/tunnel: Use print_0xhex() instead of print_string()
>   ip/tunnel: Abstract tunnel encapsulation options printing
>   gre/tunnel: Print erspan_index using print_uint()
>   vti/tunnel: Unify ikey/okey printing
>   vti6/tunnel: Unify and simplify link type help functions
>   tunnel: Return constant string without copying it
> 
>  bridge/fdb.c          |    5 +-
>  bridge/link.c         |   19 +++----
>  ip/ip6tunnel.c        |    5 +-
>  ip/iplink_bond.c      |   38 ++++++--------
>  ip/iplink_geneve.c    |   38 ++++++--------
>  ip/iplink_vxlan.c     |   49 ++++++++----------
>  ip/iproute_lwtunnel.c |   11 ++---
>  ip/iptunnel.c         |    2 +-
>  ip/link_gre.c         |  132 +++++++++++--------------------------------------
>  ip/link_gre6.c        |  103 +++++++++-----------------------------
>  ip/link_ip6tnl.c      |  105 ++++++++++-----------------------------
>  ip/link_iptnl.c       |  128 +++++++++++------------------------------------
>  ip/link_vti.c         |   42 ++++++++--------
>  ip/link_vti6.c        |   64 +++++++++++++++---------
>  ip/tunnel.c           |  116 ++++++++++++++++++++++++++++++++++++-------
>  ip/tunnel.h           |    5 ++
>  16 files changed, 340 insertions(+), 522 deletions(-)
> 

This looks fine. Thanks for following up.
Applied.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-01-19  0:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-18 14:04 [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 2/9] ip/tunnel: Correct and unify ttl/hoplimit printing Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 3/9] ip/tunnel: Simplify and unify tos printing Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 4/9] ip/tunnel: Use print_0xhex() instead of print_string() Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 5/9] ip/tunnel: Abstract tunnel encapsulation options printing Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 6/9] gre/tunnel: Print erspan_index using print_uint() Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 7/9] vti/tunnel: Unify ikey/okey printing Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 8/9] vti6/tunnel: Unify and simplify link type help functions Serhey Popovych
2018-01-18 14:04 ` [PATCH iproute2 v2 9/9] tunnel: Return constant string without copying it Serhey Popovych
2018-01-19  0:35 ` [PATCH iproute2 v2 0/9] ip/tunnel: Improve tunnel parameters printing Stephen Hemminger

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.