Linux 5.4 - bpf test build fails

Linux 5.4 - bpf test build fails

[Shuah Khan]

Hi Alexei and Daniel,

bpf test doesn't build on Linux 5.4 mainline. Do you know what's
happening here.


make -C tools/testing/selftests/bpf/

-c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
llc -march=bpf -mcpu=generic  -filetype=obj -o 
/mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o
progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
       '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
         if (BPF_CORE_READ(&out->a, &in[2].a))
             ^
./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
                        __builtin_preserve_access_index(src))
                        ^
progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
       pointer conversion passing 'int' to parameter of type 'const void *'
       [-Wint-conversion]
         if (BPF_CORE_READ(&out->a, &in[2].a))
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
                        __builtin_preserve_access_index(src))
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning and 1 error generated.
llc: error: llc: <stdin>:1:1: error: expected top-level entity
clang failed

Also

make TARGETS=bpf kselftest fails as well. Dependency between
tools/lib/bpf and the test. How can we avoid this type of
dependency or resolve it in a way it doesn't result in build
failures?

thanks,
-- Shuah

Re: Linux 5.4 - bpf test build fails

[Yonghong Song]

On 9/24/19 8:26 AM, Shuah Khan wrote:
> Hi Alexei and Daniel,
> 
> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
> happening here.
> 
> 
> make -C tools/testing/selftests/bpf/
> 
> -c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
> llc -march=bpf -mcpu=generic  -filetype=obj -o 
> /mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o 
> 
> progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
>        '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
>          if (BPF_CORE_READ(&out->a, &in[2].a))
>              ^
> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>                         __builtin_preserve_access_index(src))
>                         ^
> progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
>        pointer conversion passing 'int' to parameter of type 'const void *'
>        [-Wint-conversion]
>          if (BPF_CORE_READ(&out->a, &in[2].a))
>              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>                         __builtin_preserve_access_index(src))
>                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 warning and 1 error generated.
> llc: error: llc: <stdin>:1:1: error: expected top-level entity
> clang failed
> 
> Also
> 
> make TARGETS=bpf kselftest fails as well. Dependency between
> tools/lib/bpf and the test. How can we avoid this type of
> dependency or resolve it in a way it doesn't result in build
> failures?

Thanks, Shuah.

The clang __builtin_preserve_access_index() intrinsic is
introduced in LLVM9 (which just released last week) and
the builtin and other CO-RE features are only supported
in LLVM10 (current development branch) with more bug fixes
and added features.

I think we should do a feature test for llvm version and only
enable these tests when llvm version >= 10.

Andrii, what do you think?

> 
> thanks,
> -- Shuah

Re: Linux 5.4 - bpf test build fails

[Shuah Khan]

On 9/24/19 9:43 AM, Yonghong Song wrote:
> 
> 
> On 9/24/19 8:26 AM, Shuah Khan wrote:
>> Hi Alexei and Daniel,
>>
>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
>> happening here.
>>
>>
>> make -C tools/testing/selftests/bpf/
>>
>> -c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
>> llc -march=bpf -mcpu=generic  -filetype=obj -o
>> /mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o
>>
>> progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
>>         '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
>>           if (BPF_CORE_READ(&out->a, &in[2].a))
>>               ^
>> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>>                          __builtin_preserve_access_index(src))
>>                          ^
>> progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
>>         pointer conversion passing 'int' to parameter of type 'const void *'
>>         [-Wint-conversion]
>>           if (BPF_CORE_READ(&out->a, &in[2].a))
>>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>>                          __builtin_preserve_access_index(src))
>>                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 1 warning and 1 error generated.
>> llc: error: llc: <stdin>:1:1: error: expected top-level entity
>> clang failed
>>
>> Also
>>
>> make TARGETS=bpf kselftest fails as well. Dependency between
>> tools/lib/bpf and the test. How can we avoid this type of
>> dependency or resolve it in a way it doesn't result in build
>> failures?
> 
> Thanks, Shuah.
> 
> The clang __builtin_preserve_access_index() intrinsic is
> introduced in LLVM9 (which just released last week) and
> the builtin and other CO-RE features are only supported
> in LLVM10 (current development branch) with more bug fixes
> and added features.
> 
> I think we should do a feature test for llvm version and only
> enable these tests when llvm version >= 10.

Yes. If new tests depend on a particular llvm revision, the failing
the build is a regression. I would like to see older tests that don't
have dependency build and run.

thanks,
-- Shuah

Re: Linux 5.4 - bpf test build fails

[Cristian Marussi]

Hi Shuah

On 24/09/2019 16:26, Shuah Khan wrote:
> Hi Alexei and Daniel,
> 
> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
> happening here.
> 
> 
> make -C tools/testing/selftests/bpf/

side question, since I'm writing arm64/ tests.

my "build-testcases" following the KSFT docs are:

make kselftest
make TARGETS=arm64 kselftest
make -C tools/testing/selftests/ 
make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
make TARGETS=arm64 -C tools/testing/selftests/ 
make TARGETS=arm64 -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
./kselftest_install.sh <install-path>

(and related clean targets...)

but definitely NOT

make -C tools/testing/selftests/arm64

(for simplicity....due to the subdirs structure under tools/testing/selftests/arm64/)

am I wrong ?

Thanks

Cristian

> 
> -c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
> llc -march=bpf -mcpu=generic  -filetype=obj -o 
> /mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o
> progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
>        '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
>          if (BPF_CORE_READ(&out->a, &in[2].a))
>              ^
> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>                         __builtin_preserve_access_index(src))
>                         ^
> progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
>        pointer conversion passing 'int' to parameter of type 'const void *'
>        [-Wint-conversion]
>          if (BPF_CORE_READ(&out->a, &in[2].a))
>              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>                         __builtin_preserve_access_index(src))
>                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 warning and 1 error generated.
> llc: error: llc: <stdin>:1:1: error: expected top-level entity
> clang failed
> 
> Also
> 
> make TARGETS=bpf kselftest fails as well. Dependency between
> tools/lib/bpf and the test. How can we avoid this type of
> dependency or resolve it in a way it doesn't result in build
> failures?
> 
> thanks,
> -- Shuah
> 

Re: Linux 5.4 - bpf test build fails

[Shuah Khan]

On 9/24/19 9:52 AM, Cristian Marussi wrote:
> Hi Shuah
> 
> On 24/09/2019 16:26, Shuah Khan wrote:
>> Hi Alexei and Daniel,
>>
>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
>> happening here.
>>
>>
>> make -C tools/testing/selftests/bpf/
> 
> side question, since I'm writing arm64/ tests.
> 
> my "build-testcases" following the KSFT docs are:
> 
> make kselftest
> make TARGETS=arm64 kselftest
> make -C tools/testing/selftests/
> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
> make TARGETS=arm64 -C tools/testing/selftests/
> make TARGETS=arm64 -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
> ./kselftest_install.sh <install-path>
> 
> (and related clean targets...)
> 
> but definitely NOT
> 
> make -C tools/testing/selftests/arm64
> 
> (for simplicity....due to the subdirs structure under tools/testing/selftests/arm64/)
> 

Some people like to build their tests using:

make -C tools/testing/selftests/<testdir>

I have been continuing to support it for that reason.

Tests with subdirs can handle this case. Pleas see android and futex
tests.

thanks,
-- Shuah

Re: Linux 5.4 - bpf test build fails

[Shuah Khan]

On 9/24/19 10:03 AM, Shuah Khan wrote:
> On 9/24/19 9:52 AM, Cristian Marussi wrote:
>> Hi Shuah
>>
>> On 24/09/2019 16:26, Shuah Khan wrote:
>>> Hi Alexei and Daniel,
>>>
>>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
>>> happening here.
>>>
>>>
>>> make -C tools/testing/selftests/bpf/
>>
>> side question, since I'm writing arm64/ tests.
>>
>> my "build-testcases" following the KSFT docs are:
>>
>> make kselftest
>> make TARGETS=arm64 kselftest
>> make -C tools/testing/selftests/
>> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
>> make TARGETS=arm64 -C tools/testing/selftests/
>> make TARGETS=arm64 -C tools/testing/selftests/ 
>> INSTALL_PATH=<install-path> install
>> ./kselftest_install.sh <install-path>

Cristian,

That being said, I definitely want to see this list limited to
a few options.

One problem is that if somebody wants to do just a build, there
is no option from the main makefile. I have sent support for that
a few months ago and the patch didn't got lost it appears. I am
working on resending those patches. The same is true for install.
I sent in a patch for that a while back and I am going to resend.
These will make it easier for users.

I would really want to get to supporting only these options:

These are supported now:

make kselftest
make TARGETS=arm64 kselftest (one or more targets)

Replace the following:

make -C tools/testing/selftests/ with

make kselftes_build option from main makefile

Replace this:
make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install

with
make kselftest_install

That way we can support all the use-cases from the main Makefile

thanks,
-- Shuah

Re: Linux 5.4 - bpf test build fails

[Cristian Marussi]

Hi Shuah

On 24/09/2019 17:39, Shuah Khan wrote:
> On 9/24/19 10:03 AM, Shuah Khan wrote:
>> On 9/24/19 9:52 AM, Cristian Marussi wrote:
>>> Hi Shuah
>>>
>>> On 24/09/2019 16:26, Shuah Khan wrote:
>>>> Hi Alexei and Daniel,
>>>>
>>>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
>>>> happening here.
>>>>
>>>>
>>>> make -C tools/testing/selftests/bpf/
>>>
>>> side question, since I'm writing arm64/ tests.
>>>
>>> my "build-testcases" following the KSFT docs are:
>>>
>>> make kselftest
>>> make TARGETS=arm64 kselftest
>>> make -C tools/testing/selftests/
>>> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
>>> make TARGETS=arm64 -C tools/testing/selftests/
>>> make TARGETS=arm64 -C tools/testing/selftests/ 
>>> INSTALL_PATH=<install-path> install
>>> ./kselftest_install.sh <install-path>
> 
> Cristian,
> 
> That being said, I definitely want to see this list limited to
> a few options.
> 
> One problem is that if somebody wants to do just a build, there
> is no option from the main makefile. I have sent support for that
> a few months ago and the patch didn't got lost it appears. I am
> working on resending those patches. The same is true for install.
> I sent in a patch for that a while back and I am going to resend.
> These will make it easier for users.
> 
> I would really want to get to supporting only these options:
> 
> These are supported now:
> 
> make kselftest
> make TARGETS=arm64 kselftest (one or more targets)
> 
> Replace the following:
> 
> make -C tools/testing/selftests/ with
> 
> make kselftes_build option from main makefile
> 
> Replace this:
> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
> 
> with
> make kselftest_install

Yes these top level options would be absolutely useful to avoid multiplication
of build targets to support and test.

Moreover, currently, since there was a lot of test growing into arm64/
inside subdirs like arm64/signal, I support (still under review in fact) in the arm64/
toplevel makefile the possibility of building/installing by subdirs only, in order
to be able to limit what you want to build/install of a TARGET (resulting in quicker devel),
issuing something like:

make TARGETS=arm64 SUBTARGETS=signal -C tools/testing/selftests/

if possible, that would be useful if kept functional even in the
new schema. I mean being able to still issue:

make TARGETS=arm64 SUBTARGETS=signal kselftes_build 

with the SUBTARGETS= or whatever ENV var handling delegated to the lower level
makefiles (so not handled by the toplevel, but just let go through)

Cheers

Cristian

> 
> That way we can support all the use-cases from the main Makefile
> 
> thanks,
> -- Shuah
> 
> 

RE: Linux 5.4 - bpf test build fails

[Tim.Bird]

> -----Original Message-----
> From: Cristian Marussi on Tuesday, September 24, 2019 7:30 AM
> 
> Hi Shuah
> 
> On 24/09/2019 17:39, Shuah Khan wrote:
> > On 9/24/19 10:03 AM, Shuah Khan wrote:
> >> On 9/24/19 9:52 AM, Cristian Marussi wrote:
> >>> Hi Shuah
> >>>
> >>> On 24/09/2019 16:26, Shuah Khan wrote:
> >>>> Hi Alexei and Daniel,
> >>>>
> >>>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
> >>>> happening here.
> >>>>
> >>>>
> >>>> make -C tools/testing/selftests/bpf/
> >>>
> >>> side question, since I'm writing arm64/ tests.
> >>>
> >>> my "build-testcases" following the KSFT docs are:
> >>>
> >>> make kselftest
> >>> make TARGETS=arm64 kselftest
> >>> make -C tools/testing/selftests/
> >>> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
> >>> make TARGETS=arm64 -C tools/testing/selftests/
> >>> make TARGETS=arm64 -C tools/testing/selftests/
> >>> INSTALL_PATH=<install-path> install
> >>> ./kselftest_install.sh <install-path>
> >
> > Cristian,
> >
> > That being said, I definitely want to see this list limited to
> > a few options.
> >
> > One problem is that if somebody wants to do just a build, there
> > is no option from the main makefile. I have sent support for that
> > a few months ago and the patch didn't got lost it appears. I am
> > working on resending those patches. The same is true for install.
> > I sent in a patch for that a while back and I am going to resend.
> > These will make it easier for users.
> >
> > I would really want to get to supporting only these options:
> >
> > These are supported now:
> >
> > make kselftest
> > make TARGETS=arm64 kselftest (one or more targets)
> >
> > Replace the following:
> >
> > make -C tools/testing/selftests/ with
> >
> > make kselftes_build option from main makefile
> >
> > Replace this:
> > make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
> >
> > with
> > make kselftest_install
> 
> Yes these top level options would be absolutely useful to avoid multiplication
> of build targets to support and test.
> 
> Moreover, currently, since there was a lot of test growing into arm64/
> inside subdirs like arm64/signal, I support (still under review in fact) in the
> arm64/
> toplevel makefile the possibility of building/installing by subdirs only, in order
> to be able to limit what you want to build/install of a TARGET (resulting in
> quicker devel),
> issuing something like:
> 
> make TARGETS=arm64 SUBTARGETS=signal -C tools/testing/selftests/
> 
> if possible, that would be useful if kept functional even in the
> new schema. I mean being able to still issue:
> 
> make TARGETS=arm64 SUBTARGETS=signal kselftes_build

From a user perspective, instead of adding a new SUBTARGETS variable,
I would prefer something like the following:

make TARGET=arm64/signal kselftest_build

If you just add a single flat subsidiary namespace, then it doesn't support further
increasing the directory depth in the future.

 -- Tim

> with the SUBTARGETS= or whatever ENV var handling delegated to the lower
> level
> makefiles (so not handled by the toplevel, but just let go through)
> 
> Cheers
> 
> Cristian
> 
> >
> > That way we can support all the use-cases from the main Makefile
> >
> > thanks,
> > -- Shuah
> >
> >

Re: Linux 5.4 - bpf test build fails

[Shuah Khan]

On 9/24/19 12:07 PM, Tim.Bird@sony.com wrote:
> 
> 
>> -----Original Message-----
>> From: Cristian Marussi on Tuesday, September 24, 2019 7:30 AM
>>
>> Hi Shuah
>>
>> On 24/09/2019 17:39, Shuah Khan wrote:
>>> On 9/24/19 10:03 AM, Shuah Khan wrote:
>>>> On 9/24/19 9:52 AM, Cristian Marussi wrote:
>>>>> Hi Shuah
>>>>>
>>>>> On 24/09/2019 16:26, Shuah Khan wrote:
>>>>>> Hi Alexei and Daniel,
>>>>>>
>>>>>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
>>>>>> happening here.
>>>>>>
>>>>>>
>>>>>> make -C tools/testing/selftests/bpf/
>>>>>
>>>>> side question, since I'm writing arm64/ tests.
>>>>>
>>>>> my "build-testcases" following the KSFT docs are:
>>>>>
>>>>> make kselftest
>>>>> make TARGETS=arm64 kselftest
>>>>> make -C tools/testing/selftests/
>>>>> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
>>>>> make TARGETS=arm64 -C tools/testing/selftests/
>>>>> make TARGETS=arm64 -C tools/testing/selftests/
>>>>> INSTALL_PATH=<install-path> install
>>>>> ./kselftest_install.sh <install-path>
>>>
>>> Cristian,
>>>
>>> That being said, I definitely want to see this list limited to
>>> a few options.
>>>
>>> One problem is that if somebody wants to do just a build, there
>>> is no option from the main makefile. I have sent support for that
>>> a few months ago and the patch didn't got lost it appears. I am
>>> working on resending those patches. The same is true for install.
>>> I sent in a patch for that a while back and I am going to resend.
>>> These will make it easier for users.
>>>
>>> I would really want to get to supporting only these options:
>>>
>>> These are supported now:
>>>
>>> make kselftest
>>> make TARGETS=arm64 kselftest (one or more targets)
>>>
>>> Replace the following:
>>>
>>> make -C tools/testing/selftests/ with
>>>
>>> make kselftes_build option from main makefile
>>>
>>> Replace this:
>>> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
>>>
>>> with
>>> make kselftest_install
>>
>> Yes these top level options would be absolutely useful to avoid multiplication
>> of build targets to support and test.
>>
>> Moreover, currently, since there was a lot of test growing into arm64/
>> inside subdirs like arm64/signal, I support (still under review in fact) in the
>> arm64/
>> toplevel makefile the possibility of building/installing by subdirs only, in order
>> to be able to limit what you want to build/install of a TARGET (resulting in
>> quicker devel),
>> issuing something like:
>>
>> make TARGETS=arm64 SUBTARGETS=signal -C tools/testing/selftests/
>>
>> if possible, that would be useful if kept functional even in the
>> new schema. I mean being able to still issue:
>>
>> make TARGETS=arm64 SUBTARGETS=signal kselftes_build
> 
>  From a user perspective, instead of adding a new SUBTARGETS variable,
> I would prefer something like the following:
> 
> make TARGET=arm64/signal kselftest_build
> 
> If you just add a single flat subsidiary namespace, then it doesn't support further
> increasing the directory depth in the future.
> 

TARGETS is make variable. Adding sub-targets might not be easy without
cluttering the selftests main Makefile. I will have to look into it.

thanks,
-- Shuah

Re: Linux 5.4 - bpf test build fails

[Daniel Borkmann]

On Tue, Sep 24, 2019 at 09:48:35AM -0600, Shuah Khan wrote:
> On 9/24/19 9:43 AM, Yonghong Song wrote:
> > On 9/24/19 8:26 AM, Shuah Khan wrote:
> > > Hi Alexei and Daniel,
> > > 
> > > bpf test doesn't build on Linux 5.4 mainline. Do you know what's
> > > happening here.
> > > 
> > > make -C tools/testing/selftests/bpf/
> > > 
> > > -c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
> > > llc -march=bpf -mcpu=generic  -filetype=obj -o
> > > /mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o
> > > 
> > > progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
> > >         '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
> > >           if (BPF_CORE_READ(&out->a, &in[2].a))
> > >               ^
> > > ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
> > >                          __builtin_preserve_access_index(src))
> > >                          ^
> > > progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
> > >         pointer conversion passing 'int' to parameter of type 'const void *'
> > >         [-Wint-conversion]
> > >           if (BPF_CORE_READ(&out->a, &in[2].a))
> > >               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
> > >                          __builtin_preserve_access_index(src))
> > >                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 1 warning and 1 error generated.
> > > llc: error: llc: <stdin>:1:1: error: expected top-level entity
> > > clang failed
> > > 
> > > Also
> > > 
> > > make TARGETS=bpf kselftest fails as well. Dependency between
> > > tools/lib/bpf and the test. How can we avoid this type of
> > > dependency or resolve it in a way it doesn't result in build
> > > failures?
> > 
> > Thanks, Shuah.
> > 
> > The clang __builtin_preserve_access_index() intrinsic is
> > introduced in LLVM9 (which just released last week) and
> > the builtin and other CO-RE features are only supported
> > in LLVM10 (current development branch) with more bug fixes
> > and added features.
> > 
> > I think we should do a feature test for llvm version and only
> > enable these tests when llvm version >= 10.
> 
> Yes. If new tests depend on a particular llvm revision, the failing
> the build is a regression. I would like to see older tests that don't
> have dependency build and run.

So far we haven't made it a requirement as majority of BPF contributors
that would run/add tests in here are also on bleeding edge LLVM anyway
and other CIs like 0-day bot have simply upgraded their LLVM version
from git whenever there was a failure similar to the one here so its
ensured that really /all/ test cases are running and nothing would be
skipped. There is worry to some degree that CIs just keep sticking to
an old compiler since tests "just" pass and regressions wouldn't be
caught on new releases for those that are skipped.

That said, for the C based tests, it should actually be straight forward
to categorize them based on built-in macros like ...

$ echo | clang -dM -E -
[...]
#define __clang_major__ 10
#define __clang_minor__ 0
[...]

... given there is now also bpf-gcc, the test matrix gets bigger anyway,
so it might be worth rethinking to run the suite multiple times with
different major llvm{,gcc} versions at some point to make sure their
generated BPF bytecode keeps passing the verifier, and yell loudly if
newer features had to be skipped due to lack of recent compiler version.
This would be a super set of /just/ skipping tests and improve coverage
at the same time.

Thanks,
Daniel

Re: Linux 5.4 - bpf test build fails

[Shuah Khan]

On 9/24/19 12:49 PM, Daniel Borkmann wrote:
> On Tue, Sep 24, 2019 at 09:48:35AM -0600, Shuah Khan wrote:
>> On 9/24/19 9:43 AM, Yonghong Song wrote:
>>> On 9/24/19 8:26 AM, Shuah Khan wrote:
>>>> Hi Alexei and Daniel,
>>>>
>>>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
>>>> happening here.
>>>>
>>>> make -C tools/testing/selftests/bpf/
>>>>
>>>> -c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
>>>> llc -march=bpf -mcpu=generic  -filetype=obj -o
>>>> /mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o
>>>>
>>>> progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
>>>>          '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
>>>>            if (BPF_CORE_READ(&out->a, &in[2].a))
>>>>                ^
>>>> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>>>>                           __builtin_preserve_access_index(src))
>>>>                           ^
>>>> progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
>>>>          pointer conversion passing 'int' to parameter of type 'const void *'
>>>>          [-Wint-conversion]
>>>>            if (BPF_CORE_READ(&out->a, &in[2].a))
>>>>                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>>>>                           __builtin_preserve_access_index(src))
>>>>                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> 1 warning and 1 error generated.
>>>> llc: error: llc: <stdin>:1:1: error: expected top-level entity
>>>> clang failed
>>>>
>>>> Also
>>>>
>>>> make TARGETS=bpf kselftest fails as well. Dependency between
>>>> tools/lib/bpf and the test. How can we avoid this type of
>>>> dependency or resolve it in a way it doesn't result in build
>>>> failures?
>>>
>>> Thanks, Shuah.
>>>
>>> The clang __builtin_preserve_access_index() intrinsic is
>>> introduced in LLVM9 (which just released last week) and
>>> the builtin and other CO-RE features are only supported
>>> in LLVM10 (current development branch) with more bug fixes
>>> and added features.
>>>
>>> I think we should do a feature test for llvm version and only
>>> enable these tests when llvm version >= 10.
>>
>> Yes. If new tests depend on a particular llvm revision, the failing
>> the build is a regression. I would like to see older tests that don't
>> have dependency build and run.
> 
> So far we haven't made it a requirement as majority of BPF contributors
> that would run/add tests in here are also on bleeding edge LLVM anyway
> and other CIs like 0-day bot have simply upgraded their LLVM version
> from git whenever there was a failure similar to the one here so its
> ensured that really /all/ test cases are running and nothing would be
> skipped. There is worry to some degree that CIs just keep sticking to
> an old compiler since tests "just" pass and regressions wouldn't be
> caught on new releases for those that are skipped. >

Sure. Bleeding edge is developer mode. We still have to be concerned
about users that might not upgrade quickly.

> That said, for the C based tests, it should actually be straight forward
> to categorize them based on built-in macros like ...
> 
> $ echo | clang -dM -E -
> [...]
> #define __clang_major__ 10
> #define __clang_minor__ 0
> [...]
> 

What would nice running the tests you can run and then say some tests
aren't going to run. Is this something you can support?

> ... given there is now also bpf-gcc, the test matrix gets bigger anyway,
> so it might be worth rethinking to run the suite multiple times with
> different major llvm{,gcc} versions at some point to make sure their
> generated BPF bytecode keeps passing the verifier, and yell loudly if
> newer features had to be skipped due to lack of recent compiler version.
> This would be a super set of /just/ skipping tests and improve coverage
> at the same time.
> 

Probably. Reality is most users will just quit and add bpf to "hard to
run category" of tests.

thanks,
-- Shuah

Re: Linux 5.4 - bpf test build fails

[Daniel Borkmann]

On Tue, Sep 24, 2019 at 12:56:53PM -0600, Shuah Khan wrote:
> On 9/24/19 12:49 PM, Daniel Borkmann wrote:
> > On Tue, Sep 24, 2019 at 09:48:35AM -0600, Shuah Khan wrote:
> > > On 9/24/19 9:43 AM, Yonghong Song wrote:
> > > > On 9/24/19 8:26 AM, Shuah Khan wrote:
> > > > > Hi Alexei and Daniel,
> > > > > 
> > > > > bpf test doesn't build on Linux 5.4 mainline. Do you know what's
> > > > > happening here.
> > > > > 
> > > > > make -C tools/testing/selftests/bpf/
> > > > > 
> > > > > -c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
> > > > > llc -march=bpf -mcpu=generic  -filetype=obj -o
> > > > > /mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o
> > > > > 
> > > > > progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
> > > > >          '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
> > > > >            if (BPF_CORE_READ(&out->a, &in[2].a))
> > > > >                ^
> > > > > ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
> > > > >                           __builtin_preserve_access_index(src))
> > > > >                           ^
> > > > > progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
> > > > >          pointer conversion passing 'int' to parameter of type 'const void *'
> > > > >          [-Wint-conversion]
> > > > >            if (BPF_CORE_READ(&out->a, &in[2].a))
> > > > >                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
> > > > >                           __builtin_preserve_access_index(src))
> > > > >                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > 1 warning and 1 error generated.
> > > > > llc: error: llc: <stdin>:1:1: error: expected top-level entity
> > > > > clang failed
> > > > > 
> > > > > Also
> > > > > 
> > > > > make TARGETS=bpf kselftest fails as well. Dependency between
> > > > > tools/lib/bpf and the test. How can we avoid this type of
> > > > > dependency or resolve it in a way it doesn't result in build
> > > > > failures?
> > > > 
> > > > Thanks, Shuah.
> > > > 
> > > > The clang __builtin_preserve_access_index() intrinsic is
> > > > introduced in LLVM9 (which just released last week) and
> > > > the builtin and other CO-RE features are only supported
> > > > in LLVM10 (current development branch) with more bug fixes
> > > > and added features.
> > > > 
> > > > I think we should do a feature test for llvm version and only
> > > > enable these tests when llvm version >= 10.
> > > 
> > > Yes. If new tests depend on a particular llvm revision, the failing
> > > the build is a regression. I would like to see older tests that don't
> > > have dependency build and run.
> > 
> > So far we haven't made it a requirement as majority of BPF contributors
> > that would run/add tests in here are also on bleeding edge LLVM anyway
> > and other CIs like 0-day bot have simply upgraded their LLVM version
> > from git whenever there was a failure similar to the one here so its
> > ensured that really /all/ test cases are running and nothing would be
> > skipped. There is worry to some degree that CIs just keep sticking to
> > an old compiler since tests "just" pass and regressions wouldn't be
> > caught on new releases for those that are skipped. >
> 
> Sure. Bleeding edge is developer mode. We still have to be concerned
> about users that might not upgrade quickly.
> 
> > That said, for the C based tests, it should actually be straight forward
> > to categorize them based on built-in macros like ...
> > 
> > $ echo | clang -dM -E -
> > [...]
> > #define __clang_major__ 10
> > #define __clang_minor__ 0
> > [...]
> 
> What would nice running the tests you can run and then say some tests
> aren't going to run. Is this something you can support?

Once there is such infra in place, should be possible.

> > ... given there is now also bpf-gcc, the test matrix gets bigger anyway,
> > so it might be worth rethinking to run the suite multiple times with
> > different major llvm{,gcc} versions at some point to make sure their
> > generated BPF bytecode keeps passing the verifier, and yell loudly if
> > newer features had to be skipped due to lack of recent compiler version.
> > This would be a super set of /just/ skipping tests and improve coverage
> > at the same time.
> 
> Probably. Reality is most users will just quit and add bpf to "hard to
> run category" of tests.

I don't really worry too much about such users at this point, more important
is that we have a way to test bpf-gcc and llvm behavior side by side to
make sure behavior is consistent and to have some sort of automated CI
integration that runs BPF kselftests before we even stare at a patch for
review. These are right now the two highest prio items from BPF testing
side where we need to get to.

Thanks,
Daniel

Re: Linux 5.4 - bpf test build fails

[Shuah Khan]

On 9/24/19 1:19 PM, Daniel Borkmann wrote:
> On Tue, Sep 24, 2019 at 12:56:53PM -0600, Shuah Khan wrote:
>> On 9/24/19 12:49 PM, Daniel Borkmann wrote:
>>> On Tue, Sep 24, 2019 at 09:48:35AM -0600, Shuah Khan wrote:
>>>> On 9/24/19 9:43 AM, Yonghong Song wrote:
>>>>> On 9/24/19 8:26 AM, Shuah Khan wrote:
>>>>>> Hi Alexei and Daniel,
>>>>>>
>>>>>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
>>>>>> happening here.
>>>>>>
>>>>>> make -C tools/testing/selftests/bpf/
>>>>>>
>>>>>> -c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
>>>>>> llc -march=bpf -mcpu=generic  -filetype=obj -o
>>>>>> /mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o
>>>>>>
>>>>>> progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
>>>>>>           '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
>>>>>>             if (BPF_CORE_READ(&out->a, &in[2].a))
>>>>>>                 ^
>>>>>> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>>>>>>                            __builtin_preserve_access_index(src))
>>>>>>                            ^
>>>>>> progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
>>>>>>           pointer conversion passing 'int' to parameter of type 'const void *'
>>>>>>           [-Wint-conversion]
>>>>>>             if (BPF_CORE_READ(&out->a, &in[2].a))
>>>>>>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>> ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
>>>>>>                            __builtin_preserve_access_index(src))
>>>>>>                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>> 1 warning and 1 error generated.
>>>>>> llc: error: llc: <stdin>:1:1: error: expected top-level entity
>>>>>> clang failed
>>>>>>
>>>>>> Also
>>>>>>
>>>>>> make TARGETS=bpf kselftest fails as well. Dependency between
>>>>>> tools/lib/bpf and the test. How can we avoid this type of
>>>>>> dependency or resolve it in a way it doesn't result in build
>>>>>> failures?
>>>>>
>>>>> Thanks, Shuah.
>>>>>
>>>>> The clang __builtin_preserve_access_index() intrinsic is
>>>>> introduced in LLVM9 (which just released last week) and
>>>>> the builtin and other CO-RE features are only supported
>>>>> in LLVM10 (current development branch) with more bug fixes
>>>>> and added features.
>>>>>
>>>>> I think we should do a feature test for llvm version and only
>>>>> enable these tests when llvm version >= 10.
>>>>
>>>> Yes. If new tests depend on a particular llvm revision, the failing
>>>> the build is a regression. I would like to see older tests that don't
>>>> have dependency build and run.
>>>
>>> So far we haven't made it a requirement as majority of BPF contributors
>>> that would run/add tests in here are also on bleeding edge LLVM anyway
>>> and other CIs like 0-day bot have simply upgraded their LLVM version
>>> from git whenever there was a failure similar to the one here so its
>>> ensured that really /all/ test cases are running and nothing would be
>>> skipped. There is worry to some degree that CIs just keep sticking to
>>> an old compiler since tests "just" pass and regressions wouldn't be
>>> caught on new releases for those that are skipped. >
>>
>> Sure. Bleeding edge is developer mode. We still have to be concerned
>> about users that might not upgrade quickly.
>>
>>> That said, for the C based tests, it should actually be straight forward
>>> to categorize them based on built-in macros like ...
>>>
>>> $ echo | clang -dM -E -
>>> [...]
>>> #define __clang_major__ 10
>>> #define __clang_minor__ 0
>>> [...]
>>
>> What would nice running the tests you can run and then say some tests
>> aren't going to run. Is this something you can support?
> 
> Once there is such infra in place, should be possible.

Can't you do it in bpf run-time or during build for dependency?
You should be able to handle this as a dependency and let users
know at least.

> 
>>> ... given there is now also bpf-gcc, the test matrix gets bigger anyway,
>>> so it might be worth rethinking to run the suite multiple times with
>>> different major llvm{,gcc} versions at some point to make sure their
>>> generated BPF bytecode keeps passing the verifier, and yell loudly if
>>> newer features had to be skipped due to lack of recent compiler version.
>>> This would be a super set of /just/ skipping tests and improve coverage
>>> at the same time.
>>
>> Probably. Reality is most users will just quit and add bpf to "hard to
>> run category" of tests.
> 
> I don't really worry too much about such users at this point, more important
> is that we have a way to test bpf-gcc and llvm behavior side by side to
> make sure behavior is consistent and to have some sort of automated CI
> integration that runs BPF kselftests before we even stare at a patch for
> review. These are right now the two highest prio items from BPF testing
> side where we need to get to.
> 

What happens if CI's can't upgrade quickly and newer versions aren't
supported on test machines that are in their test rings?

thanks,
-- Shuah

Re: Linux 5.4 - bpf test build fails

[Cristian Marussi]

Hi Shuah, Tim

On 9/24/19 7:23 PM, Shuah Khan wrote:
> On 9/24/19 12:07 PM, Tim.Bird@sony.com wrote:
>>
>>
>>> -----Original Message-----
>>> From: Cristian Marussi on Tuesday, September 24, 2019 7:30 AM
>>>
>>> Hi Shuah
>>>
>>> On 24/09/2019 17:39, Shuah Khan wrote:
>>>> On 9/24/19 10:03 AM, Shuah Khan wrote:
>>>>> On 9/24/19 9:52 AM, Cristian Marussi wrote:
>>>>>> Hi Shuah
>>>>>>
>>>>>> On 24/09/2019 16:26, Shuah Khan wrote:
>>>>>>> Hi Alexei and Daniel,
>>>>>>>
>>>>>>> bpf test doesn't build on Linux 5.4 mainline. Do you know what's
>>>>>>> happening here.
>>>>>>>
>>>>>>>
>>>>>>> make -C tools/testing/selftests/bpf/
>>>>>>
>>>>>> side question, since I'm writing arm64/ tests.
>>>>>>
>>>>>> my "build-testcases" following the KSFT docs are:
>>>>>>
>>>>>> make kselftest
>>>>>> make TARGETS=arm64 kselftest
>>>>>> make -C tools/testing/selftests/
>>>>>> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
>>>>>> make TARGETS=arm64 -C tools/testing/selftests/
>>>>>> make TARGETS=arm64 -C tools/testing/selftests/
>>>>>> INSTALL_PATH=<install-path> install
>>>>>> ./kselftest_install.sh <install-path>
>>>>
>>>> Cristian,
>>>>
>>>> That being said, I definitely want to see this list limited to
>>>> a few options.
>>>>
>>>> One problem is that if somebody wants to do just a build, there
>>>> is no option from the main makefile. I have sent support for that
>>>> a few months ago and the patch didn't got lost it appears. I am
>>>> working on resending those patches. The same is true for install.
>>>> I sent in a patch for that a while back and I am going to resend.
>>>> These will make it easier for users.
>>>>
>>>> I would really want to get to supporting only these options:
>>>>
>>>> These are supported now:
>>>>
>>>> make kselftest
>>>> make TARGETS=arm64 kselftest (one or more targets)
>>>>
>>>> Replace the following:
>>>>
>>>> make -C tools/testing/selftests/ with
>>>>
>>>> make kselftes_build option from main makefile
>>>>
>>>> Replace this:
>>>> make -C tools/testing/selftests/ INSTALL_PATH=<install-path> install
>>>>
>>>> with
>>>> make kselftest_install
>>>
>>> Yes these top level options would be absolutely useful to avoid multiplication
>>> of build targets to support and test.
>>>
>>> Moreover, currently, since there was a lot of test growing into arm64/
>>> inside subdirs like arm64/signal, I support (still under review in fact) in the
>>> arm64/
>>> toplevel makefile the possibility of building/installing by subdirs only, in order
>>> to be able to limit what you want to build/install of a TARGET (resulting in
>>> quicker devel),
>>> issuing something like:
>>>
>>> make TARGETS=arm64 SUBTARGETS=signal -C tools/testing/selftests/
>>>
>>> if possible, that would be useful if kept functional even in the
>>> new schema. I mean being able to still issue:
>>>
>>> make TARGETS=arm64 SUBTARGETS=signal kselftes_build
>>
>>  From a user perspective, instead of adding a new SUBTARGETS variable,
>> I would prefer something like the following:
>>
>> make TARGET=arm64/signal kselftest_build
>>
>> If you just add a single flat subsidiary namespace, then it doesn't support further
>> increasing the directory depth in the future.
>>
> 
> TARGETS is make variable. Adding sub-targets might not be easy without
> cluttering the selftests main Makefile. I will have to look into it.
> 

I was NOT proposing in fact to introduce handling of SUBTARGETS at the toplevel kselftest
Makefile, just not to kill the possibility for interested subsystems to handle it as they
wish in their own toplevel subsystem Makefile like in arm64/Makefile in:

https://lore.kernel.org/linux-kselftest/20190910123111.33478-2-cristian.marussi@arm.com/

(probably better renaming SUBTARGETS->ARM64_SUBTARGETS in the next v7 to avoid name clashing)

Because it's painful from the development perspective not having the possibility to selectively
build/install only a subset/subdir of the chosen TARGETS; but it's a very subsystem specific issue
and not everybody need it, so I'd let the respective TARGETS subsystems handle it if they want/need.

Thanks

Cristian

> thanks,
> -- Shuah
>