Re: [PATCH v2 bpf-next 3/3] selftests/bpf: add raw_tp_test_run

[Song Liu <songliubraving@fb.com>]

> On Sep 23, 2020, at 12:40 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> 
> On Wed, Sep 23, 2020 at 9:55 AM Song Liu <songliubraving@fb.com> wrote:
>> 
>> This test runs test_run for raw_tracepoint program. The test covers ctx
>> input, retval output, and proper handling of cpu_plus field.
>> 
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> .../bpf/prog_tests/raw_tp_test_run.c          | 73 +++++++++++++++++++
>> .../bpf/progs/test_raw_tp_test_run.c          | 26 +++++++
>> 2 files changed, 99 insertions(+)
>> create mode 100644 tools/testing/selftests/bpf/prog_tests/raw_tp_test_run.c
>> create mode 100644 tools/testing/selftests/bpf/progs/test_raw_tp_test_run.c
>> 
>> diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_test_run.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_test_run.c
>> new file mode 100644
>> index 0000000000000..3c6523b61afc1
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/raw_tp_test_run.c
>> @@ -0,0 +1,73 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/* Copyright (c) 2019 Facebook */
>> +#include <test_progs.h>
>> +#include "bpf/libbpf_internal.h"
>> +#include "test_raw_tp_test_run.skel.h"
>> +
>> +static int duration;
>> +
>> +void test_raw_tp_test_run(void)
>> +{
>> +       struct bpf_prog_test_run_attr test_attr = {};
>> +       __u64 args[2] = {0x1234ULL, 0x5678ULL};
>> +       int comm_fd = -1, err, nr_online, i;
>> +       int expected_retval = 0x1234 + 0x5678;
>> +       struct test_raw_tp_test_run *skel;
>> +       char buf[] = "new_name";
>> +       bool *online = NULL;
>> +
>> +       err = parse_cpu_mask_file("/sys/devices/system/cpu/online", &online,
>> +                                 &nr_online);
>> +       if (CHECK(err, "parse_cpu_mask_file", "err %d\n", err))
>> +               return;
>> +
>> +       skel = test_raw_tp_test_run__open_and_load();
>> +       if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
>> +               return;
> 
> leaking memory here

Good catch! Fixing it in the next version. 

> 
>> +       err = test_raw_tp_test_run__attach(skel);
>> +       if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
>> +               goto cleanup;
>> +
>> +       comm_fd = open("/proc/self/comm", O_WRONLY|O_TRUNC);
>> +       if (CHECK(comm_fd < 0, "open /proc/self/comm", "err %d\n", errno))
>> +               goto cleanup;
>> +
> 
> [...]
> 
>> +SEC("raw_tp/task_rename")
>> +int BPF_PROG(rename, struct task_struct *task, char *comm)
>> +{
>> +
>> +       count++;
>> +       if ((unsigned long long) task == 0x1234 &&
>> +           (unsigned long long) comm == 0x5678) {
> 
> you can use shorter __u64?

Sure.