netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] selftests: Skip BPF seftests by default
@ 2020-12-10 18:52 Mark Brown
  2020-12-10 19:11 ` Alexei Starovoitov
  2020-12-16 22:05 ` Seth Forshee
  0 siblings, 2 replies; 8+ messages in thread
From: Mark Brown @ 2020-12-10 18:52 UTC (permalink / raw)
  To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Daniel Diaz, Veronika Kabatova, netdev, bpf,
	linux-kselftest, Guillaume Tucker, Kevin Hilman, Mark Brown

The BPF selftests have build time dependencies on cutting edge versions
of tools in the BPF ecosystem including LLVM which are more involved
to satisfy than more typical requirements like installing a package from
your distribution.  This causes issues for users looking at kselftest in
as a whole who find that a default build of kselftest fails and that
resolving this is time consuming and adds administrative overhead.  The
fast pace of BPF development and the need for a full BPF stack to do
substantial development or validation work on the code mean that people
working directly on it don't see a reasonable way to keep supporting
older environments without causing problems with the usability of the
BPF tests in BPF development so these requirements are unlikely to be
relaxed in the immediate future.

There is already support for skipping targets so in order to reduce the
barrier to entry for people interested in kselftest as a whole let's use
that to skip the BPF tests by default when people work with the top
level kselftest build system.  Users can still build the BPF selftests
as part of the wider kselftest build by specifying SKIP_TARGETS,
including setting an empty SKIP_TARGETS to build everything.  They can
also continue to build the BPF selftests individually in cases where
they are specifically focused on BPF.

This isn't ideal since it means people will need to take special steps
to build the BPF tests but the dependencies mean that realistically this
is already the case to some extent and it makes it easier for people to
pick up and work with the other selftests which is hopefully a net win.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index afbab4aeef3c..8a917cb4426a 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -77,8 +77,10 @@ TARGETS += zram
 TARGETS_HOTPLUG = cpu-hotplug
 TARGETS_HOTPLUG += memory-hotplug
 
-# User can optionally provide a TARGETS skiplist.
-SKIP_TARGETS ?=
+# User can optionally provide a TARGETS skiplist.  By default we skip
+# BPF since it has cutting edge build time dependencies which require
+# more effort to install.
+SKIP_TARGETS ?= bpf
 ifneq ($(SKIP_TARGETS),)
 	TMP := $(filter-out $(SKIP_TARGETS), $(TARGETS))
 	override TARGETS := $(TMP)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: Skip BPF seftests by default
  2020-12-10 18:52 [PATCH] selftests: Skip BPF seftests by default Mark Brown
@ 2020-12-10 19:11 ` Alexei Starovoitov
  2020-12-10 23:41   ` Shuah Khan
  2020-12-16 22:05 ` Seth Forshee
  1 sibling, 1 reply; 8+ messages in thread
From: Alexei Starovoitov @ 2020-12-10 19:11 UTC (permalink / raw)
  To: Mark Brown
  Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Daniel Diaz, Veronika Kabatova, netdev, bpf,
	linux-kselftest, Guillaume Tucker, Kevin Hilman

On Thu, Dec 10, 2020 at 06:52:33PM +0000, Mark Brown wrote:
> The BPF selftests have build time dependencies on cutting edge versions
> of tools in the BPF ecosystem including LLVM which are more involved
> to satisfy than more typical requirements like installing a package from
> your distribution.  This causes issues for users looking at kselftest in
> as a whole who find that a default build of kselftest fails and that
> resolving this is time consuming and adds administrative overhead.  The
> fast pace of BPF development and the need for a full BPF stack to do
> substantial development or validation work on the code mean that people
> working directly on it don't see a reasonable way to keep supporting
> older environments without causing problems with the usability of the
> BPF tests in BPF development so these requirements are unlikely to be
> relaxed in the immediate future.
> 
> There is already support for skipping targets so in order to reduce the
> barrier to entry for people interested in kselftest as a whole let's use
> that to skip the BPF tests by default when people work with the top
> level kselftest build system.  Users can still build the BPF selftests
> as part of the wider kselftest build by specifying SKIP_TARGETS,
> including setting an empty SKIP_TARGETS to build everything.  They can
> also continue to build the BPF selftests individually in cases where
> they are specifically focused on BPF.
> 
> This isn't ideal since it means people will need to take special steps
> to build the BPF tests but the dependencies mean that realistically this
> is already the case to some extent and it makes it easier for people to
> pick up and work with the other selftests which is hopefully a net win.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  tools/testing/selftests/Makefile | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index afbab4aeef3c..8a917cb4426a 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -77,8 +77,10 @@ TARGETS += zram
>  TARGETS_HOTPLUG = cpu-hotplug
>  TARGETS_HOTPLUG += memory-hotplug
>  
> -# User can optionally provide a TARGETS skiplist.
> -SKIP_TARGETS ?=
> +# User can optionally provide a TARGETS skiplist.  By default we skip
> +# BPF since it has cutting edge build time dependencies which require
> +# more effort to install.
> +SKIP_TARGETS ?= bpf

I'm fine with this, but I'd rather make an obvious second step right away
and move selftests/bpf into a different directory.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: Skip BPF seftests by default
  2020-12-10 19:11 ` Alexei Starovoitov
@ 2020-12-10 23:41   ` Shuah Khan
  2020-12-11 12:46     ` Mark Brown
  0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2020-12-10 23:41 UTC (permalink / raw)
  To: Alexei Starovoitov, Mark Brown
  Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Daniel Diaz, Veronika Kabatova, netdev, bpf,
	linux-kselftest, Guillaume Tucker, Kevin Hilman, Shuah Khan

On 12/10/20 12:11 PM, Alexei Starovoitov wrote:
> On Thu, Dec 10, 2020 at 06:52:33PM +0000, Mark Brown wrote:
>> The BPF selftests have build time dependencies on cutting edge versions
>> of tools in the BPF ecosystem including LLVM which are more involved
>> to satisfy than more typical requirements like installing a package from
>> your distribution.  This causes issues for users looking at kselftest in
>> as a whole who find that a default build of kselftest fails and that
>> resolving this is time consuming and adds administrative overhead.  The
>> fast pace of BPF development and the need for a full BPF stack to do
>> substantial development or validation work on the code mean that people
>> working directly on it don't see a reasonable way to keep supporting
>> older environments without causing problems with the usability of the
>> BPF tests in BPF development so these requirements are unlikely to be
>> relaxed in the immediate future.
>>
>> There is already support for skipping targets so in order to reduce the
>> barrier to entry for people interested in kselftest as a whole let's use
>> that to skip the BPF tests by default when people work with the top
>> level kselftest build system.  Users can still build the BPF selftests
>> as part of the wider kselftest build by specifying SKIP_TARGETS,
>> including setting an empty SKIP_TARGETS to build everything.  They can
>> also continue to build the BPF selftests individually in cases where
>> they are specifically focused on BPF.
>>
>> This isn't ideal since it means people will need to take special steps
>> to build the BPF tests but the dependencies mean that realistically this
>> is already the case to some extent and it makes it easier for people to
>> pick up and work with the other selftests which is hopefully a net win.
>>
>> Signed-off-by: Mark Brown <broonie@kernel.org>
>> ---
>>   tools/testing/selftests/Makefile | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>> index afbab4aeef3c..8a917cb4426a 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -77,8 +77,10 @@ TARGETS += zram
>>   TARGETS_HOTPLUG = cpu-hotplug
>>   TARGETS_HOTPLUG += memory-hotplug
>>   
>> -# User can optionally provide a TARGETS skiplist.
>> -SKIP_TARGETS ?=
>> +# User can optionally provide a TARGETS skiplist.  By default we skip
>> +# BPF since it has cutting edge build time dependencies which require
>> +# more effort to install.
>> +SKIP_TARGETS ?= bpf
> 
> I'm fine with this, but I'd rather make an obvious second step right away
> and move selftests/bpf into a different directory.
> 

Why is this an obvious second step? If people want to run bpf, they can
build and run. How does moving it out of selftests directory help? It
would become harder on users that want to run the test.

I don't support moving bpf out of selftests directory in the interest
of Linux kernel quality and validation.

Let's think big picture and kernel community as a whole.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: Skip BPF seftests by default
  2020-12-10 23:41   ` Shuah Khan
@ 2020-12-11 12:46     ` Mark Brown
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2020-12-11 12:46 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Alexei Starovoitov, Shuah Khan, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Daniel Diaz,
	Veronika Kabatova, netdev, bpf, linux-kselftest,
	Guillaume Tucker, Kevin Hilman

[-- Attachment #1: Type: text/plain, Size: 995 bytes --]

On Thu, Dec 10, 2020 at 04:41:33PM -0700, Shuah Khan wrote:
> On 12/10/20 12:11 PM, Alexei Starovoitov wrote:

> > I'm fine with this, but I'd rather make an obvious second step right away
> > and move selftests/bpf into a different directory.

> Why is this an obvious second step? If people want to run bpf, they can
> build and run. How does moving it out of selftests directory help? It
> would become harder on users that want to run the test.

> I don't support moving bpf out of selftests directory in the interest
> of Linux kernel quality and validation.

> Let's think big picture and kernel community as a whole.

Yeah, I don't see an obvious motivation for doing that either - what
problem does it solve?  For people running suites it's helpful to have
fewer testsuites and test infrastructures to integrate with.  The work
needed for the dependencies is going to be the same no matter where we
put the tests and moving out of the shared infrastructure creates some
additional work.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: Skip BPF seftests by default
  2020-12-10 18:52 [PATCH] selftests: Skip BPF seftests by default Mark Brown
  2020-12-10 19:11 ` Alexei Starovoitov
@ 2020-12-16 22:05 ` Seth Forshee
  2020-12-17 13:07   ` Mark Brown
  1 sibling, 1 reply; 8+ messages in thread
From: Seth Forshee @ 2020-12-16 22:05 UTC (permalink / raw)
  To: Mark Brown
  Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Daniel Diaz, Veronika Kabatova, netdev, bpf,
	linux-kselftest, Guillaume Tucker, Kevin Hilman

On Thu, Dec 10, 2020 at 06:52:33PM +0000, Mark Brown wrote:
> The BPF selftests have build time dependencies on cutting edge versions
> of tools in the BPF ecosystem including LLVM which are more involved
> to satisfy than more typical requirements like installing a package from
> your distribution.  This causes issues for users looking at kselftest in
> as a whole who find that a default build of kselftest fails and that
> resolving this is time consuming and adds administrative overhead.  The
> fast pace of BPF development and the need for a full BPF stack to do
> substantial development or validation work on the code mean that people
> working directly on it don't see a reasonable way to keep supporting
> older environments without causing problems with the usability of the
> BPF tests in BPF development so these requirements are unlikely to be
> relaxed in the immediate future.
> 
> There is already support for skipping targets so in order to reduce the
> barrier to entry for people interested in kselftest as a whole let's use
> that to skip the BPF tests by default when people work with the top
> level kselftest build system.  Users can still build the BPF selftests
> as part of the wider kselftest build by specifying SKIP_TARGETS,
> including setting an empty SKIP_TARGETS to build everything.  They can
> also continue to build the BPF selftests individually in cases where
> they are specifically focused on BPF.
> 
> This isn't ideal since it means people will need to take special steps
> to build the BPF tests but the dependencies mean that realistically this
> is already the case to some extent and it makes it easier for people to
> pick up and work with the other selftests which is hopefully a net win.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Why not just remove the line which adds bpf to TARGETS? This has the
same effect, but doesn't require an emtpy SKIP_TARGETS to run them. We
have testing scripts which use 'make TARGETS=bpf ...' which will have to
be updated, and I doubt we are the only ones.

I also feel like this creates confusing semantics around SKIP_TARGETS.
If I don't supply a value then I don't get the bpf selftests, but then
if I try to use SKIP_TARGETS to skip some other test suddenly I do get
them. That's counterintuitive.

I also wanted to point out that the net/test_bpf.sh selftest requires
having the test_bpf module from the bpf selftest build. So when the bpf
selftests aren't built this test is guaranteed to fail. Though it would
be nice if the net selftests didn't require building the bpf self tests
in order to pass.

Thanks,
Seth

> ---
>  tools/testing/selftests/Makefile | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index afbab4aeef3c..8a917cb4426a 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -77,8 +77,10 @@ TARGETS += zram
>  TARGETS_HOTPLUG = cpu-hotplug
>  TARGETS_HOTPLUG += memory-hotplug
>  
> -# User can optionally provide a TARGETS skiplist.
> -SKIP_TARGETS ?=
> +# User can optionally provide a TARGETS skiplist.  By default we skip
> +# BPF since it has cutting edge build time dependencies which require
> +# more effort to install.
> +SKIP_TARGETS ?= bpf
>  ifneq ($(SKIP_TARGETS),)
>  	TMP := $(filter-out $(SKIP_TARGETS), $(TARGETS))
>  	override TARGETS := $(TMP)
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: Skip BPF seftests by default
  2020-12-16 22:05 ` Seth Forshee
@ 2020-12-17 13:07   ` Mark Brown
  2020-12-17 15:53     ` Shuah Khan
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2020-12-17 13:07 UTC (permalink / raw)
  To: Seth Forshee
  Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Daniel Diaz, Veronika Kabatova, netdev, bpf,
	linux-kselftest, Guillaume Tucker, Kevin Hilman

[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]

On Wed, Dec 16, 2020 at 04:05:58PM -0600, Seth Forshee wrote:
> On Thu, Dec 10, 2020 at 06:52:33PM +0000, Mark Brown wrote:

> > as part of the wider kselftest build by specifying SKIP_TARGETS,
> > including setting an empty SKIP_TARGETS to build everything.  They can
> > also continue to build the BPF selftests individually in cases where
> > they are specifically focused on BPF.

> Why not just remove the line which adds bpf to TARGETS? This has the
> same effect, but doesn't require an emtpy SKIP_TARGETS to run them. We
> have testing scripts which use 'make TARGETS=bpf ...' which will have to
> be updated, and I doubt we are the only ones.

> I also feel like this creates confusing semantics around SKIP_TARGETS.
> If I don't supply a value then I don't get the bpf selftests, but then
> if I try to use SKIP_TARGETS to skip some other test suddenly I do get
> them. That's counterintuitive.

That's what I did first, it's also messy just differently.  If you
don't add bpf to TARGETS then if you do what's needed to get it building
it becomes inconvenient to run it as part of running everything else at
the top level since you need to enumerate all the targets.  It felt like
skipping is what we're actually doing here and it seems like those
actively working with BPF will be used to having to update things in
their environment.  People who start using SKIP_TARGETS are *probably*
going to find out about it from the Makefile anyway so will see the
default that's there.

Fundamentally having such demanding build dependencies is always going
to result in some kind of mess, it's just where we push it.

> I also wanted to point out that the net/test_bpf.sh selftest requires
> having the test_bpf module from the bpf selftest build. So when the bpf
> selftests aren't built this test is guaranteed to fail. Though it would
> be nice if the net selftests didn't require building the bpf self tests
> in order to pass.

Right, that's a separate issue - the net tests should really skip that
if they don't have BPF, as we do for other runtime detectable
dependencies.  It's nowhere near as severe as failing to build though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: Skip BPF seftests by default
  2020-12-17 13:07   ` Mark Brown
@ 2020-12-17 15:53     ` Shuah Khan
  2020-12-17 18:32       ` Shuah Khan
  0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2020-12-17 15:53 UTC (permalink / raw)
  To: Mark Brown, Seth Forshee
  Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Daniel Diaz, Veronika Kabatova, netdev, bpf,
	linux-kselftest, Guillaume Tucker, Kevin Hilman, Shuah Khan

On 12/17/20 6:07 AM, Mark Brown wrote:
> On Wed, Dec 16, 2020 at 04:05:58PM -0600, Seth Forshee wrote:
>> On Thu, Dec 10, 2020 at 06:52:33PM +0000, Mark Brown wrote:
> 
>>> as part of the wider kselftest build by specifying SKIP_TARGETS,
>>> including setting an empty SKIP_TARGETS to build everything.  They can
>>> also continue to build the BPF selftests individually in cases where
>>> they are specifically focused on BPF.
> 
>> Why not just remove the line which adds bpf to TARGETS? This has the
>> same effect, but doesn't require an emtpy SKIP_TARGETS to run them. We
>> have testing scripts which use 'make TARGETS=bpf ...' which will have to
>> be updated, and I doubt we are the only ones.
> 

I would prefer leaving bpf in the main Makefile TARGETS. This will be
useful to users that have their systems setup for bpf builds.

>> I also feel like this creates confusing semantics around SKIP_TARGETS.
>> If I don't supply a value then I don't get the bpf selftests, but then
>> if I try to use SKIP_TARGETS to skip some other test suddenly I do get
>> them. That's counterintuitive.
> 
> That's what I did first, it's also messy just differently.  If you
> don't add bpf to TARGETS then if you do what's needed to get it building
> it becomes inconvenient to run it as part of running everything else at
> the top level since you need to enumerate all the targets.  It felt like
> skipping is what we're actually doing here and it seems like those
> actively working with BPF will be used to having to update things in
> their environment.  People who start using SKIP_TARGETS are *probably*
> going to find out about it from the Makefile anyway so will see the
> default that's there.
> 
> Fundamentally having such demanding build dependencies is always going
> to result in some kind of mess, it's just where we push it.
> 
>> I also wanted to point out that the net/test_bpf.sh selftest requires
>> having the test_bpf module from the bpf selftest build. So when the bpf
>> selftests aren't built this test is guaranteed to fail. Though it would
>> be nice if the net selftests didn't require building the bpf self tests
>> in order to pass.
> 
> Right, that's a separate issue - the net tests should really skip that
> if they don't have BPF, as we do for other runtime detectable
> dependencies.  It's nowhere near as severe as failing to build though.
> 

Correct. This has to be handled as a run-time dependency check and skip
instead of fail.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: Skip BPF seftests by default
  2020-12-17 15:53     ` Shuah Khan
@ 2020-12-17 18:32       ` Shuah Khan
  0 siblings, 0 replies; 8+ messages in thread
From: Shuah Khan @ 2020-12-17 18:32 UTC (permalink / raw)
  To: Mark Brown, Seth Forshee
  Cc: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Daniel Diaz, Veronika Kabatova, netdev, bpf,
	linux-kselftest, Guillaume Tucker, Kevin Hilman, Shuah Khan

On 12/17/20 8:53 AM, Shuah Khan wrote:
> On 12/17/20 6:07 AM, Mark Brown wrote:
>> On Wed, Dec 16, 2020 at 04:05:58PM -0600, Seth Forshee wrote:
>>> On Thu, Dec 10, 2020 at 06:52:33PM +0000, Mark Brown wrote:
>>
>>>> as part of the wider kselftest build by specifying SKIP_TARGETS,
>>>> including setting an empty SKIP_TARGETS to build everything.  They can
>>>> also continue to build the BPF selftests individually in cases where
>>>> they are specifically focused on BPF.
>>

Applied to linuxkselftest fixes for rc2

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-12-17 18:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-10 18:52 [PATCH] selftests: Skip BPF seftests by default Mark Brown
2020-12-10 19:11 ` Alexei Starovoitov
2020-12-10 23:41   ` Shuah Khan
2020-12-11 12:46     ` Mark Brown
2020-12-16 22:05 ` Seth Forshee
2020-12-17 13:07   ` Mark Brown
2020-12-17 15:53     ` Shuah Khan
2020-12-17 18:32       ` 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).