B4/rhash

[mykyta5]

No description provided.

[kernel-patches-review-bot]

commit 980718cb1a37440e4463f18ce386501112cd82dc
Author: Herbert Xu <herbert@gondor.apana.org.au>

rhashtable: Use irq work for shrinking

This converts the automatic-shrinking path of __rhashtable_remove_fast_one()
from schedule_work(&ht->run_work) to irq_work_queue(&ht->run_irq_work) so
that shrinking can be triggered from NMI or irq-disabled context.

>     rhashtable: Use irq work for shrinking
>
>     Use irq work for automatic shrinking so that this may be called in NMI
>     context.
>
>     Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>     Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>

This looks like a fix for a context that was made irq-safe earlier.

The schedule_work() call in the shrink path is not safe from the
irq-disabled regions introduced when rhashtable's bucket locks were
switched from local_bh_disable()/local_bh_enable() to
local_irq_save()/local_irq_restore().

Should this include:

  Fixes: e47877c7aa82 ("rhashtable: Allow rhashtable to be used from irq-safe contexts")

---

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

In-Reply-To-Subject: rhashtable: Use irq work for shrinking
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27011687425

[kernel-patches-review-bot]

commit 09d8943141dea1b5df41d508f0e877750ae1aa63
Author: Mykyta Yatsenko <yatsenko@meta.com>

rhashtable: Add rhashtable_next_key() API

This adds rhashtable_next_key(), a best-effort iteration helper that lets
a caller advance from an arbitrary position by passing the previously
returned key, without the per-iterator state of the rhashtable_walk_*
API. The caller holds RCU and the walk follows the tbl->future_tbl chain
so in-flight rehashes are observed.

> diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
> index ef5230cece36..6f3aea498515 100644
> --- a/include/linux/rhashtable.h
> +++ b/include/linux/rhashtable.h
> @@ -650,6 +650,46 @@ static __always_inline struct rhash_head *__rhashtable_lookup(

[ ... ]

> + *   - Behavior on tables that contain duplicate keys is undefined:
> + *     duplicates may be skipped, repeated, or trap the walk in a
> + *     cycle. Callers requiring duplicate-key iteration must use
> + *     rhashtable_walk_*() instead.

[ ... ]

> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index 04b3a808fca9..dd6eaa09c55d 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -687,6 +687,75 @@ void *rhashtable_insert_slow(struct rhashtable *ht, const void *key,
> +static struct rhash_head *__rhashtable_next_in_table(
> +	struct rhashtable *ht, struct bucket_table *tbl,
> +	const void *prev_key)
> +{
> +	struct rhashtable_compare_arg arg = { .ht = ht, .key = prev_key };
> +	const struct rhashtable_params params = ht->p;
> +	struct rhash_head *he;
> +	unsigned int b = 0;
> +	bool found = false;
> +
> +	if (prev_key) {
> +		b = rht_key_hashfn(ht, tbl, prev_key, params);
> +		rht_for_each_rcu(he, tbl, b) {
> +			bool match = params.obj_cmpfn
> +				     ? !params.obj_cmpfn(&arg, rht_obj(ht, he))
> +				     : !rhashtable_compare(&arg, rht_obj(ht, he));
> +			if (found) {
> +				if (match)
> +					continue;
> +				return he;
> +			}
> +			if (match)
> +				found = true;
> +		}

Can this walk fail to make forward progress and trap an RCU-read-locked
caller in a cycle when duplicate keys are interleaved with a colliding
key in the same bucket?

This was raised as a concern in v6 and does not appear to have a code
change in response. Consider a bucket chain like:

	A2 -> B -> A1

where A1 and A2 share key A and B collides into the same bucket. With
prev_key=A, the walk matches the head A2 (found=true), then returns B.
With prev_key=B, it returns A1. With prev_key=A again, it matches the
head A2 and returns B, so the iterator oscillates between B and A
without advancing.

The duplicate-skip branch:

	if (found) {
		if (match)
			continue;
		return he;
	}

only collapses adjacent matches in the chain, so it does not cover the
interleaved A2 -> B -> A1 ordering.

The same chain can also arise from a concurrent delete and insert of key
A producing A_new -> B -> A_old, which would trap an iterator holding the
RCU read lock.

The kdoc in rhashtable.h now documents duplicate-key behavior as
undefined:

> + *   - Behavior on tables that contain duplicate keys is undefined:
> + *     duplicates may be skipped, repeated, or trap the walk in a
> + *     cycle. Callers requiring duplicate-key iteration must use
> + *     rhashtable_walk_*() instead.

and the commit message notes BPF codepaths cannot produce duplicate
keys. The code-level cycle is still reachable for any caller on a table
that does contain duplicate keys. Was a code-level guard considered for
the interleaved case, or is the documented "undefined" contract intended
to be the only mitigation?

This was discussed upstream:
https://lore.kernel.org/bpf/20260602205716.BCDBE1F00893@smtp.kernel.org/

---

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

In-Reply-To-Subject: rhashtable: Add rhashtable_next_key() API
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27011687425

[kernel-patches-review-bot]

commit 2bb32731d59830059ed6a2bd730eeb36574cbd3c
Author: Mykyta Yatsenko <yatsenko@meta.com>

bpf: Implement iteration ops for resizable hashtab

This commit implements get_next_key, batch lookup/lookup-and-delete,
for_each_map_elem callback, and the seq_file BPF iterator for
BPF_MAP_TYPE_RHASH.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 10f3a058747b..a149713d0953 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c

[ ... ]

> @@ -3020,8 +3020,79 @@ static void rhtab_map_free_internal_structs(struct bpf_map *map)
>  }
>
> +static long bpf_each_rhash_elem(struct bpf_map *map, bpf_callback_t callback_fn,
> +				void *callback_ctx, u64 flags)
> +{
> +	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
> +	void *prev_key = NULL;
> +	struct rhtab_elem *elem;
> +	int num_elems = 0;
> +	u64 ret = 0;
> +
> +	cant_migrate();
> +
> +	if (flags != 0)
> +		return -EINVAL;
> +
> +	rcu_read_lock();
> +	/*
> +	 * Best-effort iteration: if rhashtable is concurrently resized or
> +	 * elements are deleted/inserted, there may be missed or duplicate
> +	 * elements visited.
> +	 */
> +	while ((elem = rhashtable_next_key(&rhtab->ht, prev_key))) {
> +		if (IS_ERR(elem))
> +			break;

The rhashtable_next_key documentation notes that a full iteration is not
guaranteed to terminate under adversarial or sustained rehashing. Since
this loop lacks an external bound and runs under rcu_read_lock with
migration disabled, can continuous concurrent resizes cause an infinite
loop, resulting in a soft lockup or RCU stall?

If prev_key is deleted concurrently or by the callback itself,
rhashtable_next_key returns ERR_PTR(-ENOENT). Does breaking the loop here
cause the iteration to immediately abort and skip the rest of the map?

> +		num_elems++;
> +		ret = callback_fn((u64)(long)map,
> +				  (u64)(long)elem->data,
> +				  (u64)(long)rhtab_elem_value(elem, map->key_size),
> +				  (u64)(long)callback_ctx, 0);
> +		if (ret)
> +			break;
> +
> +		prev_key = elem->data;	/* valid while RCU held */
> +	}
> +	rcu_read_unlock();
> +
> +	return num_elems;
> +}

[ ... ]

> @@ -3034,6 +3105,275 @@ static u64 rhtab_map_mem_usage(const struct bpf_map *map)
>  	return sizeof(struct bpf_rhtab) + rhtab->elem_size * num_entries;
>  }
>
> +static int __rhtab_map_lookup_and_delete_batch(struct bpf_map *map,
> +					       const union bpf_attr *attr,
> +					       union bpf_attr __user *uattr,
> +					       bool do_delete)
> +{

[ ... ]

> +	rcu_read_lock();
> +
> +	/*
> +	 * Cursor stores the key of the next-to-process element (stashed by
> +	 * the previous batch). Look it up directly so the element is included
> +	 * here rather than skipped by next_key(). If the cursor was deleted
> +	 * concurrently (or by the previous do_delete batch), return -EAGAIN
> +	 * so userspace can distinguish a lost cursor from end-of-iteration
> +	 * (-ENOENT) and restart from a NULL cursor.
> +	 */
> +	if (ubatch) {
> +		elem = rhtab_lookup_elem(map, cursor);
> +		if (!elem) {
> +			rcu_read_unlock();
> +			ret = -EAGAIN;
> +			goto free;
> +		}
> +	} else {
> +		elem = rhashtable_next_key(&rhtab->ht, NULL);
> +	}
> +
> +	while (elem && !IS_ERR(elem) && total < max_count) {
> +		memcpy(dst_key, elem->data, key_size);
> +		rhtab_read_elem_value(map, dst_val, elem, elem_map_flags);
> +		check_and_init_map_value(map, dst_val);
> +
> +		if (do_delete)
> +			del_elems[total] = elem;
> +
> +		elem = rhashtable_next_key(&rhtab->ht, dst_key);
> +		dst_key += key_size;
> +		dst_val += value_size;
> +		total++;
> +
> +		/* Bail to userspace to avoid stalls. */
> +		if (need_resched())
> +			break;
> +	}

Similar to the callback iteration, if a concurrent deletion occurs and
elem receives -ENOENT, this loop breaks. If total is greater than 0 when
this happens, doesn't this falsely signal to userspace that the map is
fully iterated, leading to missed elements?

> +
> +	if (elem && !IS_ERR(elem)) {
> +		/* Stash next-to-process key as cursor for the next batch. */
> +		memcpy(cursor, elem->data, key_size);
> +		has_next_cursor = true;
> +	}
> +
> +	if (do_delete) {
> +		for (i = 0; i < total; i++)
> +			rhtab_delete_elem(rhtab, del_elems[i], NULL, 0);
> +	}
> +
> +	rcu_read_unlock();

Deferring the deletion until after the iteration leaves elements visible
in the hash table while they are being copied into user buffers. Does
this create a race window where concurrent batch operations can traverse,
copy, and yield the same elements to userspace, breaking the exactly-once
consumption semantics expected for work queues?

> +
> +	if (total == 0) {
> +		ret = -ENOENT;
> +		goto free;
> +	}
> +
> +	/* No more elements after this batch. */
> +	if (!has_next_cursor)
> +		ret = -ENOENT;
> +
> +	if (copy_to_user(ukeys, keys, (size_t)total * key_size) ||
> +	    copy_to_user(uvalues, values, (size_t)total * value_size) ||
> +	    put_user(total, &uattr->batch.count) ||
> +	    (has_next_cursor &&
> +	     copy_to_user(u64_to_user_ptr(attr->batch.out_batch),
> +			  cursor, key_size))) {
> +		ret = -EFAULT;
> +		goto free;
> +	}
> +
> +free:
> +	kfree(cursor);
> +	kvfree(keys);
> +	kvfree(values);
> +	kvfree(del_elems);
> +	return ret;
> +}

[ ... ]

These concerns were raised by sashiko-bot@kernel.org in v6 review:
https://lore.kernel.org/bpf/20260602210416.2E0631F00893@smtp.kernel.org/

---

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

In-Reply-To-Subject: bpf: Implement iteration ops for resizable hashtab
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27011687425

[kernel-patches-review-bot]

commit 19dff21d2fed6076d1ca196fa711f085ceafe756
Author: Mykyta Yatsenko <yatsenko@meta.com>

bpf: Optimize word-sized keys for resizable hashtable

Specialize the lookup/update/delete paths for keys whose size matches
sizeof(long) (4 bytes on 32-bit, 8 bytes on 64-bit). A static-const
rhashtable_params lets the compiler inline a custom XOR-fold hashfn and
a single-word equality cmpfn, eliminating the indirect jhash dispatch.
The same hashfn and cmpfn are installed into rhashtable's stored params
at rhashtable_init time, so the rehash worker, slow-path inserts, and
rhashtable_next_key() all agree with the inlined fast paths.

The seq_file BPF iterator uses rhashtable_walk_* and is unaffected.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 7b9408b8320c..b4366cad3cfa 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -2763,6 +2763,31 @@ static inline void *rhtab_elem_value(struct rhtab_elem *l, u32 key_size)
>  	return l->data + round_up(key_size, 8);
>  }
>
> +/* Specialize hash function and objcmp for long sized key */
> +static __always_inline int rhtab_key_cmp_long(struct rhashtable_compare_arg *arg,
> +					      const void *ptr)
> +{
> +	const unsigned long key1 = *(const unsigned long *)arg->key;
                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Could dereferencing this pointer directly cause alignment faults on
architectures that require strict alignment? Since BPF programs can pass
unaligned pointers such as from packet data in skb->data directly to map
helpers, using get_unaligned() might be necessary here to prevent kernel
panics on ARM32, SPARC, and other strict-alignment architectures.

This concern was raised by sashiko-bot@kernel.org in v5 and v6:
https://lore.kernel.org/bpf/20260528182522.23D121F00A3C@smtp.kernel.org/

The author acknowledged in v5 that "the same problem exists for the htab
jhash2" but did not address it in subsequent versions. The issue remains
in this version.

> +	const struct rhtab_elem *key2 = ptr;
> +
> +	return key1 != *(const unsigned long *)key2->data;
> +}
> +
> +static __always_inline u32 rhtab_hashfn_long(const void *data, u32 len, u32 seed)
> +{
> +	u64 k = *(const unsigned long *)data;
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +	return (u32)(k ^ (k >> 32)) ^ seed;
                                    ^^^^^^

Does applying the seed after folding the key make hash collisions entirely
independent of the seed?

If a BPF program or an attacker inserts keys constructed as:

    k = (Y << 32) | (X ^ Y)

These keys will all fold to exactly X before the XOR with the seed, meaning:

    f(key1) ^ seed == f(key2) ^ seed

for any random seed.

If rhashtable detects a long bucket chain and triggers a background rehash
with a new seed, won't all these colliding keys move together to the same new
bucket? This might trap the rhashtable worker in an infinite loop of
rehashing, leading to a 100 percent CPU livelock.

This concern was raised by sashiko-bot@kernel.org in v5 and v6:
https://lore.kernel.org/bpf/20260528182522.23D121F00A3C@smtp.kernel.org/

The author acknowledged in v5 that "the attack is possible in theory" but
decided to "leave for performance" without addressing the vulnerability. The
issue remains in this version.

> +}
> +
> +static const struct rhashtable_params rhtab_params_long = {
> +	.head_offset = offsetof(struct rhtab_elem, node),
> +	.key_offset  = offsetof(struct rhtab_elem, data),
> +	.key_len     = sizeof(long),
> +	.hashfn      = rhtab_hashfn_long,
> +	.obj_cmpfn   = rhtab_key_cmp_long,
> +};
> +
>  static struct bpf_map *rhtab_map_alloc(union bpf_attr *attr)
>  {
>  	struct rhashtable_params params;

[ ... ]

---

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

In-Reply-To-Subject: bpf: Optimize word-sized keys for resizable hashtable
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27011687425