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 Sun, Nov 27, 2022 at 7:07 PM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote:
>
>
>
> 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 ?

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() ?



[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