[Devel] [PATCH vz10] ve/net/neighbour: do the provisional bump on tbl->gc_entries, not tbl->entries

[Pavel Tikhomirov]

On 6/4/26 11:39, Konstantin Khorenko wrote:
> neigh_alloc()'s provisional counter bump was changed from tbl->gc_entries
> to tbl->entries while adding the per-VE neighbour limit:
> 
> 	glob_entries = atomic_inc_return(&tbl->entries) - 1;
> 
> but the success-path "atomic_inc(&tbl->entries)" was left in place, so a
> successfully created non-exempt neighbour incremented tbl->entries TWICE
> while neigh_destroy() decrements it only ONCE -> tbl->entries leaks +1 per
> neighbour. That permanently skews the hash-grow check
> (atomic_read(&tbl->entries) > (1 << hash_shift)), the gc_thresh1
> short-circuit in neigh_periodic_work(), and trips the
> pr_crit("neighbour leakage") check in neigh_table_clear() on unload.
> 
> Symmetrically tbl->gc_entries was no longer incremented in neigh_alloc()
> at all, yet the out_entries error path still decrements it and the
> gc_list machinery (neigh_mark_dead / neigh_update_gc_list) decrements it
> on removal, so gc_entries drifts negative and neigh_forced_gc()'s
> max_clean = gc_entries - gc_thresh2 goes large-negative, neutering forced
> GC.
> 
> The upstream (and pre-patch base) neigh_alloc() does the provisional bump
> on tbl->gc_entries and the single real bump on tbl->entries on success.
> Restore that: bump gc_entries provisionally. tbl->entries is now
> incremented once (success) and decremented once (neigh_destroy); the
> per-VE counter and the gc_thresh3 limit are unchanged.
> 
> Fixes: 35b80673b07b ("ve/net/neighbour: per-ct limit for neighbour entries")
> https://virtuozzo.atlassian.net/browse/VSTOR-132310
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  net/core/neighbour.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index eb6d2eff99f1..dad5fb78bc65 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -496,10 +496,10 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
>  
>  	/* If per-VE counter of neighbour entries exist
>  	 * it will be limited by tbl->gc_thresh3
> -	 * and according global counter (tbl->entries) become unlimited.
> +	 * and the global gc counter (tbl->gc_entries) become unlimited.
>  	 */
>  
> -	glob_entries = atomic_inc_return(&tbl->entries) - 1;
> +	glob_entries = atomic_inc_return(&tbl->gc_entries) - 1;
>  	cnt = get_perve_tbl_entries_counter(tbl, ve);
>  	entries = cnt ? atomic_inc_return(cnt) - 1 : glob_entries;

Should we also still increment the ve cnt on exempt_from_gc path?
Since if we don't, we would have counter imbalance after neigh_destroy()
unconditionally decrements it.

>  	gc_thresh3 = READ_ONCE(tbl->gc_thresh3);

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