Re: [PATCH] perf test: Fix perf test 42

[Thomas Richter]

On 3/23/21 7:06 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 22, 2021 at 01:53:39PM +0100, Thomas Richter escreveu:
>> For some time now the perf test 42: BPF filter returns an error
>> on bpf relocation subtest, at least on x86 and s390. This is caused by
>>
>> commit d859900c4c56 ("bpf, libbpf: support global data/bss/rodata sections")
>>
>> which introduces support for global variables in eBPF programs. At least
>> for global variables defined static.
>>
>> Perf test 42 checks that the eBPF relocation fails when the eBPF program
>> contains a global variable. It returns OK when the eBPF program
>> could not be loaded and FAILED otherwise.
>>
>> With above commit the test logic for the eBPF relocation need to change:
>> 1. The function prepare_bpf() now always succeeds, the eBPF program
>>    compiled without errors and returns a valid object pointer instead of
>>    NULL.
>> 2. There is no kprobe named sys_write, it now named ksys_write.
>> 3. The function do_test() now returns TEST_FAIL because function
>>    parse_events_load_bpf_obj() can not execute the eBPF program. The
>>    eBPF verifier complains on an invalid map pointer:
>>       libbpf: load bpf program failed: Permission denied
>>       libbpf: -- BEGIN DUMP LOG ---
>>       libbpf:
>>       0: (b7) r1 = 0
>>       1: (63) *(u32 *)(r10 -4) = r1
>>       last_idx 1 first_idx 0
>>       regs=2 stack=0 before 0: (b7) r1 = 0
>>       2: (63) *(u32 *)(r10 -8) = r1
>>       3: (bf) r2 = r10
>>       4: (07) r2 += -4
>>       5: (bf) r3 = r10
>>       6: (07) r3 += -8
>>       7: (18) r1 = 0x380006ce000
>>       9: (b7) r4 = 0
>>       10: (85) call bpf_map_update_elem#2
>>       R1 type=map_value expected=map_ptr
>>
>> Fix this by added logic to handle the kernel verifier return code:
>> 1. Add function myksys_write() to cope with successful compile.
>> 2. Use kprobe ksys_write
>> 3. Handle eBPF verifier error.
>>
>> Output after:
>>  42: BPF filter                          :
>>  42.1: Basic BPF filtering               : Ok
>>  42.2: BPF pinning                       : Ok
>>  42.3: BPF prologue generation           : Ok
>>  42.4: BPF relocation checker            : Failed
>>  #
>>
>> Output after:
>>  # ./perf test -F 42
>>  42: BPF filter                          :
>>  42.1: Basic BPF filtering               : Ok
>>  42.2: BPF pinning                       : Ok
>>  42.3: BPF prologue generation           : Ok
>>  42.4: BPF relocation checker            : Ok
>>  #
>>
>> Signed-off-by: Thomas Richter <tmricht@xxxxxxxxxxxxx>
>> ---
>>  tools/perf/tests/bpf-script-test-relocation.c |  4 ++--
>>  tools/perf/tests/bpf.c                        | 11 +++++++++++
>>  2 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/tests/bpf-script-test-relocation.c b/tools/perf/tests/bpf-script-test-relocation.c
>> index 74006e4b2d24..f8f8176ad4d1 100644
>> --- a/tools/perf/tests/bpf-script-test-relocation.c
>> +++ b/tools/perf/tests/bpf-script-test-relocation.c
>> @@ -34,8 +34,8 @@ struct bpf_map_def SEC("maps") my_table = {
>>  
>>  int this_is_a_global_val;
>>  
>> -SEC("func=sys_write")
>> -int bpf_func__sys_write(void *ctx)
>> +SEC("func=ksys_write")
>> +int bpf_func__ksys_write(void *ctx)
>>  {
>>  	int key = 0;
>>  	int value = 0;
>> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
>> index f57e075b0ed2..d60ef9472d3d 100644
>> --- a/tools/perf/tests/bpf.c
>> +++ b/tools/perf/tests/bpf.c
>> @@ -59,6 +59,11 @@ static int llseek_loop(void)
>>  
>>  #endif
>>  
>> +static int myksys_write(void)
>> +{
>> +	return 0;
>> +}
>> +
>>  static struct {
>>  	enum test_llvm__testcase prog_id;
>>  	const char *desc;
>> @@ -105,6 +110,7 @@ static struct {
>>  		.name		  = "[bpf_relocation_test]",
>>  		.msg_compile_fail = "fix 'perf test LLVM' first",
>>  		.msg_load_fail	  = "libbpf error when dealing with relocation",
>> +		.target_func	  = &myksys_write,
>>  	},
>>  };
>>  
>> @@ -258,6 +264,11 @@ static int __test__bpf(int idx)
>>  		ret = do_test(obj,
>>  			      bpf_testcase_table[idx].target_func,
>>  			      bpf_testcase_table[idx].expect_result);
>> +		if (bpf_testcase_table[idx].prog_id == LLVM_TESTCASE_BPF_RELOCATION
>> +		    && ret == TEST_FAIL) {
>> +			ret = TEST_OK;
>> +			goto out;
>> +		}
> 
> At this point, if it doesn't matter if it fails or succeeds, just drop
> this test case?
> 
> - Arnaldo
> 

Arnaldo,

dropping this test case is fine with me. Support for eBPF global variables is now
part of the linux kernel and there are test programs in the 
.../testing/selftests/bpf directory. File prog_tests/global_data.c for example.

If you agree, I will provide a patch to remove the test case 42.4.

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294