Re: [PATCH v1 net-next 2/4] selftests: netfilter: Set gc_thresh[123] for gw netns in nft_nat_zones.sh.

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In tools/testing/selftests/net/netfilter/nft_nat_zones.sh, the
  patch deletes the init_net writes of…

--

selftests: netfilter: Set gc_thresh[123] for gw netns in nft_nat_zones.sh.

Commit cda2962b6e2b ("neighbour: Namespacify neigh_tables.") made the
net.ipv[46].neigh.default.gc_thresh[123] sysctls per-netns. This patch
removes the init_net writes and their save/restore in cleanup(), and sets
the thresholds inside the gw netns instead.

> diff --git a/tools/testing/selftests/net/netfilter/nft_nat_zones.sh b/tools/testing/selftests/net/netfilter/nft_nat_zones.sh
> index 9f200f80253a7..d935c967dd13b 100755
> --- a/tools/testing/selftests/net/netfilter/nft_nat_zones.sh
> +++ b/tools/testing/selftests/net/netfilter/nft_nat_zones.sh

[ ... ]

> @@ -59,18 +40,19 @@ fi
>  
>  setup_ns gw srv
>  
> -trap cleanup EXIT
> +trap cleanup_all_ns EXIT
>  
>  ip link add veth0 netns "$gw" type veth peer name eth0 netns "$srv"
>  ip -net "$gw" link set veth0 up
>  ip -net "$srv" link set eth0 up
>  
> -sysctl -q net.ipv6.neigh.default.gc_thresh1=512  2>/dev/null
> -sysctl -q net.ipv6.neigh.default.gc_thresh2=1024 2>/dev/null
> -sysctl -q net.ipv6.neigh.default.gc_thresh3=4096 2>/dev/null
> -sysctl -q net.ipv4.neigh.default.gc_thresh1=512  2>/dev/null
> -sysctl -q net.ipv4.neigh.default.gc_thresh2=1024 2>/dev/null
> -sysctl -q net.ipv4.neigh.default.gc_thresh3=4096 2>/dev/null
> +ip netns exec "$gw" sysctl -q \
> +   net.ipv6.neigh.default.gc_thresh1=512 \
> +   net.ipv6.neigh.default.gc_thresh2=1024 \
> +   net.ipv6.neigh.default.gc_thresh3=4096 \
> +   net.ipv4.neigh.default.gc_thresh1=512 \
> +   net.ipv4.neigh.default.gc_thresh2=1024 \
> +   net.ipv4.neigh.default.gc_thresh3=4096 2>/dev/null

[Severity: Low]
Are the thresholds still raised on kernels that don't have commit
cda2962b6e2b ("neighbour: Namespacify neigh_tables.")?

Before that commit, arp_init() and ndisc_init() registered the sysctls
with neigh_sysctl_register(NULL, &arp_tbl.parms, ...) and
neigh_sysctl_register(NULL, &nd_tbl.parms, ...). So neigh/default only
existed in init_net.

On those kernels the ip netns exec "$gw" sysctl call fails, and
2>/dev/null hides the error.

The init_net write is removed along with the save/restore in cleanup().
That write used to raise gc_thresh3 to 4096 for the single shared table.
Without it, the table stays at the default 128/512/1024.

Documentation/dev-tools/kselftest.rst says mainline kselftests may be
run on older stable kernels. The comment at the top of the script says:

    # Don't increase too much, 2000 clients should work
    # just fine but script can then take several minutes with
    # KASAN/debug builds.
    maxclients=100

With maxclients raised to 2000, the shared table would need more than
2000 IPv4 entries. That is above gc_thresh3=1024, so neigh_alloc() would
report "neighbor table overflow".

Could that make the ping counter or socat checks fail on those kernels?
The default maxclients=100 doesn't trigger this.

Would it work to fall back to the init_net write, with the save/restore,
when the per-netns write fails? Another option is to do both writes.

>  
>  for i in $(seq 1 "$maxclients");do
>    setup_ns "cl$i"

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012194530.3751034-1-kuniyu%40google.com