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

[Athira Rajeev <atrajeev@linux.vnet.ibm.com>]

> On 14-May-2022, at 12:02 AM, Ian Rogers <irogers@google.com> wrote:
> 
> On Thu, May 12, 2022 at 11:16 PM Disha Goel <disgoel@linux.vnet.ibm.com> wrote:
>> 
>> 
>> 
>> -----Original Message-----
>> From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> To: acme@kernel.org, jolsa@kernel.org
>> Cc: mpe@ellerman.id.au, linux-perf-users@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, maddy@linux.vnet.ibm.com, rnsastry@linux.ibm.com, kjain@linux.ibm.com, disgoel@linux.vnet.ibm.com, irogers@google.com
>> Subject: [PATCH V2] tools/perf/tests: Skip perf BPF test if clang is not present
>> Date: Wed, 11 May 2022 17:24:38 +0530
>> 
>> 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(-)
>> 
>> 
>> Tested the patch on powerpc and powernv, perf BPF test works fine with the patch.
>> 
>> Tested-by: Disha Goel <disgoel@linux.vnet.ibm.com>
> 
> Acked-by: Ian Rogers <irogers@google.com>
> 
> Thanks!
> Ian
> 

Hi,

Thanks all for the review.

Athira

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