Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1732347 > unrolled thread

Re: [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems Shuah Khan <shuah@kernel.org> - 2017-09-14 17:10 +0200
    selftests/bpf doesn't compile Shuah Khan <shuah@kernel.org> - 2017-09-14 17:40 +0200
      Re: selftests/bpf doesn't compile Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2017-09-15 18:10 +0200
        Re: selftests/bpf doesn't compile Edward Cree <ecree@solarflare.com> - 2017-09-15 19:00 +0200
          Re: selftests/bpf doesn't compile Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2017-09-15 20:10 +0200
            Re: selftests/bpf doesn't compile Daniel Borkmann <daniel@iogearbox.net> - 2017-09-15 20:30 +0200
              Re: selftests/bpf doesn't compile Daniel Borkmann <daniel@iogearbox.net> - 2017-09-15 20:50 +0200
                Re: selftests/bpf doesn't compile Shuah Khan <shuahkh@osg.samsung.com> - 2017-09-16 00:50 +0200
                  Re: selftests/bpf doesn't compile Daniel Borkmann <daniel@iogearbox.net> - 2017-09-18 15:40 +0200
        Re: selftests/bpf doesn't compile Shuah Khan <shuahkh@osg.samsung.com> - 2017-09-15 19:10 +0200
          Re: selftests/bpf doesn't compile Shuah Khan <shuahkh@osg.samsung.com> - 2017-09-15 19:50 +0200
          Re: selftests/bpf doesn't compile Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2017-09-15 20:20 +0200
            Re: selftests/bpf doesn't compile Shuah Khan <shuahkh@osg.samsung.com> - 2017-09-16 00:40 +0200
    Re: [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems Shuah Khan <shuah@kernel.org> - 2017-09-19 16:50 +0200

#1732347 — Re: [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems

On 09/08/2017 05:05 PM, Daniel Borkmann wrote:
> On 09/09/2017 01:01 AM, Alexei Starovoitov wrote:
>> On Fri, Sep 08, 2017 at 01:19:23PM +0200, Thomas Meyer wrote:
>>> The current implementation fails to work on uniprocessor systems.
>>> Fix the parser to also handle the uniprocessor case.
>>>
>>> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
>>
>> Thanks for the fix. lgtm
>> Acked-by: Alexei Starovoitov <ast@kernel.org>
> 
> Looks good from here as well:
> 
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> 
>> This time it's ok to go via selftest tree, but next time please use net-next/net
>> to avoid conflicts.
> 
> +1
> 
>> Thanks

Thank you. I will get this into 4.14-rc2 or rc3.

thanks,
-- Shuah
>>
>>> ---
>>>   tools/testing/selftests/bpf/bpf_util.h | 17 +++++++++--------
>>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
>>> index 20ecbaa0d85d..6c53a8906eff 100644
>>> --- a/tools/testing/selftests/bpf/bpf_util.h
>>> +++ b/tools/testing/selftests/bpf/bpf_util.h
>>> @@ -12,6 +12,7 @@ static inline unsigned int bpf_num_possible_cpus(void)
>>>       unsigned int start, end, possible_cpus = 0;
>>>       char buff[128];
>>>       FILE *fp;
>>> +    int n;
>>>
>>>       fp = fopen(fcpu, "r");
>>>       if (!fp) {
>>> @@ -20,17 +21,17 @@ static inline unsigned int bpf_num_possible_cpus(void)
>>>       }
>>>
>>>       while (fgets(buff, sizeof(buff), fp)) {
>>> -        if (sscanf(buff, "%u-%u", &start, &end) == 2) {
>>> -            possible_cpus = start == 0 ? end + 1 : 0;
>>> -            break;
>>> +        n = sscanf(buff, "%u-%u", &start, &end);
>>> +        if (n == 0) {
>>> +            printf("Failed to retrieve # possible CPUs!\n");
>>> +            exit(1);
>>> +        } else if (n == 1) {
>>> +            end = start;
>>>           }
>>> +        possible_cpus = start == 0 ? end + 1 : 0;
>>> +        break;
>>>       }
>>> -
>>>       fclose(fp);
>>> -    if (!possible_cpus) {
>>> -        printf("Failed to retrieve # possible CPUs!\n");
>>> -        exit(1);
>>> -    }
>>>
>>>       return possible_cpus;
>>>   }
>>> -- 
>>> 2.11.0
>>>
> 
> 
> 

[toc] | [next] | [standalone]

#1732359 — selftests/bpf doesn't compile

Hi Alexei and Daniel,

bpf test depends on clang and fails to compile when

------------------------------------------------------
make -C tools/testing/selftests/bpf run_tests


make: clang: Command not found
Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'

With "make TARGETS=bpf kselftest" it fails earlier:


make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
Makefile:40: tools/scripts/Makefile.arch: No such file or directory
Makefile:84: tools/build/Makefile.feature: No such file or directory
Makefile:143: tools/build/Makefile.include: No such file or directory
make[3]: *** No rule to make target 'tools/build/Makefile.include'.  Stop.
make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
Makefile:69: recipe for target 'all' failed
make[1]: *** [all] Error 2
Makefile:1190: recipe for target 'kselftest' failed
make: *** [kselftest] Error 2

--------------------------------------------------------------

Is bpf test intended to be run in kselftest run? The clang dependency might
not be met on majority of the systems. Is this a hard dependency??

Would it make sense to create a special target for bpf test? We do have a few
tests that do that now. 

TARGETS_HOTPLUG = cpu-hotplug
TARGETS_HOTPLUG += memory-hotplug

I could add a special target for bpf TARGET_BPF perhaps and exclude it from
the run_tests.

Please let me know your thoughts on this.

thanks,
-- Shuah

[toc] | [prev] | [next] | [standalone]

#1732936 — Re: selftests/bpf doesn't compile

On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
> Hi Alexei and Daniel,
> 
> bpf test depends on clang and fails to compile when
> 
> ------------------------------------------------------
> make -C tools/testing/selftests/bpf run_tests
> 
> 
> make: clang: Command not found
> Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
> make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
> make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
> 
> With "make TARGETS=bpf kselftest" it fails earlier:
> 
> 
> make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
> Makefile:40: tools/scripts/Makefile.arch: No such file or directory
> Makefile:84: tools/build/Makefile.feature: No such file or directory
> Makefile:143: tools/build/Makefile.include: No such file or directory
> make[3]: *** No rule to make target 'tools/build/Makefile.include'.  Stop.
> make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
> Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
> make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
> make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
> Makefile:69: recipe for target 'all' failed
> make[1]: *** [all] Error 2
> Makefile:1190: recipe for target 'kselftest' failed
> make: *** [kselftest] Error 2
> 
> --------------------------------------------------------------
> 
> Is bpf test intended to be run in kselftest run? The clang dependency might
> not be met on majority of the systems. Is this a hard dependency??

It is a hard dependency and clang should be present on majority of the systems.
More details are in samples/bpf/README.rst
which was written long ago. Nowadays apt-get/yum will install clang
with bpf support builtin.

> Would it make sense to create a special target for bpf test? We do have a few
> tests that do that now. 
> 
> TARGETS_HOTPLUG = cpu-hotplug
> TARGETS_HOTPLUG += memory-hotplug
> 
> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
> the run_tests.

I'm not sure what was the motivation to exclude hotplug from default testing,
since I think it diminishes the value of selftests overall.
Not running all tests all the time risks breaking them.
selftest makefile refactoring broke selftests/bpf in the past,
so I strongly suggest to install clang and make sure the tests are passing
on the test servers otherwise we'd need to move selftests/bpf out of
selftests to avoid further headaches for us when selftests infra keeps
changing.

[toc] | [prev] | [next] | [standalone]

#1732950 — Re: selftests/bpf doesn't compile

On 15/09/17 17:02, Alexei Starovoitov wrote:
> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>> Is bpf test intended to be run in kselftest run? The clang dependency might
>> not be met on majority of the systems. Is this a hard dependency??
> It is a hard dependency and clang should be present on majority of the systems.
I think this is the wrong approach.  Making kselftest hard-require clang doesn't
 mean that the bpf tests will be run more often, it means that the rest of the
 kselftests will be run less often.  clang is quite big (when I tried to install
 it on one of my test servers, I didn't have enough disk space & had to go on a
 clear-out of unused packages), and most people aren't interested in the bpf
 subsystem specifically; they would rather be able to skip those tests.
I feel that as long as they know they are skipping some tests (so e.g. they
 won't consider it a sufficient test of a kselftest refactor), that's fine.
It's not even as though all of the bpf tests require clang; the (smaller) tests
 written directly in raw eBPF instructions could still be run on such a system.
 So I think we should attempt to run as much as possible but accept that clang
 may not be available and have an option to skip some tests in that case.

-Ed

[toc] | [prev] | [next] | [standalone]

#1732998 — Re: selftests/bpf doesn't compile

On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote:
> On 15/09/17 17:02, Alexei Starovoitov wrote:
> > On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
> >> Is bpf test intended to be run in kselftest run? The clang dependency might
> >> not be met on majority of the systems. Is this a hard dependency??
> > It is a hard dependency and clang should be present on majority of the systems.
> I think this is the wrong approach.  Making kselftest hard-require clang doesn't
>  mean that the bpf tests will be run more often, it means that the rest of the
>  kselftests will be run less often.  clang is quite big (when I tried to install
>  it on one of my test servers, I didn't have enough disk space & had to go on a
>  clear-out of unused packages), and most people aren't interested in the bpf
>  subsystem specifically; they would rather be able to skip those tests.
> I feel that as long as they know they are skipping some tests (so e.g. they
>  won't consider it a sufficient test of a kselftest refactor), that's fine.
> It's not even as though all of the bpf tests require clang; the (smaller) tests
>  written directly in raw eBPF instructions could still be run on such a system.
>  So I think we should attempt to run as much as possible but accept that clang
>  may not be available and have an option to skip some tests in that case.

imo the value of selftests/bpf is twofold:
1. it helps bpf developers avoid regressions
2. as part of continuous integration it helps to catch bpf regressions
that were somehow caused by changes in other parts of the kernel

If a developer didn't bother to satisfy all bpf tests dependencies
(which includes clang) and ran all tests before sending a patch,
I don't want to see such patches. It just wastes maintainers time
to review code and spot bugs that could have been caught by tests.
Collectively we invested years of work into these tests and
developers better take advantage of it by running all.

If a CI server didn't satisfy all bpf test dependencies,
I don't want such CI setup to be running and reporting results,
since it will give false sense of test coverage.
Test failures due to missing dependencies are hard failures.
We cannot skip them.

I'd like generic XDP tests to be added to selftests/bpf which
would mean that the latest iproute2 will become a hard dependency
and bpf developers and CI host owners would need to upgrade
their iproute2.
The tests either pass or fail. Skipping them due to missing
dependencies is the same as fail and in that sense I don't want
to change selftests/bpf/Makefile to make it skip clang.

[toc] | [prev] | [next] | [standalone]

#1733007 — Re: selftests/bpf doesn't compile

On 09/15/2017 08:07 PM, Alexei Starovoitov wrote:
> On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote:
>> On 15/09/17 17:02, Alexei Starovoitov wrote:
>>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>>>> Is bpf test intended to be run in kselftest run? The clang dependency might
>>>> not be met on majority of the systems. Is this a hard dependency??
>>> It is a hard dependency and clang should be present on majority of the systems.
>> I think this is the wrong approach.  Making kselftest hard-require clang doesn't
>>   mean that the bpf tests will be run more often, it means that the rest of the
>>   kselftests will be run less often.  clang is quite big (when I tried to install
>>   it on one of my test servers, I didn't have enough disk space & had to go on a
>>   clear-out of unused packages), and most people aren't interested in the bpf
>>   subsystem specifically; they would rather be able to skip those tests.
>> I feel that as long as they know they are skipping some tests (so e.g. they
>>   won't consider it a sufficient test of a kselftest refactor), that's fine.
>> It's not even as though all of the bpf tests require clang; the (smaller) tests
>>   written directly in raw eBPF instructions could still be run on such a system.
>>   So I think we should attempt to run as much as possible but accept that clang
>>   may not be available and have an option to skip some tests in that case.
>
> imo the value of selftests/bpf is twofold:
> 1. it helps bpf developers avoid regressions
> 2. as part of continuous integration it helps to catch bpf regressions
> that were somehow caused by changes in other parts of the kernel
>
> If a developer didn't bother to satisfy all bpf tests dependencies
> (which includes clang) and ran all tests before sending a patch,
> I don't want to see such patches. It just wastes maintainers time
> to review code and spot bugs that could have been caught by tests.
> Collectively we invested years of work into these tests and
> developers better take advantage of it by running all.

+1

> If a CI server didn't satisfy all bpf test dependencies,
> I don't want such CI setup to be running and reporting results,
> since it will give false sense of test coverage.
> Test failures due to missing dependencies are hard failures.
> We cannot skip them.

+1

> I'd like generic XDP tests to be added to selftests/bpf which
> would mean that the latest iproute2 will become a hard dependency
> and bpf developers and CI host owners would need to upgrade
> their iproute2.
> The tests either pass or fail. Skipping them due to missing
> dependencies is the same as fail and in that sense I don't want
> to change selftests/bpf/Makefile to make it skip clang.

I fully agree that for the BPF selftests it is very desirable
to not only test the verifier with couple of BPF insn snippets,
but to actually load and run programs that more closely resemble
real world programs. For more complex interactions these snippets
are just limited, think of tail calls, testing perf event output
helper, etc, which would all require to write these tests with
restricted C when we add them (unless we want to make writing
these tests a real pain ;) in which case no-one will bother to
write tests at all for them). Mid to long term I would definitely
like to see more programs in BPF selftests (e.g. moved over from
samples/bpf/) to increase the test coverage.

[toc] | [prev] | [next] | [standalone]

#1733017 — Re: selftests/bpf doesn't compile

On 09/15/2017 08:23 PM, Daniel Borkmann wrote:
> On 09/15/2017 08:07 PM, Alexei Starovoitov wrote:
>> On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote:
>>> On 15/09/17 17:02, Alexei Starovoitov wrote:
>>>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>>>>> Is bpf test intended to be run in kselftest run? The clang dependency might
>>>>> not be met on majority of the systems. Is this a hard dependency??
>>>> It is a hard dependency and clang should be present on majority of the systems.
>>> I think this is the wrong approach.  Making kselftest hard-require clang doesn't
>>>   mean that the bpf tests will be run more often, it means that the rest of the
>>>   kselftests will be run less often.  clang is quite big (when I tried to install
>>>   it on one of my test servers, I didn't have enough disk space & had to go on a
>>>   clear-out of unused packages), and most people aren't interested in the bpf
>>>   subsystem specifically; they would rather be able to skip those tests.
>>> I feel that as long as they know they are skipping some tests (so e.g. they
>>>   won't consider it a sufficient test of a kselftest refactor), that's fine.
>>> It's not even as though all of the bpf tests require clang; the (smaller) tests
>>>   written directly in raw eBPF instructions could still be run on such a system.
>>>   So I think we should attempt to run as much as possible but accept that clang
>>>   may not be available and have an option to skip some tests in that case.
>>
>> imo the value of selftests/bpf is twofold:
>> 1. it helps bpf developers avoid regressions
>> 2. as part of continuous integration it helps to catch bpf regressions
>> that were somehow caused by changes in other parts of the kernel
>>
>> If a developer didn't bother to satisfy all bpf tests dependencies
>> (which includes clang) and ran all tests before sending a patch,
>> I don't want to see such patches. It just wastes maintainers time
>> to review code and spot bugs that could have been caught by tests.
>> Collectively we invested years of work into these tests and
>> developers better take advantage of it by running all.
>
> +1
>
>> If a CI server didn't satisfy all bpf test dependencies,
>> I don't want such CI setup to be running and reporting results,
>> since it will give false sense of test coverage.
>> Test failures due to missing dependencies are hard failures.
>> We cannot skip them.
>
> +1

Btw, on that note, the folks from zero-day bot run the BPF kselftests
for a while now just fine and they do run them together with clang,
so they have the full, proper coverage how it should be. It's not
how it used to be in the early days, you can just go and install
llvm/clang package on all the major distros today and you get the
bpf target by default enabled already.

>> I'd like generic XDP tests to be added to selftests/bpf which
>> would mean that the latest iproute2 will become a hard dependency
>> and bpf developers and CI host owners would need to upgrade
>> their iproute2.
>> The tests either pass or fail. Skipping them due to missing
>> dependencies is the same as fail and in that sense I don't want
>> to change selftests/bpf/Makefile to make it skip clang.
>
> I fully agree that for the BPF selftests it is very desirable
> to not only test the verifier with couple of BPF insn snippets,
> but to actually load and run programs that more closely resemble
> real world programs. For more complex interactions these snippets
> are just limited, think of tail calls, testing perf event output
> helper, etc, which would all require to write these tests with
> restricted C when we add them (unless we want to make writing
> these tests a real pain ;) in which case no-one will bother to
> write tests at all for them). Mid to long term I would definitely
> like to see more programs in BPF selftests (e.g. moved over from
> samples/bpf/) to increase the test coverage.

[toc] | [prev] | [next] | [standalone]

#1733131 — Re: selftests/bpf doesn't compile

On 09/15/2017 12:48 PM, Daniel Borkmann wrote:
> On 09/15/2017 08:23 PM, Daniel Borkmann wrote:
>> On 09/15/2017 08:07 PM, Alexei Starovoitov wrote:
>>> On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote:
>>>> On 15/09/17 17:02, Alexei Starovoitov wrote:
>>>>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>>>>>> Is bpf test intended to be run in kselftest run? The clang dependency might
>>>>>> not be met on majority of the systems. Is this a hard dependency??
>>>>> It is a hard dependency and clang should be present on majority of the systems.
>>>> I think this is the wrong approach.  Making kselftest hard-require clang doesn't
>>>>   mean that the bpf tests will be run more often, it means that the rest of the
>>>>   kselftests will be run less often.  clang is quite big (when I tried to install
>>>>   it on one of my test servers, I didn't have enough disk space & had to go on a
>>>>   clear-out of unused packages), and most people aren't interested in the bpf
>>>>   subsystem specifically; they would rather be able to skip those tests.
>>>> I feel that as long as they know they are skipping some tests (so e.g. they
>>>>   won't consider it a sufficient test of a kselftest refactor), that's fine.
>>>> It's not even as though all of the bpf tests require clang; the (smaller) tests
>>>>   written directly in raw eBPF instructions could still be run on such a system.
>>>>   So I think we should attempt to run as much as possible but accept that clang
>>>>   may not be available and have an option to skip some tests in that case.
>>>
>>> imo the value of selftests/bpf is twofold:
>>> 1. it helps bpf developers avoid regressions
>>> 2. as part of continuous integration it helps to catch bpf regressions
>>> that were somehow caused by changes in other parts of the kernel
>>>
>>> If a developer didn't bother to satisfy all bpf tests dependencies
>>> (which includes clang) and ran all tests before sending a patch,
>>> I don't want to see such patches. It just wastes maintainers time
>>> to review code and spot bugs that could have been caught by tests.
>>> Collectively we invested years of work into these tests and
>>> developers better take advantage of it by running all.
>>
>> +1
>>
>>> If a CI server didn't satisfy all bpf test dependencies,
>>> I don't want such CI setup to be running and reporting results,
>>> since it will give false sense of test coverage.
>>> Test failures due to missing dependencies are hard failures.
>>> We cannot skip them.
>>
>> +1
> 
> Btw, on that note, the folks from zero-day bot run the BPF kselftests
> for a while now just fine and they do run them together with clang,
> so they have the full, proper coverage how it should be. It's not
> how it used to be in the early days, you can just go and install
> llvm/clang package on all the major distros today and you get the
> bpf target by default enabled already.
> 
>>> I'd like generic XDP tests to be added to selftests/bpf which
>>> would mean that the latest iproute2 will become a hard dependency
>>> and bpf developers and CI host owners would need to upgrade
>>> their iproute2.
>>> The tests either pass or fail. Skipping them due to missing
>>> dependencies is the same as fail and in that sense I don't want
>>> to change selftests/bpf/Makefile to make it skip clang.
>>
>> I fully agree that for the BPF selftests it is very desirable
>> to not only test the verifier with couple of BPF insn snippets,
>> but to actually load and run programs that more closely resemble
>> real world programs. For more complex interactions these snippets
>> are just limited, think of tail calls, testing perf event output
>> helper, etc, which would all require to write these tests with
>> restricted C when we add them (unless we want to make writing
>> these tests a real pain ;) in which case no-one will bother to
>> write tests at all for them). Mid to long term I would definitely
>> like to see more programs in BPF selftests (e.g. moved over from
>> samples/bpf/) to increase the test coverage.
> 

As I said in my earlier email:

Unless users choose to install clang, bpf will always fail run without
clang. So clang dependency is an issue for bpf test coverage in general.
That is your choice as to whether you want to increase the scope of
regression test coverage for bpf or not.

I fully understand you have weigh that against ease of writing tests.

We can leave things the way they are since:

- You can't force users to install clang and run bpf test. Users might
  opt for letting bpf test fail due to unmet dependency.

- You have reasons to continue use clang and you have been using it for
  a longtime.

I will try to see why make ksefltest fails on bpf even with my patch
series that addresses the source directory issue. All other tests build
and run. It is not an issue with bpf specifically, it is something that
has never been tested in this use-case.

thanks,
-- Shuah

[toc] | [prev] | [next] | [standalone]

#1734121 — Re: selftests/bpf doesn't compile

On 09/16/2017 12:41 AM, Shuah Khan wrote:
> On 09/15/2017 12:48 PM, Daniel Borkmann wrote:
>> On 09/15/2017 08:23 PM, Daniel Borkmann wrote:
>>> On 09/15/2017 08:07 PM, Alexei Starovoitov wrote:
>>>> On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote:
>>>>> On 15/09/17 17:02, Alexei Starovoitov wrote:
>>>>>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>>>>>>> Is bpf test intended to be run in kselftest run? The clang dependency might
>>>>>>> not be met on majority of the systems. Is this a hard dependency??
>>>>>> It is a hard dependency and clang should be present on majority of the systems.
>>>>> I think this is the wrong approach.  Making kselftest hard-require clang doesn't
>>>>>    mean that the bpf tests will be run more often, it means that the rest of the
>>>>>    kselftests will be run less often.  clang is quite big (when I tried to install
>>>>>    it on one of my test servers, I didn't have enough disk space & had to go on a
>>>>>    clear-out of unused packages), and most people aren't interested in the bpf
>>>>>    subsystem specifically; they would rather be able to skip those tests.
>>>>> I feel that as long as they know they are skipping some tests (so e.g. they
>>>>>    won't consider it a sufficient test of a kselftest refactor), that's fine.
>>>>> It's not even as though all of the bpf tests require clang; the (smaller) tests
>>>>>    written directly in raw eBPF instructions could still be run on such a system.
>>>>>    So I think we should attempt to run as much as possible but accept that clang
>>>>>    may not be available and have an option to skip some tests in that case.
>>>>
>>>> imo the value of selftests/bpf is twofold:
>>>> 1. it helps bpf developers avoid regressions
>>>> 2. as part of continuous integration it helps to catch bpf regressions
>>>> that were somehow caused by changes in other parts of the kernel
>>>>
>>>> If a developer didn't bother to satisfy all bpf tests dependencies
>>>> (which includes clang) and ran all tests before sending a patch,
>>>> I don't want to see such patches. It just wastes maintainers time
>>>> to review code and spot bugs that could have been caught by tests.
>>>> Collectively we invested years of work into these tests and
>>>> developers better take advantage of it by running all.
>>>
>>> +1
>>>
>>>> If a CI server didn't satisfy all bpf test dependencies,
>>>> I don't want such CI setup to be running and reporting results,
>>>> since it will give false sense of test coverage.
>>>> Test failures due to missing dependencies are hard failures.
>>>> We cannot skip them.
>>>
>>> +1
>>
>> Btw, on that note, the folks from zero-day bot run the BPF kselftests
>> for a while now just fine and they do run them together with clang,
>> so they have the full, proper coverage how it should be. It's not
>> how it used to be in the early days, you can just go and install
>> llvm/clang package on all the major distros today and you get the
>> bpf target by default enabled already.
>>
>>>> I'd like generic XDP tests to be added to selftests/bpf which
>>>> would mean that the latest iproute2 will become a hard dependency
>>>> and bpf developers and CI host owners would need to upgrade
>>>> their iproute2.
>>>> The tests either pass or fail. Skipping them due to missing
>>>> dependencies is the same as fail and in that sense I don't want
>>>> to change selftests/bpf/Makefile to make it skip clang.
>>>
>>> I fully agree that for the BPF selftests it is very desirable
>>> to not only test the verifier with couple of BPF insn snippets,
>>> but to actually load and run programs that more closely resemble
>>> real world programs. For more complex interactions these snippets
>>> are just limited, think of tail calls, testing perf event output
>>> helper, etc, which would all require to write these tests with
>>> restricted C when we add them (unless we want to make writing
>>> these tests a real pain ;) in which case no-one will bother to
>>> write tests at all for them). Mid to long term I would definitely
>>> like to see more programs in BPF selftests (e.g. moved over from
>>> samples/bpf/) to increase the test coverage.
>
> As I said in my earlier email:
>
> Unless users choose to install clang, bpf will always fail run without
> clang. So clang dependency is an issue for bpf test coverage in general.
> That is your choice as to whether you want to increase the scope of
> regression test coverage for bpf or not.
>
> I fully understand you have weigh that against ease of writing tests.
>
> We can leave things the way they are since:
>
> - You can't force users to install clang and run bpf test. Users might
>    opt for letting bpf test fail due to unmet dependency.
>
> - You have reasons to continue use clang and you have been using it for
>    a longtime.

I'm definitely for leaving it as it currently is and having clang as
hard dependency; there will be more BPF selftests over time that will
require to compile BPF progs (through clang's BPF backend) written in
restricted C, so just skipping these tests would give a false sense
of coverage. clang is pretty much needed anyway for writing more complex
programs, thus leaving requirements the way they are is the much better
option.

> I will try to see why make ksefltest fails on bpf even with my patch
> series that addresses the source directory issue. All other tests build
> and run. It is not an issue with bpf specifically, it is something that
> has never been tested in this use-case.

Great, thanks!

[toc] | [prev] | [next] | [standalone]

#1732962 — Re: selftests/bpf doesn't compile

On 09/15/2017 10:02 AM, Alexei Starovoitov wrote:
> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>> Hi Alexei and Daniel,
>>
>> bpf test depends on clang and fails to compile when
>>
>> ------------------------------------------------------
>> make -C tools/testing/selftests/bpf run_tests
>>
>>
>> make: clang: Command not found
>> Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
>> make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
>> make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
>>
>> With "make TARGETS=bpf kselftest" it fails earlier:
>>
>>
>> make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
>> Makefile:40: tools/scripts/Makefile.arch: No such file or directory
>> Makefile:84: tools/build/Makefile.feature: No such file or directory
>> Makefile:143: tools/build/Makefile.include: No such file or directory
>> make[3]: *** No rule to make target 'tools/build/Makefile.include'.  Stop.
>> make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
>> Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
>> make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
>> make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
>> Makefile:69: recipe for target 'all' failed
>> make[1]: *** [all] Error 2
>> Makefile:1190: recipe for target 'kselftest' failed
>> make: *** [kselftest] Error 2
>>
>> --------------------------------------------------------------
>>
>> Is bpf test intended to be run in kselftest run? The clang dependency might
>> not be met on majority of the systems. Is this a hard dependency??
> 
> It is a hard dependency and clang should be present on majority of the systems.
> More details are in samples/bpf/README.rst
> which was written long ago. Nowadays apt-get/yum will install clang
> with bpf support builtin.

Thanks for the clarification.

> 
>> Would it make sense to create a special target for bpf test? We do have a few
>> tests that do that now. 
>>
>> TARGETS_HOTPLUG = cpu-hotplug
>> TARGETS_HOTPLUG += memory-hotplug
>>
>> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
>> the run_test> 
> I'm not sure what was the motivation to exclude hotplug from default testing,

These are considered a bit more disruptive and were excluded a while
back. These take cpus and memory on and off-line. Also require
root access. So even if they are included in the regular run, these
won't run.

> since I think it diminishes the value of selftests overall.

I agree. We do have some timer tests that are destructive/stress that
et run using a special target. It is the idea that if somebody wants
to test all them, there is a way to do that.

In any case, I didn't think bpf falls into this category of tests that
belong in the destructive category. I am looking to understand the failures
and your take on those.

> Not running all tests all the time risks breaking them
It is balance of providing a choice to users if they don't want to
run destructive tests. For example, suspend test which requires root
access. So the idea is for users to run these by choice as opposed
to running them in the normal run and cause disruption.

> selftest makefile refactoring broke selftests/bpf in the past,

Yeah. We have had some fallout from the KBUILD_OUTPUT work that didn't
take all the use-cases into account and tests that require custom
builds such as the bpf tests. Using common build infrastructure doesn't
work for all tests.

Looks like there are other patches that went in later with lcap work.

> so I strongly suggest to install clang and make sure the tests are passing
> on the test servers 

You will have to request kernelci, stable, and It is a choice to be made by
kernelci/zero-day folks.

otherwise we'd need to move selftests/bpf out of
> selftests to avoid further headaches for us when selftests infra keeps
> changing.
> 

Let's not go to extreme options. :) I am merely looking for more information
and trying to understand the dependencies for this test.

Let's look for a constructive option to fix the build failures I am seeing.

The first failure due to clang dependency is not a problem. The second one
in the case of "make kselftest" is the one that requires some work when bpf
make is run from the main Makefile. A lots of users run tests using the
kselftest target from the mail Makefile. hence I would like to get this
working, so it would be easier to run this test on test servers.

thanks,
-- Shuah

[toc] | [prev] | [next] | [standalone]

#1732985 — Re: selftests/bpf doesn't compile

On 09/15/2017 11:00 AM, Shuah Khan wrote:
> On 09/15/2017 10:02 AM, Alexei Starovoitov wrote:
>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
>>> Hi Alexei and Daniel,
>>>
>>> bpf test depends on clang and fails to compile when
>>>
>>> ------------------------------------------------------
>>> make -C tools/testing/selftests/bpf run_tests
>>>
>>>
>>> make: clang: Command not found
>>> Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
>>> make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
>>> make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
>>>
>>> With "make TARGETS=bpf kselftest" it fails earlier:
>>>
>>>
>>> make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
>>> Makefile:40: tools/scripts/Makefile.arch: No such file or directory
>>> Makefile:84: tools/build/Makefile.feature: No such file or directory
>>> Makefile:143: tools/build/Makefile.include: No such file or directory
>>> make[3]: *** No rule to make target 'tools/build/Makefile.include'.  Stop.
>>> make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
>>> Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
>>> make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
>>> make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
>>> Makefile:69: recipe for target 'all' failed
>>> make[1]: *** [all] Error 2
>>> Makefile:1190: recipe for target 'kselftest' failed
>>> make: *** [kselftest] Error 2
>>>
>>> --------------------------------------------------------------
>>>
>>> Is bpf test intended to be run in kselftest run? The clang dependency might
>>> not be met on majority of the systems. Is this a hard dependency??
>>
>> It is a hard dependency and clang should be present on majority of the systems.
>> More details are in samples/bpf/README.rst
>> which was written long ago. Nowadays apt-get/yum will install clang
>> with bpf support builtin.
> 
> Thanks for the clarification.
> 
>>
>>> Would it make sense to create a special target for bpf test? We do have a few
>>> tests that do that now. 
>>>
>>> TARGETS_HOTPLUG = cpu-hotplug
>>> TARGETS_HOTPLUG += memory-hotplug
>>>
>>> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
>>> the run_test> 
>> I'm not sure what was the motivation to exclude hotplug from default testing,
> 
> These are considered a bit more disruptive and were excluded a while
> back. These take cpus and memory on and off-line. Also require
> root access. So even if they are included in the regular run, these
> won't run.
> 
>> since I think it diminishes the value of selftests overall.
> 
> I agree. We do have some timer tests that are destructive/stress that
> et run using a special target. It is the idea that if somebody wants
> to test all them, there is a way to do that.
> 
> In any case, I didn't think bpf falls into this category of tests that
> belong in the destructive category. I am looking to understand the failures
> and your take on those.
> 
>> Not running all tests all the time risks breaking them
> It is balance of providing a choice to users if they don't want to
> run destructive tests. For example, suspend test which requires root
> access. So the idea is for users to run these by choice as opposed
> to running them in the normal run and cause disruption.
> 
>> selftest makefile refactoring broke selftests/bpf in the past,
> 
> Yeah. We have had some fallout from the KBUILD_OUTPUT work that didn't
> take all the use-cases into account and tests that require custom
> builds such as the bpf tests. Using common build infrastructure doesn't
> work for all tests.
> 
> Looks like there are other patches that went in later with lcap work.
> 
>> so I strongly suggest to install clang and make sure the tests are passing
>> on the test servers 
> 
> You will have to request kernelci, stable, and It is a choice to be made by
> kernelci/zero-day folks.
> 
> otherwise we'd need to move selftests/bpf out of
>> selftests to avoid further headaches for us when selftests infra keeps
>> changing.
>>
> 
> Let's not go to extreme options. :) I am merely looking for more information
> and trying to understand the dependencies for this test.
> 
> Let's look for a constructive option to fix the build failures I am seeing.
> 
> The first failure due to clang dependency is not a problem.

Let me clarify that. People interested in running bpf test will have to
install clang. In that sense installing clang will solve that issue.

The hard dependency on clang does make it difficult for developers to
ensure they didn't break bpf when they make changes to the kselftest
common infrastructure.

The second one
> in the case of "make kselftest" is the one that requires some work when bpf
> make is run from the main Makefile. A lots of users run tests using the
> kselftest target from the mail Makefile. hence I would like to get this
> working, so it would be easier to run this test on test servers.
> 

Even if this is fixed, unless users choose to install clang, bpf will always
fail run without clang. So clang dependency is an issue for bpf test coverage
in general. But that is your choice as to whether you want to increase the
scope of regression test coverage for bpf or not.

thanks,
-- Shuah

[toc] | [prev] | [next] | [standalone]

#1733002 — Re: selftests/bpf doesn't compile

On Fri, Sep 15, 2017 at 11:00:31AM -0600, Shuah Khan wrote:
> >> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
> >> the run_test> 
> > I'm not sure what was the motivation to exclude hotplug from default testing,
> 
> These are considered a bit more disruptive and were excluded a while
> back. These take cpus and memory on and off-line. Also require
> root access. So even if they are included in the regular run, these
> won't run.

most of bpf tests require root access as well.

> The first failure due to clang dependency is not a problem. The second one
> in the case of "make kselftest" is the one that requires some work when bpf
> make is run from the main Makefile. A lots of users run tests using the
> kselftest target from the mail Makefile. hence I would like to get this
> working, so it would be easier to run this test on test servers.

'make kselftest' doesn't work for me at all, since I suspect it
assumes in-source build. I always use KBUILD_OUTPUT,
since I build multiple archs with different configs out of the same
source tree, so there is a bigger problem here.

$ make kselftest
make[1]: Entering directory `/data/users/ast/net-next/bld_x64'
make: Entering an unknown directory
make: *** tools/testing/selftests: No such file or directory.  Stop.
make: Leaving an unknown directory
make[1]: *** [kselftest] Error 2
make[1]: Leaving directory `/data/users/ast/net-next/bld_x64'
make: *** [sub-make] Error 2

[toc] | [prev] | [next] | [standalone]

#1733129 — Re: selftests/bpf doesn't compile

On 09/15/2017 12:14 PM, Alexei Starovoitov wrote:
> On Fri, Sep 15, 2017 at 11:00:31AM -0600, Shuah Khan wrote:
>>>> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
>>>> the run_test> 
>>> I'm not sure what was the motivation to exclude hotplug from default testing,
>>
>> These are considered a bit more disruptive and were excluded a while
>> back. These take cpus and memory on and off-line. Also require
>> root access. So even if they are included in the regular run, these
>> won't run.
> 
> most of bpf tests require root access as well.
> 
>> The first failure due to clang dependency is not a problem. The second one
>> in the case of "make kselftest" is the one that requires some work when bpf
>> make is run from the main Makefile. A lots of users run tests using the
>> kselftest target from the mail Makefile. hence I would like to get this
>> working, so it would be easier to run this test on test servers.
> 
> 'make kselftest' doesn't work for me at all, since I suspect it
> assumes in-source build. I always use KBUILD_OUTPUT,
> since I build multiple archs with different configs out of the same
> source tree, so there is a bigger problem here.

Right. I think you probably use

make KBUILD_OUTPUT=dir -C tools/testing/selftests/bpf

> 
> $ make kselftest
> make[1]: Entering directory `/data/users/ast/net-next/bld_x64'
> make: Entering an unknown directory
> make: *** tools/testing/selftests: No such file or directory.  Stop.
> make: Leaving an unknown directory
> make[1]: *** [kselftest] Error 2
> make[1]: Leaving directory `/data/users/ast/net-next/bld_x64'
> make: *** [sub-make] Error 2
> 

Right. There is patch set out to fix this problem which I sent out couple
of days ago. It is probably the case that bpf Makefile might need changes
to build when it is built from the sleftests/Makefile or the main Makefile.

thanks,
-- Shuah

[toc] | [prev] | [next] | [standalone]

#1734967

On 09/14/2017 09:01 AM, Shuah Khan wrote:
> On 09/08/2017 05:05 PM, Daniel Borkmann wrote:
>> On 09/09/2017 01:01 AM, Alexei Starovoitov wrote:
>>> On Fri, Sep 08, 2017 at 01:19:23PM +0200, Thomas Meyer wrote:
>>>> The current implementation fails to work on uniprocessor systems.
>>>> Fix the parser to also handle the uniprocessor case.
>>>>
>>>> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
>>>
>>> Thanks for the fix. lgtm
>>> Acked-by: Alexei Starovoitov <ast@kernel.org>
>>
>> Looks good from here as well:
>>
>> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
>>
>>> This time it's ok to go via selftest tree, but next time please use net-next/net
>>> to avoid conflicts.
>>
>> +1
>>
>>> Thanks
> 
> Thank you. I will get this into 4.14-rc2 or rc3.
> 

Applied to linux-kselftest fixes for 4.14-rc2

thanks,
-- Shuah

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web