On 06/04, Andrii Nakryiko wrote:

On Tue, Jun 4, 2019 at 6:45 AM Stanislav Fomichev [off-list ref] wrote:

quoted

On 06/03, Stanislav Fomichev wrote:

quoted

quoted

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.

quoted

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.

quoted

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.