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

Mykyta Yatsenko
2026-04-14T08:08:24Z

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().

>> + 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 {
>