[PATCH bpf 0/2] Check timer_off for map_in_map only when map value has timer

[PATCH bpf 0/2] Check timer_off for map_in_map only when map value has timer

[Hengqi Chen]

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(). For example:

    map_fd = bpf_map_create(BPF_MAP_TYPE_LRU_HASH, NULL, 4, 4, 1, NULL);

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. For example, map created from BPF skeleton:

    struct inner_key {
    	__u32 x;
    };

    struct inner_value {
    	__u32 y;
    };

    struct inner {
    	__uint(type, BPF_MAP_TYPE_LRU_HASH);
    	__uint(max_entries, 1);
    	__type(key, struct inner_key);
    	__type(value, struct inner_value);
    } inner SEC(".maps");

    struct {
    	__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
    	__uint(max_entries, 1);
    	__type(key, __u32);
    	__array(values, struct inner);
    } outer SEC(".maps");

Since timer_off is different, the map_in_map outer in the above case
can NOT be updated using map_fd in the first case. This patch tries
to fix such restriction.

Hengqi Chen (2):
  bpf: Check timer_off for map_in_map only when map value have timer
  selftests/bpf: Update map_in_map using map without BTF key/value info

 kernel/bpf/map_in_map.c                       |  9 ++++++-
 .../selftests/bpf/prog_tests/btf_map_in_map.c | 27 +++++++++++++++++++
 .../selftests/bpf/progs/test_btf_map_in_map.c | 22 +++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)

--
2.34.1

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

[Hengqi Chen]

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

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

[Yonghong Song]

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

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

[Hengqi Chen]

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@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?
> 

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

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

[Alexei Starovoitov]

On Sat, Nov 26, 2022 at 2:54 AM Hengqi Chen <hengqi.chen@gmail.com> 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.

Sorry, but I prefer to label this issue as 'wont-fix'.
Mixing BTF enabled and non-BTF inner maps is a corner case
that is not worth fixing.
At some point we will require all programs and maps to contain BTF.
It's necessary for introspection.
The maps as blobs of data should not be used.
Much so adding support for mixed use as inner maps.

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

[Hengqi Chen]

Hi, Alexei:

On 2022/11/28 08:44, Alexei Starovoitov wrote:
> On Sat, Nov 26, 2022 at 2:54 AM Hengqi Chen <hengqi.chen@gmail.com> 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.
> 
> Sorry, but I prefer to label this issue as 'wont-fix'.
> Mixing BTF enabled and non-BTF inner maps is a corner case

We do have such usecase. The BPF progs and maps are pinned to bpffs
using BPF object file. And the map_in_map is updated by some other
process which don't have access to such BTF info.

> that is not worth fixing.

Is there a way to get this fixed for v5.x series only ?

> At some point we will require all programs and maps to contain BTF.
> It's necessary for introspection.

We don't care much about BTF for introspection. In production, we always
have a version field and some reserved fields in the map value for backward
compatibility. The interpretation of such map values are left to upper layer.

> The maps as blobs of data should not be used.
> Much so adding support for mixed use as inner maps.

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

[Alexei Starovoitov]

On Sun, Nov 27, 2022 at 6:42 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Hi, Alexei:
>
> On 2022/11/28 08:44, Alexei Starovoitov wrote:
> > On Sat, Nov 26, 2022 at 2:54 AM Hengqi Chen <hengqi.chen@gmail.com> 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.
> >
> > Sorry, but I prefer to label this issue as 'wont-fix'.
> > Mixing BTF enabled and non-BTF inner maps is a corner case
>
> We do have such usecase. The BPF progs and maps are pinned to bpffs
> using BPF object file. And the map_in_map is updated by some other
> process which don't have access to such BTF info.
>
> > that is not worth fixing.
>
> Is there a way to get this fixed for v5.x series only ?
>
> > At some point we will require all programs and maps to contain BTF.
> > It's necessary for introspection.
>
> We don't care much about BTF for introspection. In production, we always
> have a version field and some reserved fields in the map value for backward
> compatibility. The interpretation of such map values are left to upper layer.

That "interpretation of such map values are left to upper layer"...
is exactly the reason why we will enforce BTF in the future.
Production engineers and people outside of "upper layer" sw team
has to be able to debug maps and progs.

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

[Hengqi Chen]

On 2022/11/28 10:49, Alexei Starovoitov wrote:
> On Sun, Nov 27, 2022 at 6:42 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>>
>> Hi, Alexei:
>>
>> On 2022/11/28 08:44, Alexei Starovoitov wrote:
>>> On Sat, Nov 26, 2022 at 2:54 AM Hengqi Chen <hengqi.chen@gmail.com> 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.
>>>
>>> Sorry, but I prefer to label this issue as 'wont-fix'.
>>> Mixing BTF enabled and non-BTF inner maps is a corner case
>>
>> We do have such usecase. The BPF progs and maps are pinned to bpffs
>> using BPF object file. And the map_in_map is updated by some other
>> process which don't have access to such BTF info.
>>
>>> that is not worth fixing.
>>
>> Is there a way to get this fixed for v5.x series only ?
>>
>>> At some point we will require all programs and maps to contain BTF.
>>> It's necessary for introspection.
>>
>> We don't care much about BTF for introspection. In production, we always
>> have a version field and some reserved fields in the map value for backward
>> compatibility. The interpretation of such map values are left to upper layer.
> 
> That "interpretation of such map values are left to upper layer"...
> is exactly the reason why we will enforce BTF in the future.
> Production engineers and people outside of "upper layer" sw team
> has to be able to debug maps and progs.

Fine.

In libbpf, we have:

  if (is_inner) {
  	pr_warn("map '%s': inner def can't be pinned.\n", map_name);
  	return -EINVAL;
  }


Can we lift this restriction so that we can have an easy way to access BTF info
via pinned map ?

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

[Alexei Starovoitov]

On Sun, Nov 27, 2022 at 7:07 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
>
>
> On 2022/11/28 10:49, Alexei Starovoitov wrote:
> > On Sun, Nov 27, 2022 at 6:42 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
> >>
> >> Hi, Alexei:
> >>
> >> On 2022/11/28 08:44, Alexei Starovoitov wrote:
> >>> On Sat, Nov 26, 2022 at 2:54 AM Hengqi Chen <hengqi.chen@gmail.com> 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.
> >>>
> >>> Sorry, but I prefer to label this issue as 'wont-fix'.
> >>> Mixing BTF enabled and non-BTF inner maps is a corner case
> >>
> >> We do have such usecase. The BPF progs and maps are pinned to bpffs
> >> using BPF object file. And the map_in_map is updated by some other
> >> process which don't have access to such BTF info.
> >>
> >>> that is not worth fixing.
> >>
> >> Is there a way to get this fixed for v5.x series only ?
> >>
> >>> At some point we will require all programs and maps to contain BTF.
> >>> It's necessary for introspection.
> >>
> >> We don't care much about BTF for introspection. In production, we always
> >> have a version field and some reserved fields in the map value for backward
> >> compatibility. The interpretation of such map values are left to upper layer.
> >
> > That "interpretation of such map values are left to upper layer"...
> > is exactly the reason why we will enforce BTF in the future.
> > Production engineers and people outside of "upper layer" sw team
> > has to be able to debug maps and progs.
>
> Fine.
>
> In libbpf, we have:
>
>   if (is_inner) {
>         pr_warn("map '%s': inner def can't be pinned.\n", map_name);
>         return -EINVAL;
>   }
>
>
> Can we lift this restriction so that we can have an easy way to access BTF info
> via pinned map ?

Probably. Note that __uint(pinning, LIBBPF_PIN_BY_NAME)
is the only mode libbpf understands. It's simplistic.
but why do you want to use that mode?
Just pin it directly with bpf_map__pin() ?
Or even more low level bpf_obj_pin() ?

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

[Hengqi Chen]

On 2022/11/28 11:14, Alexei Starovoitov wrote:
> On Sun, Nov 27, 2022 at 7:07 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>>
>>
>>
>> On 2022/11/28 10:49, Alexei Starovoitov wrote:
>>> On Sun, Nov 27, 2022 at 6:42 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>>>>
>>>> Hi, Alexei:
>>>>
>>>> On 2022/11/28 08:44, Alexei Starovoitov wrote:
>>>>> On Sat, Nov 26, 2022 at 2:54 AM Hengqi Chen <hengqi.chen@gmail.com> 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.
>>>>>
>>>>> Sorry, but I prefer to label this issue as 'wont-fix'.
>>>>> Mixing BTF enabled and non-BTF inner maps is a corner case
>>>>
>>>> We do have such usecase. The BPF progs and maps are pinned to bpffs
>>>> using BPF object file. And the map_in_map is updated by some other
>>>> process which don't have access to such BTF info.
>>>>
>>>>> that is not worth fixing.
>>>>
>>>> Is there a way to get this fixed for v5.x series only ?
>>>>
>>>>> At some point we will require all programs and maps to contain BTF.
>>>>> It's necessary for introspection.
>>>>
>>>> We don't care much about BTF for introspection. In production, we always
>>>> have a version field and some reserved fields in the map value for backward
>>>> compatibility. The interpretation of such map values are left to upper layer.
>>>
>>> That "interpretation of such map values are left to upper layer"...
>>> is exactly the reason why we will enforce BTF in the future.
>>> Production engineers and people outside of "upper layer" sw team
>>> has to be able to debug maps and progs.
>>
>> Fine.
>>
>> In libbpf, we have:
>>
>>   if (is_inner) {
>>         pr_warn("map '%s': inner def can't be pinned.\n", map_name);
>>         return -EINVAL;
>>   }
>>
>>
>> Can we lift this restriction so that we can have an easy way to access BTF info
>> via pinned map ?
> 
> Probably. Note that __uint(pinning, LIBBPF_PIN_BY_NAME)
> is the only mode libbpf understands. It's simplistic.
> but why do you want to use that mode?
> Just pin it directly with bpf_map__pin() ?
> Or even more low level bpf_obj_pin() ?

Will try. 

Currently, we use `__uint(pinning, LIBBPF_PIN_BY_NAME)` and let
libbpf and Cilium's ebpf go library handle all the pinning jobs.

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

[Andrii Nakryiko]

On Sun, Nov 27, 2022 at 6:42 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Hi, Alexei:
>
> On 2022/11/28 08:44, Alexei Starovoitov wrote:
> > On Sat, Nov 26, 2022 at 2:54 AM Hengqi Chen <hengqi.chen@gmail.com> 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.
> >
> > Sorry, but I prefer to label this issue as 'wont-fix'.
> > Mixing BTF enabled and non-BTF inner maps is a corner case
>
> We do have such usecase. The BPF progs and maps are pinned to bpffs
> using BPF object file. And the map_in_map is updated by some other
> process which don't have access to such BTF info.
>
> > that is not worth fixing.
>
> Is there a way to get this fixed for v5.x series only ?
>
> > At some point we will require all programs and maps to contain BTF.
> > It's necessary for introspection.
>
> We don't care much about BTF for introspection. In production, we always
> have a version field and some reserved fields in the map value for backward
> compatibility. The interpretation of such map values are left to upper layer.

All the BTF stuff aside, wouldn't this be the best and most minimal
fix? It seems to define correct semantic meaning: no timer is found
(because no BTF in this case). Easy to backport, solves the immediate
problem. This code seems to be completely reworked in bpf-next,
though, so I don't know what the situation is there.

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 7b373a5e861f..9e38cc1e136c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1118,7 +1118,7 @@ static int map_create(union bpf_attr *attr)
        spin_lock_init(&map->owner.lock);

        map->spin_lock_off = -EINVAL;
-       map->timer_off = -EINVAL;
+       map->timer_off = -ENOENT;
        if (attr->btf_key_type_id || attr->btf_value_type_id ||
            /* Even the map's value is a kernel's struct,
             * the bpf_prog.o must have BTF to begin with




>
> > The maps as blobs of data should not be used.
> > Much so adding support for mixed use as inner maps.

[PATCH bpf 2/2] selftests/bpf: Update map_in_map using map without BTF key/value info

[Hengqi Chen]

Add a selftest to ensure that inner map without BTF key/value info
can coexist with inner map with BTF key/value info in the same map_in_map.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 .../selftests/bpf/prog_tests/btf_map_in_map.c | 27 +++++++++++++++++++
 .../selftests/bpf/progs/test_btf_map_in_map.c | 22 +++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_map_in_map.c b/tools/testing/selftests/bpf/prog_tests/btf_map_in_map.c
index eb90a6b8850d..d34d91d6d9ca 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_map_in_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_map_in_map.c
@@ -154,6 +154,30 @@ static void test_diff_size(void)
 	test_btf_map_in_map__destroy(skel);
 }

+static void test_btf_key_value(void)
+{
+	struct test_btf_map_in_map *skel;
+	int err, map_fd1, map_fd2, zero = 0;
+
+	skel = test_btf_map_in_map__open_and_load();
+	if (CHECK(!skel, "skel_open", "failed to open&load skeleton\n"))
+		return;
+
+	map_fd1 = bpf_map__fd(skel->maps.inner);
+	err = bpf_map_update_elem(bpf_map__fd(skel->maps.outer), &zero, &map_fd1, 0);
+	CHECK(err, "update map_in_map using map with BTF key/value info",
+	      "cannot use inner map with BTF key/value info\n");
+
+	map_fd2 = bpf_map_create(BPF_MAP_TYPE_LRU_HASH, NULL, 4, 4, 1, NULL);
+	CHECK(map_fd2 < 0, "create map without BTF key/value info", "cannot create map\n");
+	err = bpf_map_update_elem(bpf_map__fd(skel->maps.outer), &zero, &map_fd2, 0);
+	CHECK(err, "update map_in_map using map without BTF key/value info",
+	      "cannot use inner map without BTF key/value info\n");
+
+	close(map_fd2);
+	test_btf_map_in_map__destroy(skel);
+}
+
 void test_btf_map_in_map(void)
 {
 	if (test__start_subtest("lookup_update"))
@@ -161,4 +185,7 @@ void test_btf_map_in_map(void)

 	if (test__start_subtest("diff_size"))
 		test_diff_size();
+
+	if (test__start_subtest("btf_key_value"))
+		test_btf_key_value();
 }
diff --git a/tools/testing/selftests/bpf/progs/test_btf_map_in_map.c b/tools/testing/selftests/bpf/progs/test_btf_map_in_map.c
index c218cf8989a9..8f7ca70496f2 100644
--- a/tools/testing/selftests/bpf/progs/test_btf_map_in_map.c
+++ b/tools/testing/selftests/bpf/progs/test_btf_map_in_map.c
@@ -118,6 +118,28 @@ struct outer_sockarr_sz1 {
 	.values = { (void *)&sockarr_sz1 },
 };

+struct inner_key {
+	__u32 x;
+};
+
+struct inner_value {
+	__u32 y;
+};
+
+struct inner {
+	__uint(type, BPF_MAP_TYPE_LRU_HASH);
+	__uint(max_entries, 1);
+	__type(key, struct inner_key);
+	__type(value, struct inner_value);
+} inner SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__array(values, struct inner);
+} outer SEC(".maps");
+
 int input = 0;

 SEC("raw_tp/sys_enter")
--
2.34.1