linux-kernel - Re: [PATCH] perf test: Switch basic bpf filtering test to use syscall tracepoint

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <70b60459-2e7a-1944-77dc-54fef2e00975@linux.ibm.com>
Date:   Mon, 30 Jan 2023 14:28:49 +0530
From:   kajoljain <kjain@...ux.ibm.com>
To:     "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Disha Goel <disgoel@...ux.vnet.ibm.com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH] perf test: Switch basic bpf filtering test to use syscall
 tracepoint



On 1/23/23 14:02, Naveen N. Rao wrote:
> BPF filtering tests can sometime fail. Running the test in verbose mode
> shows the following:
>   $ sudo perf test 42
>   42: BPF filter                                                      :
>   42.1: Basic BPF filtering                                           : FAILED!
>   42.2: BPF pinning                                                   : Skip
>   42.3: BPF prologue generation                                       : Skip
>   $ perf --version
>   perf version 4.18.0-425.3.1.el8.ppc64le
>   $ sudo perf test -v 42
>   42: BPF filter                                                      :
>   42.1: Basic BPF filtering                                           :
>   --- start ---
>   test child forked, pid 711060
>   ...
>   bpf: config 'func=do_epoll_wait' is ok
>   Looking at the vmlinux_path (8 entries long)
>   Using /usr/lib/debug/lib/modules/4.18.0-425.3.1.el8.ppc64le/vmlinux for symbols
>   Open Debuginfo file: /usr/lib/debug/.build-id/81/56f5a07f92ccb62c5600ba0e4aacfb5f3a7534.debug
>   Try to find probe point from debuginfo.
>   Matched function: do_epoll_wait [4ef8cb0]
>   found inline addr: 0xc00000000061dbe4
>   Probe point found: __se_compat_sys_epoll_pwait+196
>   found inline addr: 0xc00000000061d9f4
>   Probe point found: __se_sys_epoll_pwait+196
>   found inline addr: 0xc00000000061d824
>   Probe point found: __se_sys_epoll_wait+36
>   Found 3 probe_trace_events.
>   Opening /sys/kernel/tracing//kprobe_events write=1
>   ...
>   BPF filter result incorrect, expected 56, got 56 samples
>   test child finished with -1
>   ---- end ----
>   BPF filter subtest 1: FAILED!

Patch looks good to me.

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

Thanks,
Kajol Jain
> 
> The statement above about the result being incorrect looks weird, and it
> is due to that particular perf build missing commit 3e11300cdfd5f1
> ("perf test: Fix bpf test sample mismatch reporting"). In reality, due
> to commit 4b04e0decd2518 ("perf test: Fix basic bpf filtering test"),
> perf expects there to be 56*3 samples.
> 
> However, the number of samples we receive is going to be dependent on
> where the probes are installed, which is dependent on where
> do_epoll_wait gets inlined. On s390x, it looks like probes at all the
> inlined locations are hit. But, that is not the case on ppc64le.
> 
> Fix this by switching the test to instead use the syscall tracepoint.
> This ensures that we will only ever install a single event enabling us
> to reliably determine the sample count.
> 
> Reported-by: Disha Goel <disgoel@...ux.vnet.ibm.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@...ux.vnet.ibm.com>
> ---
>  tools/perf/tests/bpf-script-example.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/bpf-script-example.c b/tools/perf/tests/bpf-script-example.c
> index 7981c69ed1b456..b638cc99d5ae56 100644
> --- a/tools/perf/tests/bpf-script-example.c
> +++ b/tools/perf/tests/bpf-script-example.c
> @@ -43,7 +43,7 @@ struct {
>  	__type(value, int);
>  } flip_table SEC(".maps");
>  
> -SEC("func=do_epoll_wait")
> +SEC("syscalls:sys_enter_epoll_pwait")
>  int bpf_func__SyS_epoll_pwait(void *ctx)
>  {
>  	int ind =0;
> 
> base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9