[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1519162649-22449-6-git-send-email-serhe.popovych@gmail.com>
Date: Tue, 20 Feb 2018 23:37:27 +0200
From: Serhey Popovych <serhe.popovych@...il.com>
To: netdev@...r.kernel.org
Cc: dsahern@...il.com
Subject: [PATCH iproute2-next v2 5/7] 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 in iplink_vxcan.c and link_veth.c. Use memcpy() to save
peer device @ifm data structure as now ->ifi_index is set inside
iplist_parse(): it will be inlined by the compiler anyway.
Drop @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 | 3 +-
ip/iplink.c | 118 +++++++++++++++++++++++++----------------------------
ip/iplink_vxcan.c | 27 +++---------
ip/link_veth.c | 27 +++---------
4 files changed, 69 insertions(+), 106 deletions(-)
diff --git a/ip/ip_common.h b/ip/ip_common.h
index e4e628b..f762821 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -133,8 +133,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);
+ char **name, char **type, char **dev);
/* 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 4e9f571..e53d890 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -578,9 +578,9 @@ static void has_dev(const char *dev, int dev_index)
}
int iplink_parse(int argc, char **argv, struct iplink_req *req,
- char **name, char **type, char **link, char **dev,
- int *group, int *index)
+ char **name, char **type, char **dev)
{
+ char *link = NULL;
int ret, len;
char abuf[32];
int qlen = -1;
@@ -591,9 +591,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) {
@@ -616,14 +617,14 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
}
} 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);
@@ -816,10 +817,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;
@@ -946,80 +948,47 @@ 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;
+ goto out;
}
}
- if (!(flags & NLM_F_CREATE)) {
- if (!dev) {
+ 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)
- return nodev(dev);
+ 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;
+ if (*name == *dev)
+ *name = NULL;
} else {
- if (name != dev) {
+ if (*name != *dev) {
fprintf(stderr,
"both \"name\" and \"dev\" cannot be used when creating devices.\n");
exit(-1);
@@ -1031,18 +1000,39 @@ 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),
- IFLA_IFNAME, name, strlen(name) + 1);
+ if (*name) {
+ addattr_l(&req->n, sizeof(*req),
+ IFLA_IFNAME, *name, strlen(*name) + 1);
}
+out:
+ return ret - argc;
+}
+
+static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
+{
+ char *dev = NULL;
+ char *name = NULL;
+ 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, &name, &type, &dev);
+ if (ret < 0)
+ return ret;
if (type) {
+ struct link_util *lu;
struct rtattr *linkinfo;
char *ulinep = strchr(type, '_');
int iflatype;
@@ -1056,6 +1046,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..43c80e7 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -35,14 +35,10 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
{
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;
+ struct ifinfomsg ifm_save, *ifm, *peer_ifm;
if (strcmp(argv[0], "peer") != 0) {
usage();
@@ -50,8 +46,8 @@ 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;
+ memcpy(&ifm_save, ifm, sizeof(*ifm));
+ ifm->ifi_index = 0;
ifm->ifi_flags = 0;
ifm->ifi_change = 0;
@@ -60,27 +56,16 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
n->nlmsg_len += sizeof(struct ifinfomsg);
err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
- &name, &type, &link, &dev, &group, &index);
+ &name, &type, &dev);
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_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);
+ memcpy(peer_ifm, ifm, sizeof(*ifm));
+ memcpy(ifm, &ifm_save, sizeof(*ifm));
addattr_nest_end(n, data);
return argc - 1 - err;
diff --git a/ip/link_veth.c b/ip/link_veth.c
index a8e7cf7..bd8f36e 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -33,14 +33,10 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
{
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;
+ struct ifinfomsg ifm_save, *ifm, *peer_ifm;
if (strcmp(argv[0], "peer") != 0) {
usage();
@@ -48,8 +44,8 @@ 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;
+ memcpy(&ifm_save, ifm, sizeof(*ifm));
+ ifm->ifi_index = 0;
ifm->ifi_flags = 0;
ifm->ifi_change = 0;
@@ -58,27 +54,16 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
n->nlmsg_len += sizeof(struct ifinfomsg);
err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
- &name, &type, &link, &dev, &group, &index);
+ &name, &type, &dev);
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_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);
+ memcpy(peer_ifm, ifm, sizeof(*ifm));
+ memcpy(ifm, &ifm_save, sizeof(*ifm));
addattr_nest_end(n, data);
return argc - 1 - err;
--
1.7.10.4
Powered by blists - more mailing lists