Re: [PATCH bpf v2 1/2] bpf: bound resizable hash map iteration

[Hui Su <sh_def@163.com>]

On Thu, Sep 3, 2026 at 5:43 PM, Mykyta Yatsenko wrote:
> I'm not sure if this change fixes anything, the main issue of walking
> concurrently modified rhashtable is not changed: you still may miss
> elements or visit same elements multiple times.
>
> In some scenarios this can make things worse: imagine you start
> iterating with small map (visit_budget = 10), then 1000000 elements
> are inserted concurrently with walk, so you'll miss at least
> 1000000 - 10.
>
> To me this is a trade off/taste thing, rather than bug fix. The
> change is compact, though, I'm not against it.

Thanks for the review.

I agree that this change does not, and is not intended to, make
concurrent rhashtable iteration complete or duplicate-free. Missed and
duplicate elements remain part of the existing best-effort semantics.

The narrower issue I am trying to address is that bpf_each_rhash_elem()
currently keeps calling rhashtable_next_key() until it returns NULL.
The rhashtable_next_key() documentation states that a full iteration
may not terminate under adversarial or sustained rehashing, and
recommends that callers bound such walks externally.

The purpose of this change is therefore not to make the iteration
complete, but to provide a finite upper bound on callback invocations
under sustained rehashing.

I agree that taking a snapshot of ht.nelems makes the bound insensitive
to elements inserted after the walk starts. In the example you gave, the
walk could stop after the initial occupancy and miss most newly inserted
elements.

Using map->max_entries would preserve more of the existing behavior
under concurrent growth, at the cost of allowing a much longer walk for
a sparsely populated map.

Would map->max_entries be a more appropriate bound in your view?

Also, if you consider adding such a bound useful but not a bug fix,
would bpf-next be a more appropriate target for this change?

Thanks,
Hui