* neigh: poor scalability of forced GC when neighbour count exceeds gc_thresh3
@ 2026-06-18 8:17 Vimal Agrawal
2026-06-25 10:20 ` [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full Vimal Agrawal
0 siblings, 1 reply; 11+ messages in thread
From: Vimal Agrawal @ 2026-06-18 8:17 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Jakub Kicinski, Vimal Agrawal
While investigating a soft lockup observed during neighbour table
growth, I noticed that neighbour allocation latency increases
significantly once the number of entries exceeds gc_thresh3.
Test setup:
net.ipv6.neigh.default.gc_thresh1 = 16384
net.ipv6.neigh.default.gc_thresh2 = 32768
net.ipv6.neigh.default.gc_thresh3 = 32768
I created approximately 50,000 reachable neighbour entries and
measured time spent in __neigh_create(). Once the table size exceeds
gc_thresh3, neighbour creation latency increases dramatically (in my
testing, individual allocations can take >16 ms). Profiling shows that
most of the time is spent waiting on tbl->lock, typically held by
neigh_forced_gc().
The relevant path is:
static int neigh_forced_gc(struct neigh_table *tbl)
{
...
write_lock_bh(&tbl->lock);
list_for_each_entry_safe(n, tmp, &tbl->gc_list, gc_list) {
if (refcount_read(&n->refcnt) == 1) {
...
In my workload, most entries are active/reachable and have refcnt > 1,
so the GC walk scans a large portion of the neighbour table without
reclaiming entries. As a result, the lock can be held for a long
period while traversing the GC list.
Another observation is that once gc_thresh3 is exceeded, every new
neighbour allocation attempts a forced GC:
entries = atomic_inc_return(&tbl->gc_entries) - 1;
if (entries >= gc_thresh3 ||
(entries >= READ_ONCE(tbl->gc_thresh2) &&
time_after(now, READ_ONCE(tbl->last_flush) + 5 * HZ))) {
if (!neigh_forced_gc(tbl) && entries >= gc_thresh3) {
...
Unlike the gc_thresh2 case, there is no rate limiting once the table
is already above gc_thresh3. Under sustained neighbour creation this
results in repeated full GC scans, further increasing contention on
tbl->lock.
Has this scalability issue been discussed previously, or is there a
reason why forced GC above gc_thresh3 is intentionally not
rate-limited?
I would be interested in feedback before working on a patch.
Thanks,
Vimal Agrawal
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-18 8:17 neigh: poor scalability of forced GC when neighbour count exceeds gc_thresh3 Vimal Agrawal @ 2026-06-25 10:20 ` Vimal Agrawal 2026-06-25 15:42 ` Jakub Kicinski 2026-06-25 21:45 ` [PATCH " Kuniyuki Iwashima 0 siblings, 2 replies; 11+ messages in thread From: Vimal Agrawal @ 2026-06-25 10:20 UTC (permalink / raw) To: netdev; +Cc: kuniyu, edumazet, vimal.agrawal, avimalin Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called on every allocation attempt with no rate limiting. In workloads with mostly active/reachable entries, the GC walk traverses a large portion of the neighbour table without reclaiming entries, holding tbl->lock for an extended period. This causes severe lock contention and allocation latencies exceeding 16ms under sustained neighbour creation. Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was performed within the last second, avoiding repeated full table scans and lock acquisitions on the hot allocation path. Profiling of neigh_create() shows ~3 orders of magnitude latency improvement with this change. Link:https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> --- net/core/neighbour.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 1349c0eedb64..078842db3c5f 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) int shrunk = 0; int loop = 0; + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + HZ)) + return 0; + NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); spin_lock_bh(&tbl->lock); -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-25 10:20 ` [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full Vimal Agrawal @ 2026-06-25 15:42 ` Jakub Kicinski 2026-07-06 6:58 ` [PATCH v2 " Vimal Agrawal 2026-06-25 21:45 ` [PATCH " Kuniyuki Iwashima 1 sibling, 1 reply; 11+ messages in thread From: Jakub Kicinski @ 2026-06-25 15:42 UTC (permalink / raw) To: Vimal Agrawal; +Cc: netdev, kuniyu, edumazet, vimal.agrawal On Thu, 25 Jun 2026 10:20:20 +0000 Vimal Agrawal wrote: > Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called > on every allocation attempt with no rate limiting. In workloads with mostly > active/reachable entries, the GC walk traverses a large portion of the > neighbour table without reclaiming entries, holding tbl->lock for an > extended period. This causes severe lock contention and allocation > latencies exceeding 16ms under sustained neighbour creation. > > Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was > performed within the last second, avoiding repeated full table scans and > lock acquisitions on the hot allocation path. > > Profiling of neigh_create() shows ~3 orders of magnitude latency > improvement with this change. I'm not an expert on neigh but 1 second seems a little aggressive. Can you see if 10msec doesn't give us a similar win? > net/core/neighbour.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > index 1349c0eedb64..078842db3c5f 100644 > --- a/net/core/neighbour.c > +++ b/net/core/neighbour.c > @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) > int shrunk = 0; > int loop = 0; > > + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + HZ)) > + return 0; > + > NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); > > spin_lock_bh(&tbl->lock); ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-25 15:42 ` Jakub Kicinski @ 2026-07-06 6:58 ` Vimal Agrawal 2026-07-06 14:19 ` Paolo Abeni 0 siblings, 1 reply; 11+ messages in thread From: Vimal Agrawal @ 2026-07-06 6:58 UTC (permalink / raw) To: netdev; +Cc: kuba, kuniyu, edumazet, vimal.agrawal, avimalin Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called on every allocation attempt with no rate limiting. In workloads with mostly active/reachable entries, the GC walk traverses a large portion of the neighbour table without reclaiming entries, holding tbl->lock for an extended period. This causes severe lock contention and allocation latencies exceeding 16ms under sustained neighbour creation. Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was performed within the last 50 ms, avoiding repeated full table scans and lock acquisitions on the hot allocation path. Profiling of neigh_create() shows ~3 orders of magnitude latency improvement with this change. Link: https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> --- v2: Changed threshold from 1s (HZ) to 50ms (msecs_to_jiffies(50)) based on profiling data showing 44% -> 2.56% CPU reduction net/core/neighbour.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 1349c0eedb64..a83535d32da3 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) int shrunk = 0; int loop = 0; + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + msecs_to_jiffies(50))) + return 0; + NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); spin_lock_bh(&tbl->lock); -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-07-06 6:58 ` [PATCH v2 " Vimal Agrawal @ 2026-07-06 14:19 ` Paolo Abeni 0 siblings, 0 replies; 11+ messages in thread From: Paolo Abeni @ 2026-07-06 14:19 UTC (permalink / raw) To: Vimal Agrawal, netdev; +Cc: kuba, kuniyu, edumazet, vimal.agrawal On 7/6/26 8:58 AM, Vimal Agrawal wrote: > Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called > on every allocation attempt with no rate limiting. In workloads with mostly > active/reachable entries, the GC walk traverses a large portion of the > neighbour table without reclaiming entries, holding tbl->lock for an > extended period. This causes severe lock contention and allocation > latencies exceeding 16ms under sustained neighbour creation. > > Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was > performed within the last 50 ms, avoiding repeated full table scans and > lock acquisitions on the hot allocation path. > > Profiling of neigh_create() shows ~3 orders of magnitude latency > improvement with this change. > > Link: https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ > Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> This apparently breaks neigh self-tests: # 29.38 [+5.12] TEST: IPv4 "extern_valid" flag: Forced garbage collection [FAIL] //... # 85.91 [+5.11] TEST: IPv6 "extern_valid" flag: Forced garbage collection [FAIL] full log at: https://netdev-ctrl.bots.linux.dev/logs/vmksft/net/results/722145/13-test-neigh-sh/ /P ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-25 10:20 ` [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full Vimal Agrawal 2026-06-25 15:42 ` Jakub Kicinski @ 2026-06-25 21:45 ` Kuniyuki Iwashima 2026-06-29 7:57 ` Vimal Agrawal 1 sibling, 1 reply; 11+ messages in thread From: Kuniyuki Iwashima @ 2026-06-25 21:45 UTC (permalink / raw) To: avimalin; +Cc: edumazet, kuniyu, netdev, vimal.agrawal, kuba From: Vimal Agrawal <avimalin@gmail.com> Date: Thu, 25 Jun 2026 10:20:20 +0000 > Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called > on every allocation attempt with no rate limiting. In workloads with mostly > active/reachable entries, the GC walk traverses a large portion of the > neighbour table without reclaiming entries, holding tbl->lock for an > extended period. This causes severe lock contention and allocation > latencies exceeding 16ms under sustained neighbour creation. > > Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was > performed within the last second, avoiding repeated full table scans and > lock acquisitions on the hot allocation path. > > Profiling of neigh_create() shows ~3 orders of magnitude latency > improvement with this change. > > Link:https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ From the thread, these look misconfigured. ---8<--- net.ipv6.neigh.default.gc_thresh2 = 32768 net.ipv6.neigh.default.gc_thresh3 = 32768 ---8<--- If gc_thresh3 is larger enough, gc_thresh2 will give you 5s rate limiting. If the number of active neigh entries constantly exceeds gc_thresh3, it will be the correct gc_thresh2 for you. Also, I guess you want a new kernel param for the first neigh_hash_alloc(), which is currently fixed for 3, which is too small for some hosts. 50000 entries require neigh_hash_grow() 13 times. Can you test this on your real workload, starting from neigh_hash_shift=16 and appropriate gc_thresh2/3 ? ---8<--- diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 1349c0eedb64..a75b3750eec9 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -1817,6 +1817,22 @@ EXPORT_SYMBOL(neigh_parms_release); static struct lock_class_key neigh_table_proxy_queue_class; static struct neigh_table __rcu *neigh_tables[NEIGH_NR_TABLES] __read_mostly; +static __initdata unsigned long neigh_hash_shift = 3; + +static int __init neigh_set_hash_shift(char *str) +{ + ssize_t ret; + + if (!str) + return 0; + + ret = kstrtoul(str, 0, &neigh_hash_shift); + if (ret) + return 0; + + return 1; +} +__setup("neigh_hash_shift=", neigh_set_hash_shift); void neigh_table_init(int index, struct neigh_table *tbl) { @@ -1843,7 +1859,7 @@ void neigh_table_init(int index, struct neigh_table *tbl) panic("cannot create neighbour proc dir entry"); #endif - RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(3)); + RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(neigh_hash_shift)); phsize = (PNEIGH_HASHMASK + 1) * sizeof(struct pneigh_entry *); tbl->phash_buckets = kzalloc(phsize, GFP_KERNEL); ---8<--- > Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> > --- > net/core/neighbour.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > index 1349c0eedb64..078842db3c5f 100644 > --- a/net/core/neighbour.c > +++ b/net/core/neighbour.c > @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) > int shrunk = 0; > int loop = 0; > > + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + HZ)) > + return 0; > + > NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); > > spin_lock_bh(&tbl->lock); > -- > 2.17.1 > v ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-25 21:45 ` [PATCH " Kuniyuki Iwashima @ 2026-06-29 7:57 ` Vimal Agrawal 2026-06-29 18:05 ` Kuniyuki Iwashima 0 siblings, 1 reply; 11+ messages in thread From: Vimal Agrawal @ 2026-06-29 7:57 UTC (permalink / raw) To: Kuniyuki Iwashima, kuba; +Cc: edumazet, netdev, vimal.agrawal Hi Kuniyuki, Thank you for the feedback. However, the rate limiting issue exists independently of the threshold values. If entries genuinely exceed gc_thresh3 — regardless of what it is set to — neigh_forced_gc() is called on every allocation attempt with no rate limiting. In my workload, most entries are active/reachable with refcnt > 1, so the GC walk traverses the entire table without reclaiming anything. Increasing gc_thresh3 would make this worse, not better, as GC now has a larger table to scan on each call. Regarding neigh_hash_shift: in my workload, neigh_alloc() returns ENOBUFS before reaching do_alloc() since GC cannot reclaim any entries. kzalloc() is never called, so neigh_hash_grow() is not involved in the latency I observed. The pre-lock time check in neigh_forced_gc() is a low-cost safeguard that prevents repeated full table scans regardless of gc_thresh3 value. It does not interfere with correct GC behaviour — if entries are still above the threshold, GC runs normally. Hi Jakub, I tested with different threshold values, filling the table completely with 32k reachable entries and attempting 1000 additional allocations. Exported neigh_forced_gc so that it can be profiled no change 10ms 50ms 100ms max cpu usage % 44% 11.8% 2.56% 1.42% calls > 100us (of 1000) 101 31 13 7 At 10ms, max CPU usage is still 11.8% and 31 out of 1000 calls take more than 100us. Given that 50ms reduces this to 2.56% and 13 calls respectively, I would prefer 50ms as the threshold. However, I am open to further discussion on the right value. Thanks, Vimal On Fri, Jun 26, 2026 at 3:17 AM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > From: Vimal Agrawal <avimalin@gmail.com> > Date: Thu, 25 Jun 2026 10:20:20 +0000 > > Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called > > on every allocation attempt with no rate limiting. In workloads with mostly > > active/reachable entries, the GC walk traverses a large portion of the > > neighbour table without reclaiming entries, holding tbl->lock for an > > extended period. This causes severe lock contention and allocation > > latencies exceeding 16ms under sustained neighbour creation. > > > > Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was > > performed within the last second, avoiding repeated full table scans and > > lock acquisitions on the hot allocation path. > > > > Profiling of neigh_create() shows ~3 orders of magnitude latency > > improvement with this change. > > > > Link:https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ > > From the thread, these look misconfigured. > > ---8<--- > net.ipv6.neigh.default.gc_thresh2 = 32768 > net.ipv6.neigh.default.gc_thresh3 = 32768 > ---8<--- > > If gc_thresh3 is larger enough, gc_thresh2 will give you 5s > rate limiting. > > If the number of active neigh entries constantly exceeds > gc_thresh3, it will be the correct gc_thresh2 for you. > > Also, I guess you want a new kernel param for the first > neigh_hash_alloc(), which is currently fixed for 3, which > is too small for some hosts. > > 50000 entries require neigh_hash_grow() 13 times. > > Can you test this on your real workload, starting from > neigh_hash_shift=16 and appropriate gc_thresh2/3 ? > > ---8<--- > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > index 1349c0eedb64..a75b3750eec9 100644 > --- a/net/core/neighbour.c > +++ b/net/core/neighbour.c > @@ -1817,6 +1817,22 @@ EXPORT_SYMBOL(neigh_parms_release); > static struct lock_class_key neigh_table_proxy_queue_class; > > static struct neigh_table __rcu *neigh_tables[NEIGH_NR_TABLES] __read_mostly; > +static __initdata unsigned long neigh_hash_shift = 3; > + > +static int __init neigh_set_hash_shift(char *str) > +{ > + ssize_t ret; > + > + if (!str) > + return 0; > + > + ret = kstrtoul(str, 0, &neigh_hash_shift); > + if (ret) > + return 0; > + > + return 1; > +} > +__setup("neigh_hash_shift=", neigh_set_hash_shift); > > void neigh_table_init(int index, struct neigh_table *tbl) > { > @@ -1843,7 +1859,7 @@ void neigh_table_init(int index, struct neigh_table *tbl) > panic("cannot create neighbour proc dir entry"); > #endif > > - RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(3)); > + RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(neigh_hash_shift)); > > phsize = (PNEIGH_HASHMASK + 1) * sizeof(struct pneigh_entry *); > tbl->phash_buckets = kzalloc(phsize, GFP_KERNEL); > ---8<--- > > > > > Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> > > --- > > net/core/neighbour.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > index 1349c0eedb64..078842db3c5f 100644 > > --- a/net/core/neighbour.c > > +++ b/net/core/neighbour.c > > @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) > > int shrunk = 0; > > int loop = 0; > > > > + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + HZ)) > > + return 0; > > + > > NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); > > > > spin_lock_bh(&tbl->lock); > > -- > > 2.17.1 > > v ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-29 7:57 ` Vimal Agrawal @ 2026-06-29 18:05 ` Kuniyuki Iwashima 2026-06-30 12:01 ` Vimal Agrawal 0 siblings, 1 reply; 11+ messages in thread From: Kuniyuki Iwashima @ 2026-06-29 18:05 UTC (permalink / raw) To: Vimal Agrawal; +Cc: kuba, edumazet, netdev, vimal.agrawal On Mon, Jun 29, 2026 at 12:57 AM Vimal Agrawal <avimalin@gmail.com> wrote: > > Hi Kuniyuki, > Thank you for the feedback. > However, the rate limiting issue exists independently of the threshold > values. If entries genuinely exceed gc_thresh3 — regardless of what it > is set to — neigh_forced_gc() is called on every allocation attempt > with no rate limiting. In my workload, most entries are > active/reachable with refcnt > 1, so the GC walk traverses the entire > table without reclaiming anything. This suggests your gc_thresh2/3 do not fit your use case. If GC does not help, there is no point in running it or rate-limiting in the first place. > Increasing gc_thresh3 would make > this worse, not better, as GC now has a larger table to scan on each > call. If you just increase gc_thresh3 slightly, then yes, it won't help. > > Regarding neigh_hash_shift: in my workload, neigh_alloc() returns > ENOBUFS before reaching do_alloc() since GC cannot reclaim any > entries. kzalloc() is never called, so neigh_hash_grow() is not > involved in the latency I observed. The pre-lock time check in > neigh_forced_gc() is a low-cost safeguard that prevents repeated full > table scans regardless of gc_thresh3 value. It does not interfere with > correct GC behaviour — if entries are still above the threshold, GC > runs normally. > > > Hi Jakub, > I tested with different threshold values, filling the table completely > with 32k reachable entries and attempting 1000 additional allocations. > Exported neigh_forced_gc so that it can be profiled > no change 10ms 50ms 100ms > max cpu usage % 44% 11.8% 2.56% 1.42% > calls > 100us (of 1000) 101 31 13 7 > > At 10ms, max CPU usage is still 11.8% and 31 out of 1000 calls take > more than 100us. Given that 50ms reduces this to 2.56% and 13 calls > respectively, I would prefer 50ms as the threshold. However, I am open > to further discussion on the right value. > > Thanks, > Vimal > > > On Fri, Jun 26, 2026 at 3:17 AM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > > > From: Vimal Agrawal <avimalin@gmail.com> > > Date: Thu, 25 Jun 2026 10:20:20 +0000 > > > Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called > > > on every allocation attempt with no rate limiting. In workloads with mostly > > > active/reachable entries, the GC walk traverses a large portion of the > > > neighbour table without reclaiming entries, holding tbl->lock for an > > > extended period. This causes severe lock contention and allocation > > > latencies exceeding 16ms under sustained neighbour creation. > > > > > > Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was > > > performed within the last second, avoiding repeated full table scans and > > > lock acquisitions on the hot allocation path. > > > > > > Profiling of neigh_create() shows ~3 orders of magnitude latency > > > improvement with this change. > > > > > > Link:https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ > > > > From the thread, these look misconfigured. > > > > ---8<--- > > net.ipv6.neigh.default.gc_thresh2 = 32768 > > net.ipv6.neigh.default.gc_thresh3 = 32768 > > ---8<--- > > > > If gc_thresh3 is larger enough, gc_thresh2 will give you 5s > > rate limiting. > > > > If the number of active neigh entries constantly exceeds > > gc_thresh3, it will be the correct gc_thresh2 for you. > > > > Also, I guess you want a new kernel param for the first > > neigh_hash_alloc(), which is currently fixed for 3, which > > is too small for some hosts. > > > > 50000 entries require neigh_hash_grow() 13 times. > > > > Can you test this on your real workload, starting from > > neigh_hash_shift=16 and appropriate gc_thresh2/3 ? > > > > ---8<--- > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > index 1349c0eedb64..a75b3750eec9 100644 > > --- a/net/core/neighbour.c > > +++ b/net/core/neighbour.c > > @@ -1817,6 +1817,22 @@ EXPORT_SYMBOL(neigh_parms_release); > > static struct lock_class_key neigh_table_proxy_queue_class; > > > > static struct neigh_table __rcu *neigh_tables[NEIGH_NR_TABLES] __read_mostly; > > +static __initdata unsigned long neigh_hash_shift = 3; > > + > > +static int __init neigh_set_hash_shift(char *str) > > +{ > > + ssize_t ret; > > + > > + if (!str) > > + return 0; > > + > > + ret = kstrtoul(str, 0, &neigh_hash_shift); > > + if (ret) > > + return 0; > > + > > + return 1; > > +} > > +__setup("neigh_hash_shift=", neigh_set_hash_shift); > > > > void neigh_table_init(int index, struct neigh_table *tbl) > > { > > @@ -1843,7 +1859,7 @@ void neigh_table_init(int index, struct neigh_table *tbl) > > panic("cannot create neighbour proc dir entry"); > > #endif > > > > - RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(3)); > > + RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(neigh_hash_shift)); > > > > phsize = (PNEIGH_HASHMASK + 1) * sizeof(struct pneigh_entry *); > > tbl->phash_buckets = kzalloc(phsize, GFP_KERNEL); > > ---8<--- > > > > > > > > > Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> > > > --- > > > net/core/neighbour.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > > index 1349c0eedb64..078842db3c5f 100644 > > > --- a/net/core/neighbour.c > > > +++ b/net/core/neighbour.c > > > @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) > > > int shrunk = 0; > > > int loop = 0; > > > > > > + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + HZ)) > > > + return 0; > > > + > > > NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); > > > > > > spin_lock_bh(&tbl->lock); > > > -- > > > 2.17.1 > > > v ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-29 18:05 ` Kuniyuki Iwashima @ 2026-06-30 12:01 ` Vimal Agrawal 2026-06-30 16:36 ` Kuniyuki Iwashima 0 siblings, 1 reply; 11+ messages in thread From: Vimal Agrawal @ 2026-06-30 12:01 UTC (permalink / raw) To: Kuniyuki Iwashima; +Cc: kuba, edumazet, netdev, vimal.agrawal Hi Kuniyuki, You are correct that in this specific test case GC does not help since all entries are active/reachable. However, this is not the only scenario where entries can exceed gc_thresh3. In a real workload, the table can exceed gc_thresh3 with a mix of active and stale entries. In that case GC does help, but should not be called on every allocation attempt — once per 50ms is sufficient for GC to make progress without causing lock contention. The rate limiting also protects against the case where GC cannot reclaim anything. Without it, every allocation attempt above gc_thresh3 triggers a full table scan holding tbl->lock, even when GC has no work to do. Thanks, Vimal On Mon, Jun 29, 2026 at 11:35 PM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > On Mon, Jun 29, 2026 at 12:57 AM Vimal Agrawal <avimalin@gmail.com> wrote: > > > > Hi Kuniyuki, > > Thank you for the feedback. > > However, the rate limiting issue exists independently of the threshold > > values. If entries genuinely exceed gc_thresh3 — regardless of what it > > is set to — neigh_forced_gc() is called on every allocation attempt > > with no rate limiting. In my workload, most entries are > > active/reachable with refcnt > 1, so the GC walk traverses the entire > > table without reclaiming anything. > > This suggests your gc_thresh2/3 do not fit your use case. > > If GC does not help, there is no point in running it or rate-limiting > in the first place. > > > > Increasing gc_thresh3 would make > > this worse, not better, as GC now has a larger table to scan on each > > call. > > If you just increase gc_thresh3 slightly, then yes, it won't help. > > > > > > Regarding neigh_hash_shift: in my workload, neigh_alloc() returns > > ENOBUFS before reaching do_alloc() since GC cannot reclaim any > > entries. kzalloc() is never called, so neigh_hash_grow() is not > > involved in the latency I observed. The pre-lock time check in > > neigh_forced_gc() is a low-cost safeguard that prevents repeated full > > table scans regardless of gc_thresh3 value. It does not interfere with > > correct GC behaviour — if entries are still above the threshold, GC > > runs normally. > > > > > > Hi Jakub, > > I tested with different threshold values, filling the table completely > > with 32k reachable entries and attempting 1000 additional allocations. > > Exported neigh_forced_gc so that it can be profiled > > no change 10ms 50ms 100ms > > max cpu usage % 44% 11.8% 2.56% 1.42% > > calls > 100us (of 1000) 101 31 13 7 > > > > At 10ms, max CPU usage is still 11.8% and 31 out of 1000 calls take > > more than 100us. Given that 50ms reduces this to 2.56% and 13 calls > > respectively, I would prefer 50ms as the threshold. However, I am open > > to further discussion on the right value. > > > > Thanks, > > Vimal > > > > > > On Fri, Jun 26, 2026 at 3:17 AM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > > > > > From: Vimal Agrawal <avimalin@gmail.com> > > > Date: Thu, 25 Jun 2026 10:20:20 +0000 > > > > Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called > > > > on every allocation attempt with no rate limiting. In workloads with mostly > > > > active/reachable entries, the GC walk traverses a large portion of the > > > > neighbour table without reclaiming entries, holding tbl->lock for an > > > > extended period. This causes severe lock contention and allocation > > > > latencies exceeding 16ms under sustained neighbour creation. > > > > > > > > Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was > > > > performed within the last second, avoiding repeated full table scans and > > > > lock acquisitions on the hot allocation path. > > > > > > > > Profiling of neigh_create() shows ~3 orders of magnitude latency > > > > improvement with this change. > > > > > > > > Link:https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ > > > > > > From the thread, these look misconfigured. > > > > > > ---8<--- > > > net.ipv6.neigh.default.gc_thresh2 = 32768 > > > net.ipv6.neigh.default.gc_thresh3 = 32768 > > > ---8<--- > > > > > > If gc_thresh3 is larger enough, gc_thresh2 will give you 5s > > > rate limiting. > > > > > > If the number of active neigh entries constantly exceeds > > > gc_thresh3, it will be the correct gc_thresh2 for you. > > > > > > Also, I guess you want a new kernel param for the first > > > neigh_hash_alloc(), which is currently fixed for 3, which > > > is too small for some hosts. > > > > > > 50000 entries require neigh_hash_grow() 13 times. > > > > > > Can you test this on your real workload, starting from > > > neigh_hash_shift=16 and appropriate gc_thresh2/3 ? > > > > > > ---8<--- > > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > > index 1349c0eedb64..a75b3750eec9 100644 > > > --- a/net/core/neighbour.c > > > +++ b/net/core/neighbour.c > > > @@ -1817,6 +1817,22 @@ EXPORT_SYMBOL(neigh_parms_release); > > > static struct lock_class_key neigh_table_proxy_queue_class; > > > > > > static struct neigh_table __rcu *neigh_tables[NEIGH_NR_TABLES] __read_mostly; > > > +static __initdata unsigned long neigh_hash_shift = 3; > > > + > > > +static int __init neigh_set_hash_shift(char *str) > > > +{ > > > + ssize_t ret; > > > + > > > + if (!str) > > > + return 0; > > > + > > > + ret = kstrtoul(str, 0, &neigh_hash_shift); > > > + if (ret) > > > + return 0; > > > + > > > + return 1; > > > +} > > > +__setup("neigh_hash_shift=", neigh_set_hash_shift); > > > > > > void neigh_table_init(int index, struct neigh_table *tbl) > > > { > > > @@ -1843,7 +1859,7 @@ void neigh_table_init(int index, struct neigh_table *tbl) > > > panic("cannot create neighbour proc dir entry"); > > > #endif > > > > > > - RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(3)); > > > + RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(neigh_hash_shift)); > > > > > > phsize = (PNEIGH_HASHMASK + 1) * sizeof(struct pneigh_entry *); > > > tbl->phash_buckets = kzalloc(phsize, GFP_KERNEL); > > > ---8<--- > > > > > > > > > > > > > Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> > > > > --- > > > > net/core/neighbour.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > > > index 1349c0eedb64..078842db3c5f 100644 > > > > --- a/net/core/neighbour.c > > > > +++ b/net/core/neighbour.c > > > > @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) > > > > int shrunk = 0; > > > > int loop = 0; > > > > > > > > + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + HZ)) > > > > + return 0; > > > > + > > > > NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); > > > > > > > > spin_lock_bh(&tbl->lock); > > > > -- > > > > 2.17.1 > > > > v ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-30 12:01 ` Vimal Agrawal @ 2026-06-30 16:36 ` Kuniyuki Iwashima 2026-07-01 8:30 ` Vimal Agrawal 0 siblings, 1 reply; 11+ messages in thread From: Kuniyuki Iwashima @ 2026-06-30 16:36 UTC (permalink / raw) To: Vimal Agrawal; +Cc: kuba, edumazet, netdev, vimal.agrawal On Tue, Jun 30, 2026 at 5:01 AM Vimal Agrawal <avimalin@gmail.com> wrote: > > Hi Kuniyuki, > > You are correct that in this specific test case GC does not help since > all entries are active/reachable. However, this is not the only > scenario where entries can exceed gc_thresh3. > > In a real workload, the table can exceed gc_thresh3 with a mix of > active and stale entries. In that case GC does help, but should not be > called on every allocation attempt — once per 50ms is sufficient for > GC to make progress without causing lock contention. My mental model is that gc_thresh3 is the hard limit while gc_thresh2 is the soft limit, so if the total number of entries often exceeds gc_thresh3, it's clearly wrong. I think you need to set gc_thresh2 to a proper baseline (it sounds like your current gc_thresh3 is the one) and gc_thresh3 to gc_thresh2+X where X covers fluctuations. > > The rate limiting also protects against the case where GC cannot > reclaim anything. Without it, every allocation attempt above > gc_thresh3 triggers a full table scan holding tbl->lock, even when GC > has no work to do. > > Thanks, > Vimal > > On Mon, Jun 29, 2026 at 11:35 PM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > > > On Mon, Jun 29, 2026 at 12:57 AM Vimal Agrawal <avimalin@gmail.com> wrote: > > > > > > Hi Kuniyuki, > > > Thank you for the feedback. > > > However, the rate limiting issue exists independently of the threshold > > > values. If entries genuinely exceed gc_thresh3 — regardless of what it > > > is set to — neigh_forced_gc() is called on every allocation attempt > > > with no rate limiting. In my workload, most entries are > > > active/reachable with refcnt > 1, so the GC walk traverses the entire > > > table without reclaiming anything. > > > > This suggests your gc_thresh2/3 do not fit your use case. > > > > If GC does not help, there is no point in running it or rate-limiting > > in the first place. > > > > > > > Increasing gc_thresh3 would make > > > this worse, not better, as GC now has a larger table to scan on each > > > call. > > > > If you just increase gc_thresh3 slightly, then yes, it won't help. > > > > > > > > > > Regarding neigh_hash_shift: in my workload, neigh_alloc() returns > > > ENOBUFS before reaching do_alloc() since GC cannot reclaim any > > > entries. kzalloc() is never called, so neigh_hash_grow() is not > > > involved in the latency I observed. The pre-lock time check in > > > neigh_forced_gc() is a low-cost safeguard that prevents repeated full > > > table scans regardless of gc_thresh3 value. It does not interfere with > > > correct GC behaviour — if entries are still above the threshold, GC > > > runs normally. > > > > > > > > > Hi Jakub, > > > I tested with different threshold values, filling the table completely > > > with 32k reachable entries and attempting 1000 additional allocations. > > > Exported neigh_forced_gc so that it can be profiled > > > no change 10ms 50ms 100ms > > > max cpu usage % 44% 11.8% 2.56% 1.42% > > > calls > 100us (of 1000) 101 31 13 7 > > > > > > At 10ms, max CPU usage is still 11.8% and 31 out of 1000 calls take > > > more than 100us. Given that 50ms reduces this to 2.56% and 13 calls > > > respectively, I would prefer 50ms as the threshold. However, I am open > > > to further discussion on the right value. > > > > > > Thanks, > > > Vimal > > > > > > > > > On Fri, Jun 26, 2026 at 3:17 AM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > > > > > > > From: Vimal Agrawal <avimalin@gmail.com> > > > > Date: Thu, 25 Jun 2026 10:20:20 +0000 > > > > > Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called > > > > > on every allocation attempt with no rate limiting. In workloads with mostly > > > > > active/reachable entries, the GC walk traverses a large portion of the > > > > > neighbour table without reclaiming entries, holding tbl->lock for an > > > > > extended period. This causes severe lock contention and allocation > > > > > latencies exceeding 16ms under sustained neighbour creation. > > > > > > > > > > Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was > > > > > performed within the last second, avoiding repeated full table scans and > > > > > lock acquisitions on the hot allocation path. > > > > > > > > > > Profiling of neigh_create() shows ~3 orders of magnitude latency > > > > > improvement with this change. > > > > > > > > > > Link:https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ > > > > > > > > From the thread, these look misconfigured. > > > > > > > > ---8<--- > > > > net.ipv6.neigh.default.gc_thresh2 = 32768 > > > > net.ipv6.neigh.default.gc_thresh3 = 32768 > > > > ---8<--- > > > > > > > > If gc_thresh3 is larger enough, gc_thresh2 will give you 5s > > > > rate limiting. > > > > > > > > If the number of active neigh entries constantly exceeds > > > > gc_thresh3, it will be the correct gc_thresh2 for you. > > > > > > > > Also, I guess you want a new kernel param for the first > > > > neigh_hash_alloc(), which is currently fixed for 3, which > > > > is too small for some hosts. > > > > > > > > 50000 entries require neigh_hash_grow() 13 times. > > > > > > > > Can you test this on your real workload, starting from > > > > neigh_hash_shift=16 and appropriate gc_thresh2/3 ? > > > > > > > > ---8<--- > > > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > > > index 1349c0eedb64..a75b3750eec9 100644 > > > > --- a/net/core/neighbour.c > > > > +++ b/net/core/neighbour.c > > > > @@ -1817,6 +1817,22 @@ EXPORT_SYMBOL(neigh_parms_release); > > > > static struct lock_class_key neigh_table_proxy_queue_class; > > > > > > > > static struct neigh_table __rcu *neigh_tables[NEIGH_NR_TABLES] __read_mostly; > > > > +static __initdata unsigned long neigh_hash_shift = 3; > > > > + > > > > +static int __init neigh_set_hash_shift(char *str) > > > > +{ > > > > + ssize_t ret; > > > > + > > > > + if (!str) > > > > + return 0; > > > > + > > > > + ret = kstrtoul(str, 0, &neigh_hash_shift); > > > > + if (ret) > > > > + return 0; > > > > + > > > > + return 1; > > > > +} > > > > +__setup("neigh_hash_shift=", neigh_set_hash_shift); > > > > > > > > void neigh_table_init(int index, struct neigh_table *tbl) > > > > { > > > > @@ -1843,7 +1859,7 @@ void neigh_table_init(int index, struct neigh_table *tbl) > > > > panic("cannot create neighbour proc dir entry"); > > > > #endif > > > > > > > > - RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(3)); > > > > + RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(neigh_hash_shift)); > > > > > > > > phsize = (PNEIGH_HASHMASK + 1) * sizeof(struct pneigh_entry *); > > > > tbl->phash_buckets = kzalloc(phsize, GFP_KERNEL); > > > > ---8<--- > > > > > > > > > > > > > > > > > Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> > > > > > --- > > > > > net/core/neighbour.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > > > > index 1349c0eedb64..078842db3c5f 100644 > > > > > --- a/net/core/neighbour.c > > > > > +++ b/net/core/neighbour.c > > > > > @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) > > > > > int shrunk = 0; > > > > > int loop = 0; > > > > > > > > > > + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + HZ)) > > > > > + return 0; > > > > > + > > > > > NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); > > > > > > > > > > spin_lock_bh(&tbl->lock); > > > > > -- > > > > > 2.17.1 > > > > > v ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full 2026-06-30 16:36 ` Kuniyuki Iwashima @ 2026-07-01 8:30 ` Vimal Agrawal 0 siblings, 0 replies; 11+ messages in thread From: Vimal Agrawal @ 2026-07-01 8:30 UTC (permalink / raw) To: Kuniyuki Iwashima; +Cc: kuba, edumazet, netdev, vimal.agrawal Hi Kuniyuki, I understand the recommendation to set gc_thresh3 = gc_thresh2 + headroom. However, in field deployments the neighbour table size can be unpredictable — traffic patterns, routing changes, and network events can cause transient spikes that exceed even a well-configured gc_thresh3. We have reproduced this with 32k IXIA clients where we observed soft lockups under sustained neighbour creation. This is what originally motivated the investigation. The current behaviour when gc_thresh3 is exceeded — calling neigh_forced_gc() on every allocation with no rate limiting — is unnecessarily severe. It causes lock contention proportional to the table size on every allocation attempt, even briefly exceeding gc_thresh3 results in significant latency impact. The patch does not change GC semantics or thresholds. It simply prevents repeated full table scans within a short window, which is harmless when GC is effective and protective when it is not. This seems like a reasonable defensive improvement regardless of how thresholds are configured. Thanks, Vimal On Tue, Jun 30, 2026 at 10:06 PM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > On Tue, Jun 30, 2026 at 5:01 AM Vimal Agrawal <avimalin@gmail.com> wrote: > > > > Hi Kuniyuki, > > > > You are correct that in this specific test case GC does not help since > > all entries are active/reachable. However, this is not the only > > scenario where entries can exceed gc_thresh3. > > > > In a real workload, the table can exceed gc_thresh3 with a mix of > > active and stale entries. In that case GC does help, but should not be > > called on every allocation attempt — once per 50ms is sufficient for > > GC to make progress without causing lock contention. > > My mental model is that gc_thresh3 is the hard limit while gc_thresh2 > is the soft limit, so if the total number of entries often exceeds gc_thresh3, > it's clearly wrong. > > I think you need to set gc_thresh2 to a proper baseline (it sounds like > your current gc_thresh3 is the one) and gc_thresh3 to gc_thresh2+X > where X covers fluctuations. > > > > > > The rate limiting also protects against the case where GC cannot > > reclaim anything. Without it, every allocation attempt above > > gc_thresh3 triggers a full table scan holding tbl->lock, even when GC > > has no work to do. > > > > Thanks, > > Vimal > > > > On Mon, Jun 29, 2026 at 11:35 PM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > > > > > On Mon, Jun 29, 2026 at 12:57 AM Vimal Agrawal <avimalin@gmail.com> wrote: > > > > > > > > Hi Kuniyuki, > > > > Thank you for the feedback. > > > > However, the rate limiting issue exists independently of the threshold > > > > values. If entries genuinely exceed gc_thresh3 — regardless of what it > > > > is set to — neigh_forced_gc() is called on every allocation attempt > > > > with no rate limiting. In my workload, most entries are > > > > active/reachable with refcnt > 1, so the GC walk traverses the entire > > > > table without reclaiming anything. > > > > > > This suggests your gc_thresh2/3 do not fit your use case. > > > > > > If GC does not help, there is no point in running it or rate-limiting > > > in the first place. > > > > > > > > > > Increasing gc_thresh3 would make > > > > this worse, not better, as GC now has a larger table to scan on each > > > > call. > > > > > > If you just increase gc_thresh3 slightly, then yes, it won't help. > > > > > > > > > > > > > > Regarding neigh_hash_shift: in my workload, neigh_alloc() returns > > > > ENOBUFS before reaching do_alloc() since GC cannot reclaim any > > > > entries. kzalloc() is never called, so neigh_hash_grow() is not > > > > involved in the latency I observed. The pre-lock time check in > > > > neigh_forced_gc() is a low-cost safeguard that prevents repeated full > > > > table scans regardless of gc_thresh3 value. It does not interfere with > > > > correct GC behaviour — if entries are still above the threshold, GC > > > > runs normally. > > > > > > > > > > > > Hi Jakub, > > > > I tested with different threshold values, filling the table completely > > > > with 32k reachable entries and attempting 1000 additional allocations. > > > > Exported neigh_forced_gc so that it can be profiled > > > > no change 10ms 50ms 100ms > > > > max cpu usage % 44% 11.8% 2.56% 1.42% > > > > calls > 100us (of 1000) 101 31 13 7 > > > > > > > > At 10ms, max CPU usage is still 11.8% and 31 out of 1000 calls take > > > > more than 100us. Given that 50ms reduces this to 2.56% and 13 calls > > > > respectively, I would prefer 50ms as the threshold. However, I am open > > > > to further discussion on the right value. > > > > > > > > Thanks, > > > > Vimal > > > > > > > > > > > > On Fri, Jun 26, 2026 at 3:17 AM Kuniyuki Iwashima <kuniyu@google.com> wrote: > > > > > > > > > > From: Vimal Agrawal <avimalin@gmail.com> > > > > > Date: Thu, 25 Jun 2026 10:20:20 +0000 > > > > > > Once the neighbour table exceeds gc_thresh3, neigh_forced_gc() is called > > > > > > on every allocation attempt with no rate limiting. In workloads with mostly > > > > > > active/reachable entries, the GC walk traverses a large portion of the > > > > > > neighbour table without reclaiming entries, holding tbl->lock for an > > > > > > extended period. This causes severe lock contention and allocation > > > > > > latencies exceeding 16ms under sustained neighbour creation. > > > > > > > > > > > > Add a pre-lock check in neigh_forced_gc() to skip the GC run if one was > > > > > > performed within the last second, avoiding repeated full table scans and > > > > > > lock acquisitions on the hot allocation path. > > > > > > > > > > > > Profiling of neigh_create() shows ~3 orders of magnitude latency > > > > > > improvement with this change. > > > > > > > > > > > > Link:https://lore.kernel.org/netdev/CALkUMdSCpx_ywYCx_ePLdm6yioO1nQWx7sSM=AEgsq0kywHxTw@mail.gmail.com/ > > > > > > > > > > From the thread, these look misconfigured. > > > > > > > > > > ---8<--- > > > > > net.ipv6.neigh.default.gc_thresh2 = 32768 > > > > > net.ipv6.neigh.default.gc_thresh3 = 32768 > > > > > ---8<--- > > > > > > > > > > If gc_thresh3 is larger enough, gc_thresh2 will give you 5s > > > > > rate limiting. > > > > > > > > > > If the number of active neigh entries constantly exceeds > > > > > gc_thresh3, it will be the correct gc_thresh2 for you. > > > > > > > > > > Also, I guess you want a new kernel param for the first > > > > > neigh_hash_alloc(), which is currently fixed for 3, which > > > > > is too small for some hosts. > > > > > > > > > > 50000 entries require neigh_hash_grow() 13 times. > > > > > > > > > > Can you test this on your real workload, starting from > > > > > neigh_hash_shift=16 and appropriate gc_thresh2/3 ? > > > > > > > > > > ---8<--- > > > > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > > > > index 1349c0eedb64..a75b3750eec9 100644 > > > > > --- a/net/core/neighbour.c > > > > > +++ b/net/core/neighbour.c > > > > > @@ -1817,6 +1817,22 @@ EXPORT_SYMBOL(neigh_parms_release); > > > > > static struct lock_class_key neigh_table_proxy_queue_class; > > > > > > > > > > static struct neigh_table __rcu *neigh_tables[NEIGH_NR_TABLES] __read_mostly; > > > > > +static __initdata unsigned long neigh_hash_shift = 3; > > > > > + > > > > > +static int __init neigh_set_hash_shift(char *str) > > > > > +{ > > > > > + ssize_t ret; > > > > > + > > > > > + if (!str) > > > > > + return 0; > > > > > + > > > > > + ret = kstrtoul(str, 0, &neigh_hash_shift); > > > > > + if (ret) > > > > > + return 0; > > > > > + > > > > > + return 1; > > > > > +} > > > > > +__setup("neigh_hash_shift=", neigh_set_hash_shift); > > > > > > > > > > void neigh_table_init(int index, struct neigh_table *tbl) > > > > > { > > > > > @@ -1843,7 +1859,7 @@ void neigh_table_init(int index, struct neigh_table *tbl) > > > > > panic("cannot create neighbour proc dir entry"); > > > > > #endif > > > > > > > > > > - RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(3)); > > > > > + RCU_INIT_POINTER(tbl->nht, neigh_hash_alloc(neigh_hash_shift)); > > > > > > > > > > phsize = (PNEIGH_HASHMASK + 1) * sizeof(struct pneigh_entry *); > > > > > tbl->phash_buckets = kzalloc(phsize, GFP_KERNEL); > > > > > ---8<--- > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Vimal Agrawal <vimal.agrawal@sophos.com> > > > > > > --- > > > > > > net/core/neighbour.c | 3 +++ > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > > > > > > index 1349c0eedb64..078842db3c5f 100644 > > > > > > --- a/net/core/neighbour.c > > > > > > +++ b/net/core/neighbour.c > > > > > > @@ -260,6 +260,9 @@ static int neigh_forced_gc(struct neigh_table *tbl) > > > > > > int shrunk = 0; > > > > > > int loop = 0; > > > > > > > > > > > > + if (!time_after(jiffies, READ_ONCE(tbl->last_flush) + HZ)) > > > > > > + return 0; > > > > > > + > > > > > > NEIGH_CACHE_STAT_INC(tbl, forced_gc_runs); > > > > > > > > > > > > spin_lock_bh(&tbl->lock); > > > > > > -- > > > > > > 2.17.1 > > > > > > v ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-07-06 14:19 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-18 8:17 neigh: poor scalability of forced GC when neighbour count exceeds gc_thresh3 Vimal Agrawal 2026-06-25 10:20 ` [PATCH net-next] net: neigh: avoid calling neigh_forced_gc on every alloc when table is full Vimal Agrawal 2026-06-25 15:42 ` Jakub Kicinski 2026-07-06 6:58 ` [PATCH v2 " Vimal Agrawal 2026-07-06 14:19 ` Paolo Abeni 2026-06-25 21:45 ` [PATCH " Kuniyuki Iwashima 2026-06-29 7:57 ` Vimal Agrawal 2026-06-29 18:05 ` Kuniyuki Iwashima 2026-06-30 12:01 ` Vimal Agrawal 2026-06-30 16:36 ` Kuniyuki Iwashima 2026-07-01 8:30 ` Vimal Agrawal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox