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