Re: [PATCH bpf-next V2] bpf: Fix map_check_no_btf return code

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

 



On Thu, May 28, 2020 at 12:08 AM Jesper Dangaard Brouer
<brouer@xxxxxxxxxx> wrote:
>
> On Wed, 27 May 2020 15:59:46 -0700
> Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote:
>
> > But regardless, can you please reply on v1 thread why adding support
> > for BTF to these special maps (that do not support BTF right now)
> > won't be a better solution and won't work (as you claimed)?
>
> (I will reply here instead of on v1) and I have not claimed it won't

Well, maybe I misinterpreted your response from that thread, but not
sure how I should have interpreted it differently:

> > > For special maps, we can just enforce
> > > that BTF is 4-byte integer (or typedef of that), so that in practice
> > > you'll be defining it as:
> > >
> > > struct {
> > >     __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> > >     __type(key, u32);
> > >     __type(value, u32);
> > > } my_map SEC(".maps");
> > >
> > > and it will just work?
> >
> > Nope, this will also fail.
>
> Why? If kernel supports 4-byte integers as BTF types for key/value for
> such maps, then what would fail in this case?


> work.  It will work, but we loose an opportunity if we just allow BTF
> across the board, without using it for per map validation.

I don't see how we are losing any extensibility, honestly. Right now
we can allow simple "4-byte integer". If we need to extend it in the
future, we'll allow 4-byte integer (for backwards compatibility), and
whatever struct makes sense for extended use case. But maybe I don't
completely understand what you are after here. See below.

>
> We have an opportunity with these special maps, that do not support BTF
> right now.  The value field in these maps are actually a UAPI and also
> kABI (Binary).
>
> Simply describing the struct with BTF is nice, but only helpful to make
> the end-user understand they binary layout.  On the kernel side we are
> still stuck with a kABI that can only be end-extended and size increased.
> I want to use BTF on the kernel-side to validate and enforce that user
> provided the expected "named" layout, and possibly reject it.  This
> gives us a layer that can provide a flexible kABI.  From my current
> understanding of the kernel side code that parse/walk BTF, I we can
> actually have flexible offsets (for e.g. structs) in the binary value,
> and remap those on the kernel side.  Enforcing a named layout when we
> enable BTF for these maps, will give us binary flexibility for future
> changes.  I hope you agree?

Can you expand on this named approach and what are the use cases you
are seeing? I guess it's something like what is done for
bpf_spin_lock, where we require struct with well-defined name and
field name, etc. But please elaborate here.


My biggest grudge with changing error code is that old error code will
still be used in older kernels, so if libbpf were to help users with
more helpful message, it now needs to support both error codes,
forever, potentially depending on kernel version. This constant
splitting of logic is annoying, so I'd rather avoid it.

>
> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
>



[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