linux-kernel - Re: [PATCH 1/1] Improve missing mods error message and make shell script executable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240821180202.6da48f29@kernel.org>
Date: Wed, 21 Aug 2024 18:02:02 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: David Hunter <david.hunter.linux@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
 shuah@...nel.org, netdev@...r.kernel.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org, javier.carrasco.cruz@...il.com
Subject: Re: [PATCH 1/1] Improve missing mods error message and make shell
 script executable

On Tue, 20 Aug 2024 16:21:16 -0400 David Hunter wrote:
> Subject: [PATCH 1/1] Improve missing mods error message and make shell script executable

Would be good to add a prefix to the subject:

selftests: net:

> Make the test executable. Currently, tests in this shell script are not
> executable, so the scipt file is skipped entirely.

Could you clarify how it gets skipped? We use make [...] run_tests
in our CI and it does seem to run.

> Also, 

If you say "also" there's a good chance the commit should be split into
two..

> the error message descirbing the required modules is inaccurate.
> Currently, only  "SKIP: Need act_mirred module" is shown. As a result,
> users might only that module; however, three modules are actually
> required and if any of them are missing, the build will fail with the
> same message.
> 
> Fix the error message to show any/all modules needed for the script file
> upon failure.
> 
> Signed-off-by: David Hunter <david.hunter.linux@...il.com>
> ---
>  .../testing/selftests/net/test_ingress_egress_chaining.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>  mode change 100644 => 100755 tools/testing/selftests/net/test_ingress_egress_chaining.sh
> 
> diff --git a/tools/testing/selftests/net/test_ingress_egress_chaining.sh b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
> old mode 100644
> new mode 100755
> index 08adff6bb3b6..b1a3d68e0ec2
> --- a/tools/testing/selftests/net/test_ingress_egress_chaining.sh
> +++ b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
> @@ -13,8 +13,14 @@ if [ "$(id -u)" -ne 0 ];then
>  fi
>  
>  needed_mods="act_mirred cls_flower sch_ingress"
> +mods_missing=""
> +
> +for mod in $needed_mods; do 
> +	modinfo $mod &>/dev/null || mods_missing="$mods_missing$mod "
> +done
> +
>  for mod in $needed_mods; do

Do you have to loop again? Maybe just check if mods_missing is empty?

> -	modinfo $mod &>/dev/null || { echo "SKIP: Need act_mirred module"; exit $ksft_skip; }
> +	modinfo $mod &>/dev/null || { echo "SKIP: modules needed: $mods_missing"; exit $ksft_skip; }
>  done
-- 
pw-bot: cr