Netdev List
 help / color / mirror / Atom feed
* [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series.
@ 2026-10-12 19:44 Kuniyuki Iwashima
  2026-10-12 19:44 ` [PATCH v1 net-next 1/4] Documentation: networking: switchdev: Replace arp_tbl with arp_table(net) Kuniyuki Iwashima
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-12 19:44 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Ido Schimmel
  Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

Patch 1 updates the switchdev doc, s/arp_tbl/arp_table(net)/.

Patch 2 updates a selftest for per-netns sysctl knobs, though this
is not an actual issue since the test does not hit gc_thresh by
default.

Patches 3 & 4 are based on a suggestion by Ido Schimmel to make
neigh_managed_work CPU-friendly:

  https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/

Given it's rc7, I'll defer RTNL removal to the next cycle.


Kuniyuki Iwashima (4):
  Documentation: networking: switchdev: Replace arp_tbl with
    arp_table(net).
  selftests: netfilter: Set gc_thresh[123] for gw netns in
    nft_nat_zones.sh.
  neighbour: Schedule neigh_managed_work only when needed.
  neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.

 Documentation/netlink/specs/rt-neigh.yaml     |  2 +-
 Documentation/networking/ip-sysctl.rst        |  2 +-
 Documentation/networking/switchdev.rst        | 11 +++---
 net/core/neighbour.c                          | 26 ++++++++++----
 .../selftests/net/netfilter/nft_nat_zones.sh  | 34 +++++--------------
 5 files changed, 35 insertions(+), 40 deletions(-)

-- 
2.56.0.385.gd3acb90ef8-goog


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 net-next 1/4] Documentation: networking: switchdev: Replace arp_tbl with arp_table(net).
  2026-10-12 19:44 [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Kuniyuki Iwashima
@ 2026-10-12 19:44 ` Kuniyuki Iwashima
  2026-10-12 19:44 ` [PATCH v1 net-next 2/4] selftests: netfilter: Set gc_thresh[123] for gw netns in nft_nat_zones.sh Kuniyuki Iwashima
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-12 19:44 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Ido Schimmel
  Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

Since commit cda2962b6e2b ("neighbour: Namespacify neigh_tables."),
arp_tbl is a template for the per-netns table.

Let's apply s/arp_tbl/arp_table(net)/ to switchdev.rst.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 Documentation/networking/switchdev.rst | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/networking/switchdev.rst b/Documentation/networking/switchdev.rst
index 2966b7122f05..8e8ab6632ffe 100644
--- a/Documentation/networking/switchdev.rst
+++ b/Documentation/networking/switchdev.rst
@@ -401,14 +401,15 @@ The FIB entry's nexthop list contains the nexthop tuple (gateway, dev), but for
 the switch device to forward the packet with the correct dst mac address, the
 nexthop gateways must be resolved to the neighbor's mac address.  Neighbor mac
 address discovery comes via the ARP (or ND) process and is available via the
-arp_tbl neighbor table.  To resolve the routes nexthop gateways, the driver
-should trigger the kernel's neighbor resolution process.  See the rocker
+arp_table(net) neighbor table.  To resolve the routes nexthop gateways, the
+driver should trigger the kernel's neighbor resolution process.  See the rocker
 driver's rocker_port_ipv4_resolve() for an example.
 
-The driver can monitor for updates to arp_tbl using the netevent notifier
+The driver can monitor for updates to arp_table(net) using the netevent notifier
 NETEVENT_NEIGH_UPDATE.  The device can be programmed with resolved nexthops
-for the routes as arp_tbl updates.  The driver implements ndo_neigh_destroy
-to know when arp_tbl neighbor entries are purged from the port.
+for the routes as arp_table(net) updates.  The driver implements
+ndo_neigh_destroy to know when arp_table(net) neighbor entries are purged from
+the port.
 
 Device driver expected behavior
 -------------------------------
-- 
2.56.0.385.gd3acb90ef8-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 net-next 2/4] selftests: netfilter: Set gc_thresh[123] for gw netns in nft_nat_zones.sh.
  2026-10-12 19:44 [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Kuniyuki Iwashima
  2026-10-12 19:44 ` [PATCH v1 net-next 1/4] Documentation: networking: switchdev: Replace arp_tbl with arp_table(net) Kuniyuki Iwashima
@ 2026-10-12 19:44 ` Kuniyuki Iwashima
  2026-10-13 20:00   ` netdev-bot+sashiko
  2026-10-12 19:44 ` [PATCH v1 net-next 3/4] neighbour: Schedule neigh_managed_work only when needed Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-12 19:44 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Ido Schimmel
  Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev,
	Pablo Neira Ayuso, Florian Westphal, Phil Sutter

Since commit cda2962b6e2b ("neighbour: Namespacify neigh_tables."),
net.ipv[46].neigh.default.gc_thresh[123] are per-netns attributes.

Let's update nft_nat_zones.sh.

Note that the number of neighbour entries does not even reach the
default gc_thresh1 (128) with the default maxclients=100, and the
raised thresholds are only needed for gw netns when maxclients is
manually increased (e.g., 2000 as mentioned in the comment).

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: Phil Sutter <phil@nwl.cc>
---
 .../selftests/net/netfilter/nft_nat_zones.sh  | 34 +++++--------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/tools/testing/selftests/net/netfilter/nft_nat_zones.sh b/tools/testing/selftests/net/netfilter/nft_nat_zones.sh
index 9f200f80253a..d935c967dd13 100755
--- a/tools/testing/selftests/net/netfilter/nft_nat_zones.sh
+++ b/tools/testing/selftests/net/netfilter/nft_nat_zones.sh
@@ -31,25 +31,6 @@ ret=0
 #   NAT Gateway is supposed to do port reallocation for each of the
 #   connections.
 
-v4gc1=$(sysctl -n net.ipv4.neigh.default.gc_thresh1 2>/dev/null)
-v4gc2=$(sysctl -n net.ipv4.neigh.default.gc_thresh2 2>/dev/null)
-v4gc3=$(sysctl -n net.ipv4.neigh.default.gc_thresh3 2>/dev/null)
-v6gc1=$(sysctl -n net.ipv6.neigh.default.gc_thresh1 2>/dev/null)
-v6gc2=$(sysctl -n net.ipv6.neigh.default.gc_thresh2 2>/dev/null)
-v6gc3=$(sysctl -n net.ipv6.neigh.default.gc_thresh3 2>/dev/null)
-
-cleanup()
-{
-	cleanup_all_ns
-
-	sysctl -q net.ipv4.neigh.default.gc_thresh1="$v4gc1" 2>/dev/null
-	sysctl -q net.ipv4.neigh.default.gc_thresh2="$v4gc2" 2>/dev/null
-	sysctl -q net.ipv4.neigh.default.gc_thresh3="$v4gc3" 2>/dev/null
-	sysctl -q net.ipv6.neigh.default.gc_thresh1="$v6gc1" 2>/dev/null
-	sysctl -q net.ipv6.neigh.default.gc_thresh2="$v6gc2" 2>/dev/null
-	sysctl -q net.ipv6.neigh.default.gc_thresh3="$v6gc3" 2>/dev/null
-}
-
 checktool "nft --version" echo "run test without nft tool"
 checktool "conntrack -V" "run test without conntrack tool"
 
@@ -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
 
 for i in $(seq 1 "$maxclients");do
   setup_ns "cl$i"
-- 
2.56.0.385.gd3acb90ef8-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 net-next 3/4] neighbour: Schedule neigh_managed_work only when needed.
  2026-10-12 19:44 [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Kuniyuki Iwashima
  2026-10-12 19:44 ` [PATCH v1 net-next 1/4] Documentation: networking: switchdev: Replace arp_tbl with arp_table(net) Kuniyuki Iwashima
  2026-10-12 19:44 ` [PATCH v1 net-next 2/4] selftests: netfilter: Set gc_thresh[123] for gw netns in nft_nat_zones.sh Kuniyuki Iwashima
@ 2026-10-12 19:44 ` Kuniyuki Iwashima
  2026-10-12 19:44 ` [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000 Kuniyuki Iwashima
  2026-10-13  7:00 ` [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Ido Schimmel
  4 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-12 19:44 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Ido Schimmel
  Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

neigh_table is now per-netns.

tbl->managed_work is scheduled when netns is created and runs
periodically even though the netns does not have any NTF_MANAGED
entries.

Let's delay scheduling the work for the first NTF_MANAGED entry
and stop rescheduling if tbl->managed_list is empty.

Suggested-by: Ido Schimmel <idosch@nvidia.com>
Link: https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/neighbour.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 90335895b415..6b0569aa5d64 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -184,6 +184,15 @@ static void neigh_update_gc_list(struct neighbour *n)
 	}
 }
 
+static void neigh_add_managed_list(struct neighbour *n)
+{
+	list_add_tail(&n->managed_list, &n->tbl->managed_list);
+
+	if (list_is_singular(&n->tbl->managed_list))
+		queue_delayed_work(system_power_efficient_wq,
+				   &n->tbl->managed_work, 0);
+}
+
 static void neigh_update_managed_list(struct neighbour *n)
 {
 	bool on_managed_list, add_to_managed;
@@ -194,7 +203,7 @@ static void neigh_update_managed_list(struct neighbour *n)
 	if (!add_to_managed && on_managed_list)
 		list_del_init(&n->managed_list);
 	else if (add_to_managed && !on_managed_list)
-		list_add_tail(&n->managed_list, &n->tbl->managed_list);
+		neigh_add_managed_list(n);
 }
 
 static void neigh_update_flags(struct neighbour *neigh, u32 flags, int *notify,
@@ -708,7 +717,7 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
 	if (!exempt_from_gc)
 		list_add_tail(&n->gc_list, &n->tbl->gc_list);
 	if (n->flags & NTF_MANAGED)
-		list_add_tail(&n->managed_list, &n->tbl->managed_list);
+		neigh_add_managed_list(n);
 	if (want_ref)
 		neigh_hold(n);
 	hlist_add_head_rcu(&n->hash, &nht->hash_heads[hash_val]);
@@ -1670,10 +1679,14 @@ static void neigh_managed_work(struct work_struct *work)
 	struct neighbour *neigh;
 
 	spin_lock_bh(&tbl->lock);
+
 	list_for_each_entry(neigh, &tbl->managed_list, managed_list)
 		neigh_event_send_probe(neigh, NULL, false);
-	queue_delayed_work(system_power_efficient_wq, &tbl->managed_work,
-			   NEIGH_VAR(&tbl->parms, INTERVAL_PROBE_TIME_MS));
+
+	if (!list_empty(&tbl->managed_list))
+		queue_delayed_work(system_power_efficient_wq, &tbl->managed_work,
+				   NEIGH_VAR(&tbl->parms, INTERVAL_PROBE_TIME_MS));
+
 	spin_unlock_bh(&tbl->lock);
 }
 
@@ -1873,7 +1886,6 @@ static int neigh_table_init(struct net *net, struct neigh_table *tbl)
 
 	INIT_LIST_HEAD(&tbl->managed_list);
 	INIT_DEFERRABLE_WORK(&tbl->managed_work, neigh_managed_work);
-	queue_delayed_work(system_power_efficient_wq, &tbl->managed_work, 0);
 
 	return 0;
 
-- 
2.56.0.385.gd3acb90ef8-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
  2026-10-12 19:44 [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2026-10-12 19:44 ` [PATCH v1 net-next 3/4] neighbour: Schedule neigh_managed_work only when needed Kuniyuki Iwashima
@ 2026-10-12 19:44 ` Kuniyuki Iwashima
  2026-10-13  7:16   ` Ido Schimmel
  2026-10-13 20:00   ` netdev-bot+sashiko
  2026-10-13  7:00 ` [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Ido Schimmel
  4 siblings, 2 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-12 19:44 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Ido Schimmel
  Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

Previously, the lower bound of INTERVAL_PROBE_TIME_MS was capped
at 1s by commit ed6cd6a17896 ("net, neigh: Set lower cap for
neigh_managed_work rearming").

Later, commit 211da42eaa45 ("net, neigh: introduce
interval_probe_time_ms for periodic probe") changed the limit
to 1ms.

The limit actually did not work but was fixed with
netlink_range_validation by commit 6d79b223ec44 ("neighbour:
Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.").

neigh_table is now per-netns, and a user could hog CPU by setting
INTERVAL_PROBE_TIME_MS to a small value.

Let's floor the value at 1 second.

Suggested-by: Ido Schimmel <idosch@nvidia.com>
Link: https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 Documentation/netlink/specs/rt-neigh.yaml | 2 +-
 Documentation/networking/ip-sysctl.rst    | 2 +-
 net/core/neighbour.c                      | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/specs/rt-neigh.yaml b/Documentation/netlink/specs/rt-neigh.yaml
index c8e55c98d564..7929e667cc7a 100644
--- a/Documentation/netlink/specs/rt-neigh.yaml
+++ b/Documentation/netlink/specs/rt-neigh.yaml
@@ -342,7 +342,7 @@ attribute-sets:
         name: interval-probe-time-ms
         type: u64
         checks:
-          min: 1
+          min: 1000
           max: 86400000
 
 operations:
diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 685c84cf543d..b00bd77363d7 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -248,7 +248,7 @@ neigh/default/unres_qlen - INTEGER
 
 neigh/default/interval_probe_time_ms - INTEGER
 	The probe interval for neighbor entries with NTF_MANAGED flag,
-	the min value is 1, and the max value is 86400000 (1 day).
+	the min value is 1000 (1s), and the max value is 86400000 (1 day).
 
 	Default: 5000
 
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 6b0569aa5d64..92e3c83c0fe5 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2438,7 +2438,7 @@ static const struct nla_policy nl_neightbl_policy[NDTA_MAX+1] = {
 #define NTBL_PARM_MS_MAX	(24 * 60 * 60 * MSEC_PER_SEC)
 
 static const struct netlink_range_validation nl_ntbl_parm_ms_range = {
-	.min = 1,
+	.min = 1000,
 	.max = NTBL_PARM_MS_MAX,
 };
 
@@ -3701,7 +3701,7 @@ static int neigh_proc_dointvec_ms_jiffies_positive(const struct ctl_table *ctl,
 	struct ctl_table tmp = *ctl;
 	int ret, min, max;
 
-	min = msecs_to_jiffies(1);
+	min = HZ;
 	max = msecs_to_jiffies(NTBL_PARM_MS_MAX);
 
 	tmp.extra1 = &min;
-- 
2.56.0.385.gd3acb90ef8-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series.
  2026-10-12 19:44 [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Kuniyuki Iwashima
                   ` (3 preceding siblings ...)
  2026-10-12 19:44 ` [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000 Kuniyuki Iwashima
@ 2026-10-13  7:00 ` Ido Schimmel
  2026-10-13  7:23   ` Kuniyuki Iwashima
  4 siblings, 1 reply; 13+ messages in thread
From: Ido Schimmel @ 2026-10-13  7:00 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Simon Horman, Kuniyuki Iwashima, netdev

On Mon, Oct 12, 2026 at 07:44:55PM +0000, Kuniyuki Iwashima wrote:
> Patch 1 updates the switchdev doc, s/arp_tbl/arp_table(net)/.
> 
> Patch 2 updates a selftest for per-netns sysctl knobs, though this
> is not an actual issue since the test does not hit gc_thresh by
> default.
> 
> Patches 3 & 4 are based on a suggestion by Ido Schimmel to make
> neigh_managed_work CPU-friendly:
> 
>   https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/

Thanks for the follow-up, but I think you missed my first point... My
comment was about the GC work that doesn't have a floor at all. An
unprivileged user can set the base reachable time to 0 and
neigh_periodic_work() will hog the CPU.

> 
> Given it's rc7, I'll defer RTNL removal to the next cycle.
> 
> 
> Kuniyuki Iwashima (4):
>   Documentation: networking: switchdev: Replace arp_tbl with
>     arp_table(net).
>   selftests: netfilter: Set gc_thresh[123] for gw netns in
>     nft_nat_zones.sh.
>   neighbour: Schedule neigh_managed_work only when needed.
>   neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
> 
>  Documentation/netlink/specs/rt-neigh.yaml     |  2 +-
>  Documentation/networking/ip-sysctl.rst        |  2 +-
>  Documentation/networking/switchdev.rst        | 11 +++---
>  net/core/neighbour.c                          | 26 ++++++++++----
>  .../selftests/net/netfilter/nft_nat_zones.sh  | 34 +++++--------------
>  5 files changed, 35 insertions(+), 40 deletions(-)
> 
> -- 
> 2.56.0.385.gd3acb90ef8-goog
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
  2026-10-12 19:44 ` [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000 Kuniyuki Iwashima
@ 2026-10-13  7:16   ` Ido Schimmel
  2026-10-13  7:27     ` Kuniyuki Iwashima
  2026-10-13 20:00   ` netdev-bot+sashiko
  1 sibling, 1 reply; 13+ messages in thread
From: Ido Schimmel @ 2026-10-13  7:16 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Simon Horman, Kuniyuki Iwashima, netdev

On Mon, Oct 12, 2026 at 07:44:59PM +0000, Kuniyuki Iwashima wrote:
> Previously, the lower bound of INTERVAL_PROBE_TIME_MS was capped
> at 1s by commit ed6cd6a17896 ("net, neigh: Set lower cap for
> neigh_managed_work rearming").

Nit: INTERVAL_PROBE_TIME_MS didn't exist in ed6cd6a17896. It was only
introduced later, in 211da42eaa45.

> 
> Later, commit 211da42eaa45 ("net, neigh: introduce
> interval_probe_time_ms for periodic probe") changed the limit
> to 1ms.
> 
> The limit actually did not work but was fixed with
> netlink_range_validation by commit 6d79b223ec44 ("neighbour:
> Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.").
> 
> neigh_table is now per-netns, and a user could hog CPU by setting
> INTERVAL_PROBE_TIME_MS to a small value.
> 
> Let's floor the value at 1 second.

The bot will most likely complain that this changes existing behavior,
but:

1. The original floor was 1 second.

2. It is unlikely that anyone but syzbot is setting it below 1 second.

I would explain this in the commit message to suppress such comments.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series.
  2026-10-13  7:00 ` [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Ido Schimmel
@ 2026-10-13  7:23   ` Kuniyuki Iwashima
  0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-13  7:23 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Simon Horman, Kuniyuki Iwashima, netdev

On Tue, Oct 13, 2026 at 12:00 AM Ido Schimmel <idosch@nvidia.com> wrote:
>
> On Mon, Oct 12, 2026 at 07:44:55PM +0000, Kuniyuki Iwashima wrote:
> > Patch 1 updates the switchdev doc, s/arp_tbl/arp_table(net)/.
> >
> > Patch 2 updates a selftest for per-netns sysctl knobs, though this
> > is not an actual issue since the test does not hit gc_thresh by
> > default.
> >
> > Patches 3 & 4 are based on a suggestion by Ido Schimmel to make
> > neigh_managed_work CPU-friendly:
> >
> >   https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/
>
> Thanks for the follow-up, but I think you missed my first point...

Oops, sorry I seem to have been sidetracked by 6d79b223ec44 series...

> My
> comment was about the GC work that doesn't have a floor at all. An
> unprivileged user can set the base reachable time to 0 and
> neigh_periodic_work() will hog the CPU.

I'll add the max(HZ, ...) change in patch 5.

---
pw-bot: cr



>
> >
> > Given it's rc7, I'll defer RTNL removal to the next cycle.
> >
> >
> > Kuniyuki Iwashima (4):
> >   Documentation: networking: switchdev: Replace arp_tbl with
> >     arp_table(net).
> >   selftests: netfilter: Set gc_thresh[123] for gw netns in
> >     nft_nat_zones.sh.
> >   neighbour: Schedule neigh_managed_work only when needed.
> >   neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
> >
> >  Documentation/netlink/specs/rt-neigh.yaml     |  2 +-
> >  Documentation/networking/ip-sysctl.rst        |  2 +-
> >  Documentation/networking/switchdev.rst        | 11 +++---
> >  net/core/neighbour.c                          | 26 ++++++++++----
> >  .../selftests/net/netfilter/nft_nat_zones.sh  | 34 +++++--------------
> >  5 files changed, 35 insertions(+), 40 deletions(-)
> >
> > --
> > 2.56.0.385.gd3acb90ef8-goog
> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
  2026-10-13  7:16   ` Ido Schimmel
@ 2026-10-13  7:27     ` Kuniyuki Iwashima
  0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-13  7:27 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Ahern, Simon Horman, Kuniyuki Iwashima, netdev

On Tue, Oct 13, 2026 at 12:16 AM Ido Schimmel <idosch@nvidia.com> wrote:
>
> On Mon, Oct 12, 2026 at 07:44:59PM +0000, Kuniyuki Iwashima wrote:
> > Previously, the lower bound of INTERVAL_PROBE_TIME_MS was capped
> > at 1s by commit ed6cd6a17896 ("net, neigh: Set lower cap for
> > neigh_managed_work rearming").
>
> Nit: INTERVAL_PROBE_TIME_MS didn't exist in ed6cd6a17896. It was only
> introduced later, in 211da42eaa45.

Indeed, I'll rephrase that with neigh_managed_work().

>
> >
> > Later, commit 211da42eaa45 ("net, neigh: introduce
> > interval_probe_time_ms for periodic probe") changed the limit
> > to 1ms.
> >
> > The limit actually did not work but was fixed with
> > netlink_range_validation by commit 6d79b223ec44 ("neighbour:
> > Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.").
> >
> > neigh_table is now per-netns, and a user could hog CPU by setting
> > INTERVAL_PROBE_TIME_MS to a small value.
> >
> > Let's floor the value at 1 second.
>
> The bot will most likely complain that this changes existing behavior,
> but:
>
> 1. The original floor was 1 second.
>
> 2. It is unlikely that anyone but syzbot is setting it below 1 second.
>
> I would explain this in the commit message to suppress such comments.

Will add them in v2.

Thank you !

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 net-next 2/4] selftests: netfilter: Set gc_thresh[123] for gw netns in nft_nat_zones.sh.
  2026-10-12 19:44 ` [PATCH v1 net-next 2/4] selftests: netfilter: Set gc_thresh[123] for gw netns in nft_nat_zones.sh Kuniyuki Iwashima
@ 2026-10-13 20:00   ` netdev-bot+sashiko
  0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13 20:00 UTC (permalink / raw)
  To: kuniyu
  Cc: davem, edumazet, kuba, pabeni, dsahern, idosch, horms, kuni1840,
	netdev

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
  2026-10-12 19:44 ` [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000 Kuniyuki Iwashima
  2026-10-13  7:16   ` Ido Schimmel
@ 2026-10-13 20:00   ` netdev-bot+sashiko
  2026-10-13 20:29     ` Kuniyuki Iwashima
  1 sibling, 1 reply; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13 20:00 UTC (permalink / raw)
  To: kuniyu
  Cc: davem, edumazet, kuba, pabeni, dsahern, idosch, horms, kuni1840,
	netdev

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

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch narrows the accepted uAPI range for
  interval_probe_time_ms in every netns, including init_net.
- [Low] The commit message describes a bug fix: a namespaced user can hog
  CPU because neigh_table is now per-netns and the interval floor was 1…
- [Low] The sysctl and netlink paths now accept different minimums for
  the same parameter on kernels with HZ < 1000.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the patch
  makes it easier to hit.

--

neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.

This patch raises the minimum of NDTPA_INTERVAL_PROBE_TIME_MS from 1 ms to
1000 ms, both in the netlink range policy and in the sysctl handler. It
also updates the YAML spec and ip-sysctl.rst to match. The reason given is
that neigh_table is now per-netns, so a small interval could be used to
hog the CPU.

Link: https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/

> neigh_table is now per-netns, and a user could hog CPU by setting
> INTERVAL_PROBE_TIME_MS to a small value.
>
> Let's floor the value at 1 second.
>
> Suggested-by: Ido Schimmel <idosch@nvidia.com>
> Link: https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

[Severity: Low]

Should this have a Fixes: tag? The message describes a CPU hogging problem
caused by the per-netns conversion, which looks like this commit:

cda2962b6e2b ("neighbour: Namespacify neigh_tables.")

That commit lets RTM_SETNEIGHTBL set the default table params
(ifindex == 0) per netns.

The message mentions 211da42eaa45 and 6d79b223ec44, but it never names
cda2962b6e2b, and there is no trailer for tooling to pick up.

cda2962b6e2b seems to be in net-next only, so stable is probably not
affected.

[ ... ]

> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 6b0569aa5d643..92e3c83c0fe50 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2438,7 +2438,7 @@ static const struct nla_policy nl_neightbl_policy[NDTA_MAX+1] = {
>  #define NTBL_PARM_MS_MAX	(24 * 60 * 60 * MSEC_PER_SEC)
>  
>  static const struct netlink_range_validation nl_ntbl_parm_ms_range = {
> -	.min = 1,
> +	.min = 1000,
>  	.max = NTBL_PARM_MS_MAX,
>  };

[Severity: Medium]

Is the intent to reject these values rather than clamp them?

With .min = 1000, the range policy now rejects NDTPA_INTERVAL_PROBE_TIME_MS
values of 1..999 with -ERANGE. This applies in every netns, including
init_net.

In the same way, min = HZ in neigh_proc_dointvec_ms_jiffies_positive()
makes proc_int_conv() return -EINVAL for sysctl writes below about one
second.

So commands like these used to work and now fail:

  ip ntable change ... interval_probe_time_ms 500
  sysctl -w net.ipv4.neigh.default.interval_probe_time_ms=500

This removes the sub-second managed probing that 211da42eaa45 added. It
does so even for CAP_NET_ADMIN in init_net.

The commit message says "Let's floor the value at 1 second.", which reads
like clamping. Could the message say that values which used to be accepted
are now rejected?

Another option would be to restore the rearm-time clamp from ed6cd6a17896.
neigh_managed_work() currently uses the configured value directly:

	if (!list_empty(&tbl->managed_list))
		queue_delayed_work(system_power_efficient_wq, &tbl->managed_work,
				   NEIGH_VAR(&tbl->parms, INTERVAL_PROBE_TIME_MS));

> @@ -3701,7 +3701,7 @@ static int neigh_proc_dointvec_ms_jiffies_positive(const struct ctl_table *ctl,
>  	struct ctl_table tmp = *ctl;
>  	int ret, min, max;
>  
> -	min = msecs_to_jiffies(1);
> +	min = HZ;
>  	max = msecs_to_jiffies(NTBL_PARM_MS_MAX);

[Severity: Low]

On kernels with HZ < 1000, does the sysctl path enforce the same minimum
as netlink?

The range check runs after msecs_to_jiffies() has already rounded the user
value up to jiffies:

proc_dointvec_ms_jiffies_minmax()
  -> do_proc_int_conv_ms_jiffies_minmax()
    -> proc_int_conv()

kernel/sysctl.c:proc_int_conv() {
    ...
		ret = user_to_kern(negp, u_ptr, &tmp_k);
		if (ret)
			return ret;
		if ((tbl->extra1 && *(int *)tbl->extra1 > tmp_k) ||
    ...
}

With HZ=100, a write of 991 becomes (991 + 9) / 10 = 100 jiffies. That
equals HZ, so it is accepted. With HZ=250, any value from 997 ms up is
accepted.

RTM_SETNEIGHTBL rejects those same values with -ERANGE. Both ip-sysctl.rst
and rt-neigh.yaml now give the minimum as 1000.

Before this patch the two paths agreed: 1 jiffy for sysctl, 1 ms for
netlink.

Would a shared constant, for example msecs_to_jiffies(NTBL_PARM_MS_MIN),
keep the two paths consistent?

[Severity: Medium]

This is a pre-existing issue and was not introduced by this patch, but the
new floor makes it easier to hit.

neigh_proc_dointvec_ms_jiffies_positive() calls neigh_proc_update() whether
or not the write was accepted:

	ret = proc_dointvec_ms_jiffies_minmax(&tmp, write, buffer, lenp, ppos);
	neigh_proc_update(ctl, write);
	return ret;

neigh_proc_update() then marks the parameter as explicitly set on any
write attempt:

	if (!write)
		return;

	set_bit(index, p->data_state);

For a per-device entry, neigh_copy_dflt_parms() then skips that device:

		if (dst_p && !test_bit(index, dst_p->data_state))
			dst_p->data[index] = p->data[index];

So consider a rejected write such as this one:

  echo 500 > /proc/sys/net/ipv4/neigh/eth0/interval_probe_time_ms

It returns -EINVAL, but eth0 also stops following later changes to the
default. Writes of 1 to 999 ms used to succeed, and now they end up on
this path.

Would it make sense to skip neigh_proc_update() when the handler returns
an error? The other neigh_proc_* handlers make the same unconditional
call.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
  2026-10-13 20:00   ` netdev-bot+sashiko
@ 2026-10-13 20:29     ` Kuniyuki Iwashima
  2026-10-13 20:30       ` Kuniyuki Iwashima
  0 siblings, 1 reply; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-13 20:29 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, edumazet, kuba, pabeni, dsahern, idosch, horms, kuni1840,
	netdev

On Tue, Oct 13, 2026 at 1:00 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 (1) · Low: 2
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The patch narrows the accepted uAPI range for
>   interval_probe_time_ms in every netns, including init_net.
> - [Low] The commit message describes a bug fix: a namespaced user can hog
>   CPU because neigh_table is now per-netns and the interval floor was 1…
> - [Low] The sysctl and netlink paths now accept different minimums for
>   the same parameter on kernels with HZ < 1000.
>
> Pre-existing issues:
> - [Medium] This problem wasn't introduced by this patch, but the patch
>   makes it easier to hit.
>
> --
>
> neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
>
> This patch raises the minimum of NDTPA_INTERVAL_PROBE_TIME_MS from 1 ms to
> 1000 ms, both in the netlink range policy and in the sysctl handler. It
> also updates the YAML spec and ip-sysctl.rst to match. The reason given is
> that neigh_table is now per-netns, so a small interval could be used to
> hog the CPU.
>
> Link: https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/
>
> > neigh_table is now per-netns, and a user could hog CPU by setting
> > INTERVAL_PROBE_TIME_MS to a small value.
> >
> > Let's floor the value at 1 second.
> >
> > Suggested-by: Ido Schimmel <idosch@nvidia.com>
> > Link: https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
>
> [Severity: Low]
>
> Should this have a Fixes: tag?

No, both the commit and this change targets to net-next of this cycle.


> The message describes a CPU hogging problem
> caused by the per-netns conversion, which looks like this commit:
>
> cda2962b6e2b ("neighbour: Namespacify neigh_tables.")
>
> That commit lets RTM_SETNEIGHTBL set the default table params
> (ifindex == 0) per netns.
>
> The message mentions 211da42eaa45 and 6d79b223ec44, but it never names
> cda2962b6e2b, and there is no trailer for tooling to pick up.
>
> cda2962b6e2b seems to be in net-next only, so stable is probably not
> affected.
>
> [ ... ]
>
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index 6b0569aa5d643..92e3c83c0fe50 100644
> > --- a/net/core/neighbour.c
> > +++ b/net/core/neighbour.c
> > @@ -2438,7 +2438,7 @@ static const struct nla_policy nl_neightbl_policy[NDTA_MAX+1] = {
> >  #define NTBL_PARM_MS_MAX     (24 * 60 * 60 * MSEC_PER_SEC)
> >
> >  static const struct netlink_range_validation nl_ntbl_parm_ms_range = {
> > -     .min = 1,
> > +     .min = 1000,
> >       .max = NTBL_PARM_MS_MAX,
> >  };
>
> [Severity: Medium]
>
> Is the intent to reject these values rather than clamp them?

Yes.

>
> With .min = 1000, the range policy now rejects NDTPA_INTERVAL_PROBE_TIME_MS
> values of 1..999 with -ERANGE. This applies in every netns, including
> init_net.
>
> In the same way, min = HZ in neigh_proc_dointvec_ms_jiffies_positive()
> makes proc_int_conv() return -EINVAL for sysctl writes below about one
> second.
>
> So commands like these used to work and now fail:
>
>   ip ntable change ... interval_probe_time_ms 500
>   sysctl -w net.ipv4.neigh.default.interval_probe_time_ms=500
>
> This removes the sub-second managed probing that 211da42eaa45 added. It
> does so even for CAP_NET_ADMIN in init_net.
>
> The commit message says "Let's floor the value at 1 second.", which reads
> like clamping. Could the message say that values which used to be accepted
> are now rejected?
>
> Another option would be to restore the rearm-time clamp from ed6cd6a17896.
> neigh_managed_work() currently uses the configured value directly:
>
>         if (!list_empty(&tbl->managed_list))
>                 queue_delayed_work(system_power_efficient_wq, &tbl->managed_work,
>                                    NEIGH_VAR(&tbl->parms, INTERVAL_PROBE_TIME_MS));
>
> > @@ -3701,7 +3701,7 @@ static int neigh_proc_dointvec_ms_jiffies_positive(const struct ctl_table *ctl,
> >       struct ctl_table tmp = *ctl;
> >       int ret, min, max;
> >
> > -     min = msecs_to_jiffies(1);
> > +     min = HZ;
> >       max = msecs_to_jiffies(NTBL_PARM_MS_MAX);
>
> [Severity: Low]
>
> On kernels with HZ < 1000, does the sysctl path enforce the same minimum
> as netlink?
>
> The range check runs after msecs_to_jiffies() has already rounded the user
> value up to jiffies:
>
> proc_dointvec_ms_jiffies_minmax()
>   -> do_proc_int_conv_ms_jiffies_minmax()
>     -> proc_int_conv()
>
> kernel/sysctl.c:proc_int_conv() {
>     ...
>                 ret = user_to_kern(negp, u_ptr, &tmp_k);
>                 if (ret)
>                         return ret;
>                 if ((tbl->extra1 && *(int *)tbl->extra1 > tmp_k) ||
>     ...
> }
>
> With HZ=100, a write of 991 becomes (991 + 9) / 10 = 100 jiffies. That
> equals HZ, so it is accepted. With HZ=250, any value from 997 ms up is
> accepted.
>
> RTM_SETNEIGHTBL rejects those same values with -ERANGE. Both ip-sysctl.rst
> and rt-neigh.yaml now give the minimum as 1000.
>
> Before this patch the two paths agreed: 1 jiffy for sysctl, 1 ms for
> netlink.
>
> Would a shared constant, for example msecs_to_jiffies(NTBL_PARM_MS_MIN),
> keep the two paths consistent?

I'll use msecs_to_jiffies(HZ).


>
> [Severity: Medium]
>
> This is a pre-existing issue and was not introduced by this patch, but the
> new floor makes it easier to hit.
>
> neigh_proc_dointvec_ms_jiffies_positive() calls neigh_proc_update() whether
> or not the write was accepted:
>
>         ret = proc_dointvec_ms_jiffies_minmax(&tmp, write, buffer, lenp, ppos);
>         neigh_proc_update(ctl, write);
>         return ret;
>
> neigh_proc_update() then marks the parameter as explicitly set on any
> write attempt:
>
>         if (!write)
>                 return;
>
>         set_bit(index, p->data_state);
>
> For a per-device entry, neigh_copy_dflt_parms() then skips that device:
>
>                 if (dst_p && !test_bit(index, dst_p->data_state))
>                         dst_p->data[index] = p->data[index];
>
> So consider a rejected write such as this one:
>
>   echo 500 > /proc/sys/net/ipv4/neigh/eth0/interval_probe_time_ms
>
> It returns -EINVAL, but eth0 also stops following later changes to the
> default. Writes of 1 to 999 ms used to succeed, and now they end up on
> this path.
>
> Would it make sense to skip neigh_proc_update() when the handler returns
> an error? The other neigh_proc_* handlers make the same unconditional
> call.
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012194530.3751034-1-kuniyu%40google.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
  2026-10-13 20:29     ` Kuniyuki Iwashima
@ 2026-10-13 20:30       ` Kuniyuki Iwashima
  0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-13 20:30 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, edumazet, kuba, pabeni, dsahern, idosch, horms, kuni1840,
	netdev

On Tue, Oct 13, 2026 at 1:29 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> On Tue, Oct 13, 2026 at 1:00 PM <netdev-bot+sashiko@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 3 potential
> > issue(s) to consider.
> >
> > Critical: 0 · High: 0 · Medium: 1 (1) · Low: 2
> >
> > If you believe none of the issues are legitimate, please reply to each
> > issue of Medium severity or higher and explain why the AI is wrong.
> > If any of them are legitimate and you plan to respin, please let patchwork
> > know by including "pw-bot: cr" as a separate line at the end of your reply
> > (one such reply per series is enough).
> >
> > - [Medium] The patch narrows the accepted uAPI range for
> >   interval_probe_time_ms in every netns, including init_net.
> > - [Low] The commit message describes a bug fix: a namespaced user can hog
> >   CPU because neigh_table is now per-netns and the interval floor was 1…
> > - [Low] The sysctl and netlink paths now accept different minimums for
> >   the same parameter on kernels with HZ < 1000.
> >
> > Pre-existing issues:
> > - [Medium] This problem wasn't introduced by this patch, but the patch
> >   makes it easier to hit.
> >
> > --
> >
> > neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000.
> >
> > This patch raises the minimum of NDTPA_INTERVAL_PROBE_TIME_MS from 1 ms to
> > 1000 ms, both in the netlink range policy and in the sysctl handler. It
> > also updates the YAML spec and ip-sysctl.rst to match. The reason given is
> > that neigh_table is now per-netns, so a small interval could be used to
> > hog the CPU.
> >
> > Link: https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/
> >
> > > neigh_table is now per-netns, and a user could hog CPU by setting
> > > INTERVAL_PROBE_TIME_MS to a small value.
> > >
> > > Let's floor the value at 1 second.
> > >
> > > Suggested-by: Ido Schimmel <idosch@nvidia.com>
> > > Link: https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/
> > > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> >
> > [Severity: Low]
> >
> > Should this have a Fixes: tag?
>
> No, both the commit and this change targets to net-next of this cycle.
>
>
> > The message describes a CPU hogging problem
> > caused by the per-netns conversion, which looks like this commit:
> >
> > cda2962b6e2b ("neighbour: Namespacify neigh_tables.")
> >
> > That commit lets RTM_SETNEIGHTBL set the default table params
> > (ifindex == 0) per netns.
> >
> > The message mentions 211da42eaa45 and 6d79b223ec44, but it never names
> > cda2962b6e2b, and there is no trailer for tooling to pick up.
> >
> > cda2962b6e2b seems to be in net-next only, so stable is probably not
> > affected.
> >
> > [ ... ]
> >
> > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > > index 6b0569aa5d643..92e3c83c0fe50 100644
> > > --- a/net/core/neighbour.c
> > > +++ b/net/core/neighbour.c
> > > @@ -2438,7 +2438,7 @@ static const struct nla_policy nl_neightbl_policy[NDTA_MAX+1] = {
> > >  #define NTBL_PARM_MS_MAX     (24 * 60 * 60 * MSEC_PER_SEC)
> > >
> > >  static const struct netlink_range_validation nl_ntbl_parm_ms_range = {
> > > -     .min = 1,
> > > +     .min = 1000,
> > >       .max = NTBL_PARM_MS_MAX,
> > >  };
> >
> > [Severity: Medium]
> >
> > Is the intent to reject these values rather than clamp them?
>
> Yes.
>
> >
> > With .min = 1000, the range policy now rejects NDTPA_INTERVAL_PROBE_TIME_MS
> > values of 1..999 with -ERANGE. This applies in every netns, including
> > init_net.
> >
> > In the same way, min = HZ in neigh_proc_dointvec_ms_jiffies_positive()
> > makes proc_int_conv() return -EINVAL for sysctl writes below about one
> > second.
> >
> > So commands like these used to work and now fail:
> >
> >   ip ntable change ... interval_probe_time_ms 500
> >   sysctl -w net.ipv4.neigh.default.interval_probe_time_ms=500
> >
> > This removes the sub-second managed probing that 211da42eaa45 added. It
> > does so even for CAP_NET_ADMIN in init_net.
> >
> > The commit message says "Let's floor the value at 1 second.", which reads
> > like clamping. Could the message say that values which used to be accepted
> > are now rejected?
> >
> > Another option would be to restore the rearm-time clamp from ed6cd6a17896.
> > neigh_managed_work() currently uses the configured value directly:
> >
> >         if (!list_empty(&tbl->managed_list))
> >                 queue_delayed_work(system_power_efficient_wq, &tbl->managed_work,
> >                                    NEIGH_VAR(&tbl->parms, INTERVAL_PROBE_TIME_MS));
> >
> > > @@ -3701,7 +3701,7 @@ static int neigh_proc_dointvec_ms_jiffies_positive(const struct ctl_table *ctl,
> > >       struct ctl_table tmp = *ctl;
> > >       int ret, min, max;
> > >
> > > -     min = msecs_to_jiffies(1);
> > > +     min = HZ;
> > >       max = msecs_to_jiffies(NTBL_PARM_MS_MAX);
> >
> > [Severity: Low]
> >
> > On kernels with HZ < 1000, does the sysctl path enforce the same minimum
> > as netlink?
> >
> > The range check runs after msecs_to_jiffies() has already rounded the user
> > value up to jiffies:
> >
> > proc_dointvec_ms_jiffies_minmax()
> >   -> do_proc_int_conv_ms_jiffies_minmax()
> >     -> proc_int_conv()
> >
> > kernel/sysctl.c:proc_int_conv() {
> >     ...
> >                 ret = user_to_kern(negp, u_ptr, &tmp_k);
> >                 if (ret)
> >                         return ret;
> >                 if ((tbl->extra1 && *(int *)tbl->extra1 > tmp_k) ||
> >     ...
> > }
> >
> > With HZ=100, a write of 991 becomes (991 + 9) / 10 = 100 jiffies. That
> > equals HZ, so it is accepted. With HZ=250, any value from 997 ms up is
> > accepted.
> >
> > RTM_SETNEIGHTBL rejects those same values with -ERANGE. Both ip-sysctl.rst
> > and rt-neigh.yaml now give the minimum as 1000.
> >
> > Before this patch the two paths agreed: 1 jiffy for sysctl, 1 ms for
> > netlink.
> >
> > Would a shared constant, for example msecs_to_jiffies(NTBL_PARM_MS_MIN),
> > keep the two paths consistent?
>
> I'll use msecs_to_jiffies(HZ).

of course s/HZ/1000/ :p


>
>
> >
> > [Severity: Medium]
> >
> > This is a pre-existing issue and was not introduced by this patch, but the
> > new floor makes it easier to hit.
> >
> > neigh_proc_dointvec_ms_jiffies_positive() calls neigh_proc_update() whether
> > or not the write was accepted:
> >
> >         ret = proc_dointvec_ms_jiffies_minmax(&tmp, write, buffer, lenp, ppos);
> >         neigh_proc_update(ctl, write);
> >         return ret;
> >
> > neigh_proc_update() then marks the parameter as explicitly set on any
> > write attempt:
> >
> >         if (!write)
> >                 return;
> >
> >         set_bit(index, p->data_state);
> >
> > For a per-device entry, neigh_copy_dflt_parms() then skips that device:
> >
> >                 if (dst_p && !test_bit(index, dst_p->data_state))
> >                         dst_p->data[index] = p->data[index];
> >
> > So consider a rejected write such as this one:
> >
> >   echo 500 > /proc/sys/net/ipv4/neigh/eth0/interval_probe_time_ms
> >
> > It returns -EINVAL, but eth0 also stops following later changes to the
> > default. Writes of 1 to 999 ms used to succeed, and now they end up on
> > this path.
> >
> > Would it make sense to skip neigh_proc_update() when the handler returns
> > an error? The other neigh_proc_* handlers make the same unconditional
> > call.
> >
> > --
> > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012194530.3751034-1-kuniyu%40google.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-10-13 20:31 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 19:44 [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Kuniyuki Iwashima
2026-10-12 19:44 ` [PATCH v1 net-next 1/4] Documentation: networking: switchdev: Replace arp_tbl with arp_table(net) Kuniyuki Iwashima
2026-10-12 19:44 ` [PATCH v1 net-next 2/4] selftests: netfilter: Set gc_thresh[123] for gw netns in nft_nat_zones.sh Kuniyuki Iwashima
2026-10-13 20:00   ` netdev-bot+sashiko
2026-10-12 19:44 ` [PATCH v1 net-next 3/4] neighbour: Schedule neigh_managed_work only when needed Kuniyuki Iwashima
2026-10-12 19:44 ` [PATCH v1 net-next 4/4] neighbour: Floor INTERVAL_PROBE_TIME_MS at 1000 Kuniyuki Iwashima
2026-10-13  7:16   ` Ido Schimmel
2026-10-13  7:27     ` Kuniyuki Iwashima
2026-10-13 20:00   ` netdev-bot+sashiko
2026-10-13 20:29     ` Kuniyuki Iwashima
2026-10-13 20:30       ` Kuniyuki Iwashima
2026-10-13  7:00 ` [PATCH v1 net-next 0/4] neighbour: Misc follow-ups on per-netns neigh_table series Ido Schimmel
2026-10-13  7:23   ` Kuniyuki Iwashima

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox