[PATCH iproute2 0/4] ip/tunnel: Minor cleanups and improvements

[PATCH iproute2 0/4] ip/tunnel: Minor cleanups and improvements

[Serhey Popovych]

It is small series of fixes and code cleanups:

  1) Add space after encap-[ds]port parameter value
     in tunnel encapsulation options output.

  2) Unify ttl/hoplimit parsing routines by using
     get_u8(), handling "inherit" and "hlim" as
     alias to "hoplimit" and "ttl" parameter name.

  3) Report when unable to resolve vti tunnel link
     instead of silently quitting and use __u32
     for fwmark istead of unsigned int to match
     with rest of tunneling code.

  4) Use ll_name_to_index() instead of if_nametoindex()
     to make ip(8) output and input parameter acceptance
     coherent.

See individual patch description message for details.

Thanks,
Serhii

Serhey Popovych (4):
  tunnel: Add space between encap-dport and encap-sport in non-JSON
    output
  iptnl/ip6tnl: Unify ttl/hoplimit parsing routines
  vti/vti6: Minor improvements
  iplink: Use ll_name_to_index() instead of if_nametoindex()

 bridge/fdb.c          |    4 ++--
 bridge/mdb.c          |    2 +-
 bridge/vlan.c         |    2 +-
 ip/iplink_bond.c      |    4 ++--
 ip/iplink_bridge.c    |    2 +-
 ip/iplink_vxlan.c     |    2 +-
 ip/iproute_lwtunnel.c |    4 ++--
 ip/link_gre.c         |   12 ++++--------
 ip/link_gre6.c        |   15 ++++++++-------
 ip/link_ip6tnl.c      |   16 ++++++++--------
 ip/link_iptnl.c       |    5 +++--
 ip/link_vti.c         |    4 ++--
 ip/link_vti6.c        |    8 +++++---
 ip/tunnel.c           |    4 ++--
 14 files changed, 42 insertions(+), 42 deletions(-)

-- 
1.7.10.4

[PATCH iproute2 1/4] tunnel: Add space between encap-dport and encap-sport in non-JSON output

[Serhey Popovych]

Fixes: bad76e6b1f44 ("ip/tunnel: Abstract tunnel encapsulation options printing")
Fixes: e2d4588331fc ("ip: link_gre.c: add json output support")
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/tunnel.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ip/tunnel.c b/ip/tunnel.c
index 0414804..46c9102 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -201,11 +201,11 @@ static const char *tnl_encap_str(const char *name, int enabled, int port)
 	const char *val;
 
 	if (!port) {
-		val = "auto";
+		val = "auto ";
 	} else if (port < 0) {
 		val = "";
 	} else {
-		snprintf(b1, sizeof(b1), "%u", port - 1);
+		snprintf(b1, sizeof(b1), "%u ", port - 1);
 		val = b1;
 	}
 
-- 
1.7.10.4

[PATCH iproute2 2/4] iptnl/ip6tnl: Unify ttl/hoplimit parsing routines

[Serhey Popovych]

Handle "inherit" case properly for gre6 and ip6tnl.

Use get_u8() in gre to parse ttl/hoplimit.

Be consistent about "hlim" alias to ttl/hoplimit
support.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre.c    |   10 +++-------
 ip/link_gre6.c   |   13 +++++++------
 ip/link_ip6tnl.c |   14 +++++++-------
 ip/link_iptnl.c  |    3 ++-
 4 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index 6254e88..009ddfc 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -229,16 +229,12 @@ get_failed:
 				exit(-1);
 			}
 		} else if (!matches(*argv, "ttl") ||
-			   !matches(*argv, "hoplimit")) {
-			unsigned int uval;
-
+			   !matches(*argv, "hoplimit") ||
+			   !matches(*argv, "hlim")) {
 			NEXT_ARG();
 			if (strcmp(*argv, "inherit") != 0) {
-				if (get_unsigned(&uval, *argv, 0))
+				if (get_u8(&ttl, *argv, 0))
 					invarg("invalid TTL\n", *argv);
-				if (uval > 255)
-					invarg("TTL must be <= 255\n", *argv);
-				ttl = uval;
 			} else
 				ttl = 0;
 		} else if (!matches(*argv, "tos") ||
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 29ca3d1..3d02e16 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -238,13 +238,14 @@ get_failed:
 				exit(-1);
 			}
 		} else if (!matches(*argv, "ttl") ||
-			   !matches(*argv, "hoplimit")) {
-			__u8 uval;
-
+			   !matches(*argv, "hoplimit") ||
+			   !matches(*argv, "hlim")) {
 			NEXT_ARG();
-			if (get_u8(&uval, *argv, 0))
-				invarg("invalid TTL", *argv);
-			hop_limit = uval;
+			if (strcmp(*argv, "inherit") != 0) {
+				if (get_u8(&hop_limit, *argv, 0))
+					invarg("invalid HLIM\n", *argv);
+			} else
+				hop_limit = 0;
 		} else if (!matches(*argv, "tos") ||
 			   !matches(*argv, "tclass") ||
 			   !matches(*argv, "dsfield")) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 8f5c9bd..f6c52ac 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -196,15 +196,15 @@ get_failed:
 			link = if_nametoindex(*argv);
 			if (link == 0)
 				invarg("\"dev\" is invalid", *argv);
-		} else if (strcmp(*argv, "hoplimit") == 0 ||
-			   strcmp(*argv, "ttl") == 0 ||
+		} else if (strcmp(*argv, "ttl") == 0 ||
+			   strcmp(*argv, "hoplimit") == 0 ||
 			   strcmp(*argv, "hlim") == 0) {
-			__u8 uval;
-
 			NEXT_ARG();
-			if (get_u8(&uval, *argv, 0))
-				invarg("invalid HLIM", *argv);
-			hop_limit = uval;
+			if (strcmp(*argv, "inherit") != 0) {
+				if (get_u8(&hop_limit, *argv, 0))
+					invarg("invalid HLIM\n", *argv);
+			} else
+				hop_limit = 0;
 		} else if (strcmp(*argv, "encaplimit") == 0) {
 			NEXT_ARG();
 			if (strcmp(*argv, "none") == 0) {
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index ce3855c..a6213d2 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -204,7 +204,8 @@ get_failed:
 			if (link == 0)
 				invarg("\"dev\" is invalid", *argv);
 		} else if (strcmp(*argv, "ttl") == 0 ||
-			   strcmp(*argv, "hoplimit") == 0) {
+			   strcmp(*argv, "hoplimit") == 0 ||
+			   strcmp(*argv, "hlim") == 0) {
 			NEXT_ARG();
 			if (strcmp(*argv, "inherit") != 0) {
 				if (get_u8(&ttl, *argv, 0))
-- 
1.7.10.4

[PATCH iproute2 3/4] vti/vti6: Minor improvements

[Serhey Popovych]

In prepare of link_vti.c and link_vti6.c merge:

  1) Make @fwmark of __u32 type instead of unsigned int
     in vti to match with rest tunneling code.

  2) Report when unable to translate @link network device
     name to index instead of silently exiting in vti6.

  3) Remove newline separating local/remote attributes
     from the ikey/okey in vti6 to match vti module.

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

diff --git a/ip/link_vti.c b/ip/link_vti.c
index 1439e53..6a088c6 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -69,7 +69,7 @@ static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
 	unsigned int saddr = 0;
 	unsigned int daddr = 0;
 	unsigned int link = 0;
-	unsigned int fwmark = 0;
+	__u32 fwmark = 0;
 	int len;
 
 	if (!(n->nlmsg_flags & NLM_F_CREATE)) {
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 2a86d59..a11f389 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -144,8 +144,11 @@ get_failed:
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = if_nametoindex(*argv);
-			if (link == 0)
+			if (link == 0) {
+				fprintf(stderr, "Cannot find device \"%s\"\n",
+					*argv);
 				exit(-1);
+			}
 		} else if (strcmp(*argv, "fwmark") == 0) {
 			NEXT_ARG();
 			if (get_u32(&fwmark, *argv, 0))
@@ -157,7 +160,6 @@ get_failed:
 
 	addattr32(n, 1024, IFLA_VTI_IKEY, ikey);
 	addattr32(n, 1024, IFLA_VTI_OKEY, okey);
-
 	addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
 	addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
 	addattr32(n, 1024, IFLA_VTI_FWMARK, fwmark);
-- 
1.7.10.4

[PATCH iproute2 4/4] iplink: Use ll_name_to_index() instead of if_nametoindex()

[Serhey Popovych]

While benefit from using ll_name_to_index() with populated
cache can potentially be exploited only in few places
(e.g. bridge fdb/mdb/vlan show routines) there is another
advantage of ll_name_to_index() over plain if_nametoindex():

  in case of if_nametoindex() failure ll_name_to_index()
  will attempt to get index from common name in form "if%d"
  that may be returned from ll_index_to_name().

This makes output from ip(8) coherent with it's input.

Note that most of the code already switched from plain
if_nametoindex() to ll_name_to_index() to cached variant.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 bridge/fdb.c          |    4 ++--
 bridge/mdb.c          |    2 +-
 bridge/vlan.c         |    2 +-
 ip/iplink_bond.c      |    4 ++--
 ip/iplink_bridge.c    |    2 +-
 ip/iplink_vxlan.c     |    2 +-
 ip/iproute_lwtunnel.c |    4 ++--
 ip/link_gre.c         |    2 +-
 ip/link_gre6.c        |    2 +-
 ip/link_ip6tnl.c      |    2 +-
 ip/link_iptnl.c       |    2 +-
 ip/link_vti.c         |    2 +-
 ip/link_vti6.c        |    2 +-
 13 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index 4d55fb0..8b133f9 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -374,7 +374,7 @@ static int fdb_show(int argc, char **argv)
 
 	/*we'll keep around filter_dev for older kernels */
 	if (filter_dev) {
-		filter_index = if_nametoindex(filter_dev);
+		filter_index = ll_name_to_index(filter_dev);
 		if (filter_index == 0) {
 			fprintf(stderr, "Cannot find device \"%s\"\n",
 				filter_dev);
@@ -463,7 +463,7 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
 				invarg("invalid VNI\n", *argv);
 		} else if (strcmp(*argv, "via") == 0) {
 			NEXT_ARG();
-			via = if_nametoindex(*argv);
+			via = ll_name_to_index(*argv);
 			if (via == 0)
 				invarg("invalid device\n", *argv);
 		} else if (strcmp(*argv, "self") == 0) {
diff --git a/bridge/mdb.c b/bridge/mdb.c
index 7b7b81f..62dc8a0 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -311,7 +311,7 @@ static int mdb_show(int argc, char **argv)
 	}
 
 	if (filter_dev) {
-		filter_index = if_nametoindex(filter_dev);
+		filter_index = ll_name_to_index(filter_dev);
 		if (filter_index == 0) {
 			fprintf(stderr, "Cannot find device \"%s\"\n",
 				filter_dev);
diff --git a/bridge/vlan.c b/bridge/vlan.c
index 69fb542..f42d7e6 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -567,7 +567,7 @@ static int vlan_show(int argc, char **argv)
 	}
 
 	if (filter_dev) {
-		filter_index = if_nametoindex(filter_dev);
+		filter_index = ll_name_to_index(filter_dev);
 		if (filter_index == 0) {
 			fprintf(stderr, "Cannot find device \"%s\"\n",
 				filter_dev);
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index f01fd8d..8e8723a 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -177,7 +177,7 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 			addattr8(n, 1024, IFLA_BOND_MODE, mode);
 		} else if (matches(*argv, "active_slave") == 0) {
 			NEXT_ARG();
-			ifindex = if_nametoindex(*argv);
+			ifindex = ll_name_to_index(*argv);
 			if (!ifindex)
 				return -1;
 			addattr32(n, 1024, IFLA_BOND_ACTIVE_SLAVE, ifindex);
@@ -240,7 +240,7 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 			addattr32(n, 1024, IFLA_BOND_ARP_ALL_TARGETS, arp_all_targets);
 		} else if (matches(*argv, "primary") == 0) {
 			NEXT_ARG();
-			ifindex = if_nametoindex(*argv);
+			ifindex = ll_name_to_index(*argv);
 			if (!ifindex)
 				return -1;
 			addattr32(n, 1024, IFLA_BOND_PRIMARY, ifindex);
diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index d325098..06ec092 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -792,7 +792,7 @@ int bridge_parse_xstats(struct link_util *lu, int argc, char **argv)
 			xstats_print_attr = BRIDGE_XSTATS_MCAST;
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
-			filter_index = if_nametoindex(*argv);
+			filter_index = ll_name_to_index(*argv);
 			if (filter_index == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n",
 					*argv);
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 88b5662..7ea908e 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -151,7 +151,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 
 			NEXT_ARG();
 			check_duparg(&attrs, IFLA_VXLAN_LINK, "dev", *argv);
-			link = if_nametoindex(*argv);
+			link = ll_name_to_index(*argv);
 			if (link == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n",
 					*argv);
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index a1d36ba..da6ebb8 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -556,7 +556,7 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp,
 			NEXT_ARG();
 			if (iif_ok++)
 				duparg2("iif", *argv);
-			iif = if_nametoindex(*argv);
+			iif = ll_name_to_index(*argv);
 			if (!iif)
 				invarg("\"iif\" interface not found\n", *argv);
 			rta_addattr32(rta, len, SEG6_LOCAL_IIF, iif);
@@ -564,7 +564,7 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp,
 			NEXT_ARG();
 			if (oif_ok++)
 				duparg2("oif", *argv);
-			oif = if_nametoindex(*argv);
+			oif = ll_name_to_index(*argv);
 			if (!oif)
 				invarg("\"oif\" interface not found\n", *argv);
 			rta_addattr32(rta, len, SEG6_LOCAL_OIF, oif);
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 009ddfc..512695d 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -222,7 +222,7 @@ get_failed:
 			saddr = get_addr32(*argv);
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
-			link = if_nametoindex(*argv);
+			link = ll_name_to_index(*argv);
 			if (link == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n",
 					*argv);
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 3d02e16..29846cf 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -231,7 +231,7 @@ get_failed:
 			memcpy(&laddr, &addr.data, sizeof(laddr));
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
-			link = if_nametoindex(*argv);
+			link = ll_name_to_index(*argv);
 			if (link == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n",
 					*argv);
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index f6c52ac..fdf113e 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -193,7 +193,7 @@ get_failed:
 			memcpy(&laddr, addr.data, sizeof(laddr));
 		} else if (matches(*argv, "dev") == 0) {
 			NEXT_ARG();
-			link = if_nametoindex(*argv);
+			link = ll_name_to_index(*argv);
 			if (link == 0)
 				invarg("\"dev\" is invalid", *argv);
 		} else if (strcmp(*argv, "ttl") == 0 ||
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index a6213d2..96048e2 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -200,7 +200,7 @@ get_failed:
 			laddr = get_addr32(*argv);
 		} else if (matches(*argv, "dev") == 0) {
 			NEXT_ARG();
-			link = if_nametoindex(*argv);
+			link = ll_name_to_index(*argv);
 			if (link == 0)
 				invarg("\"dev\" is invalid", *argv);
 		} else if (strcmp(*argv, "ttl") == 0 ||
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 6a088c6..f1a1123 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -137,7 +137,7 @@ get_failed:
 			saddr = get_addr32(*argv);
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
-			link = if_nametoindex(*argv);
+			link = ll_name_to_index(*argv);
 			if (link == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n",
 					*argv);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index a11f389..c394dbc 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -143,7 +143,7 @@ get_failed:
 			memcpy(&saddr, addr.data, sizeof(saddr));
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
-			link = if_nametoindex(*argv);
+			link = ll_name_to_index(*argv);
 			if (link == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n",
 					*argv);
-- 
1.7.10.4

Re: [PATCH iproute2 0/4] ip/tunnel: Minor cleanups and improvements

[Stephen Hemminger]

On Fri, 19 Jan 2018 18:43:59 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> It is small series of fixes and code cleanups:
> 
>   1) Add space after encap-[ds]port parameter value
>      in tunnel encapsulation options output.
> 
>   2) Unify ttl/hoplimit parsing routines by using
>      get_u8(), handling "inherit" and "hlim" as
>      alias to "hoplimit" and "ttl" parameter name.
> 
>   3) Report when unable to resolve vti tunnel link
>      instead of silently quitting and use __u32
>      for fwmark istead of unsigned int to match
>      with rest of tunneling code.
> 
>   4) Use ll_name_to_index() instead of if_nametoindex()
>      to make ip(8) output and input parameter acceptance
>      coherent.
> 
> See individual patch description message for details.
> 
> Thanks,
> Serhii
> 
> Serhey Popovych (4):
>   tunnel: Add space between encap-dport and encap-sport in non-JSON
>     output
>   iptnl/ip6tnl: Unify ttl/hoplimit parsing routines
>   vti/vti6: Minor improvements
>   iplink: Use ll_name_to_index() instead of if_nametoindex()
> 
>  bridge/fdb.c          |    4 ++--
>  bridge/mdb.c          |    2 +-
>  bridge/vlan.c         |    2 +-
>  ip/iplink_bond.c      |    4 ++--
>  ip/iplink_bridge.c    |    2 +-
>  ip/iplink_vxlan.c     |    2 +-
>  ip/iproute_lwtunnel.c |    4 ++--
>  ip/link_gre.c         |   12 ++++--------
>  ip/link_gre6.c        |   15 ++++++++-------
>  ip/link_ip6tnl.c      |   16 ++++++++--------
>  ip/link_iptnl.c       |    5 +++--
>  ip/link_vti.c         |    4 ++--
>  ip/link_vti6.c        |    8 +++++---
>  ip/tunnel.c           |    4 ++--
>  14 files changed, 42 insertions(+), 42 deletions(-)

Looks good, applied.