[PATCH iproute2-next v4 0/4] iplink: Improve iplink_parse()

[PATCH iproute2-next v4 0/4] iplink: Improve iplink_parse()

[Serhey Popovych]

This is main routine to parse ip-link(8) configuration parameters.

Move all code related to command line parsing and validation to it from
iptables_modify(). As benefit we reduce number of arguments as well as
checking for most of weired cases in single place to give benefit to
iptables_parse() users.

See individual patch description message for more information.

v4
  Drop patches intended to reduce number of arguments to
  iptables_parse(): postpone to the series with real use cases.

  Save only ifi_index in iplink_vxcan.c and link_veth.c: no need
  to save whole ifinfomsg data structure.

  Note that there is no sense to introduce custom version of
  iplink_parse() to use in iplink_vxcan.c and link_veth.c because
  there is too much parameters we need to support (except VF and
  few others) making huge code duplication.

v3
  Move vxlan/veth ifinfomsg save/restore to separate patch to
  make clear change that perform most of request buffer setups
  and checks in iplink_parse().

  Update commit message descriptions and extra new line from
  "utils: Introduce and use nodev() helper routine" patch.

v2
  Terminate via exit() when failing to parse command line arguments
  to help identify failing line in batch mode.

Thanks,
Serhii

Serhey Popovych (4):
  utils: Introduce and use nodev() helper routine
  iplink: Use "dev" and "name" parameters interchangeable when possible
  iplink: Follow documented behaviour when "index" is given
  iplink: Perform most of request buffer setups and checks in
    iplink_parse()

 bridge/fdb.c          |   17 ++----
 bridge/link.c         |    8 +--
 bridge/mdb.c          |   19 ++----
 bridge/vlan.c         |    7 +--
 include/utils.h       |    1 +
 ip/ip6tunnel.c        |    6 +-
 ip/ip_common.h        |    4 +-
 ip/ipaddress.c        |    7 +--
 ip/iplink.c           |  163 +++++++++++++++++++++++++++----------------------
 ip/iplink_bond.c      |    4 +-
 ip/iplink_bridge.c    |    7 +--
 ip/iplink_vxcan.c     |   23 ++-----
 ip/iplink_vxlan.c     |    7 +--
 ip/ipmroute.c         |    7 +--
 ip/ipneigh.c          |   14 ++---
 ip/ipntable.c         |    6 +-
 ip/iproute.c          |   36 ++++-------
 ip/iproute_lwtunnel.c |    4 +-
 ip/iptunnel.c         |    6 +-
 ip/link_gre.c         |    7 +--
 ip/link_gre6.c        |    7 +--
 ip/link_ip6tnl.c      |    4 +-
 ip/link_iptnl.c       |    4 +-
 ip/link_veth.c        |   23 ++-----
 ip/link_vti.c         |    7 +--
 ip/link_vti6.c        |    7 +--
 lib/utils.c           |    6 ++
 tc/m_mirred.c         |    6 +-
 tc/tc_class.c         |   14 ++---
 tc/tc_filter.c        |   18 ++----
 tc/tc_qdisc.c         |   12 ++--
 31 files changed, 196 insertions(+), 265 deletions(-)

-- 
1.7.10.4

[PATCH iproute2-next v4 1/4] utils: Introduce and use nodev() helper routine

[Serhey Popovych]

There is a couple of places where we report error in case of no network
device is found. In all of them we output message in the same format to
stderr and either return -1 or 1 to the caller or exit with -1.

Introduce new helper function nodev() that takes name of the network
device caused error and returns -1 to it's caller. Either call exit()
or return to the caller to preserve behaviour before change.

Use -nodev() in traffic control (tc) code to return 1.

Simplify expression for checking for argument being 0/NULL in @if
statement.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 bridge/fdb.c          |   17 ++++++-----------
 bridge/link.c         |    8 +++-----
 bridge/mdb.c          |   19 ++++++-------------
 bridge/vlan.c         |    7 ++-----
 include/utils.h       |    1 +
 ip/ip6tunnel.c        |    6 ++----
 ip/ipaddress.c        |    7 +++----
 ip/iplink.c           |   13 ++++---------
 ip/iplink_bond.c      |    4 ++--
 ip/iplink_bridge.c    |    7 ++-----
 ip/iplink_vxlan.c     |    7 ++-----
 ip/ipmroute.c         |    7 +++----
 ip/ipneigh.c          |   14 +++++++-------
 ip/ipntable.c         |    6 ++----
 ip/iproute.c          |   36 ++++++++++++------------------------
 ip/iproute_lwtunnel.c |    4 ++--
 ip/iptunnel.c         |    6 ++----
 ip/link_gre.c         |    7 ++-----
 ip/link_gre6.c        |    7 ++-----
 ip/link_ip6tnl.c      |    4 ++--
 ip/link_iptnl.c       |    4 ++--
 ip/link_vti.c         |    7 ++-----
 ip/link_vti6.c        |    7 ++-----
 lib/utils.c           |    6 ++++++
 tc/m_mirred.c         |    6 ++----
 tc/tc_class.c         |   14 ++++++--------
 tc/tc_filter.c        |   18 ++++++------------
 tc/tc_qdisc.c         |   12 ++++--------
 28 files changed, 97 insertions(+), 164 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index b4f6e8b..205b4fa 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -311,11 +311,8 @@ static int fdb_show(int argc, char **argv)
 	/*we'll keep around filter_dev for older kernels */
 	if (filter_dev) {
 		filter_index = ll_name_to_index(filter_dev);
-		if (filter_index == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n",
-				filter_dev);
-			return -1;
-		}
+		if (!filter_index)
+			return nodev(filter_dev);
 		req.ifm.ifi_index = filter_index;
 	}
 
@@ -391,8 +388,8 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
 		} else if (strcmp(*argv, "via") == 0) {
 			NEXT_ARG();
 			via = ll_name_to_index(*argv);
-			if (via == 0)
-				invarg("invalid device\n", *argv);
+			if (!via)
+				exit(nodev(*argv));
 		} else if (strcmp(*argv, "self") == 0) {
 			req.ndm.ndm_flags |= NTF_SELF;
 		} else if (matches(*argv, "master") == 0) {
@@ -467,10 +464,8 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
 		addattr32(&req.n, sizeof(req), NDA_IFINDEX, via);
 
 	req.ndm.ndm_ifindex = ll_name_to_index(d);
-	if (req.ndm.ndm_ifindex == 0) {
-		fprintf(stderr, "Cannot find device \"%s\"\n", d);
-		return -1;
-	}
+	if (!req.ndm.ndm_ifindex)
+		return nodev(d);
 
 	if (rtnl_talk(&rth, &req.n, NULL) < 0)
 		return -1;
diff --git a/bridge/link.c b/bridge/link.c
index 69c08ec..579d57e 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -485,11 +485,9 @@ static int brlink_show(int argc, char **argv)
 	}
 
 	if (filter_dev) {
-		if ((filter_index = ll_name_to_index(filter_dev)) == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n",
-				filter_dev);
-			return -1;
-		}
+		filter_index = ll_name_to_index(filter_dev);
+		if (!filter_index)
+			return nodev(filter_dev);
 	}
 
 	if (show_details) {
diff --git a/bridge/mdb.c b/bridge/mdb.c
index 8c08baf..f38dc67 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -287,11 +287,8 @@ static int mdb_show(int argc, char **argv)
 
 	if (filter_dev) {
 		filter_index = ll_name_to_index(filter_dev);
-		if (filter_index == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n",
-				filter_dev);
-			return -1;
-		}
+		if (!filter_index)
+			return nodev(filter_dev);
 	}
 
 	new_json_obj(json);
@@ -360,16 +357,12 @@ static int mdb_modify(int cmd, int flags, int argc, char **argv)
 	}
 
 	req.bpm.ifindex = ll_name_to_index(d);
-	if (req.bpm.ifindex == 0) {
-		fprintf(stderr, "Cannot find device \"%s\"\n", d);
-		return -1;
-	}
+	if (!req.bpm.ifindex)
+		return nodev(d);
 
 	entry.ifindex = ll_name_to_index(p);
-	if (entry.ifindex == 0) {
-		fprintf(stderr, "Cannot find device \"%s\"\n", p);
-		return -1;
-	}
+	if (!entry.ifindex)
+		return nodev(p);
 
 	if (!inet_pton(AF_INET, grp, &entry.addr.u.ip4)) {
 		if (!inet_pton(AF_INET6, grp, &entry.addr.u.ip6)) {
diff --git a/bridge/vlan.c b/bridge/vlan.c
index 9f4a7a2..19a36b8 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -554,11 +554,8 @@ static int vlan_show(int argc, char **argv)
 
 	if (filter_dev) {
 		filter_index = ll_name_to_index(filter_dev);
-		if (filter_index == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n",
-				filter_dev);
-			return -1;
-		}
+		if (!filter_index)
+			return nodev(filter_dev);
 	}
 
 	new_json_obj(json);
diff --git a/include/utils.h b/include/utils.h
index 6bc77e7..e4389ee 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -182,6 +182,7 @@ void missarg(const char *) __attribute__((noreturn));
 void invarg(const char *, const char *) __attribute__((noreturn));
 void duparg(const char *, const char *) __attribute__((noreturn));
 void duparg2(const char *, const char *) __attribute__((noreturn));
+int nodev(const char *dev);
 int check_ifname(const char *);
 int get_ifname(char *, const char *);
 const char *get_ifname_rta(int ifindex, const struct rtattr *rta);
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index c7fa082..999408e 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -296,10 +296,8 @@ static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p)
 	}
 	if (medium) {
 		p->link = ll_name_to_index(medium);
-		if (p->link == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", medium);
-			return -1;
-		}
+		if (!p->link)
+			return nodev(medium);
 	}
 	return 0;
 }
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index d01d703..087fa68 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -2211,10 +2211,9 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 	if (!scoped && cmd != RTM_DELADDR)
 		req.ifa.ifa_scope = default_scope(&lcl);
 
-	if ((req.ifa.ifa_index = ll_name_to_index(d)) == 0) {
-		fprintf(stderr, "Cannot find device \"%s\"\n", d);
-		return -1;
-	}
+	req.ifa.ifa_index = ll_name_to_index(d);
+	if (!req.ifa.ifa_index)
+		return nodev(d);
 
 	if (valid_lftp || preferred_lftp) {
 		struct ifa_cacheinfo cinfo = {};
diff --git a/ip/iplink.c b/ip/iplink.c
index 74c377c..5471626 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -981,10 +981,8 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 		}
 
 		req.i.ifi_index = ll_name_to_index(dev);
-		if (req.i.ifi_index == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", dev);
-			return -1;
-		}
+		if (!req.i.ifi_index)
+			return nodev(dev);
 	} else {
 		/* Allow "ip link add dev" and "ip link add name" */
 		if (!name)
@@ -994,11 +992,8 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 			int ifindex;
 
 			ifindex = ll_name_to_index(link);
-			if (ifindex == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n",
-					link);
-				return -1;
-			}
+			if (!ifindex)
+				return nodev(link);
 			addattr_l(&req.n, sizeof(req), IFLA_LINK, &ifindex, 4);
 		}
 
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index 8e8723a..f906e7f 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -179,7 +179,7 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			ifindex = ll_name_to_index(*argv);
 			if (!ifindex)
-				return -1;
+				return nodev(*argv);
 			addattr32(n, 1024, IFLA_BOND_ACTIVE_SLAVE, ifindex);
 		} else if (matches(*argv, "clear_active_slave") == 0) {
 			addattr32(n, 1024, IFLA_BOND_ACTIVE_SLAVE, 0);
@@ -242,7 +242,7 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			ifindex = ll_name_to_index(*argv);
 			if (!ifindex)
-				return -1;
+				return nodev(*argv);
 			addattr32(n, 1024, IFLA_BOND_PRIMARY, ifindex);
 		} else if (matches(*argv, "primary_reselect") == 0) {
 			NEXT_ARG();
diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index 06ec092..3008e44 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -793,11 +793,8 @@ int bridge_parse_xstats(struct link_util *lu, int argc, char **argv)
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
 			filter_index = ll_name_to_index(*argv);
-			if (filter_index == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n",
-					*argv);
-				return -1;
-			}
+			if (!filter_index)
+				return nodev(*argv);
 		} else if (strcmp(*argv, "help") == 0) {
 			bridge_print_xstats_help(lu, stdout);
 			exit(0);
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index d768c07..be9f35e 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -133,11 +133,8 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
 			NEXT_ARG();
 			check_duparg(&attrs, IFLA_VXLAN_LINK, "dev", *argv);
 			link = ll_name_to_index(*argv);
-			if (link == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n",
-					*argv);
-				exit(-1);
-			}
+			if (!link)
+				exit(nodev(*argv));
 			addattr32(n, 1024, IFLA_VXLAN_LINK, link);
 		} else if (!matches(*argv, "ttl") ||
 			   !matches(*argv, "hoplimit")) {
diff --git a/ip/ipmroute.c b/ip/ipmroute.c
index aa5029b..5c232e8 100644
--- a/ip/ipmroute.c
+++ b/ip/ipmroute.c
@@ -244,10 +244,9 @@ static int mroute_list(int argc, char **argv)
 	if (id)  {
 		int idx;
 
-		if ((idx = ll_name_to_index(id)) == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", id);
-			return -1;
-		}
+		idx = ll_name_to_index(id);
+		if (!idx)
+			return nodev(id);
 		filter.iif = idx;
 	}
 
diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index 925494d..4748381 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -179,9 +179,10 @@ static int ipneigh_modify(int cmd, int flags, int argc, char **argv)
 
 	ll_init_map(&rth);
 
-	if (dev && (req.ndm.ndm_ifindex = ll_name_to_index(dev)) == 0) {
-		fprintf(stderr, "Cannot find device \"%s\"\n", dev);
-		return -1;
+	if (dev) {
+		req.ndm.ndm_ifindex = ll_name_to_index(dev);
+		if (!req.ndm.ndm_ifindex)
+			return nodev(dev);
 	}
 
 	if (rtnl_talk(&rth, &req.n, NULL) < 0)
@@ -467,10 +468,9 @@ static int do_show_or_flush(int argc, char **argv, int flush)
 	ll_init_map(&rth);
 
 	if (filter_dev) {
-		if ((filter.index = ll_name_to_index(filter_dev)) == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", filter_dev);
-			return -1;
-		}
+		filter.index = ll_name_to_index(filter_dev);
+		if (!filter.index)
+			return nodev(filter_dev);
 		addattr32(&req.n, sizeof(req), NDA_IFINDEX, filter.index);
 	}
 
diff --git a/ip/ipntable.c b/ip/ipntable.c
index 9202486..82f40f8 100644
--- a/ip/ipntable.c
+++ b/ip/ipntable.c
@@ -140,10 +140,8 @@ static int ipntable_modify(int cmd, int flags, int argc, char **argv)
 
 			NEXT_ARG();
 			ifindex = ll_name_to_index(*argv);
-			if (ifindex == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n", *argv);
-				return -1;
-			}
+			if (!ifindex)
+				return nodev(*argv);
 
 			rta_addattr32(parms_rta, sizeof(parms_buf),
 				      NDTPA_IFINDEX, ifindex);
diff --git a/ip/iproute.c b/ip/iproute.c
index e4809a4..1d8fd81 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -973,10 +973,8 @@ static int parse_one_nh(struct nlmsghdr *n, struct rtmsg *r,
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
 			rtnh->rtnh_ifindex = ll_name_to_index(*argv);
-			if (rtnh->rtnh_ifindex == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n", *argv);
-				return -1;
-			}
+			if (!rtnh->rtnh_ifindex)
+				return nodev(*argv);
 		} else if (strcmp(*argv, "weight") == 0) {
 			unsigned int w;
 
@@ -1474,10 +1472,8 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
 	if (d) {
 		int idx = ll_name_to_index(d);
 
-		if (idx == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return -1;
-		}
+		if (!idx)
+			return nodev(d);
 		addattr32(&req.n, sizeof(req), RTA_OIF, idx);
 	}
 
@@ -1866,19 +1862,15 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 
 		if (id) {
 			idx = ll_name_to_index(id);
-			if (idx == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n", id);
-				return -1;
-			}
+			if (!idx)
+				return nodev(id);
 			filter.iif = idx;
 			filter.iifmask = -1;
 		}
 		if (od) {
 			idx = ll_name_to_index(od);
-			if (idx == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n", od);
-				return -1;
-			}
+			if (!idx)
+				return nodev(od);
 			filter.oif = idx;
 			filter.oifmask = -1;
 		}
@@ -2028,18 +2020,14 @@ static int iproute_get(int argc, char **argv)
 
 		if (idev) {
 			idx = ll_name_to_index(idev);
-			if (idx == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n", idev);
-				return -1;
-			}
+			if (!idx)
+				return nodev(idev);
 			addattr32(&req.n, sizeof(req), RTA_IIF, idx);
 		}
 		if (odev) {
 			idx = ll_name_to_index(odev);
-			if (idx == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n", odev);
-				return -1;
-			}
+			if (!idx)
+				return nodev(odev);
 			addattr32(&req.n, sizeof(req), RTA_OIF, idx);
 		}
 	}
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index fa3feae..cde9b3d 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -594,7 +594,7 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp,
 				duparg2("iif", *argv);
 			iif = ll_name_to_index(*argv);
 			if (!iif)
-				invarg("\"iif\" interface not found\n", *argv);
+				exit(nodev(*argv));
 			rta_addattr32(rta, len, SEG6_LOCAL_IIF, iif);
 		} else if (strcmp(*argv, "oif") == 0) {
 			NEXT_ARG();
@@ -602,7 +602,7 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp,
 				duparg2("oif", *argv);
 			oif = ll_name_to_index(*argv);
 			if (!oif)
-				invarg("\"oif\" interface not found\n", *argv);
+				exit(nodev(*argv));
 			rta_addattr32(rta, len, SEG6_LOCAL_OIF, oif);
 		} else if (strcmp(*argv, "srh") == 0) {
 			NEXT_ARG();
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 1f04f95..d597908 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -213,10 +213,8 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 
 	if (medium) {
 		p->link = ll_name_to_index(medium);
-		if (p->link == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", medium);
-			return -1;
-		}
+		if (!p->link)
+			return nodev(medium);
 	}
 
 	if (p->i_key == 0 && IN_MULTICAST(ntohl(p->iph.daddr))) {
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 64588d7..bc1cee8 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -245,11 +245,8 @@ get_failed:
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = ll_name_to_index(*argv);
-			if (link == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n",
-					*argv);
-				exit(-1);
-			}
+			if (!link)
+				exit(nodev(*argv));
 		} else if (!matches(*argv, "ttl") ||
 			   !matches(*argv, "hoplimit") ||
 			   !matches(*argv, "hlim")) {
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index e0746bc..a6fe0b7 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -251,11 +251,8 @@ get_failed:
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = ll_name_to_index(*argv);
-			if (link == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n",
-					*argv);
-				exit(-1);
-			}
+			if (!link)
+				exit(nodev(*argv));
 		} else if (!matches(*argv, "ttl") ||
 			   !matches(*argv, "hoplimit") ||
 			   !matches(*argv, "hlim")) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 77a9090..c7fef2e 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -197,8 +197,8 @@ get_failed:
 		} else if (matches(*argv, "dev") == 0) {
 			NEXT_ARG();
 			link = ll_name_to_index(*argv);
-			if (link == 0)
-				invarg("\"dev\" is invalid", *argv);
+			if (!link)
+				exit(nodev(*argv));
 		} else if (strcmp(*argv, "ttl") == 0 ||
 			   strcmp(*argv, "hoplimit") == 0 ||
 			   strcmp(*argv, "hlim") == 0) {
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index acd9f45..57f4d0c 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -225,8 +225,8 @@ get_failed:
 		} else if (matches(*argv, "dev") == 0) {
 			NEXT_ARG();
 			link = ll_name_to_index(*argv);
-			if (link == 0)
-				invarg("\"dev\" is invalid", *argv);
+			if (!link)
+				exit(nodev(*argv));
 		} else if (strcmp(*argv, "ttl") == 0 ||
 			   strcmp(*argv, "hoplimit") == 0 ||
 			   strcmp(*argv, "hlim") == 0) {
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 99e10e8..6196a1c 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -142,11 +142,8 @@ get_failed:
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = ll_name_to_index(*argv);
-			if (link == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n",
-					*argv);
-				exit(-1);
-			}
+			if (!link)
+				exit(nodev(*argv));
 		} else if (strcmp(*argv, "fwmark") == 0) {
 			NEXT_ARG();
 			if (get_u32(&fwmark, *argv, 0))
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 1df6579..4263615 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -144,11 +144,8 @@ get_failed:
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = ll_name_to_index(*argv);
-			if (link == 0) {
-				fprintf(stderr, "Cannot find device \"%s\"\n",
-					*argv);
-				exit(-1);
-			}
+			if (!link)
+				exit(nodev(*argv));
 		} else if (strcmp(*argv, "fwmark") == 0) {
 			NEXT_ARG();
 			if (get_u32(&fwmark, *argv, 0))
diff --git a/lib/utils.c b/lib/utils.c
index 24aeddd..2b8e4e8 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -845,6 +845,12 @@ void duparg2(const char *key, const char *arg)
 	exit(-1);
 }
 
+int nodev(const char *dev)
+{
+	fprintf(stderr, "Cannot find device \"%s\"\n", dev);
+	return -1;
+}
+
 int check_ifname(const char *name)
 {
 	/* These checks mimic kernel checks in dev_valid_name */
diff --git a/tc/m_mirred.c b/tc/m_mirred.c
index eb42b7c..b25b9ac 100644
--- a/tc/m_mirred.c
+++ b/tc/m_mirred.c
@@ -193,10 +193,8 @@ parse_direction(struct action_util *a, int *argc_p, char ***argv_p,
 		ll_init_map(&rth);
 
 		idx = ll_name_to_index(d);
-		if (idx == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return -1;
-		}
+		if (!idx)
+			return nodev(d);
 
 		p.ifindex = idx;
 	}
diff --git a/tc/tc_class.c b/tc/tc_class.c
index 1b214b8..e1ca29c 100644
--- a/tc/tc_class.c
+++ b/tc/tc_class.c
@@ -142,10 +142,9 @@ static int tc_class_modify(int cmd, unsigned int flags, int argc, char **argv)
 	if (d[0])  {
 		ll_init_map(&rth);
 
-		if ((req.t.tcm_ifindex = ll_name_to_index(d)) == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
+		req.t.tcm_ifindex = ll_name_to_index(d);
+		if (!req.t.tcm_ifindex)
+			return -nodev(d);
 	}
 
 	if (rtnl_talk(&rth, &req.n, NULL) < 0)
@@ -440,10 +439,9 @@ static int tc_class_list(int argc, char **argv)
 	ll_init_map(&rth);
 
 	if (d[0]) {
-		if ((t.tcm_ifindex = ll_name_to_index(d)) == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
+		t.tcm_ifindex = ll_name_to_index(d);
+		if (!t.tcm_ifindex)
+			return -nodev(d);
 		filter_ifindex = t.tcm_ifindex;
 	}
 
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index c7701fd..c5bb0bf 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -198,10 +198,8 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
 		ll_init_map(&rth);
 
 		req->t.tcm_ifindex = ll_name_to_index(d);
-		if (req->t.tcm_ifindex == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
+		if (!req->t.tcm_ifindex)
+			return -nodev(d);
 	} else if (block_index) {
 		req->t.tcm_ifindex = TCM_IFINDEX_MAGIC_BLOCK;
 		req->t.tcm_block_index = block_index;
@@ -529,10 +527,8 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		ll_init_map(&rth);
 
 		req.t.tcm_ifindex = ll_name_to_index(d);
-		if (req.t.tcm_ifindex  == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
+		if (!req.t.tcm_ifindex)
+			return -nodev(d);
 		filter_ifindex = req.t.tcm_ifindex;
 	} else if (block_index) {
 		req.t.tcm_ifindex = TCM_IFINDEX_MAGIC_BLOCK;
@@ -695,10 +691,8 @@ static int tc_filter_list(int argc, char **argv)
 
 	if (d[0]) {
 		req.t.tcm_ifindex = ll_name_to_index(d);
-		if (req.t.tcm_ifindex == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
+		if (!req.t.tcm_ifindex)
+			return -nodev(d);
 		filter_ifindex = req.t.tcm_ifindex;
 	} else if (block_index) {
 		if (!tc_qdisc_block_exists(block_index)) {
diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
index 78b80e6..c1d2df0 100644
--- a/tc/tc_qdisc.c
+++ b/tc/tc_qdisc.c
@@ -199,10 +199,8 @@ static int tc_qdisc_modify(int cmd, unsigned int flags, int argc, char **argv)
 		ll_init_map(&rth);
 
 		idx = ll_name_to_index(d);
-		if (idx == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
+		if (!idx)
+			return -nodev(d);
 		req.t.tcm_ifindex = idx;
 	}
 
@@ -378,10 +376,8 @@ static int tc_qdisc_list(int argc, char **argv)
 
 	if (d[0]) {
 		t.tcm_ifindex = ll_name_to_index(d);
-		if (t.tcm_ifindex == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
+		if (!t.tcm_ifindex)
+			return -nodev(d);
 		filter_ifindex = t.tcm_ifindex;
 	}
 
-- 
1.7.10.4

[PATCH iproute2-next v4 2/4] iplink: Use "dev" and "name" parameters interchangeable when possible

[Serhey Popovych]

Both of them accept network device name as argument, but have different
meaning:

  dev  - is a device by it's name,
  name - name for specific device.

The only case where they treated separately is network device rename
case where need to specify both ifindex and new name. In rest of the
cases we can assume that dev == name.

With this change we do following:

  1) Kill ambiguity with both "dev" and "name" parameters given the same
     name:

       ip link {add|set} dev veth100a name veth100a ...

  2) Make sure we do not accept "name" more than once.

  3) For VF and XDP treat "name" as "dev". Fail in case of "dev" is
     given after VF and/or XDP parsing.

  4) Make veth and vxcan to accept both "name" and "dev" as their peer
     parameters, effectively following general ip-link(8) utility
     behaviour on link create:

       ip link add {name|dev} veth1a type veth peer {name|dev} veth1b

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

diff --git a/ip/iplink.c b/ip/iplink.c
index 5471626..b4307ab 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -597,9 +597,15 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			req->i.ifi_flags &= ~IFF_UP;
 		} else if (strcmp(*argv, "name") == 0) {
 			NEXT_ARG();
+			if (*name)
+				duparg("name", *argv);
 			if (check_ifname(*argv))
 				invarg("\"name\" not a valid ifname", *argv);
 			*name = *argv;
+			if (!*dev) {
+				*dev = *name;
+				dev_index = ll_name_to_index(*dev);
+			}
 		} else if (strcmp(*argv, "index") == 0) {
 			NEXT_ARG();
 			if (*index)
@@ -654,6 +660,9 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			if (xdp_parse(&argc, &argv, req, dev_index,
 				      generic, drv, offload))
 				exit(-1);
+
+			if (offload && *name == *dev)
+				*dev = NULL;
 		} else if (strcmp(*argv, "netns") == 0) {
 			NEXT_ARG();
 			if (netns != -1)
@@ -745,6 +754,9 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			if (len < 0)
 				return -1;
 			addattr_nest_end(&req->n, vflist);
+
+			if (*name == *dev)
+				*dev = NULL;
 		} else if (matches(*argv, "master") == 0) {
 			int ifindex;
 
@@ -895,7 +907,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 
 			if (strcmp(*argv, "dev") == 0)
 				NEXT_ARG();
-			if (*dev)
+			if (*dev != *name)
 				duparg2("dev", *argv);
 			if (check_ifname(*argv))
 				invarg("\"dev\" not a valid ifname", *argv);
@@ -905,6 +917,14 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 		argc--; argv++;
 	}
 
+	/* Allow "ip link add dev" and "ip link add name" */
+	if (!*name)
+		*name = *dev;
+	else if (!*dev)
+		*dev = *name;
+	else if (!strcmp(*name, *dev))
+		*name = *dev;
+
 	if (dev_index && addr_len) {
 		int halen = nl_get_ll_addr_len(dev_index);
 
@@ -983,10 +1003,16 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 		req.i.ifi_index = ll_name_to_index(dev);
 		if (!req.i.ifi_index)
 			return nodev(dev);
+
+		/* Not renaming to the same name */
+		if (name == dev)
+			name = NULL;
 	} else {
-		/* Allow "ip link add dev" and "ip link add name" */
-		if (!name)
-			name = dev;
+		if (name != dev) {
+			fprintf(stderr,
+				"both \"name\" and \"dev\" cannot be used when creating devices.\n");
+			exit(-1);
+		}
 
 		if (link) {
 			int ifindex;
-- 
1.7.10.4

[PATCH iproute2-next v4 3/4] iplink: Follow documented behaviour when "index" is given

[Serhey Popovych]

Both ip-link(8) and error message when "index" parameter is given for
set/delete case says that index can only be given during network
device creation.

Follow this documented behaviour and get rid of ambiguous behaviour in
case of both "dev" and "index" specified for ip link delete scenario
(actually "index" being ignored in favor to "dev").

Prohibit "index" when configuring/deleting group of network devices.

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

diff --git a/ip/iplink.c b/ip/iplink.c
index b4307ab..6d3ebde 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -964,6 +964,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 	argc -= ret;
 	argv += ret;
 
+	if (!(flags & NLM_F_CREATE) && index) {
+		fprintf(stderr,
+			"index can be used only when creating devices.\n");
+		exit(-1);
+	}
+
 	if (group != -1) {
 		if (dev)
 			addattr_l(&req.n, sizeof(req), IFLA_GROUP,
@@ -994,11 +1000,6 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 				"Not enough information: \"dev\" argument is required.\n");
 			exit(-1);
 		}
-		if (cmd == RTM_NEWLINK && index) {
-			fprintf(stderr,
-				"index can be used only when creating devices.\n");
-			exit(-1);
-		}
 
 		req.i.ifi_index = ll_name_to_index(dev);
 		if (!req.i.ifi_index)
-- 
1.7.10.4

[PATCH iproute2-next v4 4/4] iplink: Perform most of request buffer setups and checks in iplink_parse()

[Serhey Popovych]

To benefit other users (e.g. link_veth.c) of iplink_parse() from
additional attribute checks and setups made in iplink_modify(). This
catches most of weired cobination of parameters to peer device
configuration.

Drop @name, @dev, @link, @group and @index from iplink_parse() parameters
list: they are not needed outside.

While there change return -1 to exit(-1) for group parsing errors: we
want to stop further command processing unless -force option is given
to get error line easily.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/ip_common.h    |    4 +-
 ip/iplink.c       |  143 ++++++++++++++++++++++++++---------------------------
 ip/iplink_vxcan.c |   23 +++------
 ip/link_veth.c    |   23 +++------
 4 files changed, 82 insertions(+), 111 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index e4e628b..1b89795 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -132,9 +132,7 @@ struct link_util {
 
 struct link_util *get_link_kind(const char *kind);
 
-int iplink_parse(int argc, char **argv, struct iplink_req *req,
-		 char **name, char **type, char **link, char **dev,
-		 int *group, int *index);
+int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type);
 
 /* iplink_bridge.c */
 void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len);
diff --git a/ip/iplink.c b/ip/iplink.c
index 6d3ebde..02042ed 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -569,10 +569,11 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 	return 0;
 }
 
-int iplink_parse(int argc, char **argv, struct iplink_req *req,
-		 char **name, char **type, char **link, char **dev,
-		 int *group, int *index)
+int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 {
+	char *name = NULL;
+	char *dev = NULL;
+	char *link = NULL;
 	int ret, len;
 	char abuf[32];
 	int qlen = -1;
@@ -583,9 +584,10 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 	int numrxqueues = -1;
 	int dev_index = 0;
 	int link_netnsid = -1;
+	int index = 0;
+	int group = -1;
 	int addr_len = 0;
 
-	*group = -1;
 	ret = argc;
 
 	while (argc > 0) {
@@ -597,25 +599,25 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			req->i.ifi_flags &= ~IFF_UP;
 		} else if (strcmp(*argv, "name") == 0) {
 			NEXT_ARG();
-			if (*name)
+			if (name)
 				duparg("name", *argv);
 			if (check_ifname(*argv))
 				invarg("\"name\" not a valid ifname", *argv);
-			*name = *argv;
-			if (!*dev) {
-				*dev = *name;
-				dev_index = ll_name_to_index(*dev);
+			name = *argv;
+			if (!dev) {
+				dev = name;
+				dev_index = ll_name_to_index(dev);
 			}
 		} else if (strcmp(*argv, "index") == 0) {
 			NEXT_ARG();
-			if (*index)
+			if (index)
 				duparg("index", *argv);
-			*index = atoi(*argv);
-			if (*index <= 0)
+			index = atoi(*argv);
+			if (index <= 0)
 				invarg("Invalid \"index\" value", *argv);
 		} else if (matches(*argv, "link") == 0) {
 			NEXT_ARG();
-			*link = *argv;
+			link = *argv;
 		} else if (matches(*argv, "address") == 0) {
 			NEXT_ARG();
 			addr_len = ll_addr_a2n(abuf, sizeof(abuf), *argv);
@@ -661,8 +663,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				      generic, drv, offload))
 				exit(-1);
 
-			if (offload && *name == *dev)
-				*dev = NULL;
+			if (offload && name == dev)
+				dev = NULL;
 		} else if (strcmp(*argv, "netns") == 0) {
 			NEXT_ARG();
 			if (netns != -1)
@@ -755,8 +757,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				return -1;
 			addattr_nest_end(&req->n, vflist);
 
-			if (*name == *dev)
-				*dev = NULL;
+			if (name == dev)
+				dev = NULL;
 		} else if (matches(*argv, "master") == 0) {
 			int ifindex;
 
@@ -806,10 +808,11 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				  *argv, len);
 		} else if (strcmp(*argv, "group") == 0) {
 			NEXT_ARG();
-			if (*group != -1)
+			if (group != -1)
 				duparg("group", *argv);
-			if (rtnl_group_a2n(group, *argv))
+			if (rtnl_group_a2n(&group, *argv))
 				invarg("Invalid \"group\" value\n", *argv);
+			addattr32(&req->n, sizeof(*req), IFLA_GROUP, group);
 		} else if (strcmp(*argv, "mode") == 0) {
 			int mode;
 
@@ -907,23 +910,25 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 
 			if (strcmp(*argv, "dev") == 0)
 				NEXT_ARG();
-			if (*dev != *name)
+			if (dev != name)
 				duparg2("dev", *argv);
 			if (check_ifname(*argv))
 				invarg("\"dev\" not a valid ifname", *argv);
-			*dev = *argv;
-			dev_index = ll_name_to_index(*dev);
+			dev = *argv;
+			dev_index = ll_name_to_index(dev);
 		}
 		argc--; argv++;
 	}
 
+	ret -= argc;
+
 	/* Allow "ip link add dev" and "ip link add name" */
-	if (!*name)
-		*name = *dev;
-	else if (!*dev)
-		*dev = *name;
-	else if (!strcmp(*name, *dev))
-		*name = *dev;
+	if (!name)
+		name = dev;
+	else if (!dev)
+		dev = name;
+	else if (!strcmp(name, dev))
+		name = dev;
 
 	if (dev_index && addr_len) {
 		int halen = nl_get_ll_addr_len(dev_index);
@@ -936,73 +941,40 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 		}
 	}
 
-	return ret - argc;
-}
-
-static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
-{
-	char *dev = NULL;
-	char *name = NULL;
-	char *link = NULL;
-	char *type = NULL;
-	int index = 0;
-	int group;
-	struct link_util *lu = NULL;
-	struct iplink_req req = {
-		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
-		.n.nlmsg_flags = NLM_F_REQUEST | flags,
-		.n.nlmsg_type = cmd,
-		.i.ifi_family = preferred_family,
-	};
-	int ret;
-
-	ret = iplink_parse(argc, argv,
-			   &req, &name, &type, &link, &dev, &group, &index);
-	if (ret < 0)
-		return ret;
-
-	argc -= ret;
-	argv += ret;
-
-	if (!(flags & NLM_F_CREATE) && index) {
+	if (!(req->n.nlmsg_flags & NLM_F_CREATE) && index) {
 		fprintf(stderr,
 			"index can be used only when creating devices.\n");
 		exit(-1);
 	}
 
 	if (group != -1) {
-		if (dev)
-			addattr_l(&req.n, sizeof(req), IFLA_GROUP,
-					&group, sizeof(group));
-		else {
+		if (!dev) {
 			if (argc) {
 				fprintf(stderr,
 					"Garbage instead of arguments \"%s ...\". Try \"ip link help\".\n",
 					*argv);
-				return -1;
+				exit(-1);
 			}
-			if (flags & NLM_F_CREATE) {
+			if (req->n.nlmsg_flags & NLM_F_CREATE) {
 				fprintf(stderr,
 					"group cannot be used when creating devices.\n");
-				return -1;
+				exit(-1);
 			}
 
-			addattr32(&req.n, sizeof(req), IFLA_GROUP, group);
-			if (rtnl_talk(&rth, &req.n, NULL) < 0)
-				return -2;
-			return 0;
+			*type = NULL;
+			return ret;
 		}
 	}
 
-	if (!(flags & NLM_F_CREATE)) {
+	if (!(req->n.nlmsg_flags & NLM_F_CREATE)) {
 		if (!dev) {
 			fprintf(stderr,
 				"Not enough information: \"dev\" argument is required.\n");
 			exit(-1);
 		}
 
-		req.i.ifi_index = ll_name_to_index(dev);
-		if (!req.i.ifi_index)
+		req->i.ifi_index = ll_name_to_index(dev);
+		if (!req->i.ifi_index)
 			return nodev(dev);
 
 		/* Not renaming to the same name */
@@ -1021,18 +993,37 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 			ifindex = ll_name_to_index(link);
 			if (!ifindex)
 				return nodev(link);
-			addattr_l(&req.n, sizeof(req), IFLA_LINK, &ifindex, 4);
+			addattr32(&req->n, sizeof(*req), IFLA_LINK, ifindex);
 		}
 
-		req.i.ifi_index = index;
+		req->i.ifi_index = index;
 	}
 
 	if (name) {
-		addattr_l(&req.n, sizeof(req),
+		addattr_l(&req->n, sizeof(*req),
 			  IFLA_IFNAME, name, strlen(name) + 1);
 	}
 
+	return ret;
+}
+
+static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
+{
+	char *type = NULL;
+	struct iplink_req req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+		.n.nlmsg_flags = NLM_F_REQUEST | flags,
+		.n.nlmsg_type = cmd,
+		.i.ifi_family = preferred_family,
+	};
+	int ret;
+
+	ret = iplink_parse(argc, argv, &req, &type);
+	if (ret < 0)
+		return ret;
+
 	if (type) {
+		struct link_util *lu;
 		struct rtattr *linkinfo;
 		char *ulinep = strchr(type, '_');
 		int iflatype;
@@ -1046,6 +1037,10 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 			iflatype = IFLA_INFO_SLAVE_DATA;
 		else
 			iflatype = IFLA_INFO_DATA;
+
+		argc -= ret;
+		argv += ret;
+
 		if (lu && argc) {
 			struct rtattr *data;
 
diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index ebe9e56..8b08c9a 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -33,16 +33,11 @@ static void usage(void)
 static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
-	char *dev = NULL;
-	char *name = NULL;
-	char *link = NULL;
 	char *type = NULL;
-	int index = 0;
 	int err;
 	struct rtattr *data;
-	int group;
 	struct ifinfomsg *ifm, *peer_ifm;
-	unsigned int ifi_flags, ifi_change;
+	unsigned int ifi_flags, ifi_change, ifi_index;
 
 	if (strcmp(argv[0], "peer") != 0) {
 		usage();
@@ -52,35 +47,29 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 	ifm = NLMSG_DATA(n);
 	ifi_flags = ifm->ifi_flags;
 	ifi_change = ifm->ifi_change;
+	ifi_index = ifm->ifi_index;
 	ifm->ifi_flags = 0;
 	ifm->ifi_change = 0;
+	ifm->ifi_index = 0;
 
 	data = addattr_nest(n, 1024, VXCAN_INFO_PEER);
 
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
-	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
-			   &name, &type, &link, &dev, &group, &index);
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type);
 	if (err < 0)
 		return err;
 
 	if (type)
 		duparg("type", argv[err]);
 
-	if (name) {
-		addattr_l(n, 1024,
-			  IFLA_IFNAME, name, strlen(name) + 1);
-	}
-
 	peer_ifm = RTA_DATA(data);
-	peer_ifm->ifi_index = index;
+	peer_ifm->ifi_index = ifm->ifi_index;
 	peer_ifm->ifi_flags = ifm->ifi_flags;
 	peer_ifm->ifi_change = ifm->ifi_change;
 	ifm->ifi_flags = ifi_flags;
 	ifm->ifi_change = ifi_change;
-
-	if (group != -1)
-		addattr32(n, 1024, IFLA_GROUP, group);
+	ifm->ifi_index = ifi_index;
 
 	addattr_nest_end(n, data);
 	return argc - 1 - err;
diff --git a/ip/link_veth.c b/ip/link_veth.c
index a8e7cf7..33e8f2b 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -31,16 +31,11 @@ static void usage(void)
 static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
-	char *dev = NULL;
-	char *name = NULL;
-	char *link = NULL;
 	char *type = NULL;
-	int index = 0;
 	int err;
 	struct rtattr *data;
-	int group;
 	struct ifinfomsg *ifm, *peer_ifm;
-	unsigned int ifi_flags, ifi_change;
+	unsigned int ifi_flags, ifi_change, ifi_index;
 
 	if (strcmp(argv[0], "peer") != 0) {
 		usage();
@@ -50,35 +45,29 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 	ifm = NLMSG_DATA(n);
 	ifi_flags = ifm->ifi_flags;
 	ifi_change = ifm->ifi_change;
+	ifi_index = ifm->ifi_index;
 	ifm->ifi_flags = 0;
 	ifm->ifi_change = 0;
+	ifm->ifi_index = 0;
 
 	data = addattr_nest(n, 1024, VETH_INFO_PEER);
 
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
-	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
-			   &name, &type, &link, &dev, &group, &index);
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type);
 	if (err < 0)
 		return err;
 
 	if (type)
 		duparg("type", argv[err]);
 
-	if (name) {
-		addattr_l(n, 1024,
-			  IFLA_IFNAME, name, strlen(name) + 1);
-	}
-
 	peer_ifm = RTA_DATA(data);
-	peer_ifm->ifi_index = index;
+	peer_ifm->ifi_index = ifm->ifi_index;
 	peer_ifm->ifi_flags = ifm->ifi_flags;
 	peer_ifm->ifi_change = ifm->ifi_change;
 	ifm->ifi_flags = ifi_flags;
 	ifm->ifi_change = ifi_change;
-
-	if (group != -1)
-		addattr32(n, 1024, IFLA_GROUP, group);
+	ifm->ifi_index = ifi_index;
 
 	addattr_nest_end(n, data);
 	return argc - 1 - err;
-- 
1.7.10.4

Re: [PATCH iproute2-next v4 0/4] iplink: Improve iplink_parse()

[David Ahern]

On 3/7/18 1:40 AM, Serhey Popovych wrote:
> This is main routine to parse ip-link(8) configuration parameters.
> 
> Move all code related to command line parsing and validation to it from
> iptables_modify(). As benefit we reduce number of arguments as well as
> checking for most of weired cases in single place to give benefit to
> iptables_parse() users.
> 
> See individual patch description message for more information.
> 


applied to iproute2-next.