[Devel] [PATCH vz10 1/2] ve/net/neighbour: keep the per-VE counter in sync with tbl->gc_entries

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

On 6/15/26 17:54, Konstantin Khorenko wrote:
> The per-VE neighbour counter (ve->arp_neigh_nr / nd_neigh_nr, selected by
> get_perve_tbl_entries_counter()) was incremented only on the gc-eligible
> path of neigh_alloc() - exempt_from_gc neighbours skip it - but
> neigh_destroy() decremented it unconditionally for every neighbour. So
> each exempt_from_gc neighbour (loopback device, NUD_PERMANENT or
> NTF_EXT_LEARNED) decremented the per-VE counter without a matching
> increment, driving it negative over time. Once it is negative the per-CT
> limit check "entries >= gc_thresh3" in neigh_alloc() never trips and the
> per-container neighbour limit silently stops working.
> 
> Make the per-VE counter a true per-VE mirror of tbl->gc_entries: change
> it at exactly the same sites where gc_entries changes (it already did in
> neigh_alloc()'s provisional path and in the out_entries error path):
>  - neigh_update_gc_list(): inc/dec it together with gc_entries when the
>    neighbour joins/leaves the gc list on a state change;
>  - neigh_mark_dead(): dec it when the neighbour leaves the gc list;
>  - ___neigh_create() out_neigh_release: dec it (it previously undid only
>    gc_entries, leaking the per-VE counter on that error path);
> and drop the unconditional decrement from neigh_destroy(). The per-VE
> counter is now incremented and decremented exactly for gc-eligible
> neighbours, like gc_entries; exempt_from_gc neighbours never touch it.
> 
> get_perve_tbl_entries_counter() is moved above neigh_mark_dead() so the
> gc-list helpers can use it.
> 
> Fixes: 5f0a2a6f78f7 ("ve/net/neighbour: per-ct limit for neighbour entries")
> https://virtuozzo.atlassian.net/browse/VSTOR-132310
> Feature: net: make the neighbor entries limit per-CT
> Reported-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  net/core/neighbour.c | 47 ++++++++++++++++++++++++++++----------------
>  1 file changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index dad5fb78bc65..cf1f11a596d8 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -116,12 +116,29 @@ unsigned long neigh_rand_reach_time(unsigned long base)
>  }
>  EXPORT_SYMBOL(neigh_rand_reach_time);
>  
> +static inline atomic_t *get_perve_tbl_entries_counter(struct neigh_table *tbl,
> +							struct ve_struct *ve)
> +{
> +	switch (tbl->family) {
> +	case AF_INET:
> +		return &ve->arp_neigh_nr;
> +	case AF_INET6:
> +		return &ve->nd_neigh_nr;
> +	}
> +	return NULL;
> +}
> +
>  static void neigh_mark_dead(struct neighbour *n)
>  {
>  	n->dead = 1;
>  	if (!list_empty(&n->gc_list)) {
> +		atomic_t *cnt = get_perve_tbl_entries_counter(n->tbl,
> +						dev_net(n->dev)->owner_ve);
> +
>  		list_del_init(&n->gc_list);
>  		atomic_dec(&n->tbl->gc_entries);
> +		if (cnt)
> +			atomic_dec(cnt);
>  	}
>  	if (!list_empty(&n->managed_list))
>  		list_del_init(&n->managed_list);
> @@ -129,6 +146,8 @@ static void neigh_mark_dead(struct neighbour *n)
>  
>  static void neigh_update_gc_list(struct neighbour *n)
>  {
> +	atomic_t *cnt = get_perve_tbl_entries_counter(n->tbl,
> +					dev_net(n->dev)->owner_ve);
>  	bool on_gc_list, exempt_from_gc;
>  
>  	write_lock_bh(&n->tbl->lock);
> @@ -146,10 +165,14 @@ static void neigh_update_gc_list(struct neighbour *n)
>  	if (exempt_from_gc && on_gc_list) {
>  		list_del_init(&n->gc_list);
>  		atomic_dec(&n->tbl->gc_entries);
> +		if (cnt)
> +			atomic_dec(cnt);
>  	} else if (!exempt_from_gc && !on_gc_list) {
>  		/* add entries to the tail; cleaning removes from the front */
>  		list_add_tail(&n->gc_list, &n->tbl->gc_list);
>  		atomic_inc(&n->tbl->gc_entries);
> +		if (cnt)
> +			atomic_inc(cnt);
>  	}
>  out:
>  	write_unlock(&n->lock);
> @@ -469,18 +492,6 @@ int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
>  }
>  EXPORT_SYMBOL(neigh_ifdown);
>  
> -static inline atomic_t *get_perve_tbl_entries_counter(struct neigh_table *tbl,
> -							struct ve_struct *ve)
> -{
> -	switch (tbl->family) {
> -	case AF_INET:
> -		return &ve->arp_neigh_nr;
> -	case AF_INET6:
> -		return &ve->nd_neigh_nr;
> -	}
> -	return NULL;
> -}
> -
>  static struct neighbour *neigh_alloc(struct neigh_table *tbl,
>  				     struct net_device *dev,
>  				     u32 flags, bool exempt_from_gc)
> @@ -761,8 +772,14 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
>  out_tbl_unlock:
>  	write_unlock_bh(&tbl->lock);
>  out_neigh_release:
> -	if (!exempt_from_gc)
> +	if (!exempt_from_gc) {
> +		atomic_t *cnt = get_perve_tbl_entries_counter(tbl,
> +						dev_net(dev)->owner_ve);
> +
>  		atomic_dec(&tbl->gc_entries);
> +		if (cnt)
> +			atomic_dec(cnt);
> +	}
>  	neigh_release(n);
>  	goto out;
>  }
> @@ -927,8 +944,6 @@ static inline void neigh_parms_put(struct neigh_parms *parms)
>  void neigh_destroy(struct neighbour *neigh)
>  {
>  	struct net_device *dev = neigh->dev;
> -	struct ve_struct *ve = dev_net(dev)->owner_ve;
> -	atomic_t *cnt = get_perve_tbl_entries_counter(neigh->tbl, ve);
>  
>  	NEIGH_CACHE_STAT_INC(neigh->tbl, destroys);
>  
> @@ -954,8 +969,6 @@ void neigh_destroy(struct neighbour *neigh)
>  
>  	neigh_dbg(2, "neigh %p is destroyed\n", neigh);
>  
> -	if (cnt)
> -		 atomic_dec(cnt);
>  	atomic_dec(&neigh->tbl->entries);
>  	kfree_rcu(neigh, rcu);
>  }

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.