* [PATCH iproute2-next 0/8] iplink: Improve iplink_parse()
@ 2018-02-20 19:39 Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit() Serhey Popovych
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
This is main routine to parse ip-link(8) configuration parameters.
Main reason to improve it is to pass network device @name, @dev and
other parameters to kind specific ->parse_opt() function so they can use
this information.
For example later we will extend iplink_get() to parse netlink
attributes deeper and replace open coded rtnl_talk() in ip/tunnel
modules to simplify getting existing tunnel information.
Among main change there is a number of patches to prepare for it
that improve iplink_parse() in some way.
See individual patch description message for more information.
Thanks,
Serhii
Serhey Popovych (8):
iplink: Return from function instead of calling exit()
utils: Introduce and use nodev() helper routine
iplink: Correctly report error when network device isn't found
iplink: Use "dev" and "name" parameters interchangeable when possible
iplink: Follow documented behaviour when "index" is given
iplink: Perform most of request buffer setups and checks in
iplink_parse()
iplink: Move data structures to block of their users
iplink: Reduce number of arguments to iplink_parse()
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/ip_common.h | 17 +++-
ip/ipaddress.c | 7 +-
ip/iplink.c | 207 +++++++++++++++++++++++++++-------------------
ip/iplink_bond.c | 8 +-
ip/iplink_bond_slave.c | 4 +-
ip/iplink_bridge.c | 11 ++-
ip/iplink_bridge_slave.c | 4 +-
ip/iplink_can.c | 4 +-
ip/iplink_geneve.c | 4 +-
ip/iplink_hsr.c | 4 +-
ip/iplink_ipoib.c | 4 +-
ip/iplink_ipvlan.c | 4 +-
ip/iplink_macvlan.c | 4 +-
ip/iplink_vlan.c | 4 +-
ip/iplink_vrf.c | 5 +-
ip/iplink_vxcan.c | 39 +++------
ip/iplink_vxlan.c | 11 ++-
ip/iplink_xdp.c | 7 +-
ip/ipmacsec.c | 4 +-
ip/ipmroute.c | 7 +-
ip/ipneigh.c | 15 ++--
ip/ipntable.c | 6 +-
ip/iproute.c | 36 +++-----
ip/iproute_lwtunnel.c | 4 +-
ip/iptunnel.c | 6 +-
ip/link_gre.c | 43 +++++-----
ip/link_gre6.c | 43 +++++-----
ip/link_ip6tnl.c | 40 ++++-----
ip/link_iptnl.c | 40 ++++-----
ip/link_veth.c | 39 +++------
ip/link_vti.c | 43 +++++-----
ip/link_vti6.c | 43 +++++-----
lib/utils.c | 6 ++
tc/m_mirred.c | 6 +-
tc/tc_class.c | 14 ++--
tc/tc_filter.c | 18 ++--
tc/tc_qdisc.c | 12 +--
43 files changed, 409 insertions(+), 422 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
@ 2018-02-20 19:39 ` Serhey Popovych
2018-02-20 20:08 ` Stephen Hemminger
2018-02-20 19:39 ` [PATCH iproute2-next 2/8] utils: Introduce and use nodev() helper routine Serhey Popovych
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/iplink.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index 74c377c..a2c8108 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
NEXT_ARG();
if (xdp_parse(&argc, &argv, req, dev_index,
generic, drv, offload))
- exit(-1);
+ return -1;
} else if (strcmp(*argv, "netns") == 0) {
NEXT_ARG();
if (netns != -1)
@@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
if (!dev) {
fprintf(stderr,
"Not enough information: \"dev\" argument is required.\n");
- exit(-1);
+ return -1;
}
if (cmd == RTM_NEWLINK && index) {
fprintf(stderr,
"index can be used only when creating devices.\n");
- exit(-1);
+ return -1;
}
req.i.ifi_index = ll_name_to_index(dev);
@@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
if (!dev) {
fprintf(stderr,
"Not enough of information: \"dev\" argument is required.\n");
- exit(-1);
+ return -1;
}
if (newaddr || newbrd) {
@@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
fprintf(stderr,
"Command \"%s\" is unknown, try \"ip link help\".\n",
*argv);
- exit(-1);
+ return -1;
}
argv++; argc--;
@@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
*argv);
- exit(-1);
+ return -1;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2-next 2/8] utils: Introduce and use nodev() helper routine
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit() Serhey Popovych
@ 2018-02-20 19:39 ` Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 3/8] iplink: Correctly report error when network device isn't found Serhey Popovych
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
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.
Return to the caller instead of exit(-1) in tunnel code.
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 <serhe.popovych@gmail.com>
---
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 | 15 ++++++++-------
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, 98 insertions(+), 164 deletions(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 8b133f9..5cb7ee2 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -375,11 +375,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;
}
@@ -464,8 +461,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)
+ return nodev(*argv);
} else if (strcmp(*argv, "self") == 0) {
req.ndm.ndm_flags |= NTF_SELF;
} else if (matches(*argv, "master") == 0) {
@@ -540,10 +537,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 90c9734..1ea9c00 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -470,11 +470,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 62dc8a0..e3f6978 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -312,11 +312,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);
}
/* get mdb entries*/
@@ -418,16 +415,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 f42d7e6..3304348 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -568,11 +568,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);
}
if (!show_stats) {
diff --git a/include/utils.h b/include/utils.h
index 75ddb4a..50721f2 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 c7fa082..999408e 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 1380453..80360ee 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -2213,10 +2213,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 a2c8108..ba49a32 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 8e8723a..f906e7f 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 06ec092..3008e44 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 d768c07..0948f42 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)
+ return 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 aa5029b..5c232e8 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 0735424..9c9cd23 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -178,11 +178,13 @@ 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)
exit(2);
@@ -423,10 +425,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 2f72c98..fbbf232 100644
--- a/ip/ipntable.c
+++ b/ip/ipntable.c
@@ -139,10 +139,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 e4809a4..1d8fd81 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 fa3feae..bb61bf0 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);
+ return 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);
+ return 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 1f04f95..d597908 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 64588d7..9c5457a 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)
+ return 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 6c77038..267f79e 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -250,11 +250,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)
+ return 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 77a9090..c681c3a 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)
+ return 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 acd9f45..636e216 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)
+ return 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 99e10e8..3d3a70a 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)
+ return 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 1df6579..4c26e21 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)
+ return 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 61af123..c0535a1 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 eb42b7c..b25b9ac 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 1b214b8..e1ca29c 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 5c31a4c..f2313cd 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;
@@ -530,10 +528,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;
@@ -696,10 +692,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 2fcf04c..e9a95a4 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;
}
@@ -379,10 +377,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;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2-next 3/8] iplink: Correctly report error when network device isn't found
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit() Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 2/8] utils: Introduce and use nodev() helper routine Serhey Popovych
@ 2018-02-20 19:39 ` Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 4/8] iplink: Use "dev" and "name" parameters interchangeable when possible Serhey Popovych
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
Distinguish cases when "dev" parameter isn't given from cases where no
network device corresponding to "dev" is found.
Do not check for index validity in xdp_parse(): caller should take care
of this because has more information (e.g. when "dev" is given or not
found) for this.
Use exit(-nodev()) in has_dev() instead of return since missarg() will
call exit() and thus requires change.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/iplink.c | 16 +++++++++++++---
ip/iplink_xdp.c | 7 +------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index ba49a32..df12aca 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -569,6 +569,14 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
return 0;
}
+static void has_dev(const char *dev, int dev_index)
+{
+ if (!dev)
+ missarg("dev");
+ if (!dev_index)
+ exit(-nodev(dev));
+}
+
int iplink_parse(int argc, char **argv, struct iplink_req *req,
char **name, char **type, char **link, char **dev,
int *group, int *index)
@@ -650,6 +658,9 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
bool drv = strcmp(*argv, "xdpdrv") == 0;
bool offload = strcmp(*argv, "xdpoffload") == 0;
+ if (offload)
+ has_dev(*dev, dev_index);
+
NEXT_ARG();
if (xdp_parse(&argc, &argv, req, dev_index,
generic, drv, offload))
@@ -732,15 +743,14 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
} else if (strcmp(*argv, "vf") == 0) {
struct rtattr *vflist;
+ has_dev(*dev, dev_index);
+
NEXT_ARG();
if (get_integer(&vf, *argv, 0))
invarg("Invalid \"vf\" value\n", *argv);
vflist = addattr_nest(&req->n, sizeof(*req),
IFLA_VFINFO_LIST);
- if (dev_index == 0)
- missarg("dev");
-
len = iplink_parse_vf(vf, &argc, &argv, req, dev_index);
if (len < 0)
return -1;
diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
index 8382635..3df38b8 100644
--- a/ip/iplink_xdp.c
+++ b/ip/iplink_xdp.c
@@ -55,17 +55,12 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, __u32 ifindex,
.type = BPF_PROG_TYPE_XDP,
.argc = *argc,
.argv = *argv,
+ .ifindex = ifindex,
};
struct xdp_req xdp = {
.req = req,
};
- if (offload) {
- if (!ifindex)
- incomplete_command();
- cfg.ifindex = ifindex;
- }
-
if (!force)
xdp.flags |= XDP_FLAGS_UPDATE_IF_NOEXIST;
if (generic)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2-next 4/8] iplink: Use "dev" and "name" parameters interchangeable when possible
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
` (2 preceding siblings ...)
2018-02-20 19:39 ` [PATCH iproute2-next 3/8] iplink: Correctly report error when network device isn't found Serhey Popovych
@ 2018-02-20 19:39 ` Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 5/8] iplink: Follow documented behaviour when "index" is given Serhey Popovych
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
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(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index df12aca..131c268 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))
return -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");
+ return -1;
+ }
if (link) {
int ifindex;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2-next 5/8] iplink: Follow documented behaviour when "index" is given
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
` (3 preceding siblings ...)
2018-02-20 19:39 ` [PATCH iproute2-next 4/8] iplink: Use "dev" and "name" parameters interchangeable when possible Serhey Popovych
@ 2018-02-20 19:39 ` Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 6/8] iplink: Perform most of request buffer setups and checks in iplink_parse() Serhey Popovych
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
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 <serhe.popovych@gmail.com>
---
ip/iplink.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index 131c268..1adfaa0 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -974,6 +974,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");
+ return -1;
+ }
+
if (group != -1) {
if (dev)
addattr_l(&req.n, sizeof(req), IFLA_GROUP,
@@ -1004,11 +1010,6 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
"Not enough information: \"dev\" argument is required.\n");
return -1;
}
- if (cmd == RTM_NEWLINK && index) {
- fprintf(stderr,
- "index can be used only when creating devices.\n");
- return -1;
- }
req.i.ifi_index = ll_name_to_index(dev);
if (!req.i.ifi_index)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2-next 6/8] iplink: Perform most of request buffer setups and checks in iplink_parse()
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
` (4 preceding siblings ...)
2018-02-20 19:39 ` [PATCH iproute2-next 5/8] iplink: Follow documented behaviour when "index" is given Serhey Popovych
@ 2018-02-20 19:39 ` Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 7/8] iplink: Move data structures to block of their users Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 8/8] iplink: Reduce number of arguments to iplink_parse() Serhey Popovych
7 siblings, 0 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
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(). Using memcpy() with constant length inlined by compiler.
Drop @link, @group and @index from iplink_parse() parameters list: they
are not needed outside.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ip_common.h | 3 +-
ip/iplink.c | 115 +++++++++++++++++++++++++----------------------------
ip/iplink_vxcan.c | 27 +++----------
ip/link_veth.c | 27 +++----------
4 files changed, 68 insertions(+), 104 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 1adfaa0..9274a64 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");
return -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;
}
- 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;
}
- 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");
return -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");
return -1;
@@ -1031,18 +1000,40 @@ 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);
+ addattr_l(&req->n, sizeof(*req),
+ IFLA_LINK, &ifindex, 4);
}
- 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 +1047,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2-next 7/8] iplink: Move data structures to block of their users
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
` (5 preceding siblings ...)
2018-02-20 19:39 ` [PATCH iproute2-next 6/8] iplink: Perform most of request buffer setups and checks in iplink_parse() Serhey Popovych
@ 2018-02-20 19:39 ` Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 8/8] iplink: Reduce number of arguments to iplink_parse() Serhey Popovych
7 siblings, 0 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
This will consolidate data and code using it in single place and prepare
for upcoming ->parse_opt() method change.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre.c | 32 ++++++++++++++++----------------
ip/link_gre6.c | 32 ++++++++++++++++----------------
ip/link_ip6tnl.c | 32 ++++++++++++++++----------------
ip/link_iptnl.c | 32 ++++++++++++++++----------------
ip/link_vti.c | 32 ++++++++++++++++----------------
ip/link_vti6.c | 32 ++++++++++++++++----------------
6 files changed, 96 insertions(+), 96 deletions(-)
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 9c5457a..a43cf6a 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -66,22 +66,6 @@ static void gre_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
struct nlmsghdr *n)
{
- struct ifinfomsg *ifi = NLMSG_DATA(n);
- struct {
- struct nlmsghdr n;
- struct ifinfomsg i;
- } req = {
- .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
- .n.nlmsg_flags = NLM_F_REQUEST,
- .n.nlmsg_type = RTM_GETLINK,
- .i.ifi_family = preferred_family,
- .i.ifi_index = ifi->ifi_index,
- };
- struct nlmsghdr *answer;
- struct rtattr *tb[IFLA_MAX + 1];
- struct rtattr *linkinfo[IFLA_INFO_MAX+1];
- struct rtattr *greinfo[IFLA_GRE_MAX + 1];
- int len;
__u16 iflags = 0;
__u16 oflags = 0;
__be32 ikey = 0;
@@ -107,7 +91,23 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct {
+ struct nlmsghdr n;
+ struct ifinfomsg i;
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETLINK,
+ .i.ifi_family = preferred_family,
+ .i.ifi_index = ifi->ifi_index,
+ };
+ struct nlmsghdr *answer;
+ struct rtattr *tb[IFLA_MAX + 1];
+ struct rtattr *linkinfo[IFLA_INFO_MAX+1];
+ struct rtattr *greinfo[IFLA_GRE_MAX + 1];
const struct rtattr *rta;
+ int len;
if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 267f79e..d7e0d8e 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -77,22 +77,6 @@ static void gre_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
struct nlmsghdr *n)
{
- struct ifinfomsg *ifi = NLMSG_DATA(n);
- struct {
- struct nlmsghdr n;
- struct ifinfomsg i;
- } req = {
- .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
- .n.nlmsg_flags = NLM_F_REQUEST,
- .n.nlmsg_type = RTM_GETLINK,
- .i.ifi_family = preferred_family,
- .i.ifi_index = ifi->ifi_index,
- };
- struct nlmsghdr *answer;
- struct rtattr *tb[IFLA_MAX + 1];
- struct rtattr *linkinfo[IFLA_INFO_MAX+1];
- struct rtattr *greinfo[IFLA_GRE_MAX + 1];
- int len;
__u16 iflags = 0;
__u16 oflags = 0;
__be32 ikey = 0;
@@ -118,7 +102,23 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct {
+ struct nlmsghdr n;
+ struct ifinfomsg i;
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETLINK,
+ .i.ifi_family = preferred_family,
+ .i.ifi_index = ifi->ifi_index,
+ };
+ struct nlmsghdr *answer;
+ struct rtattr *tb[IFLA_MAX + 1];
+ struct rtattr *linkinfo[IFLA_INFO_MAX+1];
+ struct rtattr *greinfo[IFLA_GRE_MAX + 1];
const struct rtattr *rta;
+ int len;
if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index c681c3a..bffccbd 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -77,22 +77,6 @@ static void ip6tunnel_print_help(struct link_util *lu, int argc, char **argv,
static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
struct nlmsghdr *n)
{
- struct ifinfomsg *ifi = NLMSG_DATA(n);
- struct {
- struct nlmsghdr n;
- struct ifinfomsg i;
- } req = {
- .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
- .n.nlmsg_flags = NLM_F_REQUEST,
- .n.nlmsg_type = RTM_GETLINK,
- .i.ifi_family = preferred_family,
- .i.ifi_index = ifi->ifi_index,
- };
- struct nlmsghdr *answer;
- struct rtattr *tb[IFLA_MAX + 1];
- struct rtattr *linkinfo[IFLA_INFO_MAX+1];
- struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
- int len;
inet_prefix saddr, daddr;
__u8 hop_limit = DEFAULT_TNL_HOP_LIMIT;
__u8 encap_limit = IPV6_DEFAULT_TNL_ENCAP_LIMIT;
@@ -111,7 +95,23 @@ static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct {
+ struct nlmsghdr n;
+ struct ifinfomsg i;
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETLINK,
+ .i.ifi_family = preferred_family,
+ .i.ifi_index = ifi->ifi_index,
+ };
+ struct nlmsghdr *answer;
+ struct rtattr *tb[IFLA_MAX + 1];
+ struct rtattr *linkinfo[IFLA_INFO_MAX+1];
+ struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
const struct rtattr *rta;
+ int len;
if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 636e216..6c49739 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -74,22 +74,6 @@ static void iptunnel_print_help(struct link_util *lu, int argc, char **argv,
static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
struct nlmsghdr *n)
{
- struct ifinfomsg *ifi = NLMSG_DATA(n);
- struct {
- struct nlmsghdr n;
- struct ifinfomsg i;
- } req = {
- .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
- .n.nlmsg_flags = NLM_F_REQUEST,
- .n.nlmsg_type = RTM_GETLINK,
- .i.ifi_family = preferred_family,
- .i.ifi_index = ifi->ifi_index,
- };
- struct nlmsghdr *answer;
- struct rtattr *tb[IFLA_MAX + 1];
- struct rtattr *linkinfo[IFLA_INFO_MAX+1];
- struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
- int len;
inet_prefix saddr, daddr, ip6rdprefix, ip6rdrelayprefix;
__u8 pmtudisc = 1;
__u8 tos = 0;
@@ -111,7 +95,23 @@ static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&ip6rdrelayprefix);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct {
+ struct nlmsghdr n;
+ struct ifinfomsg i;
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETLINK,
+ .i.ifi_family = preferred_family,
+ .i.ifi_index = ifi->ifi_index,
+ };
+ struct nlmsghdr *answer;
+ struct rtattr *tb[IFLA_MAX + 1];
+ struct rtattr *linkinfo[IFLA_INFO_MAX+1];
+ struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
const struct rtattr *rta;
+ int len;
if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 3d3a70a..1429f07 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -47,33 +47,33 @@ static void vti_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
struct nlmsghdr *n)
{
- struct ifinfomsg *ifi = NLMSG_DATA(n);
- struct {
- struct nlmsghdr n;
- struct ifinfomsg i;
- } req = {
- .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
- .n.nlmsg_flags = NLM_F_REQUEST,
- .n.nlmsg_type = RTM_GETLINK,
- .i.ifi_family = preferred_family,
- .i.ifi_index = ifi->ifi_index,
- };
- struct nlmsghdr *answer;
- struct rtattr *tb[IFLA_MAX + 1];
- struct rtattr *linkinfo[IFLA_INFO_MAX+1];
- struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
__be32 ikey = 0;
__be32 okey = 0;
inet_prefix saddr, daddr;
unsigned int link = 0;
__u32 fwmark = 0;
- int len;
inet_prefix_reset(&saddr);
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct {
+ struct nlmsghdr n;
+ struct ifinfomsg i;
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETLINK,
+ .i.ifi_family = preferred_family,
+ .i.ifi_index = ifi->ifi_index,
+ };
+ struct nlmsghdr *answer;
+ struct rtattr *tb[IFLA_MAX + 1];
+ struct rtattr *linkinfo[IFLA_INFO_MAX+1];
+ struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
const struct rtattr *rta;
+ int len;
if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 4c26e21..2956c0e 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -49,33 +49,33 @@ static void vti6_print_help(struct link_util *lu, int argc, char **argv,
static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
struct nlmsghdr *n)
{
- struct ifinfomsg *ifi = NLMSG_DATA(n);
- struct {
- struct nlmsghdr n;
- struct ifinfomsg i;
- } req = {
- .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
- .n.nlmsg_flags = NLM_F_REQUEST,
- .n.nlmsg_type = RTM_GETLINK,
- .i.ifi_family = preferred_family,
- .i.ifi_index = ifi->ifi_index,
- };
- struct nlmsghdr *answer;
- struct rtattr *tb[IFLA_MAX + 1];
- struct rtattr *linkinfo[IFLA_INFO_MAX+1];
- struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
__be32 ikey = 0;
__be32 okey = 0;
inet_prefix saddr, daddr;
unsigned int link = 0;
__u32 fwmark = 0;
- int len;
inet_prefix_reset(&saddr);
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct {
+ struct nlmsghdr n;
+ struct ifinfomsg i;
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETLINK,
+ .i.ifi_family = preferred_family,
+ .i.ifi_index = ifi->ifi_index,
+ };
+ struct nlmsghdr *answer;
+ struct rtattr *tb[IFLA_MAX + 1];
+ struct rtattr *linkinfo[IFLA_INFO_MAX+1];
+ struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
const struct rtattr *rta;
+ int len;
if (rtnl_talk(&rth, &req.n, &answer) < 0) {
get_failed:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2-next 8/8] iplink: Reduce number of arguments to iplink_parse()
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
` (6 preceding siblings ...)
2018-02-20 19:39 ` [PATCH iproute2-next 7/8] iplink: Move data structures to block of their users Serhey Popovych
@ 2018-02-20 19:39 ` Serhey Popovych
7 siblings, 0 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 19:39 UTC (permalink / raw)
To: netdev; +Cc: dsahern
Introduce new @struct iplink_parse_args data structure to consolidate
arguments to iplink_parse(). This will reduce number of arguments
passed to it.
Pass this data structure to ->parse_opt() in iplink specific modules:
it may be used to get network device name and other information.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ip_common.h | 16 +++++++++++++---
ip/iplink.c | 34 ++++++++++++++++++++++------------
ip/iplink_bond.c | 4 +++-
ip/iplink_bond_slave.c | 4 +++-
ip/iplink_bridge.c | 4 +++-
ip/iplink_bridge_slave.c | 4 +++-
ip/iplink_can.c | 4 +++-
ip/iplink_geneve.c | 4 +++-
ip/iplink_hsr.c | 4 +++-
ip/iplink_ipoib.c | 4 +++-
ip/iplink_ipvlan.c | 4 +++-
ip/iplink_macvlan.c | 4 +++-
ip/iplink_vlan.c | 4 +++-
ip/iplink_vrf.c | 5 ++++-
ip/iplink_vxcan.c | 14 ++++++--------
ip/iplink_vxlan.c | 4 +++-
ip/ipmacsec.c | 4 +++-
ip/link_gre.c | 6 ++++--
ip/link_gre6.c | 6 ++++--
ip/link_ip6tnl.c | 6 ++++--
ip/link_iptnl.c | 6 ++++--
ip/link_veth.c | 14 ++++++--------
ip/link_vti.c | 6 ++++--
ip/link_vti6.c | 6 ++++--
24 files changed, 114 insertions(+), 57 deletions(-)
diff --git a/ip/ip_common.h b/ip/ip_common.h
index f762821..aef70de 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -112,12 +112,23 @@ struct iplink_req {
char buf[1024];
};
+struct iplink_parse_args {
+ const char *dev;
+ const char *name;
+ const char *type;
+
+ /* This definitely must be the last one and initialized
+ * by the caller of iplink_parse() that will initialize rest.
+ */
+ struct iplink_req *req;
+};
+
struct link_util {
struct link_util *next;
const char *id;
int maxattr;
int (*parse_opt)(struct link_util *, int, char **,
- struct nlmsghdr *);
+ struct iplink_parse_args *);
void (*print_opt)(struct link_util *, FILE *,
struct rtattr *[]);
void (*print_xstats)(struct link_util *, FILE *,
@@ -132,8 +143,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 **dev);
+int iplink_parse(int argc, char **argv, struct iplink_parse_args *pa);
/* 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 9274a64..3a5f724 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -10,6 +10,7 @@
*
*/
+#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
@@ -577,9 +578,12 @@ static void has_dev(const char *dev, int dev_index)
exit(-nodev(dev));
}
-int iplink_parse(int argc, char **argv, struct iplink_req *req,
- char **name, char **type, char **dev)
+int iplink_parse(int argc, char **argv, struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ const char **dev = &pa->dev;
+ const char **name = &pa->name;
+ const char **type = &pa->type;
char *link = NULL;
int ret, len;
char abuf[32];
@@ -597,6 +601,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
ret = argc;
+ memset(pa, 0, offsetof(struct iplink_parse_args, req));
+
while (argc > 0) {
if (strcmp(*argv, "up") == 0) {
req->i.ifi_change |= IFF_UP;
@@ -1017,32 +1023,32 @@ out:
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,
};
+ struct iplink_parse_args pa;
int ret;
- ret = iplink_parse(argc, argv, &req, &name, &type, &dev);
+ pa.req = &req;
+
+ ret = iplink_parse(argc, argv, &pa);
if (ret < 0)
return ret;
- if (type) {
+ if (pa.type) {
struct link_util *lu;
struct rtattr *linkinfo;
- char *ulinep = strchr(type, '_');
+ char *ulinep = strchr(pa.type, '_');
int iflatype;
linkinfo = addattr_nest(&req.n, sizeof(req), IFLA_LINKINFO);
- addattr_l(&req.n, sizeof(req), IFLA_INFO_KIND, type,
- strlen(type));
+ addattr_l(&req.n, sizeof(req), IFLA_INFO_KIND, pa.type,
+ strlen(pa.type));
- lu = get_link_kind(type);
+ lu = get_link_kind(pa.type);
if (ulinep && !strcmp(ulinep, "_slave"))
iflatype = IFLA_INFO_SLAVE_DATA;
else
@@ -1057,9 +1063,13 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
data = addattr_nest(&req.n, sizeof(req), iflatype);
if (lu->parse_opt &&
- lu->parse_opt(lu, argc, argv, &req.n))
+ lu->parse_opt(lu, argc, argv, &pa))
return -1;
+ /* Must not assume that anything except pa.req is valid
+ * after calling ->parse_opt() as one might reuse pa
+ * with iplink_parse(). See link_veth.c as an example.
+ */
addattr_nest_end(&req.n, data);
} else if (argc) {
if (matches(*argv, "help") == 0)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index f906e7f..bf03a6e 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -157,8 +157,10 @@ static void explain(void)
}
static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u8 mode, use_carrier, primary_reselect, fail_over_mac;
__u8 xmit_hash_policy, num_peer_notif, all_slaves_active;
__u8 lacp_rate, ad_select, tlb_dynamic_lb;
diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c
index 67219c6..d7e2a1e 100644
--- a/ip/iplink_bond_slave.c
+++ b/ip/iplink_bond_slave.c
@@ -120,8 +120,10 @@ static void bond_slave_print_opt(struct link_util *lu, FILE *f, struct rtattr *t
}
static int bond_slave_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u16 queue_id;
while (argc > 0) {
diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index 3008e44..9a8560a 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -80,8 +80,10 @@ void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len)
}
static int bridge_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u32 val;
while (argc > 0) {
diff --git a/ip/iplink_bridge_slave.c b/ip/iplink_bridge_slave.c
index 3fbfb87..31fde1c 100644
--- a/ip/iplink_bridge_slave.c
+++ b/ip/iplink_bridge_slave.c
@@ -292,8 +292,10 @@ static void bridge_slave_parse_on_off(char *arg_name, char *arg_val,
}
static int bridge_slave_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u8 state;
__u16 priority;
__u32 cost;
diff --git a/ip/iplink_can.c b/ip/iplink_can.c
index 587413d..36ef917 100644
--- a/ip/iplink_can.c
+++ b/ip/iplink_can.c
@@ -110,8 +110,10 @@ static void print_ctrlmode(FILE *f, __u32 cm)
}
static int can_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
struct can_bittiming bt = {}, dbt = {};
struct can_ctrlmode cm = {0, 0};
diff --git a/ip/iplink_geneve.c b/ip/iplink_geneve.c
index 1fcdd83..4b2bf29 100644
--- a/ip/iplink_geneve.c
+++ b/ip/iplink_geneve.c
@@ -55,8 +55,10 @@ static void check_duparg(__u64 *attrs, int type, const char *key,
}
static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
inet_prefix daddr;
__u32 vni = 0;
__u32 label = 0;
diff --git a/ip/iplink_hsr.c b/ip/iplink_hsr.c
index c673ccf..d020676 100644
--- a/ip/iplink_hsr.c
+++ b/ip/iplink_hsr.c
@@ -44,8 +44,10 @@ static void usage(void)
}
static int hsr_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
int ifindex;
unsigned char multicast_spec;
unsigned char protocol_version;
diff --git a/ip/iplink_ipoib.c b/ip/iplink_ipoib.c
index e69bda0..f95c79c 100644
--- a/ip/iplink_ipoib.c
+++ b/ip/iplink_ipoib.c
@@ -42,8 +42,10 @@ static int mode_arg(void)
}
static int ipoib_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u16 pkey, mode, umcast;
while (argc > 0) {
diff --git a/ip/iplink_ipvlan.c b/ip/iplink_ipvlan.c
index 8889808..2b8a3cc 100644
--- a/ip/iplink_ipvlan.c
+++ b/ip/iplink_ipvlan.c
@@ -30,8 +30,10 @@ static void ipvlan_explain(FILE *f)
}
static int ipvlan_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u16 flags = 0;
bool mflag_given = false;
diff --git a/ip/iplink_macvlan.c b/ip/iplink_macvlan.c
index b966a61..6f97415 100644
--- a/ip/iplink_macvlan.c
+++ b/ip/iplink_macvlan.c
@@ -63,8 +63,10 @@ static int flag_arg(const char *arg)
}
static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u32 mode = 0;
__u16 flags = 0;
__u32 mac_mode = 0;
diff --git a/ip/iplink_vlan.c b/ip/iplink_vlan.c
index 74f4614..6c0c12d 100644
--- a/ip/iplink_vlan.c
+++ b/ip/iplink_vlan.c
@@ -82,8 +82,10 @@ static int vlan_parse_qos_map(int *argcp, char ***argvp, struct nlmsghdr *n,
}
static int vlan_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
struct ifla_vlan_flags flags = { 0 };
__u16 id, proto;
diff --git a/ip/iplink_vrf.c b/ip/iplink_vrf.c
index e9dd0df..b943ed9 100644
--- a/ip/iplink_vrf.c
+++ b/ip/iplink_vrf.c
@@ -30,8 +30,11 @@ static void explain(void)
}
static int vrf_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
+
while (argc > 0) {
if (matches(*argv, "table") == 0) {
__u32 table;
diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index 43c80e7..a234e34 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -31,11 +31,10 @@ static void usage(void)
}
static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
- char *dev = NULL;
- char *name = NULL;
- char *type = NULL;
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
int err;
struct rtattr *data;
struct ifinfomsg ifm_save, *ifm, *peer_ifm;
@@ -45,7 +44,7 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
return -1;
}
- ifm = NLMSG_DATA(n);
+ ifm = &req->i;
memcpy(&ifm_save, ifm, sizeof(*ifm));
ifm->ifi_index = 0;
ifm->ifi_flags = 0;
@@ -55,12 +54,11 @@ 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, &dev);
+ err = iplink_parse(argc - 1, argv + 1, pa);
if (err < 0)
return err;
- if (type)
+ if (pa->type)
duparg("type", argv[err]);
peer_ifm = RTA_DATA(data);
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 0948f42..352d709 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -72,8 +72,10 @@ static void check_duparg(__u64 *attrs, int type, const char *key,
}
static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
inet_prefix saddr, daddr;
__u32 vni = 0;
__u8 learning = 1;
diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index c0b45f5..144141f 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -1132,8 +1132,10 @@ static void usage(FILE *f)
}
static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
int ret;
__u8 encoding_sa = 0xff;
__u32 window = -1;
diff --git a/ip/link_gre.c b/ip/link_gre.c
index a43cf6a..9e598e3 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -64,8 +64,10 @@ static void gre_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
}
static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u16 iflags = 0;
__u16 oflags = 0;
__be32 ikey = 0;
@@ -91,7 +93,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct ifinfomsg *ifi = &req->i;
struct {
struct nlmsghdr n;
struct ifinfomsg i;
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index d7e0d8e..87fd2cd 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -75,8 +75,10 @@ static void gre_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
}
static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__u16 iflags = 0;
__u16 oflags = 0;
__be32 ikey = 0;
@@ -102,7 +104,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct ifinfomsg *ifi = &req->i;
struct {
struct nlmsghdr n;
struct ifinfomsg i;
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index bffccbd..c3076a3 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -75,8 +75,10 @@ static void ip6tunnel_print_help(struct link_util *lu, int argc, char **argv,
}
static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
inet_prefix saddr, daddr;
__u8 hop_limit = DEFAULT_TNL_HOP_LIMIT;
__u8 encap_limit = IPV6_DEFAULT_TNL_ENCAP_LIMIT;
@@ -95,7 +97,7 @@ static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct ifinfomsg *ifi = &req->i;
struct {
struct nlmsghdr n;
struct ifinfomsg i;
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 6c49739..58491c3 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -72,8 +72,10 @@ static void iptunnel_print_help(struct link_util *lu, int argc, char **argv,
}
static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
inet_prefix saddr, daddr, ip6rdprefix, ip6rdrelayprefix;
__u8 pmtudisc = 1;
__u8 tos = 0;
@@ -95,7 +97,7 @@ static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&ip6rdrelayprefix);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct ifinfomsg *ifi = &req->i;
struct {
struct nlmsghdr n;
struct ifinfomsg i;
diff --git a/ip/link_veth.c b/ip/link_veth.c
index bd8f36e..00d5cf7 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -29,11 +29,10 @@ static void usage(void)
}
static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
- char *dev = NULL;
- char *name = NULL;
- char *type = NULL;
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
int err;
struct rtattr *data;
struct ifinfomsg ifm_save, *ifm, *peer_ifm;
@@ -43,7 +42,7 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
return -1;
}
- ifm = NLMSG_DATA(n);
+ ifm = &req->i;
memcpy(&ifm_save, ifm, sizeof(*ifm));
ifm->ifi_index = 0;
ifm->ifi_flags = 0;
@@ -53,12 +52,11 @@ 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, &dev);
+ err = iplink_parse(argc - 1, argv + 1, pa);
if (err < 0)
return err;
- if (type)
+ if (pa->type)
duparg("type", argv[err]);
peer_ifm = RTA_DATA(data);
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 1429f07..69724f0 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -45,8 +45,10 @@ static void vti_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
}
static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__be32 ikey = 0;
__be32 okey = 0;
inet_prefix saddr, daddr;
@@ -57,7 +59,7 @@ static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct ifinfomsg *ifi = &req->i;
struct {
struct nlmsghdr n;
struct ifinfomsg i;
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 2956c0e..76cc2d4 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -47,8 +47,10 @@ static void vti6_print_help(struct link_util *lu, int argc, char **argv,
}
static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
- struct nlmsghdr *n)
+ struct iplink_parse_args *pa)
{
+ struct iplink_req *req = pa->req;
+ struct nlmsghdr *n = &req->n;
__be32 ikey = 0;
__be32 okey = 0;
inet_prefix saddr, daddr;
@@ -59,7 +61,7 @@ static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
inet_prefix_reset(&daddr);
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
- struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct ifinfomsg *ifi = &req->i;
struct {
struct nlmsghdr n;
struct ifinfomsg i;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 19:39 ` [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit() Serhey Popovych
@ 2018-02-20 20:08 ` Stephen Hemminger
2018-02-20 20:17 ` Serhey Popovych
0 siblings, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2018-02-20 20:08 UTC (permalink / raw)
To: Serhey Popovych; +Cc: netdev, dsahern
On Tue, 20 Feb 2018 21:39:51 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:
> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
> ---
> ip/iplink.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/ip/iplink.c b/ip/iplink.c
> index 74c377c..a2c8108 100644
> --- a/ip/iplink.c
> +++ b/ip/iplink.c
> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
> NEXT_ARG();
> if (xdp_parse(&argc, &argv, req, dev_index,
> generic, drv, offload))
> - exit(-1);
> + return -1;
> } else if (strcmp(*argv, "netns") == 0) {
> NEXT_ARG();
> if (netns != -1)
> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
> if (!dev) {
> fprintf(stderr,
> "Not enough information: \"dev\" argument is required.\n");
> - exit(-1);
> + return -1;
> }
> if (cmd == RTM_NEWLINK && index) {
> fprintf(stderr,
> "index can be used only when creating devices.\n");
> - exit(-1);
> + return -1;
> }
>
> req.i.ifi_index = ll_name_to_index(dev);
> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
> if (!dev) {
> fprintf(stderr,
> "Not enough of information: \"dev\" argument is required.\n");
> - exit(-1);
> + return -1;
> }
>
> if (newaddr || newbrd) {
> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
> fprintf(stderr,
> "Command \"%s\" is unknown, try \"ip link help\".\n",
> *argv);
> - exit(-1);
> + return -1;
> }
>
> argv++; argc--;
> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
>
> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
> *argv);
> - exit(-1);
> + return -1;
> }
Not sure I like this. If given bad input in batch it is better to stop and exit
rather than continuing with more bad data.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 20:08 ` Stephen Hemminger
@ 2018-02-20 20:17 ` Serhey Popovych
2018-02-20 20:27 ` David Ahern
0 siblings, 1 reply; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 20:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, dsahern
[-- Attachment #1.1: Type: text/plain, Size: 2237 bytes --]
Stephen Hemminger wrote:
> On Tue, 20 Feb 2018 21:39:51 +0200
> Serhey Popovych <serhe.popovych@gmail.com> wrote:
>
>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>> ---
>> ip/iplink.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/ip/iplink.c b/ip/iplink.c
>> index 74c377c..a2c8108 100644
>> --- a/ip/iplink.c
>> +++ b/ip/iplink.c
>> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
>> NEXT_ARG();
>> if (xdp_parse(&argc, &argv, req, dev_index,
>> generic, drv, offload))
>> - exit(-1);
>> + return -1;
>> } else if (strcmp(*argv, "netns") == 0) {
>> NEXT_ARG();
>> if (netns != -1)
>> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
>> if (!dev) {
>> fprintf(stderr,
>> "Not enough information: \"dev\" argument is required.\n");
>> - exit(-1);
>> + return -1;
>> }
>> if (cmd == RTM_NEWLINK && index) {
>> fprintf(stderr,
>> "index can be used only when creating devices.\n");
>> - exit(-1);
>> + return -1;
>> }
>>
>> req.i.ifi_index = ll_name_to_index(dev);
>> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
>> if (!dev) {
>> fprintf(stderr,
>> "Not enough of information: \"dev\" argument is required.\n");
>> - exit(-1);
>> + return -1;
>> }
>>
>> if (newaddr || newbrd) {
>> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
>> fprintf(stderr,
>> "Command \"%s\" is unknown, try \"ip link help\".\n",
>> *argv);
>> - exit(-1);
>> + return -1;
>> }
>>
>> argv++; argc--;
>> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
>>
>> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
>> *argv);
>> - exit(-1);
>> + return -1;
>> }
>
> Not sure I like this. If given bad input in batch it is better to stop and exit
> rather than continuing with more bad data.
When preparing this change I think in opposite direction: we want to
continue batch mode if single line is broken.
If desired I will drop this one.
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 20:17 ` Serhey Popovych
@ 2018-02-20 20:27 ` David Ahern
2018-02-20 20:33 ` Stephen Hemminger
2018-02-20 20:44 ` Serhey Popovych
0 siblings, 2 replies; 17+ messages in thread
From: David Ahern @ 2018-02-20 20:27 UTC (permalink / raw)
To: Serhey Popovych, Stephen Hemminger; +Cc: netdev
On 2/20/18 1:17 PM, Serhey Popovych wrote:
> Stephen Hemminger wrote:
>> On Tue, 20 Feb 2018 21:39:51 +0200
>> Serhey Popovych <serhe.popovych@gmail.com> wrote:
>>
>>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>>> ---
>>> ip/iplink.c | 12 ++++++------
>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/ip/iplink.c b/ip/iplink.c
>>> index 74c377c..a2c8108 100644
>>> --- a/ip/iplink.c
>>> +++ b/ip/iplink.c
>>> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
>>> NEXT_ARG();
>>> if (xdp_parse(&argc, &argv, req, dev_index,
>>> generic, drv, offload))
>>> - exit(-1);
>>> + return -1;
>>> } else if (strcmp(*argv, "netns") == 0) {
>>> NEXT_ARG();
>>> if (netns != -1)
>>> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
>>> if (!dev) {
>>> fprintf(stderr,
>>> "Not enough information: \"dev\" argument is required.\n");
>>> - exit(-1);
>>> + return -1;
>>> }
>>> if (cmd == RTM_NEWLINK && index) {
>>> fprintf(stderr,
>>> "index can be used only when creating devices.\n");
>>> - exit(-1);
>>> + return -1;
>>> }
>>>
>>> req.i.ifi_index = ll_name_to_index(dev);
>>> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
>>> if (!dev) {
>>> fprintf(stderr,
>>> "Not enough of information: \"dev\" argument is required.\n");
>>> - exit(-1);
>>> + return -1;
>>> }
>>>
>>> if (newaddr || newbrd) {
>>> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
>>> fprintf(stderr,
>>> "Command \"%s\" is unknown, try \"ip link help\".\n",
>>> *argv);
>>> - exit(-1);
>>> + return -1;
>>> }
>>>
>>> argv++; argc--;
>>> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
>>>
>>> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
>>> *argv);
>>> - exit(-1);
>>> + return -1;
>>> }
>>
>> Not sure I like this. If given bad input in batch it is better to stop and exit
>> rather than continuing with more bad data.
>
> When preparing this change I think in opposite direction: we want to
> continue batch mode if single line is broken.
>
batch mode needs to stop on the line that fails. That said, batch still
fails with the /exit/return/ change
$ cat /tmp/ip.batch
li sh
li foo
li add veth1 type veth peer name veth2
Current command
$ ip -batch /tmp/ip.batch
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
DEFAULT group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
<link list snipped>
Command "foo" is unknown, try "ip link help".
$ echo $?
255
$ ip/ip -batch /tmp/ip.batch
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
DEFAULT group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
<link list snipped>
Command "foo" is unknown, try "ip link help".
Command failed /tmp/ip.batch:2
dsa@kenny:~/iproute2/iproute2-next.git$ echo $?
1
I like that better because it tells me the line that fails.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 20:27 ` David Ahern
@ 2018-02-20 20:33 ` Stephen Hemminger
2018-02-20 20:44 ` Roopa Prabhu
2018-02-20 20:44 ` Serhey Popovych
1 sibling, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2018-02-20 20:33 UTC (permalink / raw)
To: David Ahern; +Cc: Serhey Popovych, netdev
On Tue, 20 Feb 2018 13:27:21 -0700
David Ahern <dsahern@gmail.com> wrote:
> On 2/20/18 1:17 PM, Serhey Popovych wrote:
> > Stephen Hemminger wrote:
> >> On Tue, 20 Feb 2018 21:39:51 +0200
> >> Serhey Popovych <serhe.popovych@gmail.com> wrote:
> >>
> >>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
> >>> ---
> >>> ip/iplink.c | 12 ++++++------
> >>> 1 file changed, 6 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/ip/iplink.c b/ip/iplink.c
> >>> index 74c377c..a2c8108 100644
> >>> --- a/ip/iplink.c
> >>> +++ b/ip/iplink.c
> >>> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
> >>> NEXT_ARG();
> >>> if (xdp_parse(&argc, &argv, req, dev_index,
> >>> generic, drv, offload))
> >>> - exit(-1);
> >>> + return -1;
> >>> } else if (strcmp(*argv, "netns") == 0) {
> >>> NEXT_ARG();
> >>> if (netns != -1)
> >>> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
> >>> if (!dev) {
> >>> fprintf(stderr,
> >>> "Not enough information: \"dev\" argument is required.\n");
> >>> - exit(-1);
> >>> + return -1;
> >>> }
> >>> if (cmd == RTM_NEWLINK && index) {
> >>> fprintf(stderr,
> >>> "index can be used only when creating devices.\n");
> >>> - exit(-1);
> >>> + return -1;
> >>> }
> >>>
> >>> req.i.ifi_index = ll_name_to_index(dev);
> >>> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
> >>> if (!dev) {
> >>> fprintf(stderr,
> >>> "Not enough of information: \"dev\" argument is required.\n");
> >>> - exit(-1);
> >>> + return -1;
> >>> }
> >>>
> >>> if (newaddr || newbrd) {
> >>> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
> >>> fprintf(stderr,
> >>> "Command \"%s\" is unknown, try \"ip link help\".\n",
> >>> *argv);
> >>> - exit(-1);
> >>> + return -1;
> >>> }
> >>>
> >>> argv++; argc--;
> >>> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
> >>>
> >>> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
> >>> *argv);
> >>> - exit(-1);
> >>> + return -1;
> >>> }
> >>
> >> Not sure I like this. If given bad input in batch it is better to stop and exit
> >> rather than continuing with more bad data.
> >
> > When preparing this change I think in opposite direction: we want to
> > continue batch mode if single line is broken.
> >
>
> batch mode needs to stop on the line that fails. That said, batch still
> fails with the /exit/return/ change
>
> $ cat /tmp/ip.batch
> li sh
> li foo
> li add veth1 type veth peer name veth2
>
> Current command
> $ ip -batch /tmp/ip.batch
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
> DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> <link list snipped>
> Command "foo" is unknown, try "ip link help".
> $ echo $?
> 255
>
> $ ip/ip -batch /tmp/ip.batch
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
> DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> <link list snipped>
> Command "foo" is unknown, try "ip link help".
> Command failed /tmp/ip.batch:2
> dsa@kenny:~/iproute2/iproute2-next.git$ echo $?
> 1
>
> I like that better because it tells me the line that fails.
Normally ip batch will exit on errors.
The question is what about -force?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 20:33 ` Stephen Hemminger
@ 2018-02-20 20:44 ` Roopa Prabhu
2018-02-20 20:49 ` David Ahern
0 siblings, 1 reply; 17+ messages in thread
From: Roopa Prabhu @ 2018-02-20 20:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Ahern, Serhey Popovych, netdev
On Tue, Feb 20, 2018 at 12:33 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue, 20 Feb 2018 13:27:21 -0700
> David Ahern <dsahern@gmail.com> wrote:
>
>> On 2/20/18 1:17 PM, Serhey Popovych wrote:
>> > Stephen Hemminger wrote:
>> >> On Tue, 20 Feb 2018 21:39:51 +0200
>> >> Serhey Popovych <serhe.popovych@gmail.com> wrote:
>> >>
>> >>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>> >>> ---
>> >>> ip/iplink.c | 12 ++++++------
>> >>> 1 file changed, 6 insertions(+), 6 deletions(-)
>> >>>
>> >>> diff --git a/ip/iplink.c b/ip/iplink.c
>> >>> index 74c377c..a2c8108 100644
>> >>> --- a/ip/iplink.c
>> >>> +++ b/ip/iplink.c
>> >>> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
>> >>> NEXT_ARG();
>> >>> if (xdp_parse(&argc, &argv, req, dev_index,
>> >>> generic, drv, offload))
>> >>> - exit(-1);
>> >>> + return -1;
>> >>> } else if (strcmp(*argv, "netns") == 0) {
>> >>> NEXT_ARG();
>> >>> if (netns != -1)
>> >>> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
>> >>> if (!dev) {
>> >>> fprintf(stderr,
>> >>> "Not enough information: \"dev\" argument is required.\n");
>> >>> - exit(-1);
>> >>> + return -1;
>> >>> }
>> >>> if (cmd == RTM_NEWLINK && index) {
>> >>> fprintf(stderr,
>> >>> "index can be used only when creating devices.\n");
>> >>> - exit(-1);
>> >>> + return -1;
>> >>> }
>> >>>
>> >>> req.i.ifi_index = ll_name_to_index(dev);
>> >>> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
>> >>> if (!dev) {
>> >>> fprintf(stderr,
>> >>> "Not enough of information: \"dev\" argument is required.\n");
>> >>> - exit(-1);
>> >>> + return -1;
>> >>> }
>> >>>
>> >>> if (newaddr || newbrd) {
>> >>> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
>> >>> fprintf(stderr,
>> >>> "Command \"%s\" is unknown, try \"ip link help\".\n",
>> >>> *argv);
>> >>> - exit(-1);
>> >>> + return -1;
>> >>> }
>> >>>
>> >>> argv++; argc--;
>> >>> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
>> >>>
>> >>> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
>> >>> *argv);
>> >>> - exit(-1);
>> >>> + return -1;
>> >>> }
>> >>
>> >> Not sure I like this. If given bad input in batch it is better to stop and exit
>> >> rather than continuing with more bad data.
>> >
>> > When preparing this change I think in opposite direction: we want to
>> > continue batch mode if single line is broken.
>> >
>>
>> batch mode needs to stop on the line that fails. That said, batch still
>> fails with the /exit/return/ change
>>
>> $ cat /tmp/ip.batch
>> li sh
>> li foo
>> li add veth1 type veth peer name veth2
>>
>> Current command
>> $ ip -batch /tmp/ip.batch
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
>> DEFAULT group default qlen 1000
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>
>> <link list snipped>
>> Command "foo" is unknown, try "ip link help".
>> $ echo $?
>> 255
>>
>> $ ip/ip -batch /tmp/ip.batch
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
>> DEFAULT group default qlen 1000
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> <link list snipped>
>> Command "foo" is unknown, try "ip link help".
>> Command failed /tmp/ip.batch:2
>> dsa@kenny:~/iproute2/iproute2-next.git$ echo $?
>> 1
>>
>> I like that better because it tells me the line that fails.
>
> Normally ip batch will exit on errors.
> The question is what about -force?
on -force, it needs to continue.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 20:27 ` David Ahern
2018-02-20 20:33 ` Stephen Hemminger
@ 2018-02-20 20:44 ` Serhey Popovych
1 sibling, 0 replies; 17+ messages in thread
From: Serhey Popovych @ 2018-02-20 20:44 UTC (permalink / raw)
To: David Ahern, Stephen Hemminger; +Cc: netdev
[-- Attachment #1.1: Type: text/plain, Size: 3432 bytes --]
David Ahern wrote:
> On 2/20/18 1:17 PM, Serhey Popovych wrote:
>> Stephen Hemminger wrote:
>>> On Tue, 20 Feb 2018 21:39:51 +0200
>>> Serhey Popovych <serhe.popovych@gmail.com> wrote:
>>>
>>>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>>>> ---
>>>> ip/iplink.c | 12 ++++++------
>>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/ip/iplink.c b/ip/iplink.c
>>>> index 74c377c..a2c8108 100644
>>>> --- a/ip/iplink.c
>>>> +++ b/ip/iplink.c
>>>> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
>>>> NEXT_ARG();
>>>> if (xdp_parse(&argc, &argv, req, dev_index,
>>>> generic, drv, offload))
>>>> - exit(-1);
>>>> + return -1;
>>>> } else if (strcmp(*argv, "netns") == 0) {
>>>> NEXT_ARG();
>>>> if (netns != -1)
>>>> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
>>>> if (!dev) {
>>>> fprintf(stderr,
>>>> "Not enough information: \"dev\" argument is required.\n");
>>>> - exit(-1);
>>>> + return -1;
>>>> }
>>>> if (cmd == RTM_NEWLINK && index) {
>>>> fprintf(stderr,
>>>> "index can be used only when creating devices.\n");
>>>> - exit(-1);
>>>> + return -1;
>>>> }
>>>>
>>>> req.i.ifi_index = ll_name_to_index(dev);
>>>> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
>>>> if (!dev) {
>>>> fprintf(stderr,
>>>> "Not enough of information: \"dev\" argument is required.\n");
>>>> - exit(-1);
>>>> + return -1;
>>>> }
>>>>
>>>> if (newaddr || newbrd) {
>>>> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
>>>> fprintf(stderr,
>>>> "Command \"%s\" is unknown, try \"ip link help\".\n",
>>>> *argv);
>>>> - exit(-1);
>>>> + return -1;
>>>> }
>>>>
>>>> argv++; argc--;
>>>> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
>>>>
>>>> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
>>>> *argv);
>>>> - exit(-1);
>>>> + return -1;
>>>> }
>>>
>>> Not sure I like this. If given bad input in batch it is better to stop and exit
>>> rather than continuing with more bad data.
>>
>> When preparing this change I think in opposite direction: we want to
>> continue batch mode if single line is broken.
>>
>
> batch mode needs to stop on the line that fails. That said, batch still
> fails with the /exit/return/ change
>
> $ cat /tmp/ip.batch
> li sh
> li foo
> li add veth1 type veth peer name veth2
>
> Current command
> $ ip -batch /tmp/ip.batch
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
> DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> <link list snipped>
> Command "foo" is unknown, try "ip link help".
> $ echo $?
> 255
>
> $ ip/ip -batch /tmp/ip.batch
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
> DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> <link list snipped>
> Command "foo" is unknown, try "ip link help".
> Command failed /tmp/ip.batch:2
> dsa@kenny:~/iproute2/iproute2-next.git$ echo $?
> 1
>
> I like that better because it tells me the line that fails.
>
Okay, now is clear. Will prepare v2 according to these rules.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 20:44 ` Roopa Prabhu
@ 2018-02-20 20:49 ` David Ahern
2018-02-20 22:36 ` Roopa Prabhu
0 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2018-02-20 20:49 UTC (permalink / raw)
To: Roopa Prabhu, Stephen Hemminger; +Cc: Serhey Popovych, netdev
On 2/20/18 1:44 PM, Roopa Prabhu wrote:
> On Tue, Feb 20, 2018 at 12:33 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>> On Tue, 20 Feb 2018 13:27:21 -0700
>> David Ahern <dsahern@gmail.com> wrote:
>>
>>> On 2/20/18 1:17 PM, Serhey Popovych wrote:
>>>> Stephen Hemminger wrote:
>>>>> On Tue, 20 Feb 2018 21:39:51 +0200
>>>>> Serhey Popovych <serhe.popovych@gmail.com> wrote:
>>>>>
>>>>>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>>>>>> ---
>>>>>> ip/iplink.c | 12 ++++++------
>>>>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/ip/iplink.c b/ip/iplink.c
>>>>>> index 74c377c..a2c8108 100644
>>>>>> --- a/ip/iplink.c
>>>>>> +++ b/ip/iplink.c
>>>>>> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
>>>>>> NEXT_ARG();
>>>>>> if (xdp_parse(&argc, &argv, req, dev_index,
>>>>>> generic, drv, offload))
>>>>>> - exit(-1);
>>>>>> + return -1;
>>>>>> } else if (strcmp(*argv, "netns") == 0) {
>>>>>> NEXT_ARG();
>>>>>> if (netns != -1)
>>>>>> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
>>>>>> if (!dev) {
>>>>>> fprintf(stderr,
>>>>>> "Not enough information: \"dev\" argument is required.\n");
>>>>>> - exit(-1);
>>>>>> + return -1;
>>>>>> }
>>>>>> if (cmd == RTM_NEWLINK && index) {
>>>>>> fprintf(stderr,
>>>>>> "index can be used only when creating devices.\n");
>>>>>> - exit(-1);
>>>>>> + return -1;
>>>>>> }
>>>>>>
>>>>>> req.i.ifi_index = ll_name_to_index(dev);
>>>>>> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
>>>>>> if (!dev) {
>>>>>> fprintf(stderr,
>>>>>> "Not enough of information: \"dev\" argument is required.\n");
>>>>>> - exit(-1);
>>>>>> + return -1;
>>>>>> }
>>>>>>
>>>>>> if (newaddr || newbrd) {
>>>>>> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
>>>>>> fprintf(stderr,
>>>>>> "Command \"%s\" is unknown, try \"ip link help\".\n",
>>>>>> *argv);
>>>>>> - exit(-1);
>>>>>> + return -1;
>>>>>> }
>>>>>>
>>>>>> argv++; argc--;
>>>>>> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
>>>>>>
>>>>>> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
>>>>>> *argv);
>>>>>> - exit(-1);
>>>>>> + return -1;
>>>>>> }
>>>>>
>>>>> Not sure I like this. If given bad input in batch it is better to stop and exit
>>>>> rather than continuing with more bad data.
>>>>
>>>> When preparing this change I think in opposite direction: we want to
>>>> continue batch mode if single line is broken.
>>>>
>>>
>>> batch mode needs to stop on the line that fails. That said, batch still
>>> fails with the /exit/return/ change
>>>
>>> $ cat /tmp/ip.batch
>>> li sh
>>> li foo
>>> li add veth1 type veth peer name veth2
>>>
>>> Current command
>>> $ ip -batch /tmp/ip.batch
>>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
>>> DEFAULT group default qlen 1000
>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>>
>>> <link list snipped>
>>> Command "foo" is unknown, try "ip link help".
>>> $ echo $?
>>> 255
>>>
>>> $ ip/ip -batch /tmp/ip.batch
>>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
>>> DEFAULT group default qlen 1000
>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>> <link list snipped>
>>> Command "foo" is unknown, try "ip link help".
>>> Command failed /tmp/ip.batch:2
>>> dsa@kenny:~/iproute2/iproute2-next.git$ echo $?
>>> 1
>>>
>>> I like that better because it tells me the line that fails.
>>
>> Normally ip batch will exit on errors.
>> The question is what about -force?
>
> on -force, it needs to continue.
>
It does.
$ ip/ip -batch /tmp/ip.batch -force
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
DEFAULT group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
<snipped>
Command "foo" is unknown, try "ip link help".
Command failed /tmp/ip.batch:2
RTNETLINK answers: Operation not permitted
Command failed /tmp/ip.batch:3
Which is an improvement over today where it just exits - ignoring the force.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit()
2018-02-20 20:49 ` David Ahern
@ 2018-02-20 22:36 ` Roopa Prabhu
0 siblings, 0 replies; 17+ messages in thread
From: Roopa Prabhu @ 2018-02-20 22:36 UTC (permalink / raw)
To: David Ahern; +Cc: Stephen Hemminger, Serhey Popovych, netdev
On Tue, Feb 20, 2018 at 12:49 PM, David Ahern <dsahern@gmail.com> wrote:
> On 2/20/18 1:44 PM, Roopa Prabhu wrote:
>> On Tue, Feb 20, 2018 at 12:33 PM, Stephen Hemminger
>> <stephen@networkplumber.org> wrote:
>>> On Tue, 20 Feb 2018 13:27:21 -0700
>>> David Ahern <dsahern@gmail.com> wrote:
>>>
>>>> On 2/20/18 1:17 PM, Serhey Popovych wrote:
>>>>> Stephen Hemminger wrote:
>>>>>> On Tue, 20 Feb 2018 21:39:51 +0200
>>>>>> Serhey Popovych <serhe.popovych@gmail.com> wrote:
>>>>>>
>>>>>>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>>>>>>> ---
>>>>>>> ip/iplink.c | 12 ++++++------
>>>>>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/ip/iplink.c b/ip/iplink.c
>>>>>>> index 74c377c..a2c8108 100644
>>>>>>> --- a/ip/iplink.c
>>>>>>> +++ b/ip/iplink.c
>>>>>>> @@ -653,7 +653,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
>>>>>>> NEXT_ARG();
>>>>>>> if (xdp_parse(&argc, &argv, req, dev_index,
>>>>>>> generic, drv, offload))
>>>>>>> - exit(-1);
>>>>>>> + return -1;
>>>>>>> } else if (strcmp(*argv, "netns") == 0) {
>>>>>>> NEXT_ARG();
>>>>>>> if (netns != -1)
>>>>>>> @@ -972,12 +972,12 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
>>>>>>> if (!dev) {
>>>>>>> fprintf(stderr,
>>>>>>> "Not enough information: \"dev\" argument is required.\n");
>>>>>>> - exit(-1);
>>>>>>> + return -1;
>>>>>>> }
>>>>>>> if (cmd == RTM_NEWLINK && index) {
>>>>>>> fprintf(stderr,
>>>>>>> "index can be used only when creating devices.\n");
>>>>>>> - exit(-1);
>>>>>>> + return -1;
>>>>>>> }
>>>>>>>
>>>>>>> req.i.ifi_index = ll_name_to_index(dev);
>>>>>>> @@ -1392,7 +1392,7 @@ static int do_set(int argc, char **argv)
>>>>>>> if (!dev) {
>>>>>>> fprintf(stderr,
>>>>>>> "Not enough of information: \"dev\" argument is required.\n");
>>>>>>> - exit(-1);
>>>>>>> + return -1;
>>>>>>> }
>>>>>>>
>>>>>>> if (newaddr || newbrd) {
>>>>>>> @@ -1553,7 +1553,7 @@ static int iplink_afstats(int argc, char **argv)
>>>>>>> fprintf(stderr,
>>>>>>> "Command \"%s\" is unknown, try \"ip link help\".\n",
>>>>>>> *argv);
>>>>>>> - exit(-1);
>>>>>>> + return -1;
>>>>>>> }
>>>>>>>
>>>>>>> argv++; argc--;
>>>>>>> @@ -1648,5 +1648,5 @@ int do_iplink(int argc, char **argv)
>>>>>>>
>>>>>>> fprintf(stderr, "Command \"%s\" is unknown, try \"ip link help\".\n",
>>>>>>> *argv);
>>>>>>> - exit(-1);
>>>>>>> + return -1;
>>>>>>> }
>>>>>>
>>>>>> Not sure I like this. If given bad input in batch it is better to stop and exit
>>>>>> rather than continuing with more bad data.
>>>>>
>>>>> When preparing this change I think in opposite direction: we want to
>>>>> continue batch mode if single line is broken.
>>>>>
>>>>
>>>> batch mode needs to stop on the line that fails. That said, batch still
>>>> fails with the /exit/return/ change
>>>>
>>>> $ cat /tmp/ip.batch
>>>> li sh
>>>> li foo
>>>> li add veth1 type veth peer name veth2
>>>>
>>>> Current command
>>>> $ ip -batch /tmp/ip.batch
>>>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
>>>> DEFAULT group default qlen 1000
>>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>>>
>>>> <link list snipped>
>>>> Command "foo" is unknown, try "ip link help".
>>>> $ echo $?
>>>> 255
>>>>
>>>> $ ip/ip -batch /tmp/ip.batch
>>>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
>>>> DEFAULT group default qlen 1000
>>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>>> <link list snipped>
>>>> Command "foo" is unknown, try "ip link help".
>>>> Command failed /tmp/ip.batch:2
>>>> dsa@kenny:~/iproute2/iproute2-next.git$ echo $?
>>>> 1
>>>>
>>>> I like that better because it tells me the line that fails.
>>>
>>> Normally ip batch will exit on errors.
>>> The question is what about -force?
>>
>> on -force, it needs to continue.
>>
>
>
> It does.
>
> $ ip/ip -batch /tmp/ip.batch -force
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode
> DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> <snipped>
> Command "foo" is unknown, try "ip link help".
> Command failed /tmp/ip.batch:2
> RTNETLINK answers: Operation not permitted
> Command failed /tmp/ip.batch:3
>
> Which is an improvement over today where it just exits - ignoring the force.
cool :).
some other iproute2 commands have also received patches to continue on
-force over the last few years.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-02-20 22:36 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-20 19:39 [PATCH iproute2-next 0/8] iplink: Improve iplink_parse() Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 1/8] iplink: Return from function instead of calling exit() Serhey Popovych
2018-02-20 20:08 ` Stephen Hemminger
2018-02-20 20:17 ` Serhey Popovych
2018-02-20 20:27 ` David Ahern
2018-02-20 20:33 ` Stephen Hemminger
2018-02-20 20:44 ` Roopa Prabhu
2018-02-20 20:49 ` David Ahern
2018-02-20 22:36 ` Roopa Prabhu
2018-02-20 20:44 ` Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 2/8] utils: Introduce and use nodev() helper routine Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 3/8] iplink: Correctly report error when network device isn't found Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 4/8] iplink: Use "dev" and "name" parameters interchangeable when possible Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 5/8] iplink: Follow documented behaviour when "index" is given Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 6/8] iplink: Perform most of request buffer setups and checks in iplink_parse() Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 7/8] iplink: Move data structures to block of their users Serhey Popovych
2018-02-20 19:39 ` [PATCH iproute2-next 8/8] iplink: Reduce number of arguments to iplink_parse() Serhey Popovych
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).