[PATCH net] selftests: netdevsim: add a config file

[PATCH net] selftests: netdevsim: add a config file

[Jakub Kicinski]

netdevsim tests aren't very well integrated with kselftest,
which has its advantages and disadvantages. But regardless
of the intended integration - a config file to know what kernel
to build is very useful, add one.

Fixes: fc4c93f145d7 ("selftests: add basic netdevsim devlink flash testing")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: shuah@kernel.org
CC: jiri@resnulli.us
CC: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/drivers/net/netdevsim/config | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 tools/testing/selftests/drivers/net/netdevsim/config

diff --git a/tools/testing/selftests/drivers/net/netdevsim/config b/tools/testing/selftests/drivers/net/netdevsim/config
new file mode 100644
index 000000000000..adf45a3a78b4
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/config
@@ -0,0 +1,10 @@
+CONFIG_DUMMY=y
+CONFIG_GENEVE=m
+CONFIG_IPV6=y
+CONFIG_NETDEVSIM=m
+CONFIG_NET_SCH_MQPRIO=y
+CONFIG_NET_SCH_MULTIQ=y
+CONFIG_NET_SCH_PRIO=y
+CONFIG_PSAMPLE=y
+CONFIG_PTP_1588_CLOCK_MOCK=y
+CONFIG_VXLAN=m
-- 
2.43.0

Re: [PATCH net] selftests: netdevsim: add a config file

[Paolo Abeni]

On Tue, 2024-01-16 at 07:43 -0800, Jakub Kicinski wrote:
> netdevsim tests aren't very well integrated with kselftest,
> which has its advantages and disadvantages. 

Out of sheer ignorance I don't see the advantage?!?

> But regardless
> of the intended integration - a config file to know what kernel
> to build is very useful, add one.

With a complete integration we could more easily ask kbuild to generate
automatically the kernel config suitable for testing; what about
completing such integration?

Cheers,

Paolo

Re: [PATCH net] selftests: netdevsim: add a config file

[Jakub Kicinski]

On Tue, 16 Jan 2024 18:40:49 +0100 Paolo Abeni wrote:
> On Tue, 2024-01-16 at 07:43 -0800, Jakub Kicinski wrote:
> > netdevsim tests aren't very well integrated with kselftest,
> > which has its advantages and disadvantages.   
> 
> Out of sheer ignorance I don't see the advantage?!?
> 
> > But regardless
> > of the intended integration - a config file to know what kernel
> > to build is very useful, add one.  
> 
> With a complete integration we could more easily ask kbuild to generate
> automatically the kernel config suitable for testing; what about
> completing such integration?

My bad, I didn't have the right words at my fingertips so I deleted 
the explanation of advantages.

make run_tests doesn't give us the ability to inject logic between
each test, AFAIU. The runner for netdevsim I typed up checks after
each test whether the VM has any crashes or things got otherwise
out of whack. And if so kills the VM and starts a new one to run
the next test. For make run_tests we can still more or less zero
in on which test caused an oops or crash, but the next test will 
try to keep going. Even if we force kill it after we see a crash
I didn't see in the docs how to continue testing from a specific
point.

So all in all, yeah, uniformity is good, the hacky approach kinda
works. Converting netdevsim to make run_tests is not a priority..

Re: [PATCH net] selftests: netdevsim: add a config file

[Paolo Abeni]

On Tue, 2024-01-16 at 10:34 -0800, Jakub Kicinski wrote:
> On Tue, 16 Jan 2024 18:40:49 +0100 Paolo Abeni wrote:
> > On Tue, 2024-01-16 at 07:43 -0800, Jakub Kicinski wrote:
> > > netdevsim tests aren't very well integrated with kselftest,
> > > which has its advantages and disadvantages.   
> > 
> > Out of sheer ignorance I don't see the advantage?!?
> > 
> > > But regardless
> > > of the intended integration - a config file to know what kernel
> > > to build is very useful, add one.  
> > 
> > With a complete integration we could more easily ask kbuild to generate
> > automatically the kernel config suitable for testing; what about
> > completing such integration?
> 
> My bad, I didn't have the right words at my fingertips so I deleted 
> the explanation of advantages.
> 
> make run_tests doesn't give us the ability to inject logic between
> each test, AFAIU. The runner for netdevsim I typed up checks after
> each test whether the VM has any crashes or things got otherwise
> out of whack. And if so kills the VM and starts a new one to run
> the next test. For make run_tests we can still more or less zero
> in on which test caused an oops or crash, but the next test will 
> try to keep going. 

I see.


> Even if we force kill it after we see a crash
> I didn't see in the docs how to continue testing from a specific
> point.

I think something like the following should do:

cd tools/testing/selftests
make TARGETS="net drivers/net/bonding <...full relevant targets list>" O=<kst_dir> install
cd <kst_dir>

ARGS=""
for t in $(./run_kselftest.sh -l| sed -n '/<test name>/,$p'); do
	ARGS="$ARGS -t $t"
done
./run_kselftest.sh $ARGS # run all tests after <test name>

Probably it would be nice to add to the kselftest runner the ability to
check for kernel oops after each test and ev. stop.

> So all in all, yeah, uniformity is good, the hacky approach kinda
> works. Converting netdevsim to make run_tests is not a priority..

I agree, but also will put the all the above possible improvements in
my wishlist ;)

Cheers,

Paolo

Re: [PATCH net] selftests: netdevsim: add a config file

[Jakub Kicinski]

On Wed, 17 Jan 2024 10:32:19 +0100 Paolo Abeni wrote:
> > Even if we force kill it after we see a crash
> > I didn't see in the docs how to continue testing from a specific
> > point.  
> 
> I think something like the following should do:
> 
> cd tools/testing/selftests
> make TARGETS="net drivers/net/bonding <...full relevant targets list>" O=<kst_dir> install
> cd <kst_dir>
> 
> ARGS=""
> for t in $(./run_kselftest.sh -l| sed -n '/<test name>/,$p'); do
> 	ARGS="$ARGS -t $t"
> done
> ./run_kselftest.sh $ARGS # run all tests after <test name>
> 
> Probably it would be nice to add to the kselftest runner the ability to
> check for kernel oops after each test and ev. stop.

I wasn't aware there's a way to list tests! That should work well
enough we can run them one by one with make, that's fine.

./run_kselftest.sh only seems to work for installed tests, tho,
in tree it says:
./run_kselftest.sh: Could not find list of tests to run (~linux/tools/testing/selftests/kselftest-list.txt)
So perhaps the wishlist item would be "make list_tests"?

Re: [PATCH net] selftests: netdevsim: add a config file

[Paolo Abeni]

On Wed, 2024-01-17 at 07:39 -0800, Jakub Kicinski wrote:
> On Wed, 17 Jan 2024 10:32:19 +0100 Paolo Abeni wrote:
> > 
> > I think something like the following should do:
> > 
> > cd tools/testing/selftests
> > make TARGETS="net drivers/net/bonding <...full relevant targets list>" O=<kst_dir> install
> > cd <kst_dir>
> > 
> > ARGS=""
> > for t in $(./run_kselftest.sh -l| sed -n '/<test name>/,$p'); do
> > 	ARGS="$ARGS -t $t"
> > done
> > ./run_kselftest.sh $ARGS # run all tests after <test name>
> > 
> > Probably it would be nice to add to the kselftest runner the ability to
> > check for kernel oops after each test and ev. stop.
> 
> I wasn't aware there's a way to list tests! That should work well
> enough we can run them one by one with make, that's fine.
> 
> ./run_kselftest.sh only seems to work for installed tests, tho,
> in tree it says:
> ./run_kselftest.sh: Could not find list of tests to run (~linux/tools/testing/selftests/kselftest-list.txt)
> So perhaps the wishlist item would be "make list_tests"?

Yes, kselftest-list.txt is created on the fly by make install.

As such step could be constrained to the relevant selftests directories
(see the above code snippet), and a 'make' step is required anyway to
run the tests, what about using directly such target?

In any case I don't mean to block this patch, let me apply it...

Cheers,

Paolo

Re: [PATCH net] selftests: netdevsim: add a config file

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 16 Jan 2024 07:43:11 -0800 you wrote:
> netdevsim tests aren't very well integrated with kselftest,
> which has its advantages and disadvantages. But regardless
> of the intended integration - a config file to know what kernel
> to build is very useful, add one.
> 
> Fixes: fc4c93f145d7 ("selftests: add basic netdevsim devlink flash testing")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> [...]

Here is the summary with links:
  - [net] selftests: netdevsim: add a config file
    https://git.kernel.org/netdev/net/c/39369c9a6e09

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html