| Message ID | 1519162649-22449-3-git-send-email-serhe.popovych@gmail.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Ahern |
| Headers | show |
| Series | iplink: Improve iplink_parse() | expand |
On 2/20/18 2:37 PM, Serhey Popovych wrote: > 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. > > 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(-) > don't really see any benefit from this one.
David Ahern wrote: > On 2/20/18 2:37 PM, Serhey Popovych wrote: >> 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. >> >> 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(-) >> > > don't really see any benefit from this one. This clearly shows to user what is wrong with it's command line: either dev/name is missing while parsing VF/XDP or it specifies non existent network device (or at least ll_name_to_index() can't resolve it). Moving ifindex check from xdp_parse() to the caller is done to consolidate all dev/name parsing and validating code in single place: iplink_parse(). >
diff --git a/ip/iplink.c b/ip/iplink.c index 5471626..fc358fc 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)
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. 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(-)