Re: [PATCH] tools/perf/tests: Skip perf BPF test if clang is not present

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Thu, May 05, 2022 at 03:30:39PM +0530, Athira Rajeev escreveu:
> Perf BPF filter test fails in environment where "clang"
> is not installed.
> 
> Test failure logs:
> 
> <<>>
>  42: BPF filter                    :
>  42.1: Basic BPF filtering         : Skip
>  42.2: BPF pinning                 : FAILED!
>  42.3: BPF prologue generation     : FAILED!
> <<>>
> 
> Enabling verbose option provided debug logs which says
> clang/llvm needs to be installed. Snippet of verbose logs:
> 
> <<>>
>  42.2: BPF pinning                  :
>  --- start ---
> test child forked, pid 61423
> ERROR:	unable to find clang.
> Hint:	Try to install latest clang/llvm to support BPF.
>         Check your $PATH
> 
> <<logs_here>>
> 
> Failed to compile test case: 'Basic BPF llvm compile'
> Unable to get BPF object, fix kbuild first
> test child finished with -1
>  ---- end ----
> BPF filter subtest 2: FAILED!
> <<>>
> 
> Here subtests, "BPF pinning" and "BPF prologue generation"
> failed and logs shows clang/llvm is needed. After installing
> clang, testcase passes.
> 
> Reason on why subtest failure happens though logs has proper
> debug information:
> Main function __test__bpf calls test_llvm__fetch_bpf_obj by
> passing 4th argument as true ( 4th arguments maps to parameter
> "force" in test_llvm__fetch_bpf_obj ). But this will cause
> test_llvm__fetch_bpf_obj to skip the check for clang/llvm.
> 
> Snippet of code part which checks for clang based on
> parameter "force" in test_llvm__fetch_bpf_obj:
> 
> <<>>
> if (!force && (!llvm_param.user_set_param &&
> <<>>
> 
> Since force is set to "false", test won't get skipped and
> fails to compile test case. The BPF code compilation needs
> clang, So pass the fourth argument as "false" and also skip
> the test if reason for return is "TEST_SKIP"
> 
> After the patch:
> 
> <<>>
>  42: BPF filter                    :
>  42.1: Basic BPF filtering         : Skip
>  42.2: BPF pinning                 : Skip
>  42.3: BPF prologue generation     : Skip
> <<>>

Wouldn't it be better to add the reason for the skip, like other tests
do?

E.g.:

 23: Watchpoint                                                      :
 23.1: Read Only Watchpoint                                          : Skip (missing hardware support)
 23.2: Write Only Watchpoint                                         : Ok
 23.3: Read / Write Watchpoint                                       : Ok
 23.4: Modify Watchpoint

Something like:

After the patch:

<<>>
 42: BPF filter                    :
 42.1: Basic BPF filtering         : Skip (clang not installed)
 42.2: BPF pinning                 : Skip (clang not installed)
 42.3: BPF prologue generation     : Skip (clang not installed)
<<>>
 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
>  tools/perf/tests/bpf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 57b9591f7cbb..ae62f01239e3 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -222,11 +222,11 @@ static int __test__bpf(int idx)
>  
>  	ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz,
>  				       bpf_testcase_table[idx].prog_id,
> -				       true, NULL);
> +				       false, NULL);
>  	if (ret != TEST_OK || !obj_buf || !obj_buf_sz) {
>  		pr_debug("Unable to get BPF object, %s\n",
>  			 bpf_testcase_table[idx].msg_compile_fail);
> -		if (idx == 0)
> +		if ((idx == 0) || (ret == TEST_SKIP))
>  			return TEST_SKIP;
>  		else
>  			return TEST_FAIL;
> -- 
> 2.35.1

-- 

- Arnaldo