Re: [PATCH RFC bpf-next v2 07/18] bpf: Implement batch ops for resizable hashtab

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Tue Apr 14, 2026 at 4:08 AM EDT, Mykyta Yatsenko wrote:
>
>
> On 4/14/26 12:25 AM, Emil Tsalapatis wrote:
>> On Wed Apr 8, 2026 at 11:10 AM EDT, Mykyta Yatsenko wrote:
>>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>>
>>> Add batch operations for BPF_MAP_TYPE_RHASH.
>>>
>>> Batch operations:
>>>   * rhtab_map_lookup_batch: Bulk lookup of elements by bucket
>>>   * rhtab_map_lookup_and_delete_batch: Atomic bulk lookup and delete
>>>
>>> The batch implementation uses rhashtable_walk_enter_from() to resume
>>> iteration from the last collected key. When the buffer fills, the last
>>> key becomes the cursor for the next batch call.
>>>
>>> Also implements rhtab_map_mem_usage() to report memory consumption.
>>>
>>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>>> ---
>>>   kernel/bpf/hashtab.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++--
>>>   1 file changed, 134 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>>> index e79c194e2779..a79d434dc626 100644
>>> --- a/kernel/bpf/hashtab.c
>>> +++ b/kernel/bpf/hashtab.c
>>> @@ -3051,19 +3051,150 @@ static long bpf_each_rhash_elem(struct bpf_map *map, bpf_callback_t callback_fn,
>>>   
>>>   static u64 rhtab_map_mem_usage(const struct bpf_map *map)
>>>   {
>>> -	return 0;
>>> +	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
>>> +	u64 num_entries;
>>> +
>>> +	num_entries = atomic_read(&rhtab->ht.nelems);
>>> +	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)
>>> +{
>>> +	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
>>> +	void __user *uvalues = u64_to_user_ptr(attr->batch.values);
>>> +	void __user *ukeys = u64_to_user_ptr(attr->batch.keys);
>>> +	void __user *ubatch = u64_to_user_ptr(attr->batch.in_batch);
>>> +	void *buf = NULL, *keys = NULL, *values = NULL, *dst_key, *dst_val;
>>> +	struct rhtab_elem **del_elems = NULL;
>>> +	u32 max_count, total, key_size, value_size, i;
>>> +	struct rhashtable_iter iter;
>>> +	struct rhtab_elem *elem;
>>> +	u64 elem_map_flags, map_flags;
>>> +	int ret = 0;
>>> +
>>> +	elem_map_flags = attr->batch.elem_flags;
>>> +	if ((elem_map_flags & ~BPF_F_LOCK) ||
>>> +	    ((elem_map_flags & BPF_F_LOCK) &&
>>> +	     !btf_record_has_field(map->record, BPF_SPIN_LOCK)))
>>> +		return -EINVAL;
>>> +
>>> +	map_flags = attr->batch.flags;
>>> +	if (map_flags)
>>> +		return -EINVAL;
>>> +
>>> +	max_count = attr->batch.count;
>>> +	if (!max_count)
>>> +		return 0;
>>> +
>>> +	if (put_user(0, &uattr->batch.count))
>>> +		return -EFAULT;
>>> +
>>> +	key_size = map->key_size;
>>> +	value_size = map->value_size;
>>> +
>>> +	keys = kvmalloc_array(max_count, key_size, GFP_USER | __GFP_NOWARN);
>>> +	values = kvmalloc_array(max_count, value_size, GFP_USER | __GFP_NOWARN);
>>> +	if (do_delete)
>>> +		del_elems = kvmalloc_array(max_count, sizeof(void *),
>>> +					   GFP_USER | __GFP_NOWARN);
>>> +
>>> +	if (!keys || !values || (do_delete && !del_elems)) {
>>> +		ret = -ENOMEM;
>>> +		goto free;
>>> +	}
>>> +
>>> +	/*
>>> +	 * Use the last key from the previous batch as cursor.
>>> +	 * enter_from positions at that key's bucket, walk_next
>>> +	 * returns the successor in O(1).
>>> +	 * First call (ubatch == NULL): starts from bucket 0.
>>> +	 */
>>> +	if (ubatch) {
>>> +		buf = kmalloc(key_size, GFP_USER | __GFP_NOWARN);
>>> +		if (!buf) {
>>> +			ret = -ENOMEM;
>>> +			goto free;
>>> +		}
>>> +		if (copy_from_user(buf, ubatch, key_size)) {
>>> +			ret = -EFAULT;
>>> +			goto free;
>>> +		}
>>> +	}
>>> +
>>> +	scoped_guard(rcu) {
>> 
>> AFAICT this guard makes sure the RCU critical section extends from the
>> beginning of rhashtable_walk_enter_from all the way to walk_stop(), is
>> that correct?
>> 
>
> This guard is to make sure that when rhashtable_walk_enter_from() 
> initializes iterator, the element which it initialised with is not 
> freed. rhashtable_walk_start() calls rcu lock as well, that's why I can 
> end guard right after rhashtable_walk_start().
>

Sounds good, thanks.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

>>> +		rhashtable_walk_enter_from(&rhtab->ht, &iter, buf, rhtab->params);
>>> +		rhashtable_walk_start(&iter);
>>> +	}
>>> +
>>> +	dst_key = keys;
>>> +	dst_val = values;
>>> +	total = 0;
>>> +
>>> +	while (total < max_count) {
>>> +		elem = rhtab_iter_next(&iter);
>>> +		if (!elem)
>>> +			break;
>>> +
>>> +		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;
>>> +
>>> +		dst_key += key_size;
>>> +		dst_val += value_size;
>>> +		total++;
>>> +	}
>>> +
>>> +	if (do_delete) {
>>> +		for (i = 0; i < total; i++)
>>> +			rhtab_delete_elem(rhtab, del_elems[i]);
>>> +	}
>>> +
>>> +	rhashtable_walk_stop(&iter);
>>> +	rhashtable_walk_exit(&iter);
>>> +
>>> +	if (total == 0) {
>>> +		ret = -ENOENT;
>>> +		goto free;
>>> +	}
>>> +
>>> +	/* Signal end of table when we collected fewer than requested */
>>> +	if (total < max_count)
>>> +		ret = -ENOENT;
>>> +
>>> +	/* Write last key as cursor for the next batch call */
>>> +	if (copy_to_user(ukeys, keys, total * key_size) ||
>>> +	    copy_to_user(uvalues, values, total * value_size) ||
>>> +	    put_user(total, &uattr->batch.count) ||
>>> +	    copy_to_user(u64_to_user_ptr(attr->batch.out_batch),
>>> +			 dst_key - key_size, key_size)) {
>>> +		ret = -EFAULT;
>>> +		goto free;
>>> +	}
>>> +
>>> +free:
>>> +	kfree(buf);
>>> +	kvfree(keys);
>>> +	kvfree(values);
>>> +	kvfree(del_elems);
>>> +	return ret;
>>>   }
>>>   
>>>   static int rhtab_map_lookup_batch(struct bpf_map *map, const union bpf_attr *attr,
>>>   				  union bpf_attr __user *uattr)
>>>   {
>>> -	return 0;
>>> +	return __rhtab_map_lookup_and_delete_batch(map, attr, uattr, false);
>>>   }
>>>   
>>>   static int rhtab_map_lookup_and_delete_batch(struct bpf_map *map, const union bpf_attr *attr,
>>>   					     union bpf_attr __user *uattr)
>>>   {
>>> -	return 0;
>> Shouldn't these have been -EINVAL or -ENOSUPP in previous patches?
>> 
>>> +	return __rhtab_map_lookup_and_delete_batch(map, attr, uattr, true);
>>>   }
>>>   
>>>   struct bpf_iter_seq_rhash_map_info {
>>