On 11/21/18 12:18 PM, Yonghong Song wrote: > > > On 11/21/18 9:40 AM, Andrey Ignatov wrote: >> More and more projects use libbpf and one day it'll likely be packaged >> and distributed as DSO and that requires ABI versioning so that both >> compatible and incompatible changes to ABI can be introduced in a safe >> way in the future without breaking executables dynamically linked with a >> previous version of the library. >> >> Usual way to do ABI versioning is version script for the linker. Add >> such a script for libbpf. All global symbols currently exported via >> LIBBPF_API macro are added to the version script libbpf.map. >> >> The version name LIBBPF_0.0.1 is constructed from the name of the >> library + version specified by $(LIBBPF_VERSION) in Makefile. >> >> Version script does not duplicate the work done by LIBBPF_API macro, it >> rather complements it. The macro is used at compile time and can be used >> by compiler to do optimization that can't be done at link time, it is >> purely about global symbol visibility. The version script, in turn, is >> used at link time and takes care of ABI versioning. Both techniques are >> described in details in [1]. >> >> Whenever ABI is changed in the future, version script should be changed >> appropriately. > > Maybe we should clarify the policy of how version numbers should be > change? Each commit which changes default global symbol ABI? Each kernel > release? > >> >> [1] https://www.akkadia.org/drepper/dsohowto.pdf >> >> Signed-off-by: Andrey Ignatov <r...@fb.com> >> --- >> tools/lib/bpf/Makefile | 4 +- >> tools/lib/bpf/libbpf.map | 120 +++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 123 insertions(+), 1 deletion(-) >> create mode 100644 tools/lib/bpf/libbpf.map >> >> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile >> index 425b480bda75..d76c41fa2d39 100644 >> --- a/tools/lib/bpf/Makefile >> +++ b/tools/lib/bpf/Makefile >> @@ -145,6 +145,7 @@ include $(srctree)/tools/build/Makefile.include >> >> BPF_IN := $(OUTPUT)libbpf-in.o >> LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE)) >> +VERSION_SCRIPT := libbpf.map >> >> CMD_TARGETS = $(LIB_FILE) >> >> @@ -170,7 +171,8 @@ $(BPF_IN): force elfdep bpfdep >> $(Q)$(MAKE) $(build)=libbpf >> >> $(OUTPUT)libbpf.so: $(BPF_IN) >> - $(QUIET_LINK)$(CC) --shared $^ -o $@ >> + $(QUIET_LINK)$(CC) --shared -Wl,--version-script=$(VERSION_SCRIPT) \ >> + $^ -o $@ >> >> $(OUTPUT)libbpf.a: $(BPF_IN) >> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ >> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map >> new file mode 100644 >> index 000000000000..9fe416b68c7d >> --- /dev/null >> +++ b/tools/lib/bpf/libbpf.map >> @@ -0,0 +1,120 @@ >> +LIBBPF_0.0.1 { >> + global: >> + bpf_btf_get_fd_by_id; > > Do you think we could use this opportunities to > make naming more consistent? For example, > bpf_btf_get_fd_by_id => btf__get_fd_by_id?
I think this one is fine since it matches bpf_[map|prog]_get_fd_by_id() and it's a wrapper. >> + bpf_create_map; >> + bpf_create_map_in_map; >> + bpf_create_map_in_map_node; >> + bpf_create_map_name; >> + bpf_create_map_node; >> + bpf_create_map_xattr; >> + bpf_load_btf; >> + bpf_load_program; >> + bpf_load_program_xattr; >> + bpf_map__btf_key_type_id; >> + bpf_map__btf_value_type_id; >> + bpf_map__def; >> + bpf_map_delete_elem; > + bpf_map__fd; >> + bpf_map_get_fd_by_id; >> + bpf_map_get_next_id; >> + bpf_map_get_next_key; > + >> bpf_map__is_offload_neutral; >> + bpf_map_lookup_and_delete_elem; >> + bpf_map_lookup_elem; >> + bpf_map__name; >> + bpf_map__next; >> + bpf_map__pin; >> + bpf_map__prev; >> + bpf_map__priv; >> + bpf_map__reuse_fd; >> + bpf_map__set_ifindex; >> + bpf_map__set_priv; >> + bpf_map__unpin; >> + bpf_map_update_elem; >> + bpf_object__btf_fd; >> + bpf_object__close; >> + bpf_object__find_map_by_name; >> + bpf_object__find_map_by_offset; >> + bpf_object__find_program_by_title; >> + bpf_object__kversion; >> + bpf_object__load; >> + bpf_object__name; >> + bpf_object__next; >> + bpf_object__open; >> + bpf_object__open_buffer; >> + bpf_object__open_xattr; >> + bpf_object__pin; >> + bpf_object__pin_maps; >> + bpf_object__pin_programs; >> + bpf_object__priv; >> + bpf_object__set_priv; >> + bpf_object__unload; >> + bpf_object__unpin_maps; >> + bpf_object__unpin_programs; >> + bpf_obj_get; >> + bpf_obj_get_info_by_fd; >> + bpf_obj_pin; >> + bpf_perf_event_read_simple; >> + bpf_prog_attach; >> + bpf_prog_detach; >> + bpf_prog_detach2; >> + bpf_prog_get_fd_by_id; >> + bpf_prog_get_next_id; >> + bpf_prog_load; >> + bpf_prog_load_xattr; >> + bpf_prog_query; >> + bpf_program__fd; >> + bpf_program__is_kprobe; >> + bpf_program__is_perf_event; >> + bpf_program__is_raw_tracepoint; >> + bpf_program__is_sched_act; >> + bpf_program__is_sched_cls; >> + bpf_program__is_socket_filter; >> + bpf_program__is_tracepoint; >> + bpf_program__is_xdp; >> + bpf_program__load; >> + bpf_program__next; >> + bpf_program__nth_fd; >> + bpf_program__pin; >> + bpf_program__pin_instance; >> + bpf_program__prev; >> + bpf_program__priv; >> + bpf_program__set_expected_attach_type; >> + bpf_program__set_ifindex; >> + bpf_program__set_kprobe; >> + bpf_program__set_perf_event; >> + bpf_program__set_prep; >> + bpf_program__set_priv; >> + bpf_program__set_raw_tracepoint; >> + bpf_program__set_sched_act; >> + bpf_program__set_sched_cls; >> + bpf_program__set_socket_filter; >> + bpf_program__set_tracepoint; >> + bpf_program__set_type; >> + bpf_program__set_xdp; >> + bpf_program__title; >> + bpf_program__unload; >> + bpf_program__unpin; >> + bpf_program__unpin_instance; >> + bpf_prog_test_run; >> + bpf_raw_tracepoint_open; >> + bpf_set_link_xdp_fd; >> + bpf_task_fd_query; >> + bpf_verify_program; >> + btf__fd; >> + btf__find_by_name; >> + btf__free; >> + btf_get_from_id; > btf_get_from_id => btf__get_from_id? this one makes sense indeed. Martin, thoughts? >> + btf__name_by_offset; >> + btf__new; >> + btf__resolve_size; >> + btf__resolve_type; >> + btf__type_by_id; >> + libbpf_attach_type_by_name; >> + libbpf_get_error; >> + libbpf_prog_type_by_name; >> + libbpf_set_print; >> + libbpf_strerror; > > Anything else? We have btf__, bpf_program__ prefixes with double "_" > while with libbpf_, bpf_<wrapper> as single "_". Not sure whether they > need change or not. the convention is that syscall wrappers like bpf_map_lookup_elem() have single _ and map one-to-one to syscall commands. Double _ is for objects. That's a coding style of perf. Today btf, bpf_program, bpf_map, bpf_objects are objects. libbpf_ is a prefix for auxiliary funcs. We need to document it in a readme file.
