Hi, Yonghong:
On 2022/11/27 11:21, Yonghong Song wrote:
>
>
> On 11/26/22 2:53 AM, Hengqi Chen wrote:
>> The timer_off value could be -EINVAL or -ENOENT when map value of
>> inner map is struct and contains no bpf_timer. The EINVAL case happens
>> when the map is created without BTF key/value info, map->timer_off
>> is set to -EINVAL in map_create(). The ENOENT case happens when
>> the map is created with BTF key/value info (e.g. from BPF skeleton),
>> map->timer_off is set to -ENOENT as what btf_find_timer() returns.
>> In bpf_map_meta_equal(), we expect timer_off to be equal even if
>> map value does not contains bpf_timer. This rejects map_in_map created
>> with BTF key/value info to be updated using inner map without BTF
>> key/value info in case inner map value is struct. This commit lifts
>> such restriction.
>>
>> Fixes: 68134668c17f ("bpf: Add map side support for bpf timers.")
>> Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>
>> ---
>> kernel/bpf/map_in_map.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c
>> index 135205d0d560..0840872de486 100644
>> --- a/kernel/bpf/map_in_map.c
>> +++ b/kernel/bpf/map_in_map.c
>> @@ -80,11 +80,18 @@ void bpf_map_meta_free(struct bpf_map *map_meta)
>> bool bpf_map_meta_equal(const struct bpf_map *meta0,
>> const struct bpf_map *meta1)
>> {
>> + bool timer_off_equal;
>> +
>> + if (!map_value_has_timer(meta0) && !map_value_has_timer(meta1))
>> + timer_off_equal = true;
>> + else
>> + timer_off_equal = meta0->timer_off == meta1->timer_off;
>> +
>
> Is it possible we assign -1 to meta->timer_off directly instead of
> -EINVAL or -ENOENT, to indicate it does not exist? This will make
> this and possible other future timer_off comparison much easier?
>
These error codes are checked in verifier, so didn't touch it.
>> /* No need to compare ops because it is covered by map_type */
>> return meta0->map_type == meta1->map_type &&
>> meta0->key_size == meta1->key_size &&
>> meta0->value_size == meta1->value_size &&
>> - meta0->timer_off == meta1->timer_off &&
>> + timer_off_equal &&
>> meta0->map_flags == meta1->map_flags &&
>> bpf_map_equal_kptr_off_tab(meta0, meta1);
>> }
>> --
>> 2.34.1
