netdev - Re: [PATCH net-next] selftests: forwarding: Import top-level lib.sh through $lib

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZXt6_4WCxYoxgWqL@d3>
Date: Thu, 14 Dec 2023 17:00:31 -0500
From: Benjamin Poirier <benjamin.poirier@...il.com>
To: Hangbin Liu <liuhangbin@...il.com>,
	Patrice Duroux <patrice.duroux@...il.com>,
	Vladimir Oltean <vladimir.oltean@....com>,
	Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Cc: Petr Machata <petrm@...dia.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, Shuah Khan <shuah@...nel.org>,
	mlxsw@...dia.com, Jay Vosburgh <j.vosburgh@...il.com>
Subject: Re: [PATCH net-next] selftests: forwarding: Import top-level lib.sh
 through $lib_dir

On 2023-12-14 15:06 +0800, Hangbin Liu wrote:
> Hi Benjamin,
> 
> On Wed, Dec 13, 2023 at 04:40:53PM -0500, Benjamin Poirier wrote:
> > > Hmm.. Is it possible to write a rule in the Makefile to create the net/
> > > and net/forwarding folder so we can source the relative path directly. e.g.
> > > 
> > > ]# tree
> > > .
> > > ├── drivers
> > > │   └── net
> > > │       └── bonding
> > > │           ├── bond-arp-interval-causes-panic.sh
> > > │           ├── ...
> > > │           └── settings
> > > ├── kselftest
> > > │   ├── module.sh
> > > │   ├── prefix.pl
> > > │   └── runner.sh
> > > ├── kselftest-list.txt
> > > ├── net
> > > │   ├── forwarding
> > > │   │   └── lib.sh
> > > │   └── lib.sh
> > > └── run_kselftest.sh
> > 
> > That sounds like a good idea. I started to work on that approach but I'm
> > missing recursive inclusion. For instance
> > 
> > cd tools/testing/selftests
> > make install TARGETS="drivers/net/bonding"
> > ./kselftest_install/run_kselftest.sh -t drivers/net/bonding:dev_addr_lists.sh
> > 
> > includes net/forwarding/lib.sh but is missing net/lib.sh. I feel that my
> > 'make' skills are rusty but I guess that with enough make code, it could
> > be done. A workaround is simply to manually list the transitive
> > dependencies in TEST_SH_LIBS:
> >  TEST_SH_LIBS := \
> > -	net/forwarding/lib.sh
> > +	net/forwarding/lib.sh \
> > +	net/lib.sh
> 
> Yes, this makes the user need to make sure all the recursive inclusions listed
> here. A little inconvenient. But I "make" skill is worse than you...
> 
> > 
> > I only converted a few files to validate that the approach is viable. I
> > used the following tests:
> > drivers/net/bonding/dev_addr_lists.sh
> > net/test_vxlan_vnifiltering.sh
> > net/forwarding/pedit_ip.sh
> > 
> > Let me know what you think.
> 
> Thanks! This works for me.

I started to make the adjustments to all the tests but I got stuck on
the dsa tests. The problem is that the tests which are symlinked (like
bridge_locked_port.sh) expect to source lib.sh (net/forwarding/lib.sh)
from the same directory. That lib.sh then expects to source net/lib.sh
from the parent directory. Because `rsync --copy-unsafe-links` is used,
all those links become regular files after export so we can't rely on
`readlink -f`.

Honestly, given how the dsa tests are organized, I don't see a clean way
to support these tests without error after commit 25ae948b4478
("selftests/net: add lib.sh").

The only way that I see to run these tests via runner.sh is by using a
command like:
make -C tools/testing/selftests install TARGETS="drivers/net/dsa"
KSELFTEST_BRIDGE_LOCKED_PORT_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_BRIDGE_MDB_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_BRIDGE_MLD_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_BRIDGE_VLAN_AWARE_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_BRIDGE_VLAN_MCAST_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_BRIDGE_VLAN_UNAWARE_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_LOCAL_TERMINATION_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_NO_FORWARDING_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_TC_ACTIONS_SH_ARGS="swp1 swp2 swp3 swp4" KSELFTEST_TEST_BRIDGE_FDB_STRESS_SH_ARGS="swp1 swp2 swp3 swp4" tools/testing/selftests/kselftest_install/run_kselftest.sh

This is very cumbersome so it makes me question the value of
drivers/net/dsa/Makefile.

Patrice, Vladimir, Martin, how do you run the dsa tests?
Could we revert 6ecf206d602f ("selftests: net: dsa: Add a Makefile
which installs the selftests")?

Do you have other suggestions to avoid the following error about lib.sh:

tools/testing/selftests# make install TARGETS="drivers/net/dsa"
[...]
tools/testing/selftests# ./kselftest_install/run_kselftest.sh
TAP version 13
1..10
# timeout set to 45
# selftests: drivers/net/dsa: bridge_locked_port.sh
# lib.sh: line 41: ../lib.sh: No such file or directory
[...]