Re: Failure in test_local_storage at bpf-next

From: KP Singh <kpsingh@chromium.org>
To: Yonghong Song <yhs@fb.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	bpf <bpf@vger.kernel.org>, Andrii Nakryiko <andriin@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>
Subject: Re: Failure in test_local_storage at bpf-next
Date: Thu, 8 Oct 2020 00:07:55 +0200	[thread overview]
Message-ID: <CACYkzJ6sWE3mq1sf_m+WO3RYw-rqPd2iYmSfKdq8R-rGhy11+Q@mail.gmail.com> (raw)
In-Reply-To: <CACYkzJ6XByjNfJi6cFEPUZsfZ=9uHUYHU6cXuBPuzedj66nehA@mail.gmail.com>

On Thu, Oct 8, 2020 at 12:04 AM KP Singh <kpsingh@chromium.org> wrote:
>
> On Wed, Oct 7, 2020 at 7:33 AM Yonghong Song <yhs@fb.com> wrote:
> >
> >
> >
> > On 10/6/20 10:18 PM, Alexei Starovoitov wrote:
> > > On Tue, Oct 6, 2020 at 9:31 PM Yonghong Song <yhs@fb.com> wrote:
> > >>
> > >>
> > >>
> > >> On 10/6/20 6:23 PM, Andrii Nakryiko wrote:
> > >>> On Tue, Oct 6, 2020 at 5:31 PM KP Singh <kpsingh@chromium.org> wrote:
> > >>>>
> > >>>> I noticed that test_local_storage is broken due to a BTF error at
> > >>>> bpf-next [67ed375530e2 ("samples: bpf: Driver interrupt statistics in
> > >>>> xdpsock")]
> > >>>>
> > >>>> ./test_progs -t test_local_storage
> > >>>> libbpf: prog 'socket_post_create': relo #0: parsing [28] struct socket + 0:0.1 2
> > >>>
> > >>> This line is truncated, btw, please make sure you post the entire
> > >>> output next time.
>
> I just ran this again and it does not seem like it's truncated:
>
> ./test_progs -t test_local_storage
> libbpf: prog 'socket_post_create': relo #0: parsing [28] struct socket + 0:0.1 2
> libbpf: prog 'socket_post_create': relo #0: failed to relocate: -22
> libbpf: failed to perform CO-RE relocations: -22
> libbpf: failed to load object 'local_storage'
> libbpf: failed to load BPF skeleton 'local_storage': -22
> test_test_local_storage:FAIL:skel_load lsm skeleton failed
>
> Am I missing something?

And indeed I am. It was a bug in my script.

For the record this should have been:

root@kpsingh:~# ./test_progs -t test_local_storage
libbpf: prog 'socket_post_create': relo #0: parsing [28] struct socket
+ 0:0.1 failed: -22
libbpf: prog 'socket_post_create': relo #0: failed to relocate: -22
libbpf: failed to perform CO-RE relocations: -22
libbpf: failed to load object 'local_storage'
libbpf: failed to load BPF skeleton 'local_storage': -22
test_test_local_storage:FAIL:skel_load lsm skeleton failed
#106 test_local_storage:FAIL
Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED

Thanks everyone for looking into this and fixing it!

>
> > >>>
> > >>> But, this seems like a bug in Clang, it produced invalid access index
> > >>> string "0:0.1", there shouldn't be any other separator except ':' in
> > >>> those strings.
> > >>>
> > >>> Yonghong, can you please take a look? This seems to be a very recent
> > >>> regression, I had to update to
> > >>> 6c7d713cf5d9bb188f1e73452a256386f0288bf7 sha from not-too-outdated
> > >>> version to repro this.
> > >>
> > >> Sorry. This indeed is a llvm regression. The guilty patch is
> > >> https://reviews.llvm.org/D88855  which adds NPM (new pass manager)
> > >> support for BPF. The patch just merged this morning, thanks for catching
> > >> the bug so fast. Since NPM is not used by default and the code
> > >> refactoring looks okay, so I did not run selftests. But, yah, it does
> > >> change some semantics of the code...
> > >
> > > but llvm tests were run, of course.
> > > Looks like we need to add more of them, so they can gate the landing.
> >
> > Right, just added two more tests to gate this particular kind of
> > failure. Also just pushed a new version which is simpler compared to
> > previous version.
> >
> > >
> > >> I just put a fix at https://reviews.llvm.org/D88942 .
> > >> Hopefully to merge soon.
> > >
> > > Thanks for the quick fix!
>
> Thank you so much!
>
> > >