| Message ID | 1519304526-18848-3-git-send-email-serhe.popovych@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Ahern |
| Headers | show |
| Series | iplink: Improve iplink_parse() | expand |
On 2/22/18 6:02 AM, Serhey Popovych wrote: > @@ -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); > + I think this is actually the wrong direction. seems to me argv should be passed to xdp_parse rather than the generic, drv, offload bool's. That function can then the check on which option it is and has the knowledge about whether a device is needed or not. > NEXT_ARG(); > if (xdp_parse(&argc, &argv, req, dev_index, > generic, drv, offload))
David Ahern wrote: > On 2/22/18 6:02 AM, Serhey Popovych wrote: >> @@ -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); >> + > > I think this is actually the wrong direction. seems to me argv should be > passed to xdp_parse rather than the generic, drv, offload bool's. That > function can then the check on which option it is and has the knowledge > about whether a device is needed or not. Okay, I probably will prepare another change instead that accounts your suggestions. Will add it to v4 later. > > >> NEXT_ARG(); >> if (xdp_parse(&argc, &argv, req, dev_index, >> generic, drv, offload))
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(-)