Re: [PATCH bpf-next v3 02/10] rhashtable: Add rhashtable_walk_enter_from()

bot+bpf-ci
2026-04-24T20:45:44Z

[-- Attachment #1: Type: text/plain, Size: 4942 bytes --]

> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -693,6 +693,47 @@ void rhashtable_walk_enter(struct rhashtable *ht, struct rhashtable_iter *iter)
> EXPORT_SYMBOL_GPL(rhashtable_walk_enter);
>
> /**
> + * rhashtable_walk_enter_from - Initialise a walk starting at a key's bucket
> + * @ht: Table to walk over
> + * @iter: Hash table iterator
> + * @key: Key whose bucket to start from
> + * @params: Hash table parameters
> + *
> + * Like rhashtable_walk_enter(), but positions the iterator at the bucket
> + * containing @key. If @key is not present in the main table, the iterator is
> + * positioned at the beginning.
> + *
> + * Same constraints as rhashtable_walk_enter() apply.
> + */
> +void rhashtable_walk_enter_from(struct rhashtable *ht,
> + struct rhashtable_iter *iter,
> + const void *key,
> + const struct rhashtable_params params)
> + __must_hold(RCU)
> +{
> + struct bucket_table *tbl;
> + struct rhash_head *he;
> +
> + rhashtable_walk_enter(ht, iter);
> +
> + if (!key)
> + return;
> +
> + tbl = iter->walker.tbl;
> + if (!tbl)
> + return;
> +
> + he = __rhashtable_lookup_one(ht, tbl, key, params,
> + RHT_LOOKUP_NORMAL);
> + if (!he)
> + return;

What happens if the table is currently resizing and the key has already
been moved to the future table?

When __rhashtable_lookup_one() fails to find the key in the current
table, the function returns early, leaving the iterator positioned at
bucket 0.

For stateless iterators like BPF's get_next_key, could this cause an
infinite loop where it constantly resets to the beginning of the old
table until the resize completes?

The v2 implementation had code to check future_tbl when the key was not
found in the main table, but this was removed in v3.

Reference: https://lore.kernel.org/all/20260424201535.EA1BCC19425@smtp.kernel.org/

> +
> + iter->slot = rht_key_hashfn(ht, tbl, key, params);
> + iter->p = he;

Does this leave iter->list uninitialized for rhlist tables?

rhashtable_walk_enter() does not initialize iter->list. When
rhashtable_walk_start_check() is later called for an rhlist table, it
validates the element using:

lib/rhashtable.c:rhashtable_walk_start_check() {
...
} else if (iter->p && rhlist) {
...
if (list == iter->list) {
iter->p = p;
iter->skip = skip;
goto found;
}
...
}

Will this uninitialized read of iter->list cause the validation to fail
and silently fall back to the beginning of the bucket?

Reference: https://lore.kernel.org/all/20260424201535.EA1BCC19425@smtp.kernel.org/

> +}
> +EXPORT_SYMBOL_GPL(rhashtable_walk_enter_from);

[ ... ]

> diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
> --- a/lib/test_rhashtable.c
> +++ b/lib/test_rhashtable.c

[ ... ]

> @@ -679,6 +680,133 @@
> return err;
> }
>
> +static int __init test_walk_enter_from(void)
> +{
> + struct rhashtable ht;
> + struct test_obj objs[4];
> + struct rhashtable_iter iter;
> + struct test_obj *obj;
> + struct test_obj_val key = { };
> + int err, i, count;
> + int len_freq[4] = { 0, 0, 0, 0 };
> +
> + err = rhashtable_init(&ht, &test_rht_params);
> + if (err)
> + return err;
> +
> + /* Insert 4 elements with keys 0, 2, 4, 6 */
> + for (i = 0; i < 4; i++) {
> + objs[i].value.id = i * 2;
> + objs[i].value.tid = 0;
> + err = rhashtable_insert_fast(&ht, &objs[i].node,
> + test_rht_params);
> + if (err) {
> + pr_warn("walk_enter_from: insert %d failed: %d\n",
> + i, err);
> + goto out;
> + }
> + }

[ ... ]

> + /* Test 3: verify walk_enter_from + walk_next can iterate remaining elements */
> + for (i = 0; i < 4; i++) {
> + key.id = i * 2;
> + count = 0;
> +
> + scoped_guard(rcu) {
> + rhashtable_walk_enter_from(&ht, &iter, &key, test_rht_params);
> + rhashtable_walk_start(&iter);
> + }
> +
> + /* Count the number of elements after this one */
> + do {
> + obj = rhashtable_walk_next(&iter);
> + count++;
> + } while (obj && (!IS_ERR(obj) || (PTR_ERR(obj) == -EAGAIN)));
> +
> + rhashtable_walk_stop(&iter);
> + rhashtable_walk_exit(&iter);
> + if (count > 4) {
> + pr_warn("walk_enter_from: walked %d elements instead of max 4\n", count);
> + goto out;

Does this error path need to set the err variable?

Right now it jumps to out and returns 0, which might cause the test
suite to silently miss the failure. The err variable should be set to
-EINVAL before the goto.

Reference: https://lore.kernel.org/all/20260424201535.EA1BCC19425@smtp.kernel.org/

> + }
> + /*
> + * Collect the lengths of every walked path,
> + * we should have each length walked one time
> + */
> + len_freq[count - 1]++;
> + }

[ ... ]

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24909591959