diff mbox series

[v2,3/3] selftest/bpf: Test a perf bpf program that suppresses side effects.

Message ID 20231207163458.5554-4-khuey@kylehuey.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Combine perf and bpf for fast eval of hw breakpoint conditions | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-3 fail Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / test
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-6 fail Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for s390x-gcc / test
bpf/vmtest-bpf-next-VM_Test-8 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-12 success Logs for x86_64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-9 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-11 success Logs for x86_64-gcc / test
bpf/vmtest-bpf-next-VM_Test-10 fail Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for x86_64-llvm-16 / test
bpf/vmtest-bpf-next-VM_Test-15 success Logs for x86_64-llvm-16 / veristat
bpf/vmtest-bpf-next-VM_Test-13 fail Logs for x86_64-llvm-16 / build / build for x86_64 with llvm-16

Commit Message

Kyle Huey Dec. 7, 2023, 4:34 p.m. UTC 
The test sets a hardware breakpoint and uses a bpf program to suppress the
side effects of a perf event sample, including I/O availability signals,
SIGTRAPs, and decrementing the event counter limit, if the ip matches the
expected value. Then the function with the breakpoint is executed multiple
times to test that all effects behave as expected.

Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 .../selftests/bpf/prog_tests/perf_skip.c      | 145 ++++++++++++++++++
 .../selftests/bpf/progs/test_perf_skip.c      |  15 ++
 2 files changed, 160 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_skip.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_perf_skip.c

Comments

Andrii Nakryiko Dec. 7, 2023, 7:12 p.m. UTC | #1 
On Thu, Dec 7, 2023 at 8:35 AM Kyle Huey <me@kylehuey.com> wrote:
>
> The test sets a hardware breakpoint and uses a bpf program to suppress the
> side effects of a perf event sample, including I/O availability signals,
> SIGTRAPs, and decrementing the event counter limit, if the ip matches the
> expected value. Then the function with the breakpoint is executed multiple
> times to test that all effects behave as expected.
>
> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> ---
>  .../selftests/bpf/prog_tests/perf_skip.c      | 145 ++++++++++++++++++
>  .../selftests/bpf/progs/test_perf_skip.c      |  15 ++
>  2 files changed, 160 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_skip.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_perf_skip.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_skip.c b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> new file mode 100644
> index 000000000000..f6fa9bfd9efa
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +
> +/* We need the latest siginfo from the kernel repo. */
> +#include <asm/siginfo.h>

selftests are built with UAPI headers' copies under tools/include, so
CI did catch a real issue, I think. Try copying
include/uapi/asm-generic/siginfo.h into
tools/include/uapi/asm-generic/siginfo.h ?

> +#define __have_siginfo_t 1
> +#define __have_sigval_t 1
> +#define __have_sigevent_t 1
> +#define __siginfo_t_defined
> +#define __sigval_t_defined
> +#define __sigevent_t_defined
> +#define _BITS_SIGINFO_CONSTS_H 1
> +#define _BITS_SIGEVENT_CONSTS_H 1
> +
> +#include <test_progs.h>
> +#include "test_perf_skip.skel.h"
> +#include <linux/compiler.h>
> +#include <linux/hw_breakpoint.h>
> +#include <sys/mman.h>
> +
> +int signals_unexpected = 1;
> +int sigio_count, sigtrap_count;
> +
> +static void handle_sigio(int sig __always_unused)
> +{
> +       ASSERT_OK(signals_unexpected, "perf event not skipped");
> +       ++sigio_count;
> +}
> +
> +static void handle_sigtrap(int signum __always_unused,
> +                          siginfo_t *info,
> +                          void *ucontext __always_unused)
> +{
> +       ASSERT_OK(signals_unexpected, "perf event not skipped");
> +       ASSERT_EQ(info->si_code, TRAP_PERF, "wrong si_code");
> +       ++sigtrap_count;
> +}
> +
> +static noinline int test_function(void)
> +{
> +       asm volatile ("");
> +       return 0;
> +}
> +
> +void serial_test_perf_skip(void)
> +{
> +       struct sigaction action = {};
> +       struct sigaction previous_sigtrap;
> +       sighandler_t previous_sigio;
> +       struct test_perf_skip *skel = NULL;
> +       struct perf_event_attr attr = {};
> +       int perf_fd = -1;
> +       int err;
> +       struct f_owner_ex owner;
> +       struct bpf_link *prog_link = NULL;
> +
> +       action.sa_flags = SA_SIGINFO | SA_NODEFER;
> +       action.sa_sigaction = handle_sigtrap;
> +       sigemptyset(&action.sa_mask);
> +       if (!ASSERT_OK(sigaction(SIGTRAP, &action, &previous_sigtrap), "sigaction"))
> +               return;
> +
> +       previous_sigio = signal(SIGIO, handle_sigio);
> +
> +       skel = test_perf_skip__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "skel_load"))
> +               goto cleanup;
> +
> +       attr.type = PERF_TYPE_BREAKPOINT;
> +       attr.size = sizeof(attr);
> +       attr.bp_type = HW_BREAKPOINT_X;
> +       attr.bp_addr = (uintptr_t)test_function;
> +       attr.bp_len = sizeof(long);
> +       attr.sample_period = 1;
> +       attr.sample_type = PERF_SAMPLE_IP;
> +       attr.pinned = 1;
> +       attr.exclude_kernel = 1;
> +       attr.exclude_hv = 1;
> +       attr.precise_ip = 3;
> +       attr.sigtrap = 1;
> +       attr.remove_on_exec = 1;
> +
> +       perf_fd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, 0);
> +       if (perf_fd < 0 && (errno == ENOENT || errno == EOPNOTSUPP)) {
> +               printf("SKIP:no PERF_TYPE_BREAKPOINT/HW_BREAKPOINT_X\n");
> +               test__skip();
> +               goto cleanup;
> +       }
> +       if (!ASSERT_OK(perf_fd < 0, "perf_event_open"))
> +               goto cleanup;
> +
> +       // Configure the perf event to signal on sample.

please don't use C++ style comments

> +       err = fcntl(perf_fd, F_SETFL, O_ASYNC);
> +       if (!ASSERT_OK(err, "fcntl(F_SETFL, O_ASYNC)"))
> +               goto cleanup;
> +
> +       owner.type = F_OWNER_TID;
> +       owner.pid = syscall(__NR_gettid);
> +       err = fcntl(perf_fd, F_SETOWN_EX, &owner);
> +       if (!ASSERT_OK(err, "fcntl(F_SETOWN_EX)"))
> +               goto cleanup;
> +
> +       // Allow at most one sample. A sample rejected by bpf should
> +       // not count against this.
> +       err = ioctl(perf_fd, PERF_EVENT_IOC_REFRESH, 1);
> +       if (!ASSERT_OK(err, "ioctl(PERF_EVENT_IOC_REFRESH)"))
> +               goto cleanup;
> +
> +       prog_link = bpf_program__attach_perf_event(skel->progs.handler, perf_fd);
> +       if (!ASSERT_OK_PTR(prog_link, "bpf_program__attach_perf_event"))
> +               goto cleanup;
> +
> +       // Configure the bpf program to suppress the sample.
> +       skel->bss->ip = (uintptr_t)test_function;
> +       test_function();
> +
> +       ASSERT_EQ(sigio_count, 0, "sigio_count");
> +       ASSERT_EQ(sigtrap_count, 0, "sigtrap_count");
> +
> +       // Configure the bpf program to allow the sample.
> +       skel->bss->ip = 0;
> +       signals_unexpected = 0;
> +       test_function();
> +
> +       ASSERT_EQ(sigio_count, 1, "sigio_count");
> +       ASSERT_EQ(sigtrap_count, 1, "sigtrap_count");
> +
> +       // Test that the sample above is the only one allowed (by perf, not
> +       // by bpf)
> +       test_function();
> +
> +       ASSERT_EQ(sigio_count, 1, "sigio_count");
> +       ASSERT_EQ(sigtrap_count, 1, "sigtrap_count");
> +
> +cleanup:
> +       if (prog_link)

nit: no need for a check, bpf_link__destroy() handles NULLs just fine

> +               bpf_link__destroy(prog_link);
> +       if (perf_fd >= 0)
> +               close(perf_fd);
> +       if (skel)
> +               test_perf_skip__destroy(skel);

same, no need for NULL check

> +
> +       signal(SIGIO, previous_sigio);
> +       sigaction(SIGTRAP, &previous_sigtrap, NULL);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_perf_skip.c b/tools/testing/selftests/bpf/progs/test_perf_skip.c
> new file mode 100644
> index 000000000000..417a9db3b770
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_perf_skip.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +uintptr_t ip;
> +
> +SEC("perf_event")
> +int handler(struct bpf_perf_event_data *data)
> +{
> +       // Skip events that have the correct ip.

C++ comments


> +       return ip != PT_REGS_IP(&data->regs);
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.34.1
>
Marco Elver Dec. 7, 2023, 7:20 p.m. UTC | #2 
On Thu, 7 Dec 2023 at 20:12, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Dec 7, 2023 at 8:35 AM Kyle Huey <me@kylehuey.com> wrote:
> >
> > The test sets a hardware breakpoint and uses a bpf program to suppress the
> > side effects of a perf event sample, including I/O availability signals,
> > SIGTRAPs, and decrementing the event counter limit, if the ip matches the
> > expected value. Then the function with the breakpoint is executed multiple
> > times to test that all effects behave as expected.
> >
> > Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> > ---
> >  .../selftests/bpf/prog_tests/perf_skip.c      | 145 ++++++++++++++++++
> >  .../selftests/bpf/progs/test_perf_skip.c      |  15 ++
> >  2 files changed, 160 insertions(+)
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_skip.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/test_perf_skip.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/perf_skip.c b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > new file mode 100644
> > index 000000000000..f6fa9bfd9efa
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > @@ -0,0 +1,145 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#define _GNU_SOURCE
> > +
> > +/* We need the latest siginfo from the kernel repo. */
> > +#include <asm/siginfo.h>
>
> selftests are built with UAPI headers' copies under tools/include, so
> CI did catch a real issue, I think. Try copying
> include/uapi/asm-generic/siginfo.h into
> tools/include/uapi/asm-generic/siginfo.h ?

I believe parts of this were inspired by
tools/testing/selftests/perf_events/sigtrap_threads.c - getting the
kernel headers is allowed, as long as $(KHDR_INCLUDES) is added to
CFLAGS. See tools/testing/selftests/perf_events/Makefile. Not sure
it's appropriate for this test though, if you don't want to add
KHDR_INCLUDES for everything.
Kyle Huey Dec. 7, 2023, 10:56 p.m. UTC | #3 
On Thu, Dec 7, 2023 at 11:20 AM Marco Elver <elver@google.com> wrote:
>
> On Thu, 7 Dec 2023 at 20:12, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Dec 7, 2023 at 8:35 AM Kyle Huey <me@kylehuey.com> wrote:
> > >
> > > The test sets a hardware breakpoint and uses a bpf program to suppress the
> > > side effects of a perf event sample, including I/O availability signals,
> > > SIGTRAPs, and decrementing the event counter limit, if the ip matches the
> > > expected value. Then the function with the breakpoint is executed multiple
> > > times to test that all effects behave as expected.
> > >
> > > Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> > > ---
> > >  .../selftests/bpf/prog_tests/perf_skip.c      | 145 ++++++++++++++++++
> > >  .../selftests/bpf/progs/test_perf_skip.c      |  15 ++
> > >  2 files changed, 160 insertions(+)
> > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > >  create mode 100644 tools/testing/selftests/bpf/progs/test_perf_skip.c
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/perf_skip.c b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > > new file mode 100644
> > > index 000000000000..f6fa9bfd9efa
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > > @@ -0,0 +1,145 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#define _GNU_SOURCE
> > > +
> > > +/* We need the latest siginfo from the kernel repo. */
> > > +#include <asm/siginfo.h>
> >
> > selftests are built with UAPI headers' copies under tools/include, so
> > CI did catch a real issue, I think. Try copying
> > include/uapi/asm-generic/siginfo.h into
> > tools/include/uapi/asm-generic/siginfo.h ?
>
> I believe parts of this were inspired by
> tools/testing/selftests/perf_events/sigtrap_threads.c - getting the
> kernel headers is allowed, as long as $(KHDR_INCLUDES) is added to
> CFLAGS. See tools/testing/selftests/perf_events/Makefile. Not sure
> it's appropriate for this test though, if you don't want to add
> KHDR_INCLUDES for everything.

Yes, that's right. Namhyung's commit message for 91c97b36bd69 leads me
to believe that I should copy siginfo.h over into tools/include and
fix the perf_events self tests too.

- Kyle
Kyle Huey Dec. 8, 2023, 1:08 a.m. UTC | #4 
On Thu, Dec 7, 2023 at 2:56 PM Kyle Huey <me@kylehuey.com> wrote:
>
> On Thu, Dec 7, 2023 at 11:20 AM Marco Elver <elver@google.com> wrote:
> >
> > On Thu, 7 Dec 2023 at 20:12, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Thu, Dec 7, 2023 at 8:35 AM Kyle Huey <me@kylehuey.com> wrote:
> > > >
> > > > The test sets a hardware breakpoint and uses a bpf program to suppress the
> > > > side effects of a perf event sample, including I/O availability signals,
> > > > SIGTRAPs, and decrementing the event counter limit, if the ip matches the
> > > > expected value. Then the function with the breakpoint is executed multiple
> > > > times to test that all effects behave as expected.
> > > >
> > > > Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> > > > ---
> > > >  .../selftests/bpf/prog_tests/perf_skip.c      | 145 ++++++++++++++++++
> > > >  .../selftests/bpf/progs/test_perf_skip.c      |  15 ++
> > > >  2 files changed, 160 insertions(+)
> > > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > > >  create mode 100644 tools/testing/selftests/bpf/progs/test_perf_skip.c
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/prog_tests/perf_skip.c b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > > > new file mode 100644
> > > > index 000000000000..f6fa9bfd9efa
> > > > --- /dev/null
> > > > +++ b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > > > @@ -0,0 +1,145 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +#define _GNU_SOURCE
> > > > +
> > > > +/* We need the latest siginfo from the kernel repo. */
> > > > +#include <asm/siginfo.h>
> > >
> > > selftests are built with UAPI headers' copies under tools/include, so
> > > CI did catch a real issue, I think. Try copying
> > > include/uapi/asm-generic/siginfo.h into
> > > tools/include/uapi/asm-generic/siginfo.h ?
> >
> > I believe parts of this were inspired by
> > tools/testing/selftests/perf_events/sigtrap_threads.c - getting the
> > kernel headers is allowed, as long as $(KHDR_INCLUDES) is added to
> > CFLAGS. See tools/testing/selftests/perf_events/Makefile. Not sure
> > it's appropriate for this test though, if you don't want to add
> > KHDR_INCLUDES for everything.
>
> Yes, that's right. Namhyung's commit message for 91c97b36bd69 leads me
> to believe that I should copy siginfo.h over into tools/include and
> fix the perf_events self tests too.
>
> - Kyle

That doesn't really help (though perhaps it should be done anyway so
the selftests aren't reaching into include/) because the glibc headers
still redefine a ton of stuff in asm-generic/siginfo.h.

- Kyle
Yonghong Song Dec. 8, 2023, 3:46 a.m. UTC | #5 
On 12/7/23 5:08 PM, Kyle Huey wrote:
> On Thu, Dec 7, 2023 at 2:56 PM Kyle Huey <me@kylehuey.com> wrote:
>> On Thu, Dec 7, 2023 at 11:20 AM Marco Elver <elver@google.com> wrote:
>>> On Thu, 7 Dec 2023 at 20:12, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>> On Thu, Dec 7, 2023 at 8:35 AM Kyle Huey <me@kylehuey.com> wrote:
>>>>> The test sets a hardware breakpoint and uses a bpf program to suppress the
>>>>> side effects of a perf event sample, including I/O availability signals,
>>>>> SIGTRAPs, and decrementing the event counter limit, if the ip matches the
>>>>> expected value. Then the function with the breakpoint is executed multiple
>>>>> times to test that all effects behave as expected.
>>>>>
>>>>> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
>>>>> ---
>>>>>   .../selftests/bpf/prog_tests/perf_skip.c      | 145 ++++++++++++++++++
>>>>>   .../selftests/bpf/progs/test_perf_skip.c      |  15 ++
>>>>>   2 files changed, 160 insertions(+)
>>>>>   create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_skip.c
>>>>>   create mode 100644 tools/testing/selftests/bpf/progs/test_perf_skip.c
>>>>>
>>>>> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_skip.c b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
>>>>> new file mode 100644
>>>>> index 000000000000..f6fa9bfd9efa
>>>>> --- /dev/null
>>>>> +++ b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
>>>>> @@ -0,0 +1,145 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +#define _GNU_SOURCE
>>>>> +
>>>>> +/* We need the latest siginfo from the kernel repo. */
>>>>> +#include <asm/siginfo.h>
>>>> selftests are built with UAPI headers' copies under tools/include, so
>>>> CI did catch a real issue, I think. Try copying
>>>> include/uapi/asm-generic/siginfo.h into
>>>> tools/include/uapi/asm-generic/siginfo.h ?
>>> I believe parts of this were inspired by
>>> tools/testing/selftests/perf_events/sigtrap_threads.c - getting the
>>> kernel headers is allowed, as long as $(KHDR_INCLUDES) is added to
>>> CFLAGS. See tools/testing/selftests/perf_events/Makefile. Not sure
>>> it's appropriate for this test though, if you don't want to add
>>> KHDR_INCLUDES for everything.
>> Yes, that's right. Namhyung's commit message for 91c97b36bd69 leads me
>> to believe that I should copy siginfo.h over into tools/include and
>> fix the perf_events self tests too.
>>
>> - Kyle
> That doesn't really help (though perhaps it should be done anyway so
> the selftests aren't reaching into include/) because the glibc headers
> still redefine a ton of stuff in asm-generic/siginfo.h.

Just for testing purpose, I think you can avoid includeasm/siginfo.h and directly define necessary structures in the C file 
directly, right?

>
> - Kyle
Marco Elver Dec. 8, 2023, 8:06 a.m. UTC | #6 
On Fri, 8 Dec 2023 at 02:08, Kyle Huey <me@kylehuey.com> wrote:
>
> On Thu, Dec 7, 2023 at 2:56 PM Kyle Huey <me@kylehuey.com> wrote:
> >
> > On Thu, Dec 7, 2023 at 11:20 AM Marco Elver <elver@google.com> wrote:
> > >
> > > On Thu, 7 Dec 2023 at 20:12, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Thu, Dec 7, 2023 at 8:35 AM Kyle Huey <me@kylehuey.com> wrote:
> > > > >
> > > > > The test sets a hardware breakpoint and uses a bpf program to suppress the
> > > > > side effects of a perf event sample, including I/O availability signals,
> > > > > SIGTRAPs, and decrementing the event counter limit, if the ip matches the
> > > > > expected value. Then the function with the breakpoint is executed multiple
> > > > > times to test that all effects behave as expected.
> > > > >
> > > > > Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> > > > > ---
> > > > >  .../selftests/bpf/prog_tests/perf_skip.c      | 145 ++++++++++++++++++
> > > > >  .../selftests/bpf/progs/test_perf_skip.c      |  15 ++
> > > > >  2 files changed, 160 insertions(+)
> > > > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > > > >  create mode 100644 tools/testing/selftests/bpf/progs/test_perf_skip.c
> > > > >
> > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/perf_skip.c b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > > > > new file mode 100644
> > > > > index 000000000000..f6fa9bfd9efa
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
> > > > > @@ -0,0 +1,145 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +#define _GNU_SOURCE
> > > > > +
> > > > > +/* We need the latest siginfo from the kernel repo. */
> > > > > +#include <asm/siginfo.h>
> > > >
> > > > selftests are built with UAPI headers' copies under tools/include, so
> > > > CI did catch a real issue, I think. Try copying
> > > > include/uapi/asm-generic/siginfo.h into
> > > > tools/include/uapi/asm-generic/siginfo.h ?
> > >
> > > I believe parts of this were inspired by
> > > tools/testing/selftests/perf_events/sigtrap_threads.c - getting the
> > > kernel headers is allowed, as long as $(KHDR_INCLUDES) is added to
> > > CFLAGS. See tools/testing/selftests/perf_events/Makefile. Not sure
> > > it's appropriate for this test though, if you don't want to add
> > > KHDR_INCLUDES for everything.
> >
> > Yes, that's right. Namhyung's commit message for 91c97b36bd69 leads me
> > to believe that I should copy siginfo.h over into tools/include and
> > fix the perf_events self tests too.
> >
> > - Kyle
>
> That doesn't really help (though perhaps it should be done anyway so
> the selftests aren't reaching into include/) because the glibc headers
> still redefine a ton of stuff in asm-generic/siginfo.h.

From what I can see your test doesn't actually refer to any of the
si_perf_* fields, but only needs it for this:

> +       ASSERT_EQ(info->si_code, TRAP_PERF, "wrong si_code");

I think that's easy to fix by just defining TRAP_PERF yourself or
leaving out the assert completely. If you get an unexpected SIGTRAP
your asserts checking sigtrap_count elsewhere will fail, too. It'd
just be harder to debug.
Kyle Huey Dec. 8, 2023, 5:09 p.m. UTC | #7 
On Fri, Dec 8, 2023 at 12:07 AM Marco Elver <elver@google.com> wrote:
> I think that's easy to fix by just defining TRAP_PERF yourself

Yeah that would work here.

- Kyle
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/perf_skip.c b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
new file mode 100644
index 000000000000..f6fa9bfd9efa
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/perf_skip.c
@@ -0,0 +1,145 @@ 
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+
+/* We need the latest siginfo from the kernel repo. */
+#include <asm/siginfo.h>
+#define __have_siginfo_t 1
+#define __have_sigval_t 1
+#define __have_sigevent_t 1
+#define __siginfo_t_defined
+#define __sigval_t_defined
+#define __sigevent_t_defined
+#define _BITS_SIGINFO_CONSTS_H 1
+#define _BITS_SIGEVENT_CONSTS_H 1
+
+#include <test_progs.h>
+#include "test_perf_skip.skel.h"
+#include <linux/compiler.h>
+#include <linux/hw_breakpoint.h>
+#include <sys/mman.h>
+
+int signals_unexpected = 1;
+int sigio_count, sigtrap_count;
+
+static void handle_sigio(int sig __always_unused)
+{
+	ASSERT_OK(signals_unexpected, "perf event not skipped");
+	++sigio_count;
+}
+
+static void handle_sigtrap(int signum __always_unused,
+			   siginfo_t *info,
+			   void *ucontext __always_unused)
+{
+	ASSERT_OK(signals_unexpected, "perf event not skipped");
+	ASSERT_EQ(info->si_code, TRAP_PERF, "wrong si_code");
+	++sigtrap_count;
+}
+
+static noinline int test_function(void)
+{
+	asm volatile ("");
+	return 0;
+}
+
+void serial_test_perf_skip(void)
+{
+	struct sigaction action = {};
+	struct sigaction previous_sigtrap;
+	sighandler_t previous_sigio;
+	struct test_perf_skip *skel = NULL;
+	struct perf_event_attr attr = {};
+	int perf_fd = -1;
+	int err;
+	struct f_owner_ex owner;
+	struct bpf_link *prog_link = NULL;
+
+	action.sa_flags = SA_SIGINFO | SA_NODEFER;
+	action.sa_sigaction = handle_sigtrap;
+	sigemptyset(&action.sa_mask);
+	if (!ASSERT_OK(sigaction(SIGTRAP, &action, &previous_sigtrap), "sigaction"))
+		return;
+
+	previous_sigio = signal(SIGIO, handle_sigio);
+
+	skel = test_perf_skip__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_load"))
+		goto cleanup;
+
+	attr.type = PERF_TYPE_BREAKPOINT;
+	attr.size = sizeof(attr);
+	attr.bp_type = HW_BREAKPOINT_X;
+	attr.bp_addr = (uintptr_t)test_function;
+	attr.bp_len = sizeof(long);
+	attr.sample_period = 1;
+	attr.sample_type = PERF_SAMPLE_IP;
+	attr.pinned = 1;
+	attr.exclude_kernel = 1;
+	attr.exclude_hv = 1;
+	attr.precise_ip = 3;
+	attr.sigtrap = 1;
+	attr.remove_on_exec = 1;
+
+	perf_fd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, 0);
+	if (perf_fd < 0 && (errno == ENOENT || errno == EOPNOTSUPP)) {
+		printf("SKIP:no PERF_TYPE_BREAKPOINT/HW_BREAKPOINT_X\n");
+		test__skip();
+		goto cleanup;
+	}
+	if (!ASSERT_OK(perf_fd < 0, "perf_event_open"))
+		goto cleanup;
+
+	// Configure the perf event to signal on sample.
+	err = fcntl(perf_fd, F_SETFL, O_ASYNC);
+	if (!ASSERT_OK(err, "fcntl(F_SETFL, O_ASYNC)"))
+		goto cleanup;
+
+	owner.type = F_OWNER_TID;
+	owner.pid = syscall(__NR_gettid);
+	err = fcntl(perf_fd, F_SETOWN_EX, &owner);
+	if (!ASSERT_OK(err, "fcntl(F_SETOWN_EX)"))
+		goto cleanup;
+
+	// Allow at most one sample. A sample rejected by bpf should
+	// not count against this.
+	err = ioctl(perf_fd, PERF_EVENT_IOC_REFRESH, 1);
+	if (!ASSERT_OK(err, "ioctl(PERF_EVENT_IOC_REFRESH)"))
+		goto cleanup;
+
+	prog_link = bpf_program__attach_perf_event(skel->progs.handler, perf_fd);
+	if (!ASSERT_OK_PTR(prog_link, "bpf_program__attach_perf_event"))
+		goto cleanup;
+
+	// Configure the bpf program to suppress the sample.
+	skel->bss->ip = (uintptr_t)test_function;
+	test_function();
+
+	ASSERT_EQ(sigio_count, 0, "sigio_count");
+	ASSERT_EQ(sigtrap_count, 0, "sigtrap_count");
+
+	// Configure the bpf program to allow the sample.
+	skel->bss->ip = 0;
+	signals_unexpected = 0;
+	test_function();
+
+	ASSERT_EQ(sigio_count, 1, "sigio_count");
+	ASSERT_EQ(sigtrap_count, 1, "sigtrap_count");
+
+	// Test that the sample above is the only one allowed (by perf, not
+	// by bpf)
+	test_function();
+
+	ASSERT_EQ(sigio_count, 1, "sigio_count");
+	ASSERT_EQ(sigtrap_count, 1, "sigtrap_count");
+
+cleanup:
+	if (prog_link)
+		bpf_link__destroy(prog_link);
+	if (perf_fd >= 0)
+		close(perf_fd);
+	if (skel)
+		test_perf_skip__destroy(skel);
+
+	signal(SIGIO, previous_sigio);
+	sigaction(SIGTRAP, &previous_sigtrap, NULL);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_perf_skip.c b/tools/testing/selftests/bpf/progs/test_perf_skip.c
new file mode 100644
index 000000000000..417a9db3b770
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_perf_skip.c
@@ -0,0 +1,15 @@ 
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+uintptr_t ip;
+
+SEC("perf_event")
+int handler(struct bpf_perf_event_data *data)
+{
+	// Skip events that have the correct ip.
+	return ip != PT_REGS_IP(&data->regs);
+}
+
+char _license[] SEC("license") = "GPL";