* [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems @ 2017-09-08 11:19 Thomas Meyer 2017-09-08 23:01 ` Alexei Starovoitov 0 siblings, 1 reply; 19+ messages in thread From: Thomas Meyer @ 2017-09-08 11:19 UTC (permalink / raw) To: linux-kernel, linux-kselftest, shuah; +Cc: Thomas Meyer 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> --- 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems 2017-09-08 11:19 [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems Thomas Meyer @ 2017-09-08 23:01 ` Alexei Starovoitov 2017-09-08 23:05 ` Daniel Borkmann 0 siblings, 1 reply; 19+ messages in thread From: Alexei Starovoitov @ 2017-09-08 23:01 UTC (permalink / raw) To: Thomas Meyer; +Cc: linux-kernel, linux-kselftest, shuah, Daniel Borkmann 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> This time it's ok to go via selftest tree, but next time please use net-next/net to avoid conflicts. Thanks > --- > 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 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems 2017-09-08 23:01 ` Alexei Starovoitov @ 2017-09-08 23:05 ` Daniel Borkmann 2017-09-14 15:01 ` Shuah Khan 0 siblings, 1 reply; 19+ messages in thread From: Daniel Borkmann @ 2017-09-08 23:05 UTC (permalink / raw) To: Alexei Starovoitov, Thomas Meyer; +Cc: linux-kernel, linux-kselftest, shuah 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 > >> --- >> 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 >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems 2017-09-08 23:05 ` Daniel Borkmann @ 2017-09-14 15:01 ` Shuah Khan 2017-09-14 15:33 ` selftests/bpf doesn't compile Shuah Khan 2017-09-19 14:45 ` [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems Shuah Khan 0 siblings, 2 replies; 19+ messages in thread From: Shuah Khan @ 2017-09-14 15:01 UTC (permalink / raw) To: Daniel Borkmann, Alexei Starovoitov, Thomas Meyer Cc: linux-kernel, linux-kselftest, Shuah Khan, Shuah Khan 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 >>> > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* selftests/bpf doesn't compile 2017-09-14 15:01 ` Shuah Khan @ 2017-09-14 15:33 ` Shuah Khan 2017-09-15 16:02 ` Alexei Starovoitov 2019-01-04 17:16 ` Geert Uytterhoeven 2017-09-19 14:45 ` [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems Shuah Khan 1 sibling, 2 replies; 19+ messages in thread From: Shuah Khan @ 2017-09-14 15:33 UTC (permalink / raw) To: Daniel Borkmann, Alexei Starovoitov, Thomas Meyer Cc: linux-kernel, linux-kselftest, Shuah Khan, Networking 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-14 15:33 ` selftests/bpf doesn't compile Shuah Khan @ 2017-09-15 16:02 ` Alexei Starovoitov 2017-09-15 16:58 ` Edward Cree 2017-09-15 17:00 ` Shuah Khan 2019-01-04 17:16 ` Geert Uytterhoeven 1 sibling, 2 replies; 19+ messages in thread From: Alexei Starovoitov @ 2017-09-15 16:02 UTC (permalink / raw) To: Shuah Khan Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest, Shuah Khan, Networking 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 16:02 ` Alexei Starovoitov @ 2017-09-15 16:58 ` Edward Cree 2017-09-15 18:07 ` Alexei Starovoitov 2017-09-15 17:00 ` Shuah Khan 1 sibling, 1 reply; 19+ messages in thread From: Edward Cree @ 2017-09-15 16:58 UTC (permalink / raw) To: Alexei Starovoitov, Shuah Khan Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest, Shuah Khan, Networking 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 16:58 ` Edward Cree @ 2017-09-15 18:07 ` Alexei Starovoitov 2017-09-15 18:23 ` Daniel Borkmann 0 siblings, 1 reply; 19+ messages in thread From: Alexei Starovoitov @ 2017-09-15 18:07 UTC (permalink / raw) To: Edward Cree Cc: Shuah Khan, Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest, Shuah Khan, Networking 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 18:07 ` Alexei Starovoitov @ 2017-09-15 18:23 ` Daniel Borkmann 2017-09-15 18:48 ` Daniel Borkmann 0 siblings, 1 reply; 19+ messages in thread From: Daniel Borkmann @ 2017-09-15 18:23 UTC (permalink / raw) To: Alexei Starovoitov, Edward Cree Cc: Shuah Khan, Thomas Meyer, linux-kernel, linux-kselftest, Shuah Khan, Networking 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 18:23 ` Daniel Borkmann @ 2017-09-15 18:48 ` Daniel Borkmann 2017-09-15 22:41 ` Shuah Khan 0 siblings, 1 reply; 19+ messages in thread From: Daniel Borkmann @ 2017-09-15 18:48 UTC (permalink / raw) To: Alexei Starovoitov, Edward Cree Cc: Shuah Khan, Thomas Meyer, linux-kernel, linux-kselftest, Shuah Khan, Networking 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 18:48 ` Daniel Borkmann @ 2017-09-15 22:41 ` Shuah Khan 2017-09-18 13:31 ` Daniel Borkmann 0 siblings, 1 reply; 19+ messages in thread From: Shuah Khan @ 2017-09-15 22:41 UTC (permalink / raw) To: Daniel Borkmann, Alexei Starovoitov, Edward Cree Cc: Shuah Khan, Thomas Meyer, linux-kernel, linux-kselftest, Networking, Shuah Khan 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 22:41 ` Shuah Khan @ 2017-09-18 13:31 ` Daniel Borkmann 0 siblings, 0 replies; 19+ messages in thread From: Daniel Borkmann @ 2017-09-18 13:31 UTC (permalink / raw) To: Shuah Khan, Alexei Starovoitov, Edward Cree Cc: Shuah Khan, Thomas Meyer, linux-kernel, linux-kselftest, Networking 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! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 16:02 ` Alexei Starovoitov 2017-09-15 16:58 ` Edward Cree @ 2017-09-15 17:00 ` Shuah Khan 2017-09-15 17:44 ` Shuah Khan 2017-09-15 18:14 ` Alexei Starovoitov 1 sibling, 2 replies; 19+ messages in thread From: Shuah Khan @ 2017-09-15 17:00 UTC (permalink / raw) To: Alexei Starovoitov, Shuah Khan Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest, Networking, Shuah Khan 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 17:00 ` Shuah Khan @ 2017-09-15 17:44 ` Shuah Khan 2017-09-15 18:14 ` Alexei Starovoitov 1 sibling, 0 replies; 19+ messages in thread From: Shuah Khan @ 2017-09-15 17:44 UTC (permalink / raw) To: Alexei Starovoitov, Shuah Khan Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest, Networking, Shuah Khan 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 17:00 ` Shuah Khan 2017-09-15 17:44 ` Shuah Khan @ 2017-09-15 18:14 ` Alexei Starovoitov 2017-09-15 22:32 ` Shuah Khan 1 sibling, 1 reply; 19+ messages in thread From: Alexei Starovoitov @ 2017-09-15 18:14 UTC (permalink / raw) To: Shuah Khan Cc: Shuah Khan, Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest, Networking 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-15 18:14 ` Alexei Starovoitov @ 2017-09-15 22:32 ` Shuah Khan 0 siblings, 0 replies; 19+ messages in thread From: Shuah Khan @ 2017-09-15 22:32 UTC (permalink / raw) To: Alexei Starovoitov Cc: Shuah Khan, Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest, Networking, Shuah Khan 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2017-09-14 15:33 ` selftests/bpf doesn't compile Shuah Khan 2017-09-15 16:02 ` Alexei Starovoitov @ 2019-01-04 17:16 ` Geert Uytterhoeven 2019-01-04 19:07 ` shuah 1 sibling, 1 reply; 19+ messages in thread From: Geert Uytterhoeven @ 2019-01-04 17:16 UTC (permalink / raw) To: Shuah Khan Cc: Daniel Borkmann, Alexei Starovoitov, Thomas Meyer, Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Networking, Masahiro Yamada, linux-kbuild, open list:GPIO SUBSYSTEM Hi Shuah, CC kbuild, gpio On Thu, Sep 14, 2017 at 5:34 PM Shuah Khan <shuah@kernel.org> wrote: > 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' The above failure is indeed due to missing clang. > 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 This is due to srctree being "." instead of the actual source tree, when invoked as "make kselftest". When using "make -C tools/testing/selftests", srctree is correct. tools/testing/selftests/bpf/Makefile has: $(BPFOBJ): force $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ to enter the tools/lib/bpf directory to force a build of libbpf.a Note that tools/gpio has the same issue. There seem to be _four_ different ways to build kselftests (Documentation/dev-tools/kselftest.rst): make kselftest make O=/path/to/output kselftest make -C tools/testing/selftests make O=/path/to/output -C tools/testing/selftests I'm not so fond of the latter two, as they basically run make from somewhere inside the tree, which complicates things. I believe we don't support this anywhere else. Each of the four seem to have (different) issues. Especially when you throw cross-compiling into the mix. And care about where installed headers end up (yes, kselftest calls headers_install internally). I'm working on fixes for some of them, but I don't know how to fix the srctree issue. Anyone with a suggestion? Thanks! > 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 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: selftests/bpf doesn't compile 2019-01-04 17:16 ` Geert Uytterhoeven @ 2019-01-04 19:07 ` shuah 0 siblings, 0 replies; 19+ messages in thread From: shuah @ 2019-01-04 19:07 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Daniel Borkmann, Alexei Starovoitov, Thomas Meyer, Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Networking, Masahiro Yamada, linux-kbuild, open list:GPIO SUBSYSTEM, shuah Hi Geert, On 1/4/19 10:16 AM, Geert Uytterhoeven wrote: > Hi Shuah, > > CC kbuild, gpio > > On Thu, Sep 14, 2017 at 5:34 PM Shuah Khan <shuah@kernel.org> wrote: >> 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' > > The above failure is indeed due to missing clang. > >> 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 > > This is due to srctree being "." instead of the actual source tree, > when invoked as "make kselftest". > When using "make -C tools/testing/selftests", srctree is correct. > > tools/testing/selftests/bpf/Makefile has: > > $(BPFOBJ): force > $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ > > to enter the tools/lib/bpf directory to force a build of libbpf.a > > Note that tools/gpio has the same issue. > > There seem to be _four_ different ways to build kselftests > (Documentation/dev-tools/kselftest.rst): > > make kselftest > make O=/path/to/output kselftest > make -C tools/testing/selftests > make O=/path/to/output -C tools/testing/selftests > > I'm not so fond of the latter two, as they basically run make from > somewhere inside the tree, which complicates things. I believe we don't > support this anywhere else. > > Each of the four seem to have (different) issues. Especially when you > throw cross-compiling into the mix. And care about where installed > headers end up (yes, kselftest calls headers_install internally). > > I'm working on fixes for some of them, but I don't know how to fix the > srctree issue. > I will take a look at the srctree problem and fix it. I have had to fix a few individual test Makefiles after ksefltest headers_install went in. It will be in a week or two. I am taking some time off this week and next week other than for occasional email checking. thanks, -- Shuah ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems 2017-09-14 15:01 ` Shuah Khan 2017-09-14 15:33 ` selftests/bpf doesn't compile Shuah Khan @ 2017-09-19 14:45 ` Shuah Khan 1 sibling, 0 replies; 19+ messages in thread From: Shuah Khan @ 2017-09-19 14:45 UTC (permalink / raw) To: Daniel Borkmann, Alexei Starovoitov, Thomas Meyer Cc: linux-kernel, linux-kselftest, Shuah Khan, Shuah Khan 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-01-04 19:07 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-08 11:19 [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems Thomas Meyer 2017-09-08 23:01 ` Alexei Starovoitov 2017-09-08 23:05 ` Daniel Borkmann 2017-09-14 15:01 ` Shuah Khan 2017-09-14 15:33 ` selftests/bpf doesn't compile Shuah Khan 2017-09-15 16:02 ` Alexei Starovoitov 2017-09-15 16:58 ` Edward Cree 2017-09-15 18:07 ` Alexei Starovoitov 2017-09-15 18:23 ` Daniel Borkmann 2017-09-15 18:48 ` Daniel Borkmann 2017-09-15 22:41 ` Shuah Khan 2017-09-18 13:31 ` Daniel Borkmann 2017-09-15 17:00 ` Shuah Khan 2017-09-15 17:44 ` Shuah Khan 2017-09-15 18:14 ` Alexei Starovoitov 2017-09-15 22:32 ` Shuah Khan 2019-01-04 17:16 ` Geert Uytterhoeven 2019-01-04 19:07 ` shuah 2017-09-19 14:45 ` [PATCH] selftests/bpf: Make bpf_util work on uniprocessor systems Shuah Khan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).