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

[Konstantin Khorenko]

On 6/15/26 16:03, Pavel Tikhomirov wrote:
> 
> 
> 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.

Yes, you are right.
But if we increment the ve cnt on exempt_from_gc path, the counter becomes ->entries, not the ->gc_entries,
so i will go another way - will inc/dec per-CT ->gc_entries in the same places where now global ->gc_entries are managed.

> 
>>  	gc_thresh3 = READ_ONCE(tbl->gc_thresh3);
>