On Fri, Sep 17, 2021 at 4:39 PM [off-list ref] wrote:

On 09/17, Andrii Nakryiko wrote:

quoted

On Fri, Sep 17, 2021 at 4:11 PM [off-list ref] wrote:

quoted

On 09/17, Andrii Nakryiko wrote:

quoted

On Fri, Sep 17, 2021 at 10:26 AM [off-list ref] wrote:

quoted

On 09/16, Andrii Nakryiko wrote:

quoted

Implement strict ELF section name handling for BPF programs. It

utilizes

quoted

quoted

`libbpf_set_strict_mode()` framework and adds new flag:
LIBBPF_STRICT_SEC_NAME.

quoted

If this flag is set, libbpf will enforce exact section name

matching

quoted

quoted

for

quoted

quoted

a lot of program types that previously allowed just partial prefix
match. E.g., if previously SEC("xdp_whatever_i_want") was

allowed, now

quoted

quoted

quoted

quoted

in strict mode only SEC("xdp") will be accepted, which makes

SEC("")

quoted

quoted

quoted

quoted

definitions cleaner and more structured. SEC() now won't be used

as

quoted

quoted

yet

quoted

quoted

another way to uniquely encode BPF program identifier (for that
C function name is better and is guaranteed to be unique within
bpf_object). Now SEC() is strictly BPF program type and,

depending on

quoted

quoted

quoted

quoted

program type, extra load/attach parameter specification.

quoted

Libbpf completely supports multiple BPF programs in the same ELF
section, so multiple BPF programs of the same type/specification

easily

quoted

quoted

co-exist together within the same bpf_object scope.

quoted

Additionally, a new (for now internal) convention is introduced:

section

quoted

quoted

name that can be a stand-alone exact BPF program type

specificator,

quoted

quoted

but

quoted

quoted

also could have extra parameters after '/' delimiter. An example

of

quoted

quoted

such

quoted

quoted

section is "struct_ops", which can be specified by itself, but

also

quoted

quoted

quoted

quoted

allows to specify the intended operation to be attached to, e.g.,
"struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not

allowed.

quoted

quoted

Such section definition is specified as "struct_ops+".

quoted

This change is part of libbpf 1.0 effort ([0], [1]).

quoted

   [0] Closes: https://github.com/libbpf/libbpf/issues/271
   [1]

https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling

quoted

quoted

quoted

quoted

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
  tools/lib/bpf/libbpf.c        | 135

++++++++++++++++++++++------------

quoted

quoted

  tools/lib/bpf/libbpf_legacy.h |   9 +++
  2 files changed, 98 insertions(+), 46 deletions(-)

quoted

[...]

quoted

quoted

quoted

quoted

quoted

+     /*
+      * Enforce strict BPF program section (SEC()) names.
+      * E.g., while prefiously SEC("xdp_whatever") or

SEC("perf_event_blah")

quoted

quoted

were
+      * allowed, with LIBBPF_STRICT_SEC_PREFIX this will become
+      * unrecognized by libbpf and would have to be just

SEC("xdp")

quoted

quoted

and

quoted

quoted

+      * SEC("xdp") and SEC("perf_event").
+      */
+     LIBBPF_STRICT_SEC_NAME = 0x04,

To clarify: I'm assuming, as discussed, we'll still support that

old,

quoted

quoted

quoted

non-conforming naming in libbpf 1.0, right? It just won't be enabled
by default.

quoted

No, we won't. All those opt-in strict flags will be turned on
permanently in libbpf 1.0. But I'm adding an ability to provide custom
callbacks to handle whatever (reasonable) BPF program section names.
So if someone has a real important case needing custom handling, it's
not a big problem to implement that logic on their own. If someone is
just resisting making their code conforming, well... Stay on the old
fixed version, write a callback, or just do the mechanical rename, how
hard can that be? We are dropping bpf_program__find_program_by_title()
in libbpf 1.0, that API is meaningless with multiple programs per
section, so you'd have to update your logic to either skeleton or
bpf_program__find_program_by_name() anyways.

I see. I was assuming some of them would stay, iirc Toke also was asking
for this one to stay (or was it the old maps format?). FTR, I'm not
resisting any changes, I'm willing to invest some time to update our
callers, just trying to understand what my options are. We do have some
cases where we depend on the section names, so maybe I should just
switch from bpf_program__title to bpf_program__name (and do appropriate
renaming).

quoted

Switching to name over title (section name) is a good idea for sure.

quoted

quoted

RE skeleton: I'm not too eager to adopt it, I'll wait for version 2 :-)

quoted

Honest curiosity, what's wrong with the current version of skeleton?
Can you please expand on this?

Nothing wrong, I'm just joking, but I guess we went with a different path
long time ago where we ship .o more-or-less independently and it's hard to
adopt skeletons at this point (where .o is bundled with the userspace part).
We also ship several .o's compiled for different kernel versions because
prior to BTF we didn't have a good way todo '#ifdef KERNEL_X_Y_Z' and
some of that still lives on (but probably should die eventually).

We also have some applications with "old model", though they've used
their custom embedding logic to not distribute separate .o files. We
are working on switching them to BPF skeletons this half, though. Note
that BPF skeleton generation logic recently added a way to get raw ELF
bytes without necessarily instantiating the skeleton itself (see
<xxx>__elf_bytes()). There are also plans to add small C++
constructor/destructor shims to make it harder for C++ apps to miss
the need to destroy the skeleton, which helps with BPF skeleton
adoption in C++ applications.

Maybe we should do some talk on some conference on the way we ship
bpf to the fleet. Again, it's not like we don't like to change anything
here, but there is some existing "legacy" infra that has some
assumptions that might not easy to change in a short period of time.

Sure, some sort of lighting talk would be interesting. The problem is
that there needs to be a persistent and energetic initiative to break
all assumptions and switch to newer (arguably, better) ways. Doesn't
happen by itself ;)