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

[kajoljain <kjain@linux.ibm.com>]

On 5/11/22 17:24, Athira Rajeev wrote:
> 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
> <<>>
> 
> Fixes: ba1fae431e74 ("perf test: Add 'perf test BPF'")
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> Changelog:
>  Addressed review comments from Arnaldo by adding
>  reason for skipping of testcase.
> 
>  tools/perf/tests/bpf.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
Patch looks fine to me.

Reviewed-by: Kajol Jain<kjain@linux.ibm.com>

Thanks,
Kajol Jain

> 
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 57b9591f7cbb..17c023823713 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;
> @@ -364,9 +364,11 @@ static int test__bpf_prologue_test(struct test_suite *test __maybe_unused,
>  static struct test_case bpf_tests[] = {
>  #ifdef HAVE_LIBBPF_SUPPORT
>  	TEST_CASE("Basic BPF filtering", basic_bpf_test),
> -	TEST_CASE("BPF pinning", bpf_pinning),
> +	TEST_CASE_REASON("BPF pinning", bpf_pinning,
> +			"clang isn't installed or environment missing BPF support"),
>  #ifdef HAVE_BPF_PROLOGUE
> -	TEST_CASE("BPF prologue generation", bpf_prologue_test),
> +	TEST_CASE_REASON("BPF prologue generation", bpf_prologue_test,
> +			"clang isn't installed or environment missing BPF support"),
>  #else
>  	TEST_CASE_REASON("BPF prologue generation", bpf_prologue_test, "not compiled in"),
>  #endif