diff mbox series

[iproute2-next,2/3] devlink: print missing params even if an unknown one is present

Message ID 20240703131521.60284-3-przemyslaw.kitszel@intel.com (mailing list archive)
State Accepted
Commit 77241a525bdce6630ba1fdfb1c8756fa0cc827d6
Delegated to: David Ahern
Headers show
Series minor improvements to makefile, devlink | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Przemek Kitszel July 3, 2024, 1:15 p.m. UTC
Print all of the missing parameters, also in the presence of unknown ones.

Take for example a correct command:
    $ devlink resource set pci/0000:01:00.0 path /kvd/linear size 98304
And remove the "size" keyword:
    $ devlink resource set pci/0000:01:00.0 path /kvd/linear 98304
That yields output:
    Resource size expected.
    Unknown option "98304"

Prior to the patch only the last line of output was present. And if user
would forgot also the "path" keyword, there will be additional line:
    Resource path expected.
in the stderr.

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 devlink/devlink.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

Comments

Michal Kubiak July 4, 2024, 1:33 p.m. UTC | #1
On Wed, Jul 03, 2024 at 03:15:20PM +0200, Przemek Kitszel wrote:
> Print all of the missing parameters, also in the presence of unknown ones.
> 
> Take for example a correct command:
>     $ devlink resource set pci/0000:01:00.0 path /kvd/linear size 98304
> And remove the "size" keyword:
>     $ devlink resource set pci/0000:01:00.0 path /kvd/linear 98304
> That yields output:
>     Resource size expected.
>     Unknown option "98304"
> 
> Prior to the patch only the last line of output was present. And if user
> would forgot also the "path" keyword, there will be additional line:
>     Resource path expected.
> in the stderr.
> 
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>  devlink/devlink.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index 57bcc9658bdb..9907712e3ad0 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -1680,26 +1680,28 @@ static const struct dl_args_metadata dl_args_required[] = {
>  static int dl_args_finding_required_validate(uint64_t o_required,
>  					     uint64_t o_found)
>  {
> -	uint64_t o_flag;
> -	int i;
> +	uint64_t o_flag, o_missing = 0;
> +	int i, err = 0;
>  
>  	for (i = 0; i < ARRAY_SIZE(dl_args_required); i++) {
>  		o_flag = dl_args_required[i].o_flag;
>  		if ((o_required & o_flag) && !(o_found & o_flag)) {
> +			o_missing |= o_flag;
>  			pr_err("%s\n", dl_args_required[i].err_msg);
> -			return -ENOENT;
> +			err = -ENOENT;
>  		}
>  	}
> -	if (o_required & ~o_found) {
> +	if (o_required & ~(o_found | o_missing)) {
>  		pr_err("BUG: unknown argument required but not found\n");
>  		return -EINVAL;
>  	}
> -	return 0;
> +	return err;
>  }
>  
>  static int dl_argv_parse(struct dl *dl, uint64_t o_required,
>  			 uint64_t o_optional)
>  {
> +	const char *unknown_option = NULL;
>  	struct dl_opts *opts = &dl->opts;
>  	uint64_t o_all = o_required | o_optional;
>  	char *str = dl_argv_next(dl);
> @@ -2313,8 +2315,9 @@ static int dl_argv_parse(struct dl *dl, uint64_t o_required,
>  			o_found |= DL_OPT_PORT_FN_MAX_IO_EQS;
>  
>  		} else {
> -			pr_err("Unknown option \"%s\"\n", dl_argv(dl));
> -			return -EINVAL;
> +			if (!unknown_option)
> +				unknown_option = dl_argv(dl);
> +			dl_arg_inc(dl);
>  		}
>  	}
>  
> @@ -2325,7 +2328,15 @@ static int dl_argv_parse(struct dl *dl, uint64_t o_required,
>  
>  	opts->present = o_found;
>  
> -	return dl_args_finding_required_validate(o_required, o_found);
> +	err = dl_args_finding_required_validate(o_required, o_found);
> +
> +	if (unknown_option) {
> +		pr_err("Unknown option \"%s\"\n", unknown_option);
> +		if (!err)
> +			return -EINVAL;
> +	}
> +
> +	return err;
>  }
>  
>  static int dl_argv_dry_parse(struct dl *dl, uint64_t o_required,
> -- 
> 2.39.3
> 
> 

Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
diff mbox series

Patch

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 57bcc9658bdb..9907712e3ad0 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1680,26 +1680,28 @@  static const struct dl_args_metadata dl_args_required[] = {
 static int dl_args_finding_required_validate(uint64_t o_required,
 					     uint64_t o_found)
 {
-	uint64_t o_flag;
-	int i;
+	uint64_t o_flag, o_missing = 0;
+	int i, err = 0;
 
 	for (i = 0; i < ARRAY_SIZE(dl_args_required); i++) {
 		o_flag = dl_args_required[i].o_flag;
 		if ((o_required & o_flag) && !(o_found & o_flag)) {
+			o_missing |= o_flag;
 			pr_err("%s\n", dl_args_required[i].err_msg);
-			return -ENOENT;
+			err = -ENOENT;
 		}
 	}
-	if (o_required & ~o_found) {
+	if (o_required & ~(o_found | o_missing)) {
 		pr_err("BUG: unknown argument required but not found\n");
 		return -EINVAL;
 	}
-	return 0;
+	return err;
 }
 
 static int dl_argv_parse(struct dl *dl, uint64_t o_required,
 			 uint64_t o_optional)
 {
+	const char *unknown_option = NULL;
 	struct dl_opts *opts = &dl->opts;
 	uint64_t o_all = o_required | o_optional;
 	char *str = dl_argv_next(dl);
@@ -2313,8 +2315,9 @@  static int dl_argv_parse(struct dl *dl, uint64_t o_required,
 			o_found |= DL_OPT_PORT_FN_MAX_IO_EQS;
 
 		} else {
-			pr_err("Unknown option \"%s\"\n", dl_argv(dl));
-			return -EINVAL;
+			if (!unknown_option)
+				unknown_option = dl_argv(dl);
+			dl_arg_inc(dl);
 		}
 	}
 
@@ -2325,7 +2328,15 @@  static int dl_argv_parse(struct dl *dl, uint64_t o_required,
 
 	opts->present = o_found;
 
-	return dl_args_finding_required_validate(o_required, o_found);
+	err = dl_args_finding_required_validate(o_required, o_found);
+
+	if (unknown_option) {
+		pr_err("Unknown option \"%s\"\n", unknown_option);
+		if (!err)
+			return -EINVAL;
+	}
+
+	return err;
 }
 
 static int dl_argv_dry_parse(struct dl *dl, uint64_t o_required,