Netdev List
 help / color / mirror / Atom feed
* 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

* 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

* [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

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