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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2022/11/28 10:49, Alexei Starovoitov wrote:
> On Sun, Nov 27, 2022 at 6:42 PM Hengqi Chen <hengqi.chen@xxxxxxxxx> 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@xxxxxxxxx> 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 ?



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux