[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1520412039-17154-5-git-send-email-serhe.popovych@gmail.com>
Date: Wed, 7 Mar 2018 10:40:39 +0200
From: Serhey Popovych <serhe.popovych@...il.com>
To: netdev@...r.kernel.org
Cc: dsahern@...il.com
Subject: [PATCH iproute2-next v4 4/4] iplink: Perform most of request buffer setups and checks in iplink_parse()
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@...il.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
Powered by blists - more mailing lists