diff mbox series

[iproute2-next,v2,3/7] iplink: Use "dev" and "name" parameters interchangeable when possible

Message ID 1519162649-22449-4-git-send-email-serhe.popovych@gmail.com
State Superseded, archived
Delegated to: David Ahern
Headers show
Series iplink: Improve iplink_parse() | expand

Commit Message

Serhey Popovych Feb. 20, 2018, 9:37 p.m. UTC 
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 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(-)

Comments

Stephen Hemminger Feb. 20, 2018, 11:04 p.m. UTC | #1 
On Tue, 20 Feb 2018 23:37:25 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> 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 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.

I would rather keep name as only being a valid argument for set command.
And reject use of:

   ip li show name eth0
Serhey Popovych Feb. 21, 2018, 6:35 a.m. UTC | #2 
Stephen Hemminger wrote:
> On Tue, 20 Feb 2018 23:37:25 +0200
> Serhey Popovych <serhe.popovych@gmail.com> wrote:
> 
>> 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 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.
> 
> I would rather keep name as only being a valid argument for set command.
> And reject use of:
> 
>    ip li show name eth0
> 

But ip-link(8) man page is full of examples where "name" is used instead
of "dev". I suppose there are lot of configuration we can broke when
explicitly disabling "name" as an alternative to "dev".

Especially on the first lines in man page we have:

ip link add [ link DEVICE ] [ name ] NAME
...

and there is no reference to "dev".

What do you think?
diff mbox series

Patch

diff --git a/ip/iplink.c b/ip/iplink.c
index fc358fc..1359c0f 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -605,9 +605,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)
@@ -665,6 +671,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)
@@ -755,6 +764,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;
 
@@ -905,7 +917,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);
@@ -915,6 +927,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);
 
@@ -993,10 +1013,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;