On Fri, Apr 03, 2020 at 04:56:01PM -0700, Matt Cover wrote: > > I think doing BTF annotation for EXPORT_SYMBOL_BPF(bpf_icmp_send); is trivial. > > I've been looking into this more; here is what I'm thinking. > > 1. Export symbols for bpf the same as modules, but into one or more > special namespaces. > > Exported symbols recently gained namespaces. > https://lore.kernel.org/linux-usb/20190906103235.197072-1-maennich@xxxxxxxxxx/ > Documentation/kbuild/namespaces.rst > > This makes the in-kernel changes needed for export super simple. > > #define EXPORT_SYMBOL_BPF(sym) EXPORT_SYMBOL_NS(sym, BPF_PROG) > #define EXPORT_SYMBOL_BPF_GPL(sym) EXPORT_SYMBOL_NS_GPL(sym, BPF_PROG) > > BPF_PROG is our special namespace above. We can easily add > BPF_PROG_ACQUIRES and BPF_PROG_RELEASES for those types of > unstable helpers. > > Exports for bpf progs are then as simple as for modules. > > EXPORT_SYMBOL_BPF(bpf_icmp_send); > > Documenting these namespaces as not for use by modules should be > enough; an explicit import statement to use namespaced symbols is > already required. Explicitly preventing module use in > MODULE_IMPORT_NS or modpost are also options if we feel more is > needed. > > 2. Teach pahole's (dwarves') dwarf loader to parse __ksymtab*. > > I've got a functional wip which retrieves the namespace from the > __kstrtab ELF section. Working to differentiate between __ksymtab > and __ksymtab_gpl symbols next. Good news is this info is readily > available in vmlinux and module .o files. The interface here will > probably end up similar to dwarves' elf_symtab__*, but with an > struct elf_ksymtab per __ksymtab* section (all pointing to the > same __kstrtab section though). > > 3. Teach pahole's btf encoder to encode the following bools: export, > gpl_only, acquires, releases. > > I'm envisioning this info will end up in a new struct > btf_func_proto in btf.h. Perhaps like this. > > struct btf_func_proto { > /* "info" bits arrangement > * bit 0: exported (callable by bpf prog) > * bit 1: gpl_only (only callable from GPL licensed bpf prog) > * bit 2: acquires (acquires and returns a refcounted pointer) > * bit 3: releases (first argument, a refcounted pointer, > is released) > * bits 4-31: unused > */ > __u32 info; > }; > > Currently, a "struct btf_type" of type BTF_KIND_FUNC_PROTO is > directly followed by vlen struct btf_param/s. I'm hoping we can > insert btf_func_proto before the first btf_param or after the > last. If that's not workable, adding a new type, > BTF_KIND_FUNC_EXPORT, is another idea. I don't see why 1 and 2 are necessary. What is the value of true export_symbol here? What is the value of namespaced true export_symbol? Imo it only adds memory overhead to vmlinux. The same information is available in BTF as a _name_. What is the point to replicate it into kcrc? Imo kcrc is a poor protection mechanism that is already worse that BTF. I really don't see a value going that route. I think just encoding the intent to export into BTF is enough. Option 3 above looks like overkill too. Just name convention would do. We already use different prefixes to encode certain BTFs (see struct_ops and btf_trace). Just say when BTF func_proto starts with "export_" it means it's exported. It would be trivial for users to grep as well: bpftool btf dump file ./vmlinux |grep export_ > > The crcs could be used to improve the developer experience when > using unstable helpers. crc don't add any additional value on top of BTF. BTF types has to match exactly. It's like C compiler checking that you can call a function with correct proto.
