[PATCH bpf-next 0/2] libbpf: ABI versioning

[PATCH bpf-next 0/2] libbpf: ABI versioning

[Andrey Ignatov]

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

[PATCH bpf-next 1/2] libbpf: Add version script for DSO

[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_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

[PATCH bpf-next 2/2] libbpf: Verify versioned symbols

[Andrey Ignatov]

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

Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO

[Yonghong Song]

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:
> +		*;
> +};
> 

Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO

[Alexei Starovoitov]

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.

Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO

[Daniel Borkmann]

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

Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO

[Martin Lau]

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.

Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO

[Andrey Ignatov]

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

Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO

[Andrey Ignatov]

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