| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <45175828-13df-6cf1-6817-ddffa381f0ec@fb.com>
Date: Wed, 21 Nov 2018 22:22:14 +0000
From: Alexei Starovoitov <ast@...com>
To: Yonghong Song <yhs@...com>, Andrey Ignatov <rdna@...com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "ast@...nel.org" <ast@...nel.org>,
"daniel@...earbox.net" <daniel@...earbox.net>,
Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
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@...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.
Powered by blists - more mailing lists