Re: [PATCH RFC bpf-next v2 05/18] bpf: Implement get_next_key and free_internal_structs for resizable hashtab

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 4/13/26 11:44 PM, Emil Tsalapatis wrote:
> On Wed Apr 8, 2026 at 11:10 AM EDT, Mykyta Yatsenko wrote:
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> Implement rhtab_map_get_next_key() and rhtab_map_free_internal_structs()
>> of the BPF resizable hashtable. Both are only called from syscall, so
>> it's safe to use rhashtable walk API that uses spinlocks internally.
>>
>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>> ---
>>   kernel/bpf/hashtab.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 77 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>> index ea7314cc3703..7eee450a321e 100644
>> --- a/kernel/bpf/hashtab.c
>> +++ b/kernel/bpf/hashtab.c
>> @@ -2921,13 +2921,89 @@ static int rhtab_map_gen_lookup(struct bpf_map *map, struct bpf_insn *insn_buf)
>>   	return insn - insn_buf;
>>   }
>>   
>> +/* Helper to get next element, handling -EAGAIN during resize */
>> +static struct rhtab_elem *rhtab_iter_next(struct rhashtable_iter *iter)
>> +{
>> +	struct rhtab_elem *elem;
>> +
>> +	while ((elem = rhashtable_walk_next(iter))) {
>> +		if (IS_ERR(elem)) {
>> +			if (PTR_ERR(elem) == -EAGAIN)
>> +				continue;
>> +			return NULL;
>> +		}
>> +		return elem;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>>   static void rhtab_map_free_internal_structs(struct bpf_map *map)
>>   {
>> +	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
>> +	struct rhashtable_iter iter;
>> +	struct rhtab_elem *elem;
>> +
>> +	if (!bpf_map_has_internal_structs(map))
>> +		return;
>> +
>> +	/*
>> +	 * An element can be processed twice if rhashtable resized concurrently.
>> +	 * Special structs freeing handles duplicate cancel_and_free.
>> +	 */
>> +	rhashtable_walk_enter(&rhtab->ht, &iter);
>> +	rhashtable_walk_start(&iter);
>> +
>> +	for (elem = rhtab_iter_next(&iter); elem; elem = rhtab_iter_next(&iter))
>> +		bpf_map_free_internal_structs(map, rhtab_elem_value(elem, map->key_size));
>> +
>> +	rhashtable_walk_stop(&iter);
>> +	rhashtable_walk_exit(&iter);
>>   }
>>   
>>   static int rhtab_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
>>   {
>> -	return -EOPNOTSUPP;
>> +	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
>> +	struct rhashtable_iter iter;
>> +	struct rhtab_elem *elem;
>> +	bool key_found;
>> +	int ret = -ENOENT;
>> +
>> +	/*
>> +	 * Hold RCU across enter_from + walk_start to prevent the
>> +	 * element cached by enter_from from being freed before
>> +	 * walk_start re-acquires RCU.
>> +	 */
>> +	guard(rcu)();
>> +	rhashtable_walk_enter_from(&rhtab->ht, &iter, key, rhtab->params);
>> +	key_found = key && iter.p;
>> +	rhashtable_walk_start(&iter);
>> +
>> +	elem = rhtab_iter_next(&iter);
>> +	if (elem) {
>> +		memcpy(next_key, elem->data, map->key_size);
>> +		ret = 0;
>> +	}
>> +
>> +	rhashtable_walk_stop(&iter);
>> +	rhashtable_walk_exit(&iter);
>> +
>> +	if (ret == 0 || key_found)
>> +		return ret;
>> +
>> +	/* Key was not found restart from the beginning */
>> +	rhashtable_walk_enter(&rhtab->ht, &iter);
>> +	rhashtable_walk_start(&iter);
>> +
>> +	elem = rhtab_iter_next(&iter);
>> +	if (elem) {
>> +		memcpy(next_key, elem->data, map->key_size);
>> +		ret = 0;
>> +	}
>> +
>> +	rhashtable_walk_stop(&iter);
>> +	rhashtable_walk_exit(&iter);
> Nit: The two attempts are partly identical, can you try to
> roll them together and use a goto to retry? Or do you think
> this will be too messy?
> 

It felt nicer just to write this code sequentially without any gotos or 
loops. No strong opinion here.

> Otherwise:
> 
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> 
>> +	return ret;
>>   }
>>   
>>   static void rhtab_map_seq_show_elem(struct bpf_map *map, void *key, struct seq_file *m)
>