libbpf: ABI versioning
Subsystems (7)
cover libbpf: ABI versioning
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
Signature · 1 line
2.17.1
PATCH 1/2 libbpf: Add version script for DSO
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
--- 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
tools/lib/bpf/Makefile +3 -1
@@ -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 $@ $^
tools/lib/bpf/libbpf.map new file +120 -0
120 lines not on this page · raw message
Signature · 1 line
2.17.1
Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
Quoted 20 lines · Andrey Ignatov
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_APImacro 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_APImacro, 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].
Quoted 2 lines
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?
Quoted 38 lines
[1] https://www.akkadia.org/drepper/dsohowto.pdf
Signed-off-by Andrey Ignatov <[email protected]>--- 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.maptools/lib/bpf/Makefile +3
-1@@ -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 $@ $^tools/lib/bpf/libbpf.map new file +0
-0
tools/lib/bpf/libbpf.map line 3
Quoted 3 lines
tools/lib/bpf/libbpf.map new file +3
-0@@ -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?
Quoted 92 lines
tools/lib/bpf/libbpf.map new file +92
-0+ 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;
tools/lib/bpf/libbpf.map line 107
Quoted 10 lines
tools/lib/bpf/libbpf.map new file +10
-0+ 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?
tools/lib/bpf/libbpf.map line 117
Quoted 10 lines
tools/lib/bpf/libbpf.map new file +10
-0+ 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.
Quoted 4 lines
tools/lib/bpf/libbpf.map new file +3
-0+ local: + *; +};
Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
Quoted 60 lines · Yonghong Song
Quoted 20 lines · Andrey Ignatov
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_APImacro 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_APImacro, 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].Quoted 2 lines
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?
Quoted 30 lines
[1] https://www.akkadia.org/drepper/dsohowto.pdf
Signed-off-by Andrey Ignatov <[email protected]>--- 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.maptools/lib/bpf/Makefile +3
-1@@ -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 $@
Quoted 15 lines
tools/lib/bpf/libbpf.map line 3
Quoted 11 lines
tools/lib/bpf/Makefile +0
-0$(OUTPUT)libbpf.a: $(BPF_IN) $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^tools/lib/bpf/libbpf.map new file +3
-0@@ -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.
Quoted 95 lines
Quoted 95 lines
tools/lib/bpf/libbpf.map new file +95
-0+ 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;
Quoted 8 lines
tools/lib/bpf/libbpf.map line 107
Quoted 7 lines
tools/lib/bpf/libbpf.map new file +7
-0+ 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?
Quoted 6 lines
Quoted 6 lines
tools/lib/bpf/libbpf.map new file +6
-0+ btf__name_by_offset; + btf__new; + btf__resolve_size; + btf__resolve_type; + btf__type_by_id; + libbpf_attach_type_by_name;
Quoted 8 lines
tools/lib/bpf/libbpf.map line 117
Quoted 4 lines
tools/lib/bpf/libbpf.map new file +4
-0+ libbpf_get_error; + libbpf_prog_type_by_name; + libbpf_set_print; + libbpf_strerror;Anything else? We have
btf__,bpf_program__prefixes with double "_" while withlibbpf_, 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.
Daniel Borkmann
maintainerRe: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
Quoted 22 lines · Alexei Starovoitov
Quoted 21 lines · Yonghong Song
Quoted 20 lines · Andrey Ignatov
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_APImacro 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_APImacro, 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].
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
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?
Quoted 170 lines
Quoted 29 lines
Quoted 29 lines
[1] https://www.akkadia.org/drepper/dsohowto.pdf
Signed-off-by Andrey Ignatov <[email protected]>--- 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.maptools/lib/bpf/Makefile +3
-1@@ -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 $@Quoted 15 lines
tools/lib/bpf/libbpf.map line 3
Quoted 11 lines
tools/lib/bpf/Makefile +0
-0$(OUTPUT)libbpf.a: $(BPF_IN) $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^tools/lib/bpf/libbpf.map new file +3
-0@@ -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.Quoted 95 lines
Quoted 95 lines
tools/lib/bpf/libbpf.map new file +95
-0+ 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;Quoted 8 lines
tools/lib/bpf/libbpf.map line 107
Quoted 7 lines
tools/lib/bpf/libbpf.map new file +7
-0+ 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?
Quoted 6 lines
Quoted 6 lines
tools/lib/bpf/libbpf.map new file +6
-0+ btf__name_by_offset; + btf__new; + btf__resolve_size; + btf__resolve_type; + btf__type_by_id; + libbpf_attach_type_by_name;Quoted 8 lines
tools/lib/bpf/libbpf.map line 117
Quoted 4 lines
tools/lib/bpf/libbpf.map new file +4
-0+ libbpf_get_error; + libbpf_prog_type_by_name; + libbpf_set_print; + libbpf_strerror;Anything else? We have
btf__,bpf_program__prefixes with double "_" while withlibbpf_, bpf_<wrapper> as single "_". Not sure whether they need change or not.
Quoted 5 lines
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_objectsare 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.)
Quoted 1 line
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
Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
Daniel Borkmann <[email protected]> [Thu, 2018-11-22 02:28 -0800]:
Quoted 26 lines · Alexei Starovoitov
Quoted 25 lines · Yonghong Song
Quoted 24 lines · Andrey Ignatov
Quoted 22 lines
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_APImacro 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_APImacro, 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.
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
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.
Quoted 185 lines
Quoted 170 lines
Quoted 29 lines
Quoted 29 lines
Signed-off-by Andrey Ignatov <[email protected]>--- 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.maptools/lib/bpf/Makefile +3
-1@@ -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 $@Quoted 15 lines
tools/lib/bpf/libbpf.map line 3
Quoted 11 lines
tools/lib/bpf/Makefile +0
-0$(OUTPUT)libbpf.a: $(BPF_IN) $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^tools/lib/bpf/libbpf.map new file +3
-0@@ -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.Quoted 95 lines
Quoted 95 lines
tools/lib/bpf/libbpf.map new file +95
-0+ 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;Quoted 8 lines
tools/lib/bpf/libbpf.map line 107
Quoted 7 lines
tools/lib/bpf/libbpf.map new file +7
-0+ 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?
Quoted 6 lines
Quoted 6 lines
tools/lib/bpf/libbpf.map new file +6
-0+ btf__name_by_offset; + btf__new; + btf__resolve_size; + btf__resolve_type; + btf__type_by_id; + libbpf_attach_type_by_name;Quoted 8 lines
tools/lib/bpf/libbpf.map line 117
Quoted 4 lines
tools/lib/bpf/libbpf.map new file +4
-0+ libbpf_get_error; + libbpf_prog_type_by_name; + libbpf_set_print; + libbpf_strerror;Anything else? We have
btf__,bpf_program__prefixes with double "_" while withlibbpf_, bpf_<wrapper> as single "_". Not sure whether they need change or not.Quoted 5 lines
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_objectsare 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()andbpf_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 newbpf_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.)
Quoted 4 lines
Quoted 1 line
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).
Quoted 3 lines
Thanks,
Daniel
Signature · 1 line
Andrey Ignatov
Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
In reply to Alexei StarovoitovQuoted 77 lines · Alexei Starovoitov
Quoted 60 lines · Yonghong Song
Quoted 20 lines · Andrey Ignatov
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_APImacro 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_APImacro, 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].Quoted 2 lines
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?
Quoted 30 lines
[1] https://www.akkadia.org/drepper/dsohowto.pdf
Signed-off-by Andrey Ignatov <[email protected]>--- 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.maptools/lib/bpf/Makefile +3
-1@@ -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 $@Quoted 15 lines
tools/lib/bpf/libbpf.map line 3
Quoted 11 lines
tools/lib/bpf/Makefile +0
-0$(OUTPUT)libbpf.a: $(BPF_IN) $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^tools/lib/bpf/libbpf.map new file +3
-0@@ -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?
Quoted 3 lines
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.
Quoted 105 lines
Quoted 96 lines
Quoted 96 lines
tools/lib/bpf/libbpf.map new file +95
-0+ 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;Quoted 8 lines
tools/lib/bpf/libbpf.map line 105
Quoted 7 lines
tools/lib/bpf/libbpf.map new file +7
-0+ 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?
Quoted 1 line
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:
Quoted 1 line
From
32a1489619184d7965f235a86f082f2710a2ba3cMon Sep 17 00:00:00 2001
From: Martin KaFai Lau <[email protected]>
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.
--- 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(-)
tools/bpf/bpftool/map.c +2 -2
@@ -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;
tools/bpf/bpftool/prog.c +1 -1
@@ -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; }
tools/lib/bpf/btf.c +1 -1
@@ -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);
tools/lib/bpf/btf.h +1 -1
@@ -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);
tools/testing/selftests/bpf/test_btf.c +1 -1
@@ -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;
Signature · 1 line
2.17.1
Quoted 23 lines
Quoted 15 lines
tools/lib/bpf/libbpf.map line 115
Quoted 11 lines
tools/lib/bpf/libbpf.map new file +10
-0+ 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 withlibbpf_, 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_objectsare objects.libbpf_is a prefix for auxiliary funcs.We need to document it in a readme file.
Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
Martin Lau <[email protected]> [Fri, 2018-11-23 10:44 -0800]:
Quoted 189 lines · Alexei Starovoitov
Quoted 77 lines · Yonghong Song
Quoted 60 lines · Andrey Ignatov
Quoted 20 lines
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_APImacro 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_APImacro, 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].Quoted 2 lines
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?
Quoted 30 lines
[1] https://www.akkadia.org/drepper/dsohowto.pdf
Signed-off-by Andrey Ignatov <[email protected]>--- 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.maptools/lib/bpf/Makefile +3
-1@@ -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 $@Quoted 15 lines
tools/lib/bpf/libbpf.map line 3
Quoted 11 lines
tools/lib/bpf/Makefile +0
-0$(OUTPUT)libbpf.a: $(BPF_IN) $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^tools/lib/bpf/libbpf.map new file +3
-0@@ -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?Quoted 3 lines
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 otherget_fd_by_id()interfaces.Quoted 105 lines
Quoted 104 lines
Quoted 93 lines
tools/lib/bpf/libbpf.map new file +92
-0+ 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;tools/lib/bpf/libbpf.map line 105
Quoted 10 lines
tools/lib/bpf/libbpf.map new file +10
-0+ 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?
Quoted 7 lines
Quoted 1 line
this one makes sense indeed. Martin, thoughts?
Agree. We overlooked this in the earlier
func_infopatch 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.
Quoted 116 lines
From
32a1489619184d7965f235a86f082f2710a2ba3cMon Sep 17 00:00:00 2001
From: Martin KaFai Lau <[email protected]>
Date: Wed, 21 Nov 2018 09:21:17 -0800
Subject: [PATCH 3/8] bpf: libbpf: Name changing forbtf_get_from_ids/btf_get_from_id/btf__get_from_id/ to restore the API naming convention.
Signed-off-by Martin KaFai Lau <[email protected]>--- 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(-)tools/bpf/bpftool/map.c +2
-2@@ -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;tools/bpf/bpftool/prog.c +1
-1@@ -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; }tools/lib/bpf/btf.c +1
-1@@ -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);tools/lib/bpf/btf.h +1
-1@@ -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);tools/testing/selftests/bpf/test_btf.c +1
-1@@ -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;Signature · 1 line
2.17.1Quoted 23 lines
Quoted 15 lines
tools/lib/bpf/libbpf.map line 115
Quoted 11 lines
tools/lib/bpf/libbpf.map new file +10
-0+ 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 withlibbpf_, 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_objectsare objects.libbpf_is a prefix for auxiliary funcs.We need to document it in a readme file.
Signature · 1 line
Andrey Ignatov
PATCH 2/2 libbpf: Verify versioned symbols
In reply to Andrey IgnatovSince 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
--- tools/lib/bpf/Makefile | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
tools/lib/bpf/Makefile +18 -1
@@ -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'; \
Signature · 1 line
2.17.1