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

2018-11-23 Thread Andrey Ignatov
Daniel Borkmann  [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=DwICaQ=5VD0RTtNlTh3ycd41b3MUw=3jAokpHyGuCuJ834j-tttQ=DaYaGCQXLC7Lqf82VhtHjSPrf6R4RdDMKrDDR2T9XPA=nN4Sz6re4n-pP50ICk8s0M-nu_535bblSiVPeEdGiFk=
> >>>
> >>> Signed-off-by: Andrey Ignatov 
> >>> ---
> >>>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 ..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;
> >>> + 

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

2018-11-23 Thread Andrey Ignatov
Martin Lau  [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 
> > >> ---
> > >>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 ..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;
> > >> +

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

2018-11-23 Thread 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 
> >> ---
> >>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 ..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;
> >> +   

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

2018-11-22 Thread 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 
>>> ---
>>>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 ..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;
>>> +   

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

2018-11-21 Thread 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 
>> ---
>>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 ..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;
>> +

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

2018-11-21 Thread 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 
> ---
>   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 ..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;
> +  

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

2018-11-21 Thread 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 
---
 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 ..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;
+