Re: [PATCH bpf 1/2] bpf: Check timer_off for map_in_map only when map value have timer

[Yonghong Song <yhs@meta.com>]

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@gmail.com>
> ---
>   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?

>   	/* 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