[PATCH net v2] selftests/net: run tun tests in a dedicated network namespace

Edoardo Canepa posted 1 patch 1 week, 3 days ago
There is a newer version of this series
tools/testing/selftests/net/tun.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH net v2] selftests/net: run tun tests in a dedicated network namespace
Posted by Edoardo Canepa 1 week, 3 days ago
The tun_vnet_udptnl fixture creates a fresh tap device and installs an
IPv6 outer neighbor entry as NUD_PERMANENT before sending packets.  On
systems where systemd-udevd is running and a systemd .link file sets

    MACAddressPolicy=persistent

(the default shipped by systemd in 99-default.link, so this is what
most systemd-based hosts inherit), systemd-udevd's net_setup_link
builtin asynchronously sends an RTM_SETLINK to reassign the freshly
created tap device's MAC to a machine-persistent value.  When that
netlink message races the test's ip_neigh_add() call, the address
change kicks the following path:

    do_setlink
    -> netif_set_mac_address
       -> call_netdevice_notifiers_info
          -> ndisc_netdev_event
             -> neigh_changeaddr
                -> neigh_flush_dev(tbl, dev, /* skip_perm = */ false)

which flushes every neighbor entry on the interface, including the one
the test just installed as NUD_PERMANENT.  The subsequent packet
therefore hits __neigh_create(), triggers NDISC, and times out with:

  tun.c:947:send_gso_packet:Expected ret (0) == variant->data_size (1423)
  tun.c:948:send_gso_packet:Expected r_num_mss (0) == variant->r_num_mss (2)

The failure is non-deterministic and can affect both directions.  Both
recv_gso_packet and send_gso_packet variants can hit it; the failure
reproduces on a plain systemd-based VM with no containers, and is
triggered whenever the udev worker's RTM_SETLINK lands after the test
has installed its neighbor entry.

Fix by calling unshare(CLONE_NEWNET) from both fixture setups.  The
harness runs each test in its own forked process, so every test gets a
private network namespace that is torn down with it, and all tap and
geneve devices are created in a namespace that systemd-udevd (running
in the init netns) does not watch, so its RTM_SETLINK never fires
against them.

Creating a network namespace needs CAP_SYS_ADMIN in the current user
namespace and CONFIG_NET_NS=y, neither of which the tests required
before.  Where they are unavailable the unshare() is reported with
SKIP() rather than aborting, so the binary still emits a full TAP
stream and a runner can tell "network namespaces unavailable" apart
from a real tun/tap regression.

Verified on a plain systemd-based VM running the affected kernel, with
the tap and geneve devices removed between iterations so that each one
starts from a clean state.  1000 repeated invocations of

  tun -r tun_vnet_udptnl.4in6_nogsosz_1byte.recv_gso_packet

produce 266 failures without the fix and zero failures with it, and a
full run of the test binary fails in 20 out of 20 attempts without the
fix and in zero out of 20 with it.

Note that without the fix a failure is not self-contained: the fixture
setup aborts before FIXTURE_TEARDOWN runs, so the tap and geneve
devices are left behind in the init netns and every later run fails
right away in geneve_create().  Running in a private namespace also
removes that, since the namespace is torn down with the test process.

Reported-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
Closes: https://bugs.launchpad.net/bugs/2158217
Fixes: 24e59f26eef2 ("selftest: tun: Add helpers for GSO over UDP tunnel")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Edoardo Canepa <edoardo.canepa@canonical.com>
---
v2:
  - Add the unshare(CLONE_NEWNET) to FIXTURE_SETUP(tun) and
    FIXTURE_SETUP(tun_vnet_udptnl) instead of replacing
    TEST_HARNESS_MAIN with a hand-written main(), as suggested by
    Jakub.
  - Report an unshare() failure with SKIP() instead of aborting the
    binary before the harness starts, so the TAP stream stays complete
    and a runner can tell "no network namespaces" apart from a real
    tun/tap regression (raised by Sashiko).
  - Mention the new CAP_SYS_ADMIN / CONFIG_NET_NS prerequisite in the
    commit message (raised by Sashiko).
  - Use the Assisted-by: format documented in
    Documentation/process/coding-assistants.rst.
  - Redo the measurements in the commit message.  The v1 numbers were
    taken without cleaning up the tap and geneve devices that a failed
    run leaves behind, which made runs after the first failure fail in
    geneve_create() rather than on the race being fixed here.
v1: https://lore.kernel.org/netdev/20260905085318.3416670-1-edoardo.canepa@canonical.com/
 tools/testing/selftests/net/tun.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
index abe488bac50b..6db21dad0efe 100644
--- a/tools/testing/selftests/net/tun.c
+++ b/tools/testing/selftests/net/tun.c
@@ -4,6 +4,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <sched.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -488,6 +489,10 @@ FIXTURE(tun)
 
 FIXTURE_SETUP(tun)
 {
+	if (unshare(CLONE_NEWNET))
+		SKIP(return, "Cannot create network namespace: %s",
+		     strerror(errno));
+
 	memset(self->ifname, 0, sizeof(self->ifname));
 
 	self->fd = tun_alloc(self->ifname);
@@ -732,6 +737,10 @@ FIXTURE_SETUP(tun_vnet_udptnl)
 	struct sockaddr_storage ssa, dsa;
 	void *sip, *dip, *smac, *dmac;
 
+	if (unshare(CLONE_NEWNET))
+		SKIP(return, "Cannot create network namespace: %s",
+		     strerror(errno));
+
 	flags = (variant->is_tap ? IFF_TAP : IFF_TUN) | IFF_VNET_HDR |
 		IFF_MULTI_QUEUE | IFF_NO_PI;
 	features = TUN_F_CSUM | TUN_F_UDP_TUNNEL_GSO |
-- 
2.53.0
Re: [PATCH net v2] selftests/net: run tun tests in a dedicated network namespace
Posted by netdev-bot+sashiko@kernel.org 1 week ago
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] The commit message's closing note ("Note that without the fix
  a failure is not self-contained: the fixture setup aborts before…
- [Low] The commit message uses `Assisted-by: Claude:claude-opus-5` while
  the v2 changelog claims "Use the Assisted-by: format documented in…

--

selftests/net: run tun tests in a dedicated network namespace

The tun and tun_vnet_udptnl fixtures now call unshare(CLONE_NEWNET) in
their setup so each forked test process gets a private network namespace,
which keeps systemd-udevd's RTM_SETLINK (MACAddressPolicy=persistent) from
flushing the NUD_PERMANENT neighbor entry the test installs.  If unshare()
fails the test is reported with SKIP() instead of aborting.

Closes: https://bugs.launchpad.net/bugs/2158217

Both observations below are about the commit message only, so the diff
hunks are snipped.

[ ... ]

>     Note that without the fix a failure is not self-contained: the fixture
>     setup aborts before FIXTURE_TEARDOWN runs, so the tap and geneve
>     devices are left behind in the init netns and every later run fails
>     right away in geneve_create().  Running in a private namespace also
>     removes that, since the namespace is torn down with the test process.

[Severity: Medium]
Does this paragraph describe the same failure the message quotes earlier?

The quoted diagnostics

  tun.c:947:send_gso_packet:Expected ret (0) == variant->data_size (1423)
  tun.c:948:send_gso_packet:Expected r_num_mss (0) == variant->r_num_mss (2)

map (with the nine lines this patch adds above them) to the two checks at
the end of TEST_F(tun_vnet_udptnl, send_gso_packet):

	ret = receive_gso_packet_from_tunnel(self, variant, &r_num_mss);
	EXPECT_EQ(ret, variant->data_size);
	EXPECT_EQ(r_num_mss, variant->r_num_mss);

That is after FIXTURE_SETUP(tun_vnet_udptnl) has completed, not an abort
during setup.  EXPECT_EQ only records KSFT_FAIL and continues, so the test
body returns normally and __TEST_F_IMPL in kselftest_harness.h has already
cleared no_teardown and then calls the teardown unconditionally:

			*_metadata->no_teardown = false; \
			fixture_name##_##test_name(_metadata, self, variant->data); \
			_metadata->teardown_fn(false, _metadata, self, variant->data); \

FIXTURE_TEARDOWN(tun_vnet_udptnl) then does run:

	ret = ip_link_del(param_dev_geneve_name);
	EXPECT_EQ(ret, 0);

	ret = tun_delete(self->ifname);
	EXPECT_EQ(ret, 0);

A harness timeout does not look like the mechanism either: udp_socket_open()
sets SO_RCVTIMEO to TIMEOUT_SEC (10 s) and receive_gso_packet_from_tun()
select()s with the same 10 s bound, both under TEST_TIMEOUT_DEFAULT (30 s).
The tap is also opened without IFF_PERSIST, so it goes away when the child
closes its fd.

Could the leftover geneve1 and tap devices that were observed have come from
a different abort or setup-failure path than the neighbor flush described
here?  Since the v2 measurement methodology ("with the tap and geneve devices
removed between iterations") is justified by this paragraph, it would help to
either identify that path or drop the claim.

>     Assisted-by: Claude:claude-opus-5

[Severity: Low]
This isn't a bug, but the v2 changelog says "Use the Assisted-by: format
documented in Documentation/process/coding-assistants.rst", and that
document specifies:

  Assisted-by: LLM [TOOL1] [TOOL2]

* ``[TOOL1] [TOOL2]`` are optional specialized analysis tools used
  (e.g., coccinelle, sparse, smatch, clang-tidy)

The literal LLM token is required and the optional trailing fields are
analysis tools rather than a vendor or model name, so the trailer as written
does not match either part.  Should this read "Assisted-by: LLM"?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914211921.3786609-1-edoardo.canepa%40canonical.com