First 3 patches are more-or-less cleanups/preparations.
Patches 4/5 are fixes for netns file descriptors leaks/open.
Patch 6 was sent to me/contributed off-list by Mohammad, who wants 32-bit
kernels to run TCP-AO.
Patch 7 is a workaround/fix for slow VMs. Albeit, I can't reproduce
the issue, but I hope it will fix netdev flakes for connect-deny-*
tests.
And the biggest change is adding TCP-AO tracepoints to selftests.
I think it's a good addition by the following reasons:
- The related tracepoints are now tested;
- It allows tcp-ao selftests to raise expectations on the kernel
behavior - up from the syscalls exit statuses + net counters.
- Provides tracepoints usage samples.
As tracepoints are not a stable ABI, any kernel changes done to them
will be reflected to the selftests, which also will allow users
to see how to change their code. It's quite better than parsing dmesg
(what BGP was doing pre-tracepoints, ugh).
Somewhat arguably, the code parses trace_pipe, rather than uses
libtraceevent (which any sane user should do). The reason behind that is
the same as for rt-netlink macros instead of libmnl: I'm trying
to minimize the library dependencies of the selftests. And the
performance of formatting text in kernel and parsing it again in a test
is not critical.
Current output sample:
> ok 73 Trace events matched expectations: 13 tcp_hash_md5_required[2] tcp_hash_md5_unexpected[4] tcp_hash_ao_required[3] tcp_ao_key_not_found[4]
Previously, tracepoints selftests were part of kernel tcp tracepoints
submission [1], but since then the code was quite changed:
- Now generic tracing setup is in lib/ftrace.c, separate from
lib/ftrace-tcp.c which utilizes TCP trace points. This separation
allows future selftests to trace non-TCP events, i.e. to find out
an skb's drop reason, which was useful in the creation of TCP-CLOSE
stress-test (not in this patch set, but used in attempt to reproduce
the issue from [2]).
- Another change is that in the previous submission the trace events
where used only to detect unexpected TCP-AO/TCP-MD5 events. In this
version the selftests will fail if an expected trace event didn't
appear.
Let's see how reliable this is on the netdev bot - it obviously passes
on my testing, but potentially may require a temporary XFAIL patch
if it misbehaves on a slow VM.
[1] https://lore.kernel.org/lkml/20240224-tcp-ao-tracepoints-v1-0-15f31b7f30a7@…
[2] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=3…
Signed-off-by: Dmitry Safonov <0x7f454c46(a)gmail.com>
---
In v4 mostly worked on non-appearing events on netdev test VM.
- Set up x86 VM with the config from netdev & run stress-ng didn't
reproduce the isssue.
- Spread more error messages if tracing pthread fails to start
- Added conditional wait for tracer thread, just before destruction, in
case it didn't had a time slice to run and parse trace events.
- Addressed some of checkpatch.pl --strict warnings (with
nits from Simon Horman)
- Link to v3: https://lore.kernel.org/r/20240815-tcp-ao-selftests-upd-6-12-v3-0-7bd2e22bb…
Changes in v3:
- Corrected the selftests printing of tcp header flags, parsed from
trace points
- Fixed an issue with VRF kconfig checks (and tests)
- Made check for unexpected trace events XFAIL, yet looking into the
reason behind the fail
- Link to v2: https://lore.kernel.org/r/20240802-tcp-ao-selftests-upd-6-12-v2-0-370c99358…
Changes in v2:
- Fixed two issues with parsing TCP-AO events: the socket state and TCP
segment flags. Hopefully, won't fail on netdev.
- Reword patch 1 & 2 messages to be more informative and at some degree
formal (Paolo)
- Since commit e33a02ed6a4f ("selftests: Add printf attribute to
kselftest prints") it's possible to use __printf instead of "raw" gcc
attribute - switch using that, as checkpatch suggests.
- Link to v1: https://lore.kernel.org/r/20240730-tcp-ao-selftests-upd-6-12-v1-0-ffd4bf15d…
---
Dmitry Safonov (7):
selftests/net: Clean-up double assignment
selftests/net: Provide test_snprintf() helper
selftests/net: Be consistent in kconfig checks
selftests/net: Open /proc/thread-self in open_netns()
selftests/net: Don't forget to close nsfd after switch_save_ns()
selftests/net: Synchronize client/server before counters checks
selftests/net: Add trace events matching to tcp_ao
Mohammad Nassiri (1):
selftests/tcp_ao: Fix printing format for uint64_t
tools/testing/selftests/net/tcp_ao/Makefile | 3 +-
tools/testing/selftests/net/tcp_ao/bench-lookups.c | 2 +-
tools/testing/selftests/net/tcp_ao/config | 1 +
tools/testing/selftests/net/tcp_ao/connect-deny.c | 25 +-
tools/testing/selftests/net/tcp_ao/connect.c | 6 +-
tools/testing/selftests/net/tcp_ao/icmps-discard.c | 2 +-
.../testing/selftests/net/tcp_ao/key-management.c | 18 +-
tools/testing/selftests/net/tcp_ao/lib/aolib.h | 178 ++++++-
.../testing/selftests/net/tcp_ao/lib/ftrace-tcp.c | 559 +++++++++++++++++++++
tools/testing/selftests/net/tcp_ao/lib/ftrace.c | 543 ++++++++++++++++++++
tools/testing/selftests/net/tcp_ao/lib/kconfig.c | 31 +-
tools/testing/selftests/net/tcp_ao/lib/setup.c | 17 +-
tools/testing/selftests/net/tcp_ao/lib/sock.c | 1 -
tools/testing/selftests/net/tcp_ao/lib/utils.c | 26 +
tools/testing/selftests/net/tcp_ao/restore.c | 30 +-
tools/testing/selftests/net/tcp_ao/rst.c | 2 +-
tools/testing/selftests/net/tcp_ao/self-connect.c | 19 +-
tools/testing/selftests/net/tcp_ao/seq-ext.c | 28 +-
.../selftests/net/tcp_ao/setsockopt-closed.c | 6 +-
tools/testing/selftests/net/tcp_ao/unsigned-md5.c | 35 +-
20 files changed, 1465 insertions(+), 67 deletions(-)
---
base-commit: f9db28bb09f46087580f2a8da54bb0aab59a8024
change-id: 20240730-tcp-ao-selftests-upd-6-12-4d3e53a74f3f
Best regards,
--
Dmitry Safonov <0x7f454c46(a)gmail.com>
Here are different fixes:
Patch 1 closes the subflow after having received a FIN, instead of
leaving it half-closed until the end of the MPTCP connection. A fix for
v5.12.
Patch 2 validates the previous patch.
Patch 3 is a fix for a recent fix to check both directions for the
backup flag. It can follow the 'Fixes' commit and be backported up to
v5.7.
Patch 4 adds a missing \n at the end of pr_debug(), causing debug
messages to be displayed with a delay, which confuses the debugger. A
fix for v5.6.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe(a)kernel.org>
---
Note: Peter's email address has been removed from the Cc list, because
it is bouncing.
---
Matthieu Baerts (NGI0) (4):
mptcp: close subflow when receiving TCP+FIN
selftests: mptcp: join: cannot rm sf if closed
mptcp: sched: check both backup in retrans
mptcp: pr_debug: add missing \n at the end
net/mptcp/fastopen.c | 4 +-
net/mptcp/options.c | 50 ++++++++++-----------
net/mptcp/pm.c | 28 ++++++------
net/mptcp/pm_netlink.c | 20 ++++-----
net/mptcp/protocol.c | 59 +++++++++++++------------
net/mptcp/protocol.h | 4 +-
net/mptcp/sched.c | 4 +-
net/mptcp/sockopt.c | 4 +-
net/mptcp/subflow.c | 56 ++++++++++++-----------
tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 ++---
10 files changed, 122 insertions(+), 118 deletions(-)
---
base-commit: 31a972959ae57691a1e4f539399b2674ae576086
change-id: 20240826-net-mptcp-close-extra-sf-fin-19d4e5aa4c9c
Best regards,
--
Matthieu Baerts (NGI0) <matttbe(a)kernel.org>
Make the test executable. Currently, tests in this shell script are not
executable, so the scipt file is skipped entirely.
Also, the error message descirbing the required modules is inaccurate.
Currently, only "SKIP: Need act_mirred module" is shown. As a result,
users might only that module; however, three modules are actually
required and if any of them are missing, the build will fail with the
same message.
Fix the error message to show any/all modules needed for the script file
upon failure.
Signed-off-by: David Hunter <david.hunter.linux(a)gmail.com>
---
.../testing/selftests/net/test_ingress_egress_chaining.sh | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
mode change 100644 => 100755 tools/testing/selftests/net/test_ingress_egress_chaining.sh
diff --git a/tools/testing/selftests/net/test_ingress_egress_chaining.sh b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
old mode 100644
new mode 100755
index 08adff6bb3b6..b1a3d68e0ec2
--- a/tools/testing/selftests/net/test_ingress_egress_chaining.sh
+++ b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
@@ -13,8 +13,14 @@ if [ "$(id -u)" -ne 0 ];then
fi
needed_mods="act_mirred cls_flower sch_ingress"
+mods_missing=""
+
+for mod in $needed_mods; do
+ modinfo $mod &>/dev/null || mods_missing="$mods_missing$mod "
+done
+
for mod in $needed_mods; do
- modinfo $mod &>/dev/null || { echo "SKIP: Need act_mirred module"; exit $ksft_skip; }
+ modinfo $mod &>/dev/null || { echo "SKIP: modules needed: $mods_missing"; exit $ksft_skip; }
done
ns="ns$((RANDOM%899+100))"
--
2.43.0
The error message descirbing the required modules is inaccurate.
Currently, only "SKIP: Need act_mirred module" is printed when any of
the modules are missing. As a result, users might only include that
module; however, three modules are required.
Fix the error message to show any/all modules needed for the script file
to properly execute.
Signed-off-by: David Hunter <david.hunter.linux(a)gmail.com>
---
V1 --> V2
- included subject prefixes
- Split the patch into two separate patches (one for each issue)
- fixed typos in message body
- removed second, unnecessary for loop
---
.../selftests/net/test_ingress_egress_chaining.sh | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/test_ingress_egress_chaining.sh b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
index 08adff6bb3b6..d4b97662849b 100644
--- a/tools/testing/selftests/net/test_ingress_egress_chaining.sh
+++ b/tools/testing/selftests/net/test_ingress_egress_chaining.sh
@@ -13,10 +13,19 @@ if [ "$(id -u)" -ne 0 ];then
fi
needed_mods="act_mirred cls_flower sch_ingress"
+mods_missing=""
+numb_mods_needed=0;
+
for mod in $needed_mods; do
- modinfo $mod &>/dev/null || { echo "SKIP: Need act_mirred module"; exit $ksft_skip; }
+ modinfo $mod &>/dev/null ||
+ { mods_missing="$mods_missing$mod " ; numb_mods_needed=$(expr $numb_mods_needed + 1);}
done
+if [[ $numb_mods_needed>0 ]]; then
+ echo "SKIP: $numb_mods_needed modules needed: $mods_missing" ; exit $ksft_skip;
+fi
+
+
ns="ns$((RANDOM%899+100))"
veth1="veth1$((RANDOM%899+100))"
veth2="veth2$((RANDOM%899+100))"
--
2.43.0
This test neglects to put ports down on cleanup. Fix it.
Fixes: 90b9566aa5cd ("selftests: forwarding: add a test for local_termination.sh")
Signed-off-by: Petr Machata <petrm(a)nvidia.com>
---
tools/testing/selftests/net/forwarding/local_termination.sh | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh b/tools/testing/selftests/net/forwarding/local_termination.sh
index c5b0cbc85b3e..9b5a63519b94 100755
--- a/tools/testing/selftests/net/forwarding/local_termination.sh
+++ b/tools/testing/selftests/net/forwarding/local_termination.sh
@@ -278,6 +278,10 @@ bridge()
cleanup()
{
pre_cleanup
+
+ ip link set $h2 down
+ ip link set $h1 down
+
vrf_cleanup
}
--
2.45.0
This test neglects to put ports down on cleanup. Fix it.
Fixes: 476a4f05d9b8 ("selftests: forwarding: add a no_forwarding.sh test")
Signed-off-by: Petr Machata <petrm(a)nvidia.com>
---
tools/testing/selftests/net/forwarding/no_forwarding.sh | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/no_forwarding.sh b/tools/testing/selftests/net/forwarding/no_forwarding.sh
index af3b398d13f0..9e677aa64a06 100755
--- a/tools/testing/selftests/net/forwarding/no_forwarding.sh
+++ b/tools/testing/selftests/net/forwarding/no_forwarding.sh
@@ -233,6 +233,9 @@ cleanup()
{
pre_cleanup
+ ip link set dev $swp2 down
+ ip link set dev $swp1 down
+
h2_destroy
h1_destroy
--
2.45.0
Recently, a defer helper was added to Python selftests. The idea is to keep
cleanup commands close to their dirtying counterparts, thereby making it
more transparent what is cleaning up what, making it harder to miss a
cleanup, and make the whole cleanup business exception safe. All these
benefits are applicable to bash as well, exception safety can be
interpreted in terms of safety vs. a SIGINT.
This patchset therefore introduces a framework of several helpers that
serve to schedule cleanups in bash selftests.
As a personal remark. More than once was I bit by stop_traffic not getting
invoked because I C-c'd a traffic scheduler selftest at the wrong time.
This would leave behind a running mausezahn that would break follow-up runs
of the script that I was just debugging, making me question my sanity.
("How did this one extra debug print break the full script? And when I
remove it again, _why is it still broken_?") This is an attempt at
squashing this whole class of problems.
Patch #1 has more details about the primitives being introduced.
Patches #2 to #5 the convert several selftests to give an idea of how it
looks in practice.
Petr Machata (5):
selftests: forwarding: Introduce deferred commands
selftests: mlxsw: sch_red_core: Use defer for test cleanup
selftests: mlxsw: sch_red_core: Use defer for stopping traffic
selftests: mlxsw: sch_red_*: Use defer for qdisc management
selftests: sch_tbf_core: Use defer for stopping traffic
.../drivers/net/mlxsw/sch_red_core.sh | 131 +++++++-----------
.../drivers/net/mlxsw/sch_red_ets.sh | 32 ++---
.../drivers/net/mlxsw/sch_red_root.sh | 24 +++-
tools/testing/selftests/net/forwarding/lib.sh | 83 +++++++++++
.../selftests/net/forwarding/sch_tbf_core.sh | 3 +-
5 files changed, 170 insertions(+), 103 deletions(-)
--
2.45.0
This option makes IP6_NF_IPTABLES_LEGACY user selectable, giving
users the option to configure iptables without enabling any other
config.
Signed-off-by: Breno Leitao <leitao(a)debian.org>
---
net/ipv6/netfilter/Kconfig | 22 ++++++++++++----------
tools/testing/selftests/net/config | 5 +++++
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
index f3c8e2d918e1..dad0a50d3ef4 100644
--- a/net/ipv6/netfilter/Kconfig
+++ b/net/ipv6/netfilter/Kconfig
@@ -8,7 +8,13 @@ menu "IPv6: Netfilter Configuration"
# old sockopt interface and eval loop
config IP6_NF_IPTABLES_LEGACY
- tristate
+ tristate "Legacy IP6 tables support"
+ depends on INET && IPV6
+ select NETFILTER_XTABLES
+ default n
+ help
+ ip6tables is a general, extensible packet identification legacy framework.
+ This is not needed if you are using iptables over nftables (iptables-nft).
config NF_SOCKET_IPV6
tristate "IPv6 socket lookup support"
@@ -190,7 +196,7 @@ config IP6_NF_TARGET_HL
config IP6_NF_FILTER
tristate "Packet filtering"
default m if NETFILTER_ADVANCED=n
- select IP6_NF_IPTABLES_LEGACY
+ depends on IP6_NF_IPTABLES_LEGACY
tristate
help
Packet filtering defines a table `filter', which has a series of
@@ -227,7 +233,7 @@ config IP6_NF_TARGET_SYNPROXY
config IP6_NF_MANGLE
tristate "Packet mangling"
default m if NETFILTER_ADVANCED=n
- select IP6_NF_IPTABLES_LEGACY
+ depends on IP6_NF_IPTABLES_LEGACY
help
This option adds a `mangle' table to iptables: see the man page for
iptables(8). This table is used for various packet alterations
@@ -237,7 +243,7 @@ config IP6_NF_MANGLE
config IP6_NF_RAW
tristate 'raw table support (required for TRACE)'
- select IP6_NF_IPTABLES_LEGACY
+ depends on IP6_NF_IPTABLES_LEGACY
help
This option adds a `raw' table to ip6tables. This table is the very
first in the netfilter framework and hooks in at the PREROUTING
@@ -249,9 +255,7 @@ config IP6_NF_RAW
# security table for MAC policy
config IP6_NF_SECURITY
tristate "Security table"
- depends on SECURITY
- depends on NETFILTER_ADVANCED
- select IP6_NF_IPTABLES_LEGACY
+ depends on SECURITY && NETFILTER_ADVANCED && IP6_NF_IPTABLES_LEGACY
help
This option adds a `security' table to iptables, for use
with Mandatory Access Control (MAC) policy.
@@ -260,10 +264,8 @@ config IP6_NF_SECURITY
config IP6_NF_NAT
tristate "ip6tables NAT support"
- depends on NF_CONNTRACK
- depends on NETFILTER_ADVANCED
+ depends on NF_CONNTRACK && NETFILTER_ADVANCED && IP6_NF_IPTABLES_LEGACY
select NF_NAT
- select IP6_NF_IPTABLES_LEGACY
select NETFILTER_XT_NAT
help
This enables the `nat' table in ip6tables. This allows masquerading,
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 90e997cfa12e..e534144c75ea 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -35,12 +35,16 @@ CONFIG_IPV6_SIT=y
CONFIG_IP_DCCP=m
CONFIG_NF_NAT=m
CONFIG_IP6_NF_IPTABLES=m
+CONFIG_IP6_NF_IPTABLES_LEGACY=m
CONFIG_IP_NF_IPTABLES=m
CONFIG_IP_NF_IPTABLES_LEGACY=m
CONFIG_IP_NF_FILTER=m
CONFIG_IP_NF_TARGET_REJECT=m
CONFIG_IP_NF_TARGET_MASQUERADE=m
CONFIG_IP_NF_MANGLE=m
+CONFIG_IP6_NF_MANGLE=m
+CONFIG_IP6_NF_FILTER=m
+CONFIG_IP6_NF_TARGET_REJECT=m
CONFIG_IP6_NF_NAT=m
CONFIG_IP6_NF_RAW=m
CONFIG_IP_NF_NAT=m
@@ -61,6 +65,7 @@ CONFIG_NF_TABLES=m
CONFIG_NF_TABLES_IPV6=y
CONFIG_NF_TABLES_IPV4=y
CONFIG_NF_REJECT_IPV4=y
+CONFIG_NF_REJECT_IPV6=y
CONFIG_NFT_NAT=m
CONFIG_NETFILTER_XT_MATCH_LENGTH=m
CONFIG_NET_ACT_CSUM=m
--
2.43.5
Based on feedback from Linus[1] and follow-up discussions, change the
suggested file naming for KUnit tests.
Link: https://lore.kernel.org/lkml/CAHk-=wgim6pNiGTBMhP8Kd3tsB7_JTAuvNJ=XYd3wPvvk… [1]
Reviewed-by: John Hubbard <jhubbard(a)nvidia.com>
Signed-off-by: Kees Cook <kees(a)kernel.org>
---
v3: additional clarification
v2: https://lore.kernel.org/all/20240720165441.it.320-kees@kernel.org/
Cc: David Gow <davidgow(a)google.com>
Cc: Brendan Higgins <brendan.higgins(a)linux.dev>
Cc: Rae Moar <rmoar(a)google.com>
Cc: John Hubbard <jhubbard(a)nvidia.com>
Cc: Jonathan Corbet <corbet(a)lwn.net>
Cc: Linus Torvalds <torvalds(a)linux-foundation.org>
Cc: linux-kselftest(a)vger.kernel.org
Cc: kunit-dev(a)googlegroups.com
Cc: linux-doc(a)vger.kernel.org
Cc: linux-kernel(a)vger.kernel.org
Cc: linux-hardening(a)vger.kernel.org
---
Documentation/dev-tools/kunit/style.rst | 29 +++++++++++++++++--------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/Documentation/dev-tools/kunit/style.rst b/Documentation/dev-tools/kunit/style.rst
index b6d0d7359f00..eac81a714a29 100644
--- a/Documentation/dev-tools/kunit/style.rst
+++ b/Documentation/dev-tools/kunit/style.rst
@@ -188,15 +188,26 @@ For example, a Kconfig entry might look like:
Test File and Module Names
==========================
-KUnit tests can often be compiled as a module. These modules should be named
-after the test suite, followed by ``_test``. If this is likely to conflict with
-non-KUnit tests, the suffix ``_kunit`` can also be used.
+KUnit tests are often compiled as a separate module. To avoid conflicting
+with regular modules, KUnit modules should be named after the test suite,
+followed by ``_kunit`` (e.g. if "foobar" is the core module, then
+"foobar_kunit" is the KUnit test module).
-The easiest way of achieving this is to name the file containing the test suite
-``<suite>_test.c`` (or, as above, ``<suite>_kunit.c``). This file should be
-placed next to the code under test.
+Test source files, whether compiled as a separate module or an
+``#include`` in another source file, are best kept in a ``tests/``
+subdirectory to not conflict with other source files (e.g. for
+tab-completion).
+
+Note that the ``_test`` suffix has also been used in some existing
+tests. The ``_kunit`` suffix is preferred, as it makes the distinction
+between KUnit and non-KUnit tests clearer.
+
+So for the common case, name the file containing the test suite
+``tests/<suite>_kunit.c``. The ``tests`` directory should be placed at
+the same level as the code under test. For example, tests for
+``lib/string.c`` live in ``lib/tests/string_kunit.c``.
If the suite name contains some or all of the name of the test's parent
-directory, it may make sense to modify the source filename to reduce redundancy.
-For example, a ``foo_firmware`` suite could be in the ``foo/firmware_test.c``
-file.
+directory, it may make sense to modify the source filename to reduce
+redundancy. For example, a ``foo_firmware`` suite could be in the
+``foo/tests/firmware_kunit.c`` file.
--
2.34.1