* [PATCH bpf-next 0/2] libbpf: ABI versioning @ 2018-11-21 17:40 Andrey Ignatov 2018-11-21 17:40 ` [PATCH bpf-next 1/2] libbpf: Add version script for DSO Andrey Ignatov 2018-11-21 17:40 ` [PATCH bpf-next 2/2] libbpf: Verify versioned symbols Andrey Ignatov 0 siblings, 2 replies; 9+ messages in thread From: Andrey Ignatov @ 2018-11-21 17:40 UTC (permalink / raw) To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team This patch set adds ABI versioning to libbpf. Patch 1 adds version script and has more details on why it's needed. Patch 2 adds simple check that all global symbols are versioned. Andrey Ignatov (2): libbpf: Add version script for DSO libbpf: Verify versioned symbols tools/lib/bpf/Makefile | 23 +++++++- tools/lib/bpf/libbpf.map | 120 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 tools/lib/bpf/libbpf.map -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next 1/2] libbpf: Add version script for DSO 2018-11-21 17:40 [PATCH bpf-next 0/2] libbpf: ABI versioning Andrey Ignatov @ 2018-11-21 17:40 ` Andrey Ignatov 2018-11-21 20:18 ` Yonghong Song 2018-11-21 17:40 ` [PATCH bpf-next 2/2] libbpf: Verify versioned symbols Andrey Ignatov 1 sibling, 1 reply; 9+ messages in thread From: Andrey Ignatov @ 2018-11-21 17:40 UTC (permalink / raw) To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team 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. [1] https://www.akkadia.org/drepper/dsohowto.pdf Signed-off-by: Andrey Ignatov <rdna@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; + 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__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; + local: + *; +}; -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO 2018-11-21 17:40 ` [PATCH bpf-next 1/2] libbpf: Add version script for DSO Andrey Ignatov @ 2018-11-21 20:18 ` Yonghong Song 2018-11-21 22:22 ` Alexei Starovoitov 0 siblings, 1 reply; 9+ messages in thread From: Yonghong Song @ 2018-11-21 20:18 UTC (permalink / raw) To: Andrey Ignatov, netdev@vger.kernel.org Cc: ast@kernel.org, daniel@iogearbox.net, Kernel Team 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 <rdna@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? > + 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? > + 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. > + local: > + *; > +}; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO 2018-11-21 20:18 ` Yonghong Song @ 2018-11-21 22:22 ` Alexei Starovoitov 2018-11-22 10:27 ` Daniel Borkmann 2018-11-23 18:44 ` Martin Lau 0 siblings, 2 replies; 9+ messages in thread From: Alexei Starovoitov @ 2018-11-21 22:22 UTC (permalink / raw) To: Yonghong Song, Andrey Ignatov, netdev@vger.kernel.org Cc: ast@kernel.org, daniel@iogearbox.net, Kernel Team 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 <rdna@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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO 2018-11-21 22:22 ` Alexei Starovoitov @ 2018-11-22 10:27 ` Daniel Borkmann 2018-11-24 0:35 ` Andrey Ignatov 2018-11-23 18:44 ` Martin Lau 1 sibling, 1 reply; 9+ messages in thread From: Daniel Borkmann @ 2018-11-22 10:27 UTC (permalink / raw) To: Alexei Starovoitov, Yonghong Song, Andrey Ignatov, netdev@vger.kernel.org Cc: ast@kernel.org, Kernel Team On 11/21/2018 11:22 PM, Alexei Starovoitov wrote: > 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, could you add a documentation file into tools/lib/bpf/ where we keep note on this process? >>> [1] https://www.akkadia.org/drepper/dsohowto.pdf >>> >>> Signed-off-by: Andrey Ignatov <rdna@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. With that in mind, should we also change prototype for things like bpf_set_link_xdp_fd() and bpf_perf_event_read_simple()? It's not a syscall wrapper mapping one-to-one either. Would libbpf_ prefix be a better fit in here, or some new bpf_misc__ prefix for everything else which is not directly related to libbpf internals and neither to the existing objects? (bpf_misc__ would probably be my slight preference fwiw.) > We need to document it in a readme file. Fully agree, this should probably go to that same document so that this convention is clear to everyone extending libbpf. Thanks, Daniel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO 2018-11-22 10:27 ` Daniel Borkmann @ 2018-11-24 0:35 ` Andrey Ignatov 0 siblings, 0 replies; 9+ messages in thread From: Andrey Ignatov @ 2018-11-24 0:35 UTC (permalink / raw) To: Alexei Starovoitov, Yonghong Song, Daniel Borkmann Cc: netdev@vger.kernel.org, ast@kernel.org, Kernel Team Daniel Borkmann <daniel@iogearbox.net> [Thu, 2018-11-22 02:28 -0800]: > On 11/21/2018 11:22 PM, Alexei Starovoitov wrote: > > 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, could you add a documentation file into tools/lib/bpf/ where we > keep note on this process? That makes sense. I'll add documentation. I think it'll take time to figure out a policy to maintain ABI that works well (like when to bump version, etc). I'll describe what is reasonable from my point of view so that we have a starting point and we can refine / adjust it to reality later. > >>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__www.akkadia.org_drepper_dsohowto.pdf&d=DwICaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=3jAokpHyGuCuJ834j-tttQ&m=DaYaGCQXLC7Lqf82VhtHjSPrf6R4RdDMKrDDR2T9XPA&s=nN4Sz6re4n-pP50ICk8s0M-nu_535bblSiVPeEdGiFk&e= > >>> > >>> Signed-off-by: Andrey Ignatov <rdna@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. > > With that in mind, should we also change prototype for things like > bpf_set_link_xdp_fd() and bpf_perf_event_read_simple()? It's not a > syscall wrapper mapping one-to-one either. > > Would libbpf_ prefix be a better fit in here, or some new bpf_misc__ > prefix for everything else which is not directly related to libbpf > internals and neither to the existing objects? (bpf_misc__ would > probably be my slight preference fwiw.) > > > We need to document it in a readme file. > > Fully agree, this should probably go to that same document so that > this convention is clear to everyone extending libbpf. These both are good points. I'll try to document API naming convetion to the extent I have context on this and combine it with ABI documentation. As for bpf_set_link_xdp_fd() and bpf_perf_event_read_simple() I'm not really sure since I hardly ever use. I can rename things that we're sure about (like Martin's s/btf_get_from_id/btf__get_from_id/ above, that I'll include in v2). > > Thanks, > Daniel -- Andrey Ignatov ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO 2018-11-21 22:22 ` Alexei Starovoitov 2018-11-22 10:27 ` Daniel Borkmann @ 2018-11-23 18:44 ` Martin Lau 2018-11-23 20:36 ` Andrey Ignatov 1 sibling, 1 reply; 9+ messages in thread From: Martin Lau @ 2018-11-23 18:44 UTC (permalink / raw) To: Alexei Starovoitov Cc: Yonghong Song, Andrey Ignatov, netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, Kernel Team On Wed, Nov 21, 2018 at 02:22:14PM -0800, Alexei Starovoitov wrote: > 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 <rdna@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. Agree with keeping btf's get_fd_by_id() name to match with other get_fd_by_id() interfaces. > > >> + 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? Agree. We overlooked this in the earlier func_info patch set. I also have this change locally in my current working patchset. I think it makes sense to have the change go with this libbpf version patchset as a cleanup effort. It is what I have. Feel free to reuse any of it: >From 32a1489619184d7965f235a86f082f2710a2ba3c Mon Sep 17 00:00:00 2001 From: Martin KaFai Lau <kafai@fb.com> Date: Wed, 21 Nov 2018 09:21:17 -0800 Subject: [PATCH 3/8] bpf: libbpf: Name changing for btf_get_from_id s/btf_get_from_id/btf__get_from_id/ to restore the API naming convention. Signed-off-by: Martin KaFai Lau <kafai@fb.com> --- tools/bpf/bpftool/map.c | 4 ++-- tools/bpf/bpftool/prog.c | 2 +- tools/lib/bpf/btf.c | 2 +- tools/lib/bpf/btf.h | 2 +- tools/testing/selftests/bpf/test_btf.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c index a1ae2a3e9fef..96be42f288f5 100644 --- a/tools/bpf/bpftool/map.c +++ b/tools/bpf/bpftool/map.c @@ -711,7 +711,7 @@ static int do_dump(int argc, char **argv) prev_key = NULL; - err = btf_get_from_id(info.btf_id, &btf); + err = btf__get_from_id(info.btf_id, &btf); if (err) { p_err("failed to get btf"); goto exit_free; @@ -855,7 +855,7 @@ static int do_lookup(int argc, char **argv) } /* here means bpf_map_lookup_elem() succeeded */ - err = btf_get_from_id(info.btf_id, &btf); + err = btf__get_from_id(info.btf_id, &btf); if (err) { p_err("failed to get btf"); goto exit_free; diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 37b1daf19da6..521a1073d1b4 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -622,7 +622,7 @@ static int do_dump(int argc, char **argv) goto err_free; } - if (info.btf_id && btf_get_from_id(info.btf_id, &btf)) { + if (info.btf_id && btf__get_from_id(info.btf_id, &btf)) { p_err("failed to get btf"); goto err_free; } diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c index 13ddc4bd24ee..eadcf8dfd295 100644 --- a/tools/lib/bpf/btf.c +++ b/tools/lib/bpf/btf.c @@ -415,7 +415,7 @@ const char *btf__name_by_offset(const struct btf *btf, __u32 offset) return NULL; } -int btf_get_from_id(__u32 id, struct btf **btf) +int btf__get_from_id(__u32 id, struct btf **btf) { struct bpf_btf_info btf_info = { 0 }; __u32 len = sizeof(btf_info); diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h index 386b2ffc32a3..2991b9f93e16 100644 --- a/tools/lib/bpf/btf.h +++ b/tools/lib/bpf/btf.h @@ -69,7 +69,7 @@ LIBBPF_API __s64 btf__resolve_size(const struct btf *btf, __u32 type_id); LIBBPF_API int btf__resolve_type(const struct btf *btf, __u32 type_id); LIBBPF_API int btf__fd(const struct btf *btf); LIBBPF_API const char *btf__name_by_offset(const struct btf *btf, __u32 offset); -LIBBPF_API int btf_get_from_id(__u32 id, struct btf **btf); +LIBBPF_API int btf__get_from_id(__u32 id, struct btf **btf); struct btf_ext *btf_ext__new(__u8 *data, __u32 size, btf_print_fn_t err_log); void btf_ext__free(struct btf_ext *btf_ext); diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c index 7b1b160d6e67..aa779f48cc2b 100644 --- a/tools/testing/selftests/bpf/test_btf.c +++ b/tools/testing/selftests/bpf/test_btf.c @@ -2604,7 +2604,7 @@ static int do_test_file(unsigned int test_num) goto done; } - err = btf_get_from_id(info.btf_id, &btf); + err = btf__get_from_id(info.btf_id, &btf); if (CHECK(err, "cannot get btf from kernel, err: %d", err)) goto done; -- 2.17.1 > > >> + 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. ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO 2018-11-23 18:44 ` Martin Lau @ 2018-11-23 20:36 ` Andrey Ignatov 0 siblings, 0 replies; 9+ messages in thread From: Andrey Ignatov @ 2018-11-23 20:36 UTC (permalink / raw) To: Martin Lau Cc: Alexei Starovoitov, Yonghong Song, netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, Kernel Team Martin Lau <kafai@fb.com> [Fri, 2018-11-23 10:44 -0800]: > On Wed, Nov 21, 2018 at 02:22:14PM -0800, Alexei Starovoitov wrote: > > 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 <rdna@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. > Agree with keeping btf's get_fd_by_id() name to match with > other get_fd_by_id() interfaces. > > > > > >> + 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? > Agree. We overlooked this in the earlier func_info patch set. > I also have this change locally in my current working patchset. > > I think it makes sense to have the change go with this libbpf version > patchset as a cleanup effort. It is what I have. Feel free > to reuse any of it: Ok, sounds good, let's rename it. I'll add this patch as is in v2 of my patch set. Thanks Martin. > From 32a1489619184d7965f235a86f082f2710a2ba3c Mon Sep 17 00:00:00 2001 > From: Martin KaFai Lau <kafai@fb.com> > Date: Wed, 21 Nov 2018 09:21:17 -0800 > Subject: [PATCH 3/8] bpf: libbpf: Name changing for btf_get_from_id > > s/btf_get_from_id/btf__get_from_id/ to restore the API naming convention. > > Signed-off-by: Martin KaFai Lau <kafai@fb.com> > --- > tools/bpf/bpftool/map.c | 4 ++-- > tools/bpf/bpftool/prog.c | 2 +- > tools/lib/bpf/btf.c | 2 +- > tools/lib/bpf/btf.h | 2 +- > tools/testing/selftests/bpf/test_btf.c | 2 +- > 5 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c > index a1ae2a3e9fef..96be42f288f5 100644 > --- a/tools/bpf/bpftool/map.c > +++ b/tools/bpf/bpftool/map.c > @@ -711,7 +711,7 @@ static int do_dump(int argc, char **argv) > > prev_key = NULL; > > - err = btf_get_from_id(info.btf_id, &btf); > + err = btf__get_from_id(info.btf_id, &btf); > if (err) { > p_err("failed to get btf"); > goto exit_free; > @@ -855,7 +855,7 @@ static int do_lookup(int argc, char **argv) > } > > /* here means bpf_map_lookup_elem() succeeded */ > - err = btf_get_from_id(info.btf_id, &btf); > + err = btf__get_from_id(info.btf_id, &btf); > if (err) { > p_err("failed to get btf"); > goto exit_free; > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c > index 37b1daf19da6..521a1073d1b4 100644 > --- a/tools/bpf/bpftool/prog.c > +++ b/tools/bpf/bpftool/prog.c > @@ -622,7 +622,7 @@ static int do_dump(int argc, char **argv) > goto err_free; > } > > - if (info.btf_id && btf_get_from_id(info.btf_id, &btf)) { > + if (info.btf_id && btf__get_from_id(info.btf_id, &btf)) { > p_err("failed to get btf"); > goto err_free; > } > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 13ddc4bd24ee..eadcf8dfd295 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -415,7 +415,7 @@ const char *btf__name_by_offset(const struct btf *btf, __u32 offset) > return NULL; > } > > -int btf_get_from_id(__u32 id, struct btf **btf) > +int btf__get_from_id(__u32 id, struct btf **btf) > { > struct bpf_btf_info btf_info = { 0 }; > __u32 len = sizeof(btf_info); > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > index 386b2ffc32a3..2991b9f93e16 100644 > --- a/tools/lib/bpf/btf.h > +++ b/tools/lib/bpf/btf.h > @@ -69,7 +69,7 @@ LIBBPF_API __s64 btf__resolve_size(const struct btf *btf, __u32 type_id); > LIBBPF_API int btf__resolve_type(const struct btf *btf, __u32 type_id); > LIBBPF_API int btf__fd(const struct btf *btf); > LIBBPF_API const char *btf__name_by_offset(const struct btf *btf, __u32 offset); > -LIBBPF_API int btf_get_from_id(__u32 id, struct btf **btf); > +LIBBPF_API int btf__get_from_id(__u32 id, struct btf **btf); > > struct btf_ext *btf_ext__new(__u8 *data, __u32 size, btf_print_fn_t err_log); > void btf_ext__free(struct btf_ext *btf_ext); > diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c > index 7b1b160d6e67..aa779f48cc2b 100644 > --- a/tools/testing/selftests/bpf/test_btf.c > +++ b/tools/testing/selftests/bpf/test_btf.c > @@ -2604,7 +2604,7 @@ static int do_test_file(unsigned int test_num) > goto done; > } > > - err = btf_get_from_id(info.btf_id, &btf); > + err = btf__get_from_id(info.btf_id, &btf); > if (CHECK(err, "cannot get btf from kernel, err: %d", err)) > goto done; > > -- > 2.17.1 > > > > > >> + 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. -- Andrey Ignatov ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next 2/2] libbpf: Verify versioned symbols 2018-11-21 17:40 [PATCH bpf-next 0/2] libbpf: ABI versioning Andrey Ignatov 2018-11-21 17:40 ` [PATCH bpf-next 1/2] libbpf: Add version script for DSO Andrey Ignatov @ 2018-11-21 17:40 ` Andrey Ignatov 1 sibling, 0 replies; 9+ messages in thread From: Andrey Ignatov @ 2018-11-21 17:40 UTC (permalink / raw) To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team Since ABI versioning info is kept separately from the code it's easy to forget to update it while adding a new API. Add simple verification that all global symbols exported with LIBBPF_API are versioned in libbpf.map version script. The idea is to check that number of global symbols in libbpf-in.o, that is the input to the linker, matches with number of unique versioned symbols in libbpf.so, that is the output of the linker. If these numbers don't match, it may mean some symbol was not versioned and make will fail. "Unique" means that if a symbol is present in more than one version of ABI due to ABI changes, it'll be counted once. Another option to calculate number of global symbols in the "input" could be to count number of LIBBPF_ABI entries in C headers but it seems to be fragile. Example of output when a symbol is missing in version script: ... LD libbpf-in.o LINK libbpf.a LINK libbpf.so Warning: Num of global symbols in libbpf-in.o (115) does NOT match with num of versioned symbols in libbpf.so (114). Please make sure all LIBBPF_API symbols are versioned in libbpf.map. make: *** [check_abi] Error 1 Signed-off-by: Andrey Ignatov <rdna@fb.com> --- tools/lib/bpf/Makefile | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile index d76c41fa2d39..742ef0d5210e 100644 --- a/tools/lib/bpf/Makefile +++ b/tools/lib/bpf/Makefile @@ -147,13 +147,18 @@ BPF_IN := $(OUTPUT)libbpf-in.o LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE)) VERSION_SCRIPT := libbpf.map +GLOBAL_SYM_COUNT = $(shell readelf -s $(BPF_IN) | \ + awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {s++} END{print s}') +VERSIONED_SYM_COUNT = $(shell readelf -s $(OUTPUT)libbpf.so | \ + grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l) + CMD_TARGETS = $(LIB_FILE) TARGETS = $(CMD_TARGETS) all: fixdep all_cmd -all_cmd: $(CMD_TARGETS) +all_cmd: $(CMD_TARGETS) check $(BPF_IN): force elfdep bpfdep @(test -f ../../include/uapi/linux/bpf.h -a -f ../../../include/uapi/linux/bpf.h && ( \ @@ -177,6 +182,18 @@ $(OUTPUT)libbpf.so: $(BPF_IN) $(OUTPUT)libbpf.a: $(BPF_IN) $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ +check: check_abi + +check_abi: $(OUTPUT)libbpf.so + @if [ "$(GLOBAL_SYM_COUNT)" != "$(VERSIONED_SYM_COUNT)" ]; then \ + echo "Warning: Num of global symbols in $(BPF_IN)" \ + "($(GLOBAL_SYM_COUNT)) does NOT match with num of" \ + "versioned symbols in $^ ($(VERSIONED_SYM_COUNT))." \ + "Please make sure all LIBBPF_API symbols are" \ + "versioned in $(VERSION_SCRIPT)." >&2; \ + exit 1; \ + fi + define do_install if [ ! -d '$(DESTDIR_SQ)$2' ]; then \ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \ -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-11-24 11:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-21 17:40 [PATCH bpf-next 0/2] libbpf: ABI versioning Andrey Ignatov 2018-11-21 17:40 ` [PATCH bpf-next 1/2] libbpf: Add version script for DSO Andrey Ignatov 2018-11-21 20:18 ` Yonghong Song 2018-11-21 22:22 ` Alexei Starovoitov 2018-11-22 10:27 ` Daniel Borkmann 2018-11-24 0:35 ` Andrey Ignatov 2018-11-23 18:44 ` Martin Lau 2018-11-23 20:36 ` Andrey Ignatov 2018-11-21 17:40 ` [PATCH bpf-next 2/2] libbpf: Verify versioned symbols Andrey Ignatov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).