Re: [PATCH bpf-next v2 02/20] bpf: Parse bpf_dynptr in map key

[Hou Tao <houtao@huaweicloud.com>]

Hi,

On 2/14/2025 1:59 AM, Alexei Starovoitov wrote:
> On Sat, Jan 25, 2025 at 2:59 AM Hou Tao <houtao@huaweicloud.com> wrote:
>> From: Hou Tao <houtao1@huawei.com>
>>
>> To support variable-length key or strings in map key, use bpf_dynptr to
>> represent these variable-length objects and save these bpf_dynptr
>> fields in the map key. As shown in the examples below, a map key with an
>> integer and a string is defined:

SNIP
>> @@ -271,7 +271,14 @@ struct bpf_map {
>>         u64 map_extra; /* any per-map-type extra fields */
>>         u32 map_flags;
>>         u32 id;
>> +       /* BTF record for special fields in map value. bpf_dynptr is disallowed
>> +        * at present.
>> +        */
> Maybe drop 'at present' to fit on one line.
> I would also capitalize Value to make the difference more obvious...

Will do.
>
>>         struct btf_record *record;
>> +       /* BTF record for special fields in map key. Only bpf_dynptr is allowed
>> +        * at present.
> ...with this line. Key.

Will do.
>
>> +

SNIP
>> +       btf_record_free(key_rec);
>>         btf_record_free(rec);
>>         /* Delay freeing of btf for maps, as map_free callback may need
>>          * struct_meta info which will be freed with btf_put().
>> @@ -1180,6 +1188,8 @@ int map_check_no_btf(const struct bpf_map *map,
>>         return -ENOTSUPP;
>>  }
>>
>> +#define MAX_DYNPTR_CNT_IN_MAP_KEY 1
> I remember we discussed to allow 2 dynptr-s in a key.
> And in patch 11 you already do:
> +       record = map->key_record;
> +       for (i = 0; i < record->cnt; i++) {
>
> so the support for multiple dynptr-s is almost there?

I misunderstood the discussion. However, the change is simple. Only need
to change it from 1 to 2 because the following patches has already
supported multiple dynptrs.