On Fri, Sep 17, 2021 at 4:11 PM <sdf@xxxxxxxxxx> wrote: > > On 09/17, Andrii Nakryiko wrote: > > On Fri, Sep 17, 2021 at 10:26 AM <sdf@xxxxxxxxxx> wrote: > > > > > > On 09/16, Andrii Nakryiko wrote: > > > > Implement strict ELF section name handling for BPF programs. It > > utilizes > > > > `libbpf_set_strict_mode()` framework and adds new flag: > > > > LIBBPF_STRICT_SEC_NAME. > > > > > > > If this flag is set, libbpf will enforce exact section name matching > > for > > > > a lot of program types that previously allowed just partial prefix > > > > match. E.g., if previously SEC("xdp_whatever_i_want") was allowed, now > > > > in strict mode only SEC("xdp") will be accepted, which makes SEC("") > > > > definitions cleaner and more structured. SEC() now won't be used as > > yet > > > > 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 > > > > program type, extra load/attach parameter specification. > > > > > > > Libbpf completely supports multiple BPF programs in the same ELF > > > > section, so multiple BPF programs of the same type/specification > > easily > > > > co-exist together within the same bpf_object scope. > > > > > > > Additionally, a new (for now internal) convention is introduced: > > section > > > > name that can be a stand-alone exact BPF program type specificator, > > but > > > > also could have extra parameters after '/' delimiter. An example of > > such > > > > section is "struct_ops", which can be specified by itself, but also > > > > 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. > > > > Such section definition is specified as "struct_ops+". > > > > > > > This change is part of libbpf 1.0 effort ([0], [1]). > > > > > > > [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 > > > > > > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > > > --- > > > > tools/lib/bpf/libbpf.c | 135 > > ++++++++++++++++++++++------------ > > > > tools/lib/bpf/libbpf_legacy.h | 9 +++ > > > > 2 files changed, 98 insertions(+), 46 deletions(-) > > > [...] > > > > + /* > > > > + * Enforce strict BPF program section (SEC()) names. > > > > + * E.g., while prefiously SEC("xdp_whatever") or > > SEC("perf_event_blah") > > > > were > > > > + * allowed, with LIBBPF_STRICT_SEC_PREFIX this will become > > > > + * unrecognized by libbpf and would have to be just SEC("xdp") > > and > > > > + * SEC("xdp") and SEC("perf_event"). > > > > + */ > > > > + LIBBPF_STRICT_SEC_NAME = 0x04, > > > > > > To clarify: I'm assuming, as discussed, we'll still support that old, > > > non-conforming naming in libbpf 1.0, right? It just won't be enabled > > > by default. > > > 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). Switching to name over title (section name) is a good idea for sure. > > RE skeleton: I'm not too eager to adopt it, I'll wait for version 2 :-) Honest curiosity, what's wrong with the current version of skeleton? Can you please expand on this? > > > > > > > > Btw, forgot to update you, I've enabled LIBBPF_STRICT_DIRECT_ERRS and > > > LIBBPF_STRICT_CLEAN_PTRS and everything seems to be working fine 🤞 > > > Great! The problem is that you would see the difference only when > > actual runtime failure happens. So I'd still recommend auditing the > > code, if possible.