From fe99adbca4cbd48cfa8f50d3617fe7485424501e Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Wed, 7 Mar 2018 10:40:36 +0200 Subject: [PATCH 1/4] utils: Introduce and use nodev() helper routine 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 --- 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 b4f6e8b3..205b4faa 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 69c08ec7..579d57e7 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 8c08baf5..f38dc67c 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 9f4a7a2b..19a36b80 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 6bc77e74..e4389ee4 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 c7fa0822..999408ed 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 d01d7030..087fa68a 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 74c377c8..5471626f 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 8e8723a9..f906e7f1 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 06ec092f..3008e44b 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 d768c07e..be9f35e4 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 aa5029b4..5c232e8a 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 925494db..47483817 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 92024864..82f40f87 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 e4809a43..1d8fd815 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 fa3feaea..cde9b3d2 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 1f04f95a..d597908f 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 64588d75..bc1cee8f 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 e0746bc3..a6fe0b73 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 77a90900..c7fef2e0 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 acd9f45b..57f4d0c7 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 99e10e8a..6196a1c9 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 1df6579b..4263615b 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 24aeddd8..2b8e4e8e 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 eb42b7c1..b25b9acc 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 1b214b82..e1ca29cf 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 c7701fd8..c5bb0bff 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 78b80e67..c1d2df01 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; } From a24315ba4688a31262dc3846178b09912c0c8081 Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Wed, 7 Mar 2018 10:40:37 +0200 Subject: [PATCH 2/4] iplink: Use "dev" and "name" parameters interchangeable when possible 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 --- ip/iplink.c | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/ip/iplink.c b/ip/iplink.c index 5471626f..b4307ab7 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; From b06a29603accea41ef699b6224fae80eba9dead4 Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Wed, 7 Mar 2018 10:40:38 +0200 Subject: [PATCH 3/4] iplink: Follow documented behaviour when "index" is given 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 --- ip/iplink.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ip/iplink.c b/ip/iplink.c index b4307ab7..6d3ebdee 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) From c58213f69c294c75ae6bd1ae16af7e0df29cf187 Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Wed, 7 Mar 2018 10:40:39 +0200 Subject: [PATCH 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 --- 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 e4e628b5..1b89795c 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 6d3ebdee..02042ed6 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 ebe9e568..8b08c9a7 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 a8e7cf7f..33e8f2b1 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;