Message ID | 20190322234755.29306-2-daniel@iogearbox.net |
---|---|
State | Changes Requested |
Delegated to: | BPF Maintainers |
Headers | show |
Series | libbpf fix up and clarify version info | expand |
On 03/23, Daniel Borkmann wrote: > Even though libbpf's versioning script for the linker (libbpf.map) > is pointing to 0.0.2, the BPF_EXTRAVERSION in the Makefile has > not been updated along with it and is therefore still on 0.0.1. > > While fixing up, I also noticed that the generated shared object > versioning information is missing, typical convention is to have > a linker name (libbpf.so), soname (libbpf.so.0) and real name > (libbpf.so.0.0.2) for library management. This is based upon the > LIBBPF_VERSION as well. > > The build will then produce the following bpf libraries: Thank you for taking care of this! Couple of nits/questions below. > > # ll libbpf* > libbpf.a > libbpf.so -> libbpf.so.0.0.2 > libbpf.so.0 -> libbpf.so.0.0.2 > libbpf.so.0.0.2 > > And the they are also properly installed: > > # rm -rf /tmp/bld; mkdir /tmp/bld; make -j$(nproc) O=/tmp/bld install > > Auto-detecting system features: > ... libelf: [ on ] > ... bpf: [ on ] > > CC /tmp/bld/libbpf.o > CC /tmp/bld/bpf.o > CC /tmp/bld/nlattr.o > CC /tmp/bld/btf.o > CC /tmp/bld/libbpf_errno.o > CC /tmp/bld/str_error.o > CC /tmp/bld/netlink.o > CC /tmp/bld/bpf_prog_linfo.o > CC /tmp/bld/libbpf_probes.o > CC /tmp/bld/xsk.o > LD /tmp/bld/libbpf-in.o > LINK /tmp/bld/libbpf.a > LINK /tmp/bld/libbpf.so.0.0.2 > LINK /tmp/bld/test_libbpf > INSTALL /tmp/bld/libbpf.a > INSTALL /tmp/bld/libbpf.so.0.0.2 > > # ll /usr/local/lib64/libbpf.* > /usr/local/lib64/libbpf.a > /usr/local/lib64/libbpf.so -> libbpf.so.0.0.2 > /usr/local/lib64/libbpf.so.0 -> libbpf.so.0.0.2 > /usr/local/lib64/libbpf.so.0.0.2 > > Fixes: 1bf4b05810fe ("tools: bpftool: add probes for eBPF program types") > Fixes: 1b76c13e4b36 ("bpf tools: Introduce 'bpf' library and add bpf feature check") > Reported-by: Stanislav Fomichev <sdf@google.com> > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > --- > tools/lib/bpf/Makefile | 42 ++++++++++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 14 deletions(-) > > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile > index 61aaacf..ada2e90a9 100644 > --- a/tools/lib/bpf/Makefile > +++ b/tools/lib/bpf/Makefile > @@ -3,7 +3,7 @@ > > BPF_VERSION = 0 > BPF_PATCHLEVEL = 0 > -BPF_EXTRAVERSION = 1 > +BPF_EXTRAVERSION = 2 > > MAKEFLAGS += --no-print-directory > > @@ -79,8 +79,6 @@ export prefix libdir src obj > libdir_SQ = $(subst ','\'',$(libdir)) > libdir_relative_SQ = $(subst ','\'',$(libdir_relative)) > > -LIB_FILE = libbpf.a libbpf.so > - > VERSION = $(BPF_VERSION) > PATCHLEVEL = $(BPF_PATCHLEVEL) > EXTRAVERSION = $(BPF_EXTRAVERSION) > @@ -88,7 +86,10 @@ EXTRAVERSION = $(BPF_EXTRAVERSION) > OBJ = $@ > N = > > -LIBBPF_VERSION = $(BPF_VERSION).$(BPF_PATCHLEVEL).$(BPF_EXTRAVERSION) > +LIBBPF_VERSION = $(BPF_VERSION).$(BPF_PATCHLEVEL).$(BPF_EXTRAVERSION) > + > +LIB_TARGET = libbpf.a libbpf.so.$(LIBBPF_VERSION) > +LIB_FILE = libbpf.a libbpf.so* > > # Set compile option CFLAGS > ifdef EXTRA_CFLAGS > @@ -128,16 +129,18 @@ all: > export srctree OUTPUT CC LD CFLAGS V > include $(srctree)/tools/build/Makefile.include > > -BPF_IN := $(OUTPUT)libbpf-in.o > -LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE)) > -VERSION_SCRIPT := libbpf.map > +BPF_IN := $(OUTPUT)libbpf-in.o > +VERSION_SCRIPT := libbpf.map > + > +LIB_TARGET := $(addprefix $(OUTPUT),$(LIB_TARGET)) > +LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE)) > > GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN) | \ > awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {s++} END{print s}') > VERSIONED_SYM_COUNT = $(shell readelf -s --wide $(OUTPUT)libbpf.so | \ > grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l) > > -CMD_TARGETS = $(LIB_FILE) > +CMD_TARGETS = $(LIB_TARGET) > > CXX_TEST_TARGET = $(OUTPUT)test_libbpf > > @@ -170,9 +173,13 @@ $(BPF_IN): force elfdep bpfdep > echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/if_xdp.h' differs from latest version at 'include/uapi/linux/if_xdp.h'" >&2 )) || true > $(Q)$(MAKE) $(build)=libbpf > > -$(OUTPUT)libbpf.so: $(BPF_IN) > - $(QUIET_LINK)$(CC) --shared -Wl,--version-script=$(VERSION_SCRIPT) \ > - $^ -o $@ > +$(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION) > + > +$(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) > + $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_VERSION) \ Shouldn't it be -soname,libbpf.so.$(VERSION) ? > + -Wl,--version-script=$(VERSION_SCRIPT) $^ -o $@ > + @ln -sf $(@F) $(OUTPUT)libbpf.so > + @ln -sf $(@F) $(OUTPUT)libbpf.so.$(VERSION) > > $(OUTPUT)libbpf.a: $(BPF_IN) > $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ > @@ -192,6 +199,12 @@ check_abi: $(OUTPUT)libbpf.so > exit 1; \ > fi > > +define do_install_mkdir > + if [ ! -d '$(DESTDIR_SQ)$1' ]; then \ > + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \ > + fi > +endef > + > define do_install > if [ ! -d '$(DESTDIR_SQ)$2' ]; then \ > $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \ > @@ -200,8 +213,9 @@ define do_install > endef > > install_lib: all_cmd > - $(call QUIET_INSTALL, $(LIB_FILE)) \ > - $(call do_install,$(LIB_FILE),$(libdir_SQ)) > + $(call QUIET_INSTALL, $(LIB_TARGET)) \ > + $(call do_install_mkdir,$(libdir_SQ)); \ > + cp -fpR $(LIB_FILE) $(DESTDIR)$(libdir_SQ) > > install_headers: > $(call QUIET_INSTALL, headers) \ > @@ -219,7 +233,7 @@ config-clean: > > clean: > $(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \ > - *.o *~ *.a *.so .*.d .*.cmd LIBBPF-CFLAGS > + *.o *~ *.a *.so *.so.$(VERSION) .*.d .*.cmd LIBBPF-CFLAGS What about libbpf.so.$(LIBBPF_VERSION) ? > $(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf > > > -- > 2.9.5 >
On 03/23/2019 01:09 AM, Stanislav Fomichev wrote: > On 03/23, Daniel Borkmann wrote: [...] >> +$(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) >> + $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_VERSION) \ > Shouldn't it be -soname,libbpf.so.$(VERSION) ? The above is generating libbpf.so.0.0.2, and with the below two we generate symlinks to it. libbpf.so.$(VERSION) would result in libbpf.so.0 otherwise which we want to be a symlink instead. The workflow I've been following is similar to fe316723a810 ("tools lib traceevent: Add version for traceevent shared object"). >> + -Wl,--version-script=$(VERSION_SCRIPT) $^ -o $@ >> + @ln -sf $(@F) $(OUTPUT)libbpf.so >> + @ln -sf $(@F) $(OUTPUT)libbpf.so.$(VERSION) >> >> $(OUTPUT)libbpf.a: $(BPF_IN) >> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ >> @@ -192,6 +199,12 @@ check_abi: $(OUTPUT)libbpf.so >> exit 1; \ >> fi >> >> +define do_install_mkdir >> + if [ ! -d '$(DESTDIR_SQ)$1' ]; then \ >> + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \ >> + fi >> +endef >> + >> define do_install >> if [ ! -d '$(DESTDIR_SQ)$2' ]; then \ >> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \ >> @@ -200,8 +213,9 @@ define do_install >> endef >> >> install_lib: all_cmd >> - $(call QUIET_INSTALL, $(LIB_FILE)) \ >> - $(call do_install,$(LIB_FILE),$(libdir_SQ)) >> + $(call QUIET_INSTALL, $(LIB_TARGET)) \ >> + $(call do_install_mkdir,$(libdir_SQ)); \ >> + cp -fpR $(LIB_FILE) $(DESTDIR)$(libdir_SQ) >> >> install_headers: >> $(call QUIET_INSTALL, headers) \ >> @@ -219,7 +233,7 @@ config-clean: >> >> clean: >> $(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \ >> - *.o *~ *.a *.so .*.d .*.cmd LIBBPF-CFLAGS >> + *.o *~ *.a *.so *.so.$(VERSION) .*.d .*.cmd LIBBPF-CFLAGS > What about libbpf.so.$(LIBBPF_VERSION) ? That is already cleaned via $(TARGETS), so this is taken care of. >> $(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf >> >> >> -- >> 2.9.5 >>
On 03/23, Daniel Borkmann wrote: > On 03/23/2019 01:09 AM, Stanislav Fomichev wrote: > > On 03/23, Daniel Borkmann wrote: > [...] > >> +$(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) > >> + $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_VERSION) \ > > Shouldn't it be -soname,libbpf.so.$(VERSION) ? > > The above is generating libbpf.so.0.0.2, and with the below two we generate > symlinks to it. libbpf.so.$(VERSION) would result in libbpf.so.0 otherwise > which we want to be a symlink instead. The workflow I've been following is > similar to fe316723a810 ("tools lib traceevent: Add version for traceevent > shared object"). Sorry, I was not clear enough, I was talking about -Wl,-soname linker option. Shouldn't it contain "major" version? $ readelf -d libtraceevent.so.1.1.0 | grep SONAME 0x000000000000000e (SONAME) Library soname: [libtraceevent.so.1] With your patch applied: $ readelf -d libbpf.so.0.0.2 | grep SONAME 0x000000000000000e (SONAME) Library soname: [libbpf.so.0.0.2] ^^^^^^^^^^^^^ libbpf.so.0 ? > > >> + -Wl,--version-script=$(VERSION_SCRIPT) $^ -o $@ > >> + @ln -sf $(@F) $(OUTPUT)libbpf.so > >> + @ln -sf $(@F) $(OUTPUT)libbpf.so.$(VERSION) > >> > >> $(OUTPUT)libbpf.a: $(BPF_IN) > >> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ > >> @@ -192,6 +199,12 @@ check_abi: $(OUTPUT)libbpf.so > >> exit 1; \ > >> fi > >> > >> +define do_install_mkdir > >> + if [ ! -d '$(DESTDIR_SQ)$1' ]; then \ > >> + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \ > >> + fi > >> +endef > >> + > >> define do_install > >> if [ ! -d '$(DESTDIR_SQ)$2' ]; then \ > >> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \ > >> @@ -200,8 +213,9 @@ define do_install > >> endef > >> > >> install_lib: all_cmd > >> - $(call QUIET_INSTALL, $(LIB_FILE)) \ > >> - $(call do_install,$(LIB_FILE),$(libdir_SQ)) > >> + $(call QUIET_INSTALL, $(LIB_TARGET)) \ > >> + $(call do_install_mkdir,$(libdir_SQ)); \ > >> + cp -fpR $(LIB_FILE) $(DESTDIR)$(libdir_SQ) > >> > >> install_headers: > >> $(call QUIET_INSTALL, headers) \ > >> @@ -219,7 +233,7 @@ config-clean: > >> > >> clean: > >> $(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \ > >> - *.o *~ *.a *.so .*.d .*.cmd LIBBPF-CFLAGS > >> + *.o *~ *.a *.so *.so.$(VERSION) .*.d .*.cmd LIBBPF-CFLAGS > > What about libbpf.so.$(LIBBPF_VERSION) ? > > That is already cleaned via $(TARGETS), so this is taken care of. > > >> $(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf > >> > >> > >> -- > >> 2.9.5 > >> >
On 03/23/2019 01:28 AM, Stanislav Fomichev wrote: > On 03/23, Daniel Borkmann wrote: >> On 03/23/2019 01:09 AM, Stanislav Fomichev wrote: >>> On 03/23, Daniel Borkmann wrote: >> [...] >>>> +$(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) >>>> + $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_VERSION) \ >>> Shouldn't it be -soname,libbpf.so.$(VERSION) ? >> >> The above is generating libbpf.so.0.0.2, and with the below two we generate >> symlinks to it. libbpf.so.$(VERSION) would result in libbpf.so.0 otherwise >> which we want to be a symlink instead. The workflow I've been following is >> similar to fe316723a810 ("tools lib traceevent: Add version for traceevent >> shared object"). > Sorry, I was not clear enough, I was talking about -Wl,-soname linker option. > Shouldn't it contain "major" version? > > $ readelf -d libtraceevent.so.1.1.0 | grep SONAME > 0x000000000000000e (SONAME) Library soname: [libtraceevent.so.1] > > With your patch applied: > $ readelf -d libbpf.so.0.0.2 | grep SONAME > 0x000000000000000e (SONAME) Library soname: [libbpf.so.0.0.2] > > ^^^^^^^^^^^^^ > libbpf.so.0 ? Heh, agree, good catch indeed. v2 coming.
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile index 61aaacf..ada2e90a9 100644 --- a/tools/lib/bpf/Makefile +++ b/tools/lib/bpf/Makefile @@ -3,7 +3,7 @@ BPF_VERSION = 0 BPF_PATCHLEVEL = 0 -BPF_EXTRAVERSION = 1 +BPF_EXTRAVERSION = 2 MAKEFLAGS += --no-print-directory @@ -79,8 +79,6 @@ export prefix libdir src obj libdir_SQ = $(subst ','\'',$(libdir)) libdir_relative_SQ = $(subst ','\'',$(libdir_relative)) -LIB_FILE = libbpf.a libbpf.so - VERSION = $(BPF_VERSION) PATCHLEVEL = $(BPF_PATCHLEVEL) EXTRAVERSION = $(BPF_EXTRAVERSION) @@ -88,7 +86,10 @@ EXTRAVERSION = $(BPF_EXTRAVERSION) OBJ = $@ N = -LIBBPF_VERSION = $(BPF_VERSION).$(BPF_PATCHLEVEL).$(BPF_EXTRAVERSION) +LIBBPF_VERSION = $(BPF_VERSION).$(BPF_PATCHLEVEL).$(BPF_EXTRAVERSION) + +LIB_TARGET = libbpf.a libbpf.so.$(LIBBPF_VERSION) +LIB_FILE = libbpf.a libbpf.so* # Set compile option CFLAGS ifdef EXTRA_CFLAGS @@ -128,16 +129,18 @@ all: export srctree OUTPUT CC LD CFLAGS V include $(srctree)/tools/build/Makefile.include -BPF_IN := $(OUTPUT)libbpf-in.o -LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE)) -VERSION_SCRIPT := libbpf.map +BPF_IN := $(OUTPUT)libbpf-in.o +VERSION_SCRIPT := libbpf.map + +LIB_TARGET := $(addprefix $(OUTPUT),$(LIB_TARGET)) +LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE)) GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN) | \ awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {s++} END{print s}') VERSIONED_SYM_COUNT = $(shell readelf -s --wide $(OUTPUT)libbpf.so | \ grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l) -CMD_TARGETS = $(LIB_FILE) +CMD_TARGETS = $(LIB_TARGET) CXX_TEST_TARGET = $(OUTPUT)test_libbpf @@ -170,9 +173,13 @@ $(BPF_IN): force elfdep bpfdep echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/if_xdp.h' differs from latest version at 'include/uapi/linux/if_xdp.h'" >&2 )) || true $(Q)$(MAKE) $(build)=libbpf -$(OUTPUT)libbpf.so: $(BPF_IN) - $(QUIET_LINK)$(CC) --shared -Wl,--version-script=$(VERSION_SCRIPT) \ - $^ -o $@ +$(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION) + +$(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN) + $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_VERSION) \ + -Wl,--version-script=$(VERSION_SCRIPT) $^ -o $@ + @ln -sf $(@F) $(OUTPUT)libbpf.so + @ln -sf $(@F) $(OUTPUT)libbpf.so.$(VERSION) $(OUTPUT)libbpf.a: $(BPF_IN) $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^ @@ -192,6 +199,12 @@ check_abi: $(OUTPUT)libbpf.so exit 1; \ fi +define do_install_mkdir + if [ ! -d '$(DESTDIR_SQ)$1' ]; then \ + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \ + fi +endef + define do_install if [ ! -d '$(DESTDIR_SQ)$2' ]; then \ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \ @@ -200,8 +213,9 @@ define do_install endef install_lib: all_cmd - $(call QUIET_INSTALL, $(LIB_FILE)) \ - $(call do_install,$(LIB_FILE),$(libdir_SQ)) + $(call QUIET_INSTALL, $(LIB_TARGET)) \ + $(call do_install_mkdir,$(libdir_SQ)); \ + cp -fpR $(LIB_FILE) $(DESTDIR)$(libdir_SQ) install_headers: $(call QUIET_INSTALL, headers) \ @@ -219,7 +233,7 @@ config-clean: clean: $(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \ - *.o *~ *.a *.so .*.d .*.cmd LIBBPF-CFLAGS + *.o *~ *.a *.so *.so.$(VERSION) .*.d .*.cmd LIBBPF-CFLAGS $(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf
Even though libbpf's versioning script for the linker (libbpf.map) is pointing to 0.0.2, the BPF_EXTRAVERSION in the Makefile has not been updated along with it and is therefore still on 0.0.1. While fixing up, I also noticed that the generated shared object versioning information is missing, typical convention is to have a linker name (libbpf.so), soname (libbpf.so.0) and real name (libbpf.so.0.0.2) for library management. This is based upon the LIBBPF_VERSION as well. The build will then produce the following bpf libraries: # ll libbpf* libbpf.a libbpf.so -> libbpf.so.0.0.2 libbpf.so.0 -> libbpf.so.0.0.2 libbpf.so.0.0.2 And the they are also properly installed: # rm -rf /tmp/bld; mkdir /tmp/bld; make -j$(nproc) O=/tmp/bld install Auto-detecting system features: ... libelf: [ on ] ... bpf: [ on ] CC /tmp/bld/libbpf.o CC /tmp/bld/bpf.o CC /tmp/bld/nlattr.o CC /tmp/bld/btf.o CC /tmp/bld/libbpf_errno.o CC /tmp/bld/str_error.o CC /tmp/bld/netlink.o CC /tmp/bld/bpf_prog_linfo.o CC /tmp/bld/libbpf_probes.o CC /tmp/bld/xsk.o LD /tmp/bld/libbpf-in.o LINK /tmp/bld/libbpf.a LINK /tmp/bld/libbpf.so.0.0.2 LINK /tmp/bld/test_libbpf INSTALL /tmp/bld/libbpf.a INSTALL /tmp/bld/libbpf.so.0.0.2 # ll /usr/local/lib64/libbpf.* /usr/local/lib64/libbpf.a /usr/local/lib64/libbpf.so -> libbpf.so.0.0.2 /usr/local/lib64/libbpf.so.0 -> libbpf.so.0.0.2 /usr/local/lib64/libbpf.so.0.0.2 Fixes: 1bf4b05810fe ("tools: bpftool: add probes for eBPF program types") Fixes: 1b76c13e4b36 ("bpf tools: Introduce 'bpf' library and add bpf feature check") Reported-by: Stanislav Fomichev <sdf@google.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> --- tools/lib/bpf/Makefile | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-)