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
