Re: [PATCH net-next v2 2/2] selftests/net: integrate packetdrill with ksft

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jakub,

On 07/09/2024 02:04, Jakub Kicinski wrote:
> On Fri, 06 Sep 2024 19:28:08 -0400 Willem de Bruijn wrote:
>>>> No, we opted for this design exactly to use existing kselftest infra,
>>>> rather than reimplementing that in our wrapper, as I did in the RFC.  
>>>
>>> OK, I understood from the discussions from the RFC that by using the
>>> kselftest infra, the tests would be automatically executed in dedicated
>>> netns, and it could also help running tests in parallel. That sounded
>>> great to me, but that's not the case by default from what I see.  
>>
>> Perhaps that's something to change in the defaults for run_tests.
>>
>> Since the infra exist, that is preferable over reimplementing it for
>> one particular subset of tests.
>>
>> Or if not all kselftests can run in netns (quite likely), this needs
>> to be opt-in. Then a variable defined in the Makefile perhaps. To
>> tell kselftest to enable the feature for this target.
> 
> Indeed, I was thinking along the same lines.

Yes, I was also thinking about a variable defined in the Makefile.

Because I suppose this variable will not be added in this cycle, and if
a v3 is planned, would it be OK to simply prefix the 'packetdrill'
commands with "unshare -n"? That would be similar to what is already
done in Netfilter, and it prevents messing up with other tests/host
settings?

> We're closing net-next in a week, it'd be great to have the baseline
> ksft interpreter mechanism in place in the next couple of days. 
> The exact implementation of packetdrill/ksft_runner.sh can be changed
> later as needed, and the current one works fine for NIPA.

It is fine for me if the v2 is applied. The suggested modifications were
small, not blocking, fixes can be sent after (or could be ignored if
preferred).

> Hopefully we can also discuss at LPC/netconf what to do about libraries
> (where setup / cleanup code could live).

Good idea!

> Looking at MPTCP tests - do
> they work out of tree? I see mptcp_lib.sh does:
> 
> . "$(dirname "${0}")/../lib.sh"
> . "$(dirname "${0}")/../net_helper.sh"
> 
> but lib/sh and net_helper.sh are not listed in the Makefile. So they
> won't get packaged...

Indeed, I noticed that when I reviewed Willem's series. It's only
recently that we started to use these files. I already fixed these two
on my side, I will share patches later after some testing.

> We should make sure we support running the tests with make run_tests
> and in installed mode. 
> 
> If we agree that the current situation with support for library code is
> far from ideal, I think we have three(ish) directions to explore:
> 
>  1  build netns handling into runner.sh
>    + already mostly there
>    + simpler tests, no need to worry about netns, it just happens
>    - not all tests need netns (HW-adjacent tests especially)
>    - netns setup is the main thing we need but not the only thing,
>      wait helpers, python code, etc. also need to be handled

Indeed, almost there. 'unshare' could also be used in runner.sh to
simplify things.

Please note that for some tests, multiple netns are needed, e.g. to
split the client and the server (and routers) in different netns. I
don't think such setups should be handled by runner.sh.

If the use of a dedicated netns is triggered by a variable in the
Makefile, it also means that all tests listed in this Makefile should
work the same way, which is not always the case.

On the other hand, creating the netns is not difficult, even easier with
the helper from lib.sh, and 'unshare -n' might be enough for simple cases.

>  2a improve library bundling at the ksft level
>    + we already have a net/lib "meta-target", it kinda works
>    + hopefully in a way that lets us Python
>    - no idea how

If the idea is only to launch tests in a dedicated netns, can we not
change the shebang to launch the test script with:

  unshare (...) <interpreter>

>  2b put all the code in kselftest/, like ktap_helpers.sh ?
>    + easy to do
>    + helps other subsystems
>    - could cause git conflicts

For me, that would be great to share more helpers between subsystems,
but that looks difficult from a maintenance point of view.

>    - won't help Python?
> 
>  3  give up on target proliferation; on a quick count we have 15 targets
>     in ksft for various bits of networking, faaar more than anyone else
>    + fewer targets limits the need for libraries, libraries local to
>      the target are trivial to handle
>    - ksft has no other form of "grouping" tests, if we collapse into 
>      a small number of targets it will be hard to run a group of tests

It is good to have targets, to easily run a group of tests related to a
modification that has just been done, and to limit the size of the
required kernel config, etc. Probably easier to have different libs per
target/subsystem, and when something can be re-used elsewhere, it can be
extracted to a more generic lib maybe?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux