Re: [PATCH net-next] selftests: drv-net: warn if LOCAL_V6 has DAD or not kept on link down

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

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

- [Medium] The patch documents a new environment invariant
  (README.rst:73-80: the local IPv6 address must be added with `nodad`…

--

selftests: drv-net: warn if LOCAL_V6 has DAD or not kept on link down

This documents that the drv-net environment is expected to configure
LOCAL_V6 with nodad and with keep_addr_on_down enabled, and adds a check
in NetDrvEpEnv._check_env() that prints warnings when either is missing.
The motivation is intermittent bind() failures in napi_id.py on HW
runners.

> diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
> index c6bed9a985bc..3fe49bce4f3a 100644
> --- a/tools/testing/selftests/drivers/net/README.rst
> +++ b/tools/testing/selftests/drivers/net/README.rst
> @@ -70,6 +70,15 @@ LOCAL_V4, LOCAL_V6, REMOTE_V4, REMOTE_V6
>  
>  Local and remote endpoint IP addresses.
>  
> +Tests reconfigure the device freely, including taking the link down, and
> +expect the addresses to work immediately afterwards. IPv6 needs help::
> +
> +  ip -6 address add 2001:db8:1::1/64 dev eth0 nodad
> +  sysctl -w net.ipv6.conf.eth0.keep_addr_on_down=1
> +
> +Without those the address is flushed when the link goes down, or spends
> +a second unusable while waiting for DAD to finish.
> +
>  LOCAL_PREFIX_V6
>  ~~~~~~~~~~~~~~~
>  
> diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
> index 25903f580b40..6262080a8bf4 100644
> --- a/tools/testing/selftests/drivers/net/lib/py/env.py
> +++ b/tools/testing/selftests/drivers/net/lib/py/env.py
> @@ -239,6 +239,25 @@ class NetDrvEpEnv(NetDrvEnvBase):
>          if missing:
>              raise Exception("Invalid environment, missing configuration:", missing,
>                              "Please see tools/testing/selftests/drivers/net/README.rst")
> +        if "LOCAL_V6" in self.env:
> +            self._check_v6_env()

[Severity: Medium]
Do the in-tree environment providers meet the invariant that is being
documented here?

Two cases look like they do not:

1) NetDrvEpEnv.create_local() (the netdevsim path in the same file) adds
   the addresses with nodad, but never enables keep_addr_on_down on the
   nsim devices or on 'all':

     ip(f"   addr add dev {self._ns.nsims[0].ifname} {self.nsim_v4_pfx}1/24")
     ip(f"-6 addr add dev {self._ns.nsims[0].ifname} {self.nsim_v6_pfx}1/64 nodad")
     ip(f"   link set dev {self._ns.nsims[0].ifname} up")

   Both the devconf_all and per-device defaults are 0, and
   addrconf_ifdown() drops permanent global addresses when the aggregate
   is not positive:

   net/ipv6/addrconf.c:addrconf_ifdown() {
	...
	int _keep_addr = READ_ONCE(net->ipv6.devconf_all->keep_addr_on_down);

	if (!_keep_addr)
		_keep_addr = READ_ONCE(idev->cnf.keep_addr_on_down);

	keep_addr = (_keep_addr > 0);
	...
   }

   So a test that follows the new documentation and bounces the link
   would lose the nsim LOCAL_V6 address. Note also that this path never
   reaches _check_v6_env(), since the call is only made from the branch
   that requires NETIF, so there is no warning either.

2) tools/testing/selftests/net/lib/ksft_setup_loopback.sh sets up the
   env that this code validates, and adds LOCAL_V6 without nodad and
   without touching keep_addr_on_down:

     ip -netns "${SERVER_NS}" addr add $LOCAL_V6/64 dev server
     ...
     export NETIF=server

   Would every run driven by that script now print both new warnings,
   pointing at in-tree setup code rather than at a user
   misconfiguration? Should the script and create_local() be updated
   along with the documentation?

>  
>      def resolve_remote_ifc(self):
>          v4 = v6 = None

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908181956.1357684-1-kuba%40kernel.org