All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing
@ 2018-01-12 17:39 Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 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.

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: Use print_string() and simplify encap option printing
  gre/tunnel: Print erspan_index using print_uint()
  ip/tunnel: Minor cleanups in print routines
  vti/tunnel: Unify ikey/okey printing
  vti6/tunnel: Unify and simplify link type help functions

 bridge/fdb.c          |    4 +-
 bridge/link.c         |   18 +++-----
 ip/ip6tunnel.c        |    5 +-
 ip/iplink_bond.c      |   25 ++--------
 ip/iplink_geneve.c    |   38 +++++++--------
 ip/iplink_vxlan.c     |   42 +++++++----------
 ip/iproute_lwtunnel.c |   11 ++---
 ip/iptunnel.c         |    2 +-
 ip/link_gre.c         |  117 +++++++++++++++++++---------------------------
 ip/link_gre6.c        |   94 ++++++++++++++++++-------------------
 ip/link_ip6tnl.c      |   89 +++++++++++++++++------------------
 ip/link_iptnl.c       |  123 ++++++++++++++++++++-----------------------------
 ip/link_vti.c         |   36 ++++++---------
 ip/link_vti6.c        |   60 +++++++++++++-----------
 ip/tunnel.c           |   24 ++++++++++
 ip/tunnel.h           |    1 +
 16 files changed, 313 insertions(+), 376 deletions(-)

-- 
1.7.10.4

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

* [PATCH iproute2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname()
  2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
@ 2018-01-12 17:39 ` Serhey Popovych
  2018-01-17 18:53   ` Stephen Hemminger
  2018-01-12 17:39 ` [PATCH iproute2 2/9] ip/tunnel: Correct and unify ttl/hoplimit printing Serhey Popovych
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 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          |    4 ++--
 bridge/link.c         |   18 ++++++------------
 ip/iplink_bond.c      |   25 ++++---------------------
 ip/iplink_vxlan.c     |    8 ++------
 ip/iproute_lwtunnel.c |    7 ++-----
 ip/link_gre.c         |   12 +++++-------
 ip/link_gre6.c        |   12 +++++-------
 ip/link_ip6tnl.c      |   12 +++++-------
 ip/link_iptnl.c       |   12 +++++-------
 ip/link_vti.c         |    7 ++-----
 ip/link_vti6.c        |   10 ++++------
 11 files changed, 42 insertions(+), 85 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index 376713b..2cc0268 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -219,10 +219,10 @@ 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];
+			const char *ifname;
 
 			if (!tb[NDA_LINK_NETNSID] &&
-			    if_indextoname(ifindex, ifname)) {
+			    (ifname = ll_index_to_name(ifindex))) {
 				if (jw_global)
 					jsonw_string_field(jw_global, "viaIf",
 							   ifname);
diff --git a/bridge/link.c b/bridge/link.c
index e2371d0..9c846c9 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,9 @@ 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 +143,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..45900c8 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -382,19 +382,9 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	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);
+		const char *n = ll_index_to_name(ifindex);
 
-		if (n)
-			print_string(PRINT_ANY,
-				     "active_slave",
-				     "active_slave %s ",
-				     n);
-		else
-			print_uint(PRINT_ANY,
-				   "active_slave_index",
-				   "active_slave %u ",
-				   ifindex);
+		print_string(PRINT_ANY, "active_slave", "active_slave %s ", n);
 	}
 
 	if (tb[IFLA_BOND_MIIMON])
@@ -481,16 +471,9 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_BOND_PRIMARY] &&
 	    (ifindex = rta_getattr_u32(tb[IFLA_BOND_PRIMARY]))) {
-		char buf[IFNAMSIZ];
-		const char *n = if_indextoname(ifindex, buf);
+		const char *n = ll_index_to_name(ifindex);
 
-		if (n)
-			print_string(PRINT_ANY, "primary", "primary %s ", n);
-		else
-			print_uint(PRINT_ANY,
-				   "primary_index",
-				   "primary %u ",
-				   ifindex);
+		print_string(PRINT_ANY, "primary", "primary %s ", n);
 	}
 
 	if (tb[IFLA_BOND_PRIMARY_RESELECT]) {
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 661eaa7..a6c964a 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -397,7 +397,6 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	unsigned int link;
 	__u8 tos;
 	__u32 maxaddr;
-	char s2[64];
 
 	if (!tb)
 		return;
@@ -469,12 +468,9 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_VXLAN_LINK] &&
 	    (link = rta_getattr_u32(tb[IFLA_VXLAN_LINK]))) {
-		const char *n = if_indextoname(link, s2);
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	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..d876bad 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -359,6 +359,7 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
+	unsigned int link;
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 
@@ -380,14 +381,11 @@ 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])) {
-		unsigned int link = rta_getattr_u32(tb[IFLA_GRE_LINK]);
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_GRE_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_GRE_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_GRE_TTL]) {
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 55bd1fb..93f3a96 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -377,6 +377,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
+	unsigned int link;
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 	unsigned int flags = 0;
@@ -414,14 +415,11 @@ 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])) {
-		unsigned int link = rta_getattr_u32(tb[IFLA_GRE_LINK]);
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_GRE_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_GRE_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_GRE_TTL]) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index bbc7878..0cfb2c6 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -334,6 +334,7 @@ get_failed:
 static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	char s2[64];
+	unsigned int link;
 	int flags = 0;
 	__u32 flowinfo = 0;
 
@@ -377,14 +378,11 @@ 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])) {
-		unsigned int link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]);
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_IPTUN_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_IPTUN_TTL]) {
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 24a0f0c..3204e5e 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -364,6 +364,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";
+	unsigned int link;
 	__u16 prefixlen, type;
 
 	if (!tb)
@@ -407,14 +408,11 @@ 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])) {
-		unsigned int link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]);
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_IPTUN_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_int(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_IPTUN_TTL]) {
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 2b0fab2..7ae2e3c 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -194,12 +194,9 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_VTI_LINK] &&
 	    (link = rta_getattr_u32(tb[IFLA_VTI_LINK]))) {
-		const char *n = if_indextoname(link, s2);
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_VTI_IKEY] &&
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 74c246d..f7c40d3 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -190,13 +190,11 @@ 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] &&
+	    (link = rta_getattr_u32(tb[IFLA_VTI_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_VTI_IKEY]) {
-- 
1.7.10.4

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

* [PATCH iproute2 2/9] ip/tunnel: Correct and unify ttl/hoplimit printing
  2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
@ 2018-01-12 17:39 ` Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 3/9] ip/tunnel: Simplify and unify tos printing Serhey Popovych
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 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 a6c964a..bac326d 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -395,6 +395,7 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	__u32 vni;
 	unsigned int link;
+	__u8 ttl = 0;
 	__u8 tos;
 	__u32 maxaddr;
 
@@ -525,14 +526,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 d876bad..2f6be20 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 link;
 	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]);
@@ -388,16 +389,12 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
-	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 93f3a96..9b08656 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -383,6 +383,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;
@@ -422,14 +423,12 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
-	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 0cfb2c6..bd06e46 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -337,6 +337,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 	unsigned int link;
 	int flags = 0;
 	__u32 flowinfo = 0;
+	__u8 ttl = 0;
 
 	if (!tb)
 		return;
@@ -385,12 +386,12 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
-	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 3204e5e..e19cb21 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";
 	unsigned int link;
 	__u16 prefixlen, type;
+	__u8 ttl = 0;
 
 	if (!tb)
 		return;
@@ -415,16 +416,12 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
-	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 3/9] ip/tunnel: Simplify and unify tos printing
  2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 2/9] ip/tunnel: Correct and unify ttl/hoplimit printing Serhey Popovych
@ 2018-01-12 17:39 ` Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 4/9] ip/tunnel: Use print_0xhex() instead of print_string() Serhey Popovych
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 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 bac326d..e292369 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -396,7 +396,7 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	__u32 vni;
 	unsigned int link;
 	__u8 ttl = 0;
-	__u8 tos;
+	__u8 tos = 0;
 	__u32 maxaddr;
 
 	if (!tb)
@@ -514,16 +514,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 2f6be20..11a131f 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -363,6 +363,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]);
@@ -396,21 +397,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 e19cb21..fd08bd9 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -367,6 +367,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 	unsigned int link;
 	__u16 prefixlen, type;
 	__u8 ttl = 0;
+	__u8 tos = 0;
 
 	if (!tb)
 		return;
@@ -423,20 +424,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 4/9] ip/tunnel: Use print_0xhex() instead of print_string()
  2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (2 preceding siblings ...)
  2018-01-12 17:39 ` [PATCH iproute2 3/9] ip/tunnel: Simplify and unify tos printing Serhey Popovych
@ 2018-01-12 17:39 ` Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 5/9] ip/tunnel: Use print_string() and simplify encap option printing Serhey Popovych
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 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    |   14 ++++----------
 ip/link_vti6.c   |   13 ++++---------
 6 files changed, 32 insertions(+), 49 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index 11a131f..2ae2194 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -442,9 +442,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 9b08656..9576354 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -496,18 +496,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 bd06e46..9594c3e 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -437,6 +437,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",
@@ -446,19 +452,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 fd08bd9..fcb0795 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";
@@ -454,7 +453,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);
@@ -481,6 +480,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]);
@@ -544,16 +552,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 7ae2e3c..eebf542 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -168,6 +168,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;
+	__u32 fwmark;
 	unsigned int link;
 	char s2[IFNAMSIZ];
 
@@ -208,16 +209,9 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	    (key = rta_getattr_u32(tb[IFLA_VTI_OKEY])))
 		print_0xhex(PRINT_ANY, "okey", "okey %#x ", ntohl(key));
 
-	if (tb[IFLA_VTI_FWMARK]) {
-		__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);
-		}
-	}
+	if (tb[IFLA_VTI_FWMARK] &&
+	    (fwmark = rta_getattr_u32(tb[IFLA_VTI_FWMARK])))
+		print_0xhex(PRINT_ANY, "fwmark", "fwmark 0x%x ", fwmark);
 }
 
 static void vti_print_help(struct link_util *lu, int argc, char **argv,
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index f7c40d3..29a7062 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -168,6 +168,7 @@ 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;
+	__u32 fwmark;
 	unsigned int link;
 	char s2[64];
 
@@ -207,15 +208,9 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_string(PRINT_ANY, "okey", "okey %s ", s2);
 	}
 
-	if (tb[IFLA_VTI_FWMARK]) {
-		__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);
-		}
-	}
+	if (tb[IFLA_VTI_FWMARK] &&
+	    (fwmark = rta_getattr_u32(tb[IFLA_VTI_FWMARK])))
+		print_0xhex(PRINT_ANY, "fwmark", "fwmark 0x%x ", fwmark);
 }
 
 struct link_util vti6_link_util = {
-- 
1.7.10.4

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

* [PATCH iproute2 5/9] ip/tunnel: Use print_string() and simplify encap option printing
  2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (3 preceding siblings ...)
  2018-01-12 17:39 ` [PATCH iproute2 4/9] ip/tunnel: Use print_0xhex() instead of print_string() Serhey Popovych
@ 2018-01-12 17:39 ` Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 6/9] gre/tunnel: Print erspan_index using print_uint() Serhey Popovych
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 UTC (permalink / raw)
  To: netdev

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

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

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre.c    |   54 +++++++++++++++++++++---------------------------------
 ip/link_gre6.c   |   46 ++++++++++++++++++++++------------------------
 ip/link_ip6tnl.c |   40 +++++++++++++++++++---------------------
 ip/link_iptnl.c  |   52 ++++++++++++++++++++++------------------------------
 ip/tunnel.c      |   24 ++++++++++++++++++++++++
 ip/tunnel.h      |    1 +
 6 files changed, 109 insertions(+), 108 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index 2ae2194..b70f73b 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -491,50 +491,38 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		}
 
 		if (is_json_context()) {
-			print_uint(PRINT_JSON,
-				   "sport",
-				   NULL,
-				   sport ? ntohs(sport) : 0);
+			print_uint(PRINT_JSON, "sport", NULL, ntohs(sport));
 			print_uint(PRINT_JSON, "dport", NULL, ntohs(dport));
-
-			print_bool(PRINT_JSON,
-				   "csum",
-				   NULL,
+			print_bool(PRINT_JSON, "csum", NULL,
 				   flags & TUNNEL_ENCAP_FLAG_CSUM);
-
-			print_bool(PRINT_JSON,
-				   "csum6",
-				   NULL,
+			print_bool(PRINT_JSON, "csum6", NULL,
 				   flags & TUNNEL_ENCAP_FLAG_CSUM6);
-
-			print_bool(PRINT_JSON,
-				   "remcsum",
-				   NULL,
+			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));
+			int t;
 
-			fprintf(f, "encap-dport %u ", ntohs(dport));
+			t = sport ? ntohs(sport) + 1 : 0;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("sport", 1, t));
 
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM)
-				fputs("encap-csum ", f);
-			else
-				fputs("noencap-csum ", f);
+			t = ntohs(dport) + 1;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("dport", 1, t));
 
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM6)
-				fputs("encap-csum6 ", f);
-			else
-				fputs("noencap-csum6 ", f);
+			t = flags & TUNNEL_ENCAP_FLAG_CSUM;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("csum", t, -1));
 
-			if (flags & TUNNEL_ENCAP_FLAG_REMCSUM)
-				fputs("encap-remcsum ", f);
-			else
-				fputs("noencap-remcsum ", f);
+			t = flags & TUNNEL_ENCAP_FLAG_CSUM6;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("csum6", t, -1));
+
+			t = flags & TUNNEL_ENCAP_FLAG_REMCSUM;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("remcsum", t, -1));
 		}
 	}
 }
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 9576354..41180bb 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -538,40 +538,38 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		}
 
 		if (is_json_context()) {
-			print_uint(PRINT_JSON,
-				   "sport",
-				   NULL,
-				   sport ? ntohs(sport) : 0);
+			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);
+				   flags & TUNNEL_ENCAP_FLAG_CSUM);
 			print_bool(PRINT_JSON, "csum6", NULL,
-					   flags & TUNNEL_ENCAP_FLAG_CSUM6);
+				   flags & TUNNEL_ENCAP_FLAG_CSUM6);
 			print_bool(PRINT_JSON, "remcsum", NULL,
-					   flags & TUNNEL_ENCAP_FLAG_REMCSUM);
+				   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));
+			int t;
 
-			fprintf(f, "encap-dport %u ", ntohs(dport));
+			t = sport ? ntohs(sport) + 1 : 0;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("sport", 1, t));
 
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM)
-				fputs("encap-csum ", f);
-			else
-				fputs("noencap-csum ", f);
+			t = ntohs(dport) + 1;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("dport", 1, t));
 
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM6)
-				fputs("encap-csum6 ", f);
-			else
-				fputs("noencap-csum6 ", f);
+			t = flags & TUNNEL_ENCAP_FLAG_CSUM;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("csum", t, -1));
 
-			if (flags & TUNNEL_ENCAP_FLAG_REMCSUM)
-				fputs("encap-remcsum ", f);
-			else
-				fputs("noencap-remcsum ", f);
+			t = flags & TUNNEL_ENCAP_FLAG_CSUM6;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("csum6", t, -1));
+
+			t = flags & TUNNEL_ENCAP_FLAG_REMCSUM;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("remcsum", t, -1));
 		}
 	}
 }
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 9594c3e..aa6f6fa 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -479,10 +479,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		}
 
 		if (is_json_context()) {
-			print_uint(PRINT_JSON,
-				   "sport",
-				   NULL,
-				   sport ? ntohs(sport) : 0);
+			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);
@@ -490,29 +487,30 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 				   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));
+			int t;
 
-			fprintf(f, "encap-dport %u ", ntohs(dport));
+			t = sport ? ntohs(sport) + 1 : 0;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("sport", 1, t));
 
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM)
-				fputs("encap-csum ", f);
-			else
-				fputs("noencap-csum ", f);
+			t = ntohs(dport) + 1;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("dport", 1, t));
 
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM6)
-				fputs("encap-csum6 ", f);
-			else
-				fputs("noencap-csum6 ", f);
+			t = flags & TUNNEL_ENCAP_FLAG_CSUM;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("csum", t, -1));
 
-			if (flags & TUNNEL_ENCAP_FLAG_REMCSUM)
-				fputs("encap-remcsum ", f);
-			else
-				fputs("noencap-remcsum ", f);
+			t = flags & TUNNEL_ENCAP_FLAG_CSUM6;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("csum6", t, -1));
+
+			t = flags & TUNNEL_ENCAP_FLAG_REMCSUM;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("remcsum", t, -1));
 		}
 	}
 }
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index fcb0795..83a524f 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -510,46 +510,38 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		}
 
 		if (is_json_context()) {
-			print_uint(PRINT_JSON,
-				   "sport",
-				   NULL,
-				   sport ? ntohs(sport) : 0);
+			print_uint(PRINT_JSON, "sport", NULL, ntohs(sport));
 			print_uint(PRINT_JSON, "dport", NULL, ntohs(dport));
-			print_bool(PRINT_JSON,
-				   "csum",
-				   NULL,
+			print_bool(PRINT_JSON, "csum", NULL,
 				   flags & TUNNEL_ENCAP_FLAG_CSUM);
-			print_bool(PRINT_JSON,
-				   "csum6",
-				   NULL,
+			print_bool(PRINT_JSON, "csum6", NULL,
 				   flags & TUNNEL_ENCAP_FLAG_CSUM6);
-			print_bool(PRINT_JSON,
-				   "remcsum",
-				   NULL,
+			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));
+			int t;
 
-			fprintf(f, "encap-dport %u ", ntohs(dport));
+			t = sport ? ntohs(sport) + 1 : 0;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("sport", 1, t));
 
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM)
-				fputs("encap-csum ", f);
-			else
-				fputs("noencap-csum ", f);
+			t = ntohs(dport) + 1;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("dport", 1, t));
 
-			if (flags & TUNNEL_ENCAP_FLAG_CSUM6)
-				fputs("encap-csum6 ", f);
-			else
-				fputs("noencap-csum6 ", f);
+			t = flags & TUNNEL_ENCAP_FLAG_CSUM;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("csum", t, -1));
 
-			if (flags & TUNNEL_ENCAP_FLAG_REMCSUM)
-				fputs("encap-remcsum ", f);
-			else
-				fputs("noencap-remcsum ", f);
+			t = flags & TUNNEL_ENCAP_FLAG_CSUM6;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("csum6", t, -1));
+
+			t = flags & TUNNEL_ENCAP_FLAG_REMCSUM;
+			print_string(PRINT_FP, NULL, "%s",
+				     tnl_encap_optstr("remcsum", t, -1));
 		}
 	}
 }
diff --git a/ip/tunnel.c b/ip/tunnel.c
index f860103..1305e12 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -200,6 +200,30 @@ __be32 tnl_parse_key(const char *name, const char *key)
 	return htonl(uval);
 }
 
+const char *tnl_encap_optstr(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;
+}
+
+
 /* 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..7af3592 100644
--- a/ip/tunnel.h
+++ b/ip/tunnel.h
@@ -32,6 +32,7 @@ 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);
+const char *tnl_encap_optstr(const char *name, int enabled, int port);
 void tnl_print_stats(const char *buf);
 
 #endif
-- 
1.7.10.4

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

* [PATCH iproute2 6/9] gre/tunnel: Print erspan_index using print_uint()
  2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (4 preceding siblings ...)
  2018-01-12 17:39 ` [PATCH iproute2 5/9] ip/tunnel: Use print_string() and simplify encap option printing Serhey Popovych
@ 2018-01-12 17:39 ` Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 7/9] ip/tunnel: Minor cleanups in print routines Serhey Popovych
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 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 b70f73b..a7d1cd1 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -464,7 +464,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);
 	}
 
 	if (tb[IFLA_GRE_ENCAP_TYPE] &&
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 41180bb..200846e 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -512,7 +512,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);
 	}
 
 	if (tb[IFLA_GRE_ENCAP_TYPE] &&
-- 
1.7.10.4

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

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

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

Unify encapsulation type check.

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

diff --git a/ip/link_gre.c b/ip/link_gre.c
index a7d1cd1..b4cde62 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -450,6 +450,8 @@ static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 
 static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
+	__u16 type;
+
 	if (!tb)
 		return;
 
@@ -469,8 +471,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	}
 
 	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]);
+	    (type = rta_getattr_u16(tb[IFLA_GRE_ENCAP_TYPE])) != TUNNEL_ENCAP_NONE) {
 		__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]);
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 200846e..557151f 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -381,6 +381,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;
+	__u16 type;
 	__u32 flowinfo = 0;
 	struct in6_addr in6_addr_any = IN6ADDR_ANY_INIT;
 	__u8 ttl = 0;
@@ -518,15 +519,14 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	}
 
 	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]);
+	    (type = rta_getattr_u16(tb[IFLA_GRE_ENCAP_TYPE])) != TUNNEL_ENCAP_NONE) {
 		__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");
@@ -535,7 +535,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			print_string(PRINT_ANY, "type", "%s ", "gue");
 			break;
 		default:
-			print_null(PRINT_ANY, "type", "unknown ", NULL);
+			print_null(PRINT_ANY, "type", "%s ", "unknown");
 			break;
 		}
 
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index aa6f6fa..51c73dc 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];
 	unsigned int link;
 	int flags = 0;
+	__u16 type;
 	__u32 flowinfo = 0;
 	__u8 ttl = 0;
 
@@ -458,8 +459,7 @@ 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]);
+	    (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]);
@@ -474,7 +474,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			print_string(PRINT_ANY, "type", "%s ", "gue");
 			break;
 		default:
-			print_null(PRINT_ANY, "type", "unknown ", NULL);
+			print_null(PRINT_ANY, "type", "%s ", "unknown");
 			break;
 		}
 
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 83a524f..17d28ec 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -505,7 +505,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 			print_string(PRINT_ANY, "type", "%s ", "gue");
 			break;
 		default:
-			print_null(PRINT_ANY, "type", "unknown ", NULL);
+			print_null(PRINT_ANY, "type", "%s ", "unknown");
 			break;
 		}
 
-- 
1.7.10.4

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

* [PATCH iproute2 8/9] vti/tunnel: Unify ikey/okey printing
  2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (6 preceding siblings ...)
  2018-01-12 17:39 ` [PATCH iproute2 7/9] ip/tunnel: Minor cleanups in print routines Serhey Popovych
@ 2018-01-12 17:39 ` Serhey Popovych
  2018-01-12 17:39 ` [PATCH iproute2 9/9] vti6/tunnel: Unify and simplify link type help functions Serhey Popovych
  8 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 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).

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

diff --git a/ip/link_vti.c b/ip/link_vti.c
index eebf542..5fccf74 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -170,7 +170,7 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	__u32 key;
 	__u32 fwmark;
 	unsigned int link;
-	char s2[IFNAMSIZ];
+	char s2[64];
 
 	if (!tb)
 		return;
@@ -201,13 +201,16 @@ 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));
-
+	    (key = rta_getattr_u32(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);
+	}
 
 	if (tb[IFLA_VTI_OKEY] &&
-	    (key = rta_getattr_u32(tb[IFLA_VTI_OKEY])))
-		print_0xhex(PRINT_ANY, "okey", "okey %#x ", ntohl(key));
+	    (key = rta_getattr_u32(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);
+	}
 
 	if (tb[IFLA_VTI_FWMARK] &&
 	    (fwmark = rta_getattr_u32(tb[IFLA_VTI_FWMARK])))
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 29a7062..a4ad650 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -168,6 +168,7 @@ 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;
+	__u32 key;
 	__u32 fwmark;
 	unsigned int link;
 	char s2[64];
@@ -198,12 +199,14 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
-	if (tb[IFLA_VTI_IKEY]) {
+	if (tb[IFLA_VTI_IKEY] &&
+	    (key = rta_getattr_u32(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);
 	}
 
-	if (tb[IFLA_VTI_OKEY]) {
+	if (tb[IFLA_VTI_OKEY] &&
+	    (key = rta_getattr_u32(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);
 	}
-- 
1.7.10.4

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

* [PATCH iproute2 9/9] vti6/tunnel: Unify and simplify link type help functions
  2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
                   ` (7 preceding siblings ...)
  2018-01-12 17:39 ` [PATCH iproute2 8/9] vti/tunnel: Unify ikey/okey printing Serhey Popovych
@ 2018-01-12 17:39 ` Serhey Popovych
  8 siblings, 0 replies; 11+ messages in thread
From: Serhey Popovych @ 2018-01-12 17:39 UTC (permalink / raw)
  To: netdev

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

  o commit 8b47135474cd (ip: link: Unify link type help functions a bit)
  o 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 a4ad650..b9fe83a 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);
 }
 
@@ -216,9 +221,16 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_0xhex(PRINT_ANY, "fwmark", "fwmark 0x%x ", fwmark);
 }
 
+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

* Re: [PATCH iproute2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname()
  2018-01-12 17:39 ` [PATCH iproute2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
@ 2018-01-17 18:53   ` Stephen Hemminger
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2018-01-17 18:53 UTC (permalink / raw)
  To: Serhey Popovych; +Cc: netdev

On Fri, 12 Jan 2018 19:39:26 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

This looks fine, but minor nuisances from checkpatch

> diff --git a/bridge/fdb.c b/bridge/fdb.c
> index 376713b..2cc0268 100644
> --- a/bridge/fdb.c
> +++ b/bridge/fdb.c
> @@ -219,10 +219,10 @@ 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];
> +			const char *ifname;
>  
>  			if (!tb[NDA_LINK_NETNSID] &&
> -			    if_indextoname(ifindex, ifname)) {
> +			    (ifname = ll_index_to_name(ifindex))) {
>  				if (jw_global)

Please rearrange to avoid assignment in conditional.
			ifname = ll_index_to_name(ifindex);
			if (ifname && !tb[NDA_LINK_NETNSID]) {


> @@ -135,14 +132,9 @@ 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");

Break long line here.


ERROR: do not use assignment in if condition
#265: FILE: ip/link_gre6.c:418:
+	if (tb[IFLA_GRE_LINK] &&

ERROR: do not use assignment in if condition
#296: FILE: ip/link_ip6tnl.c:381:
+	if (tb[IFLA_IPTUN_LINK] &&

ERROR: do not use assignment in if condition
#327: FILE: ip/link_iptnl.c:411:
+	if (tb[IFLA_IPTUN_LINK] &&

ERROR: do not use assignment in if condition
#368: FILE: ip/link_vti6.c:193:
+	if (tb[IFLA_VTI_LINK] &&

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

end of thread, other threads:[~2018-01-17 18:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-12 17:39 [PATCH iproute2 0/9] ip/tunnel: Improve tunnel parameters printing Serhey Popovych
2018-01-12 17:39 ` [PATCH iproute2 1/9] iplink: Use ll_index_to_name() instead of if_indextoname() Serhey Popovych
2018-01-17 18:53   ` Stephen Hemminger
2018-01-12 17:39 ` [PATCH iproute2 2/9] ip/tunnel: Correct and unify ttl/hoplimit printing Serhey Popovych
2018-01-12 17:39 ` [PATCH iproute2 3/9] ip/tunnel: Simplify and unify tos printing Serhey Popovych
2018-01-12 17:39 ` [PATCH iproute2 4/9] ip/tunnel: Use print_0xhex() instead of print_string() Serhey Popovych
2018-01-12 17:39 ` [PATCH iproute2 5/9] ip/tunnel: Use print_string() and simplify encap option printing Serhey Popovych
2018-01-12 17:39 ` [PATCH iproute2 6/9] gre/tunnel: Print erspan_index using print_uint() Serhey Popovych
2018-01-12 17:39 ` [PATCH iproute2 7/9] ip/tunnel: Minor cleanups in print routines Serhey Popovych
2018-01-12 17:39 ` [PATCH iproute2 8/9] vti/tunnel: Unify ikey/okey printing Serhey Popovych
2018-01-12 17:39 ` [PATCH iproute2 9/9] vti6/tunnel: Unify and simplify link type help functions Serhey Popovych

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.