Re: [RFC] bpf: Issue with bpf_fentry_test7 call

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Dec 15, 2023 at 01:22:35PM -0800, Andrii Nakryiko wrote:
> On Fri, Dec 15, 2023 at 6:42 AM Yonghong Song <yonghong.song@linux.dev> wrote:
> >
> >
> > On 12/15/23 6:24 AM, Jiri Olsa wrote:
> > > On Fri, Dec 15, 2023 at 10:16:27AM +0100, Jiri Olsa wrote:
> > >> hi,
> > >> The bpf CI is broken due to clang emitting 2 functions for
> > >> bpf_fentry_test7:
> > >>
> > >>    # cat available_filter_functions | grep bpf_fentry_test7
> > >>    bpf_fentry_test7
> > >>    bpf_fentry_test7.specialized.1
> > >>
> > >> The tests attach to 'bpf_fentry_test7' while the function with
> > >> '.specialized.1' suffix is executed in bpf_prog_test_run_tracing.
> > >>
> > >> It looks like clang optimalization that comes from passing 0
> > >> as argument and returning it directly in bpf_fentry_test7.
> > >>
> > >> I'm not sure there's a way to disable this, so far I came
> > >> up with solution below that passes real pointer, but I think
> > >> that was not the original intention for the test.
> > >>
> > >> We had issue with this function back in august:
> > >>    32337c0a2824 bpf: Prevent inlining of bpf_fentry_test7()
> > >>
> > >> I'm not sure why it started to show now? was clang updated for CI?
> > >>
> > >> I'll try to find out more, but any clang ideas are welcome ;-)
> > >>
> > >> thanks,
> > >> jirka
> > >
> > > hm, there seems to be fix in bpf-next for this one:
> > >
> > >    b16904fd9f01 bpf: Fix a few selftest failures due to llvm18 change
> >
> > Maybe submit a patch to https://github.com/kernel-patches/vmtest/tree/master/ci/diffs?
> > That is typically the place to have temporary patches to workaround ci failures.
> >
> 
> To get bpf/master back to green CI I did it meanwhile ([0]). Jiri,
> please check the PR to be familiar with the process for the future
> similar mitigations, thanks.
> 
>   [0] https://github.com/kernel-patches/vmtest/pull/258

great, thanks

jirka

> 
> > >
> > > jirka
> > >
> > >>
> > >> ---
> > >> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > >> index c9fdcc5cdce1..33208eec9361 100644
> > >> --- a/net/bpf/test_run.c
> > >> +++ b/net/bpf/test_run.c
> > >> @@ -543,7 +543,7 @@ struct bpf_fentry_test_t {
> > >>   int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
> > >>   {
> > >>      asm volatile ("");
> > >> -    return (long)arg;
> > >> +    return 0;
> > >>   }
> > >>
> > >>   int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg)
> > >> @@ -668,7 +668,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
> > >>                  bpf_fentry_test4((void *)7, 8, 9, 10) != 34 ||
> > >>                  bpf_fentry_test5(11, (void *)12, 13, 14, 15) != 65 ||
> > >>                  bpf_fentry_test6(16, (void *)17, 18, 19, (void *)20, 21) != 111 ||
> > >> -                bpf_fentry_test7((struct bpf_fentry_test_t *)0) != 0 ||
> > >> +                bpf_fentry_test7(&arg) != 0 ||
> > >>                  bpf_fentry_test8(&arg) != 0 ||
> > >>                  bpf_fentry_test9(&retval) != 0)
> > >>                      goto out;
> > >> diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c
> > >> index 52a550d281d9..95c5c34ccaa8 100644
> > >> --- a/tools/testing/selftests/bpf/progs/fentry_test.c
> > >> +++ b/tools/testing/selftests/bpf/progs/fentry_test.c
> > >> @@ -64,7 +64,7 @@ __u64 test7_result = 0;
> > >>   SEC("fentry/bpf_fentry_test7")
> > >>   int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
> > >>   {
> > >> -    if (!arg)
> > >> +    if (arg)
> > >>              test7_result = 1;
> > >>      return 0;
> > >>   }
> > >> diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
> > >> index 8f1ccb7302e1..ffb30236ca02 100644
> > >> --- a/tools/testing/selftests/bpf/progs/fexit_test.c
> > >> +++ b/tools/testing/selftests/bpf/progs/fexit_test.c
> > >> @@ -65,7 +65,7 @@ __u64 test7_result = 0;
> > >>   SEC("fexit/bpf_fentry_test7")
> > >>   int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
> > >>   {
> > >> -    if (!arg)
> > >> +    if (arg)
> > >>              test7_result = 1;
> > >>      return 0;
> > >>   }