bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@fomichev.me>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Alexei Starovoitov <ast@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <Kernel-team@fb.com>
Subject: Re: [RFC PATCH bpf-next 6/8] libbpf: allow specifying map definitions using BTF
Date: Tue, 4 Jun 2019 14:07:10 -0700	[thread overview]
Message-ID: <20190604210710.GA17053@mini-arch> (raw)
In-Reply-To: <CAEf4BzZEqmnwL0MvEkM7iH3qKJ+TF7=yCKJRAAb34m4+B-1Zcg@mail.gmail.com>

On 06/04, Andrii Nakryiko wrote:
> On Tue, Jun 4, 2019 at 6:45 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 06/03, Stanislav Fomichev wrote:
> > > > BTF is mandatory for _any_ new feature.
> > > If something is easy to support without asking everyone to upgrade to
> > > a bleeding edge llvm, why not do it?
> > > So much for backwards compatibility and flexibility.
> > >
> > > > It's for introspection and debuggability in the first place.
> > > > Good debugging is not optional.
> > > Once llvm 8+ is everywhere, sure, but we are not there yet (I'm talking
> > > about upstream LTS distros like ubuntu/redhat).
> > But putting this aside, one thing that I didn't see addressed in the
> > cover letter is: what is the main motivation for the series?
> > Is it to support iproute2 map definitions (so cilium can switch to libbpf)?
> 
> In general, the motivation is to arrive at a way to support
> declaratively defining maps in such a way, that:
> - captures type information (for debuggability/introspection) in
> coherent and hard-to-screw-up way;
> - allows to support missing useful features w/ good syntax (e.g.,
> natural map-in-map case vs current completely manual non-declarative
> way for libbpf);

[..]
> - ultimately allow iproute2 to use libbpf as unified loader (and thus
> the need to support its existing features, like
> BPF_MAP_TYPE_PROG_ARRAY initialization, pinning, map-in-map);
So prog_array tail call info would be encoded in the magic struct instead of
a __section_tail(whatever) macros that iproute2 is using? Does it
mean that the programs that target iproute2 would have to be rewritten?
Or we don't have a goal to provide source-level compatibility?

In general, supporting iproute2 seems like the most compelling
reason to use BTF given current state of llvm+btf adoption.
BPF_ANNOTATE_KV_PAIR and map-in-map syntax while ugly, is not the major
paint point (imho); but I agree, with BTF both of those things
look much better.

That's why I was trying to understand whether we can start with using
BTF to support _existing_ iproute2 format and then, once it's working,
generalize it (and kill bpf_map_def or make it a subset of generic BTF).
That way we are not implementing another way to support pinning/tail
calls, but enabling iproute2 to use libbpf.

But feel free to ignore all my nonsense above; I don't really have any
major concerns with the new generic format rather than discoverability
(the docs might help) and a mandate that everyone switches to it immediately.

> The only missing feature that can be supported reasonably with
> bpf_map_def is pinning (as it's just another int field), but all the
> other use cases requires awkward approach of matching arbitrary IDs,
> which feels like a bad way forward.
> 
> 
> > If that's the case, maybe explicitly focus on that? Once we have
> > proof-of-concept working for iproute2 mode, we can extend it to everything.

  reply	other threads:[~2019-06-04 21:07 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-31 20:21 [RFC PATCH bpf-next 0/8] BTF-defined BPF map definitions Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 1/8] libbpf: add common min/max macro to libbpf_internal.h Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 2/8] libbpf: extract BTF loading and simplify ELF parsing logic Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 3/8] libbpf: refactor map initialization Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 4/8] libbpf: identify maps by section index in addition to offset Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 5/8] libbpf: split initialization and loading of BTF Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 6/8] libbpf: allow specifying map definitions using BTF Andrii Nakryiko
2019-05-31 21:28   ` Stanislav Fomichev
2019-05-31 22:58     ` Andrii Nakryiko
2019-06-03  0:33       ` Jakub Kicinski
2019-06-03 21:54         ` Andrii Nakryiko
2019-06-03 23:34           ` Jakub Kicinski
2019-06-03 16:32       ` Stanislav Fomichev
2019-06-03 22:03         ` Andrii Nakryiko
2019-06-04  1:02           ` Stanislav Fomichev
2019-06-04  1:07             ` Alexei Starovoitov
2019-06-04  4:29               ` Stanislav Fomichev
2019-06-04 13:45                 ` Stanislav Fomichev
2019-06-04 17:31                   ` Andrii Nakryiko
2019-06-04 21:07                     ` Stanislav Fomichev [this message]
2019-06-04 21:22                       ` Andrii Nakryiko
2019-06-06 21:09                     ` Daniel Borkmann
2019-06-06 23:02                       ` Andrii Nakryiko
2019-06-06 23:27                         ` Alexei Starovoitov
2019-06-07  0:10                           ` Jakub Kicinski
2019-06-07  0:27                             ` Alexei Starovoitov
2019-06-07  1:02                               ` Jakub Kicinski
2019-06-10  1:17                                 ` explicit maps. Was: " Alexei Starovoitov
2019-06-10 21:15                                   ` Jakub Kicinski
2019-06-10 23:48                                   ` Andrii Nakryiko
2019-06-03 22:34   ` Andrii Nakryiko
2019-06-06 16:42   ` Lorenz Bauer
2019-06-06 22:34     ` Andrii Nakryiko
2019-06-17  9:07       ` Lorenz Bauer
2019-06-17 20:59         ` Andrii Nakryiko
2019-06-20  9:27           ` Lorenz Bauer
2019-06-21  4:05             ` Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 7/8] selftests/bpf: add test for BTF-defined maps Andrii Nakryiko
2019-05-31 20:21 ` [RFC PATCH bpf-next 8/8] selftests/bpf: switch tests to BTF-defined map definitions Andrii Nakryiko
2019-06-11  4:34 [RFC PATCH bpf-next 0/8] BTF-defined BPF " Andrii Nakryiko
2019-06-11  4:35 ` [RFC PATCH bpf-next 6/8] libbpf: allow specifying map definitions using BTF Andrii Nakryiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190604210710.GA17053@mini-arch \
    --to=sdf@fomichev.me \
    --cc=Kernel-team@fb.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).