Re: [PATCH iproute2-next v2 2/7] iplink: Correctly report error when network device isn't found

[Serhey Popovych]

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 
>> ---
>>  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().

> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH iproute2-next v2 2/7] iplink: Correctly report error when network device isn't found

[David Ahern]

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

[PATCH iproute2-next v2 2/7] iplink: Correctly report error when network device isn't found

[Serhey Popovych]

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 
---
 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 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(, , 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(,  *argv, 0))
invarg("Invalid \"vf\" value\n", *argv);
 
vflist = addattr_nest(>n, sizeof(*req),
  IFLA_VFINFO_LIST);
-   if (dev_index == 0)
-   missarg("dev");
-
len = iplink_parse_vf(vf, , , 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