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 <[email protected]>
> ---
> 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.
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
