On Tue, Nov 09, 2021 at 07:42:29AM -0800, Andrii Nakryiko wrote: > On Mon, Nov 8, 2021 at 7:44 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Sun, Nov 07, 2021 at 10:13:09PM -0800, Andrii Nakryiko wrote: > > > Add test_progs-shared flavor to compile against libbpf as a shared > > > library. This is useful to make sure that libbpf's backwards/forward > > > compatibility guarantees are upheld. Currently this has to be checked > > > locally, but in the future we'll automate at least some scenarios as > > > part of libbpf CI runs. > > > > > > Biggest change is how either libbpf.a or libbpf.so is passed to the > > > compiler, which is controled on per-flavor through a new TRUNNER_LIBBPF > > > parameter. All the places that depend on libbpf artifacts (headers, > > > library itself, etc) to be built are moved to order-only dependency on > > > $(BPFOBJ). rpath is used to specify relative location to where libbpf.so > > > should be so that when test_progs-shared is run under QEMU, libbpf.so is > > > still going to be discovered correctly. > > > > > > Few selftests are using or testing internal libbpf APIs, so are not > > > compatible with shared library use of libbpf. Filter them out for shared > > > flavor. > > ... > > > +# Define test_progs-shared test runner. > > > +TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE > > > +TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS > > > +TRUNNER_EXTRA_CFLAGS := -Wl,-rpath=$(subst $(CURDIR)/,,$(dir $(BPFOBJ))) > > > +TRUNNER_LIBBPF := $(patsubst %libbpf.a,%libbpf.so,$(BPFOBJ)) > > > +TRUNNER_TESTS_BLACKLIST := cpu_mask.c hashmap.c perf_buffer.c raw_tp_test_run.c > > > +$(eval $(call DEFINE_TEST_RUNNER,test_progs,shared)) > > > +TRUNNER_TESTS_BLACKLIST := > > > > It's a good idea to add libbpf.so test, but going through test_progs is imo overkill. > > No reason to run more than one test with shared lib. > > If it links fine it's pretty much certain that it will work. > > Maybe convert test_maps into shared only? CI runs it already. > > If some APIs are not used from a user app, their symbols won't be used > during dynamic linking, so they won't be tested neither for linking > nor functionally. E.g., in this case all the btf_dump__new(), > btf__dedup(), etc, invocations won't be verified even at dynamic > linking time. At least that's my understanding from looking at > generated ELF binaries. And that was the sole motivator (at least > initially) to add -shared flavor, so that I can make sure it works. > Learned a bit about symbol versioning and symbol visibility from that, > so definitely not a waste of time. > > But more broadly, what's the concern? User-space part compilation time > for test_progs is extremely fast, it's the BPF object compilation that > is slow. I'll send another Makefile change to eliminate the third > compilation variant for BPF source code in the next few days (will > roll it into this patch set unless it lands first), so that will go > away. exactly. Rebuilding the tests for 3rd variant is a build time waste. Even linking test_progs as shared is misleading. There is no need to run such flavor. I suspect test_progs doesn't use 100% of libbpf api. test_maps is using less than test_progs. If the goal is to cover all of libbpf then how about adding all calls from libbpf.map to test_maps or standalone new binary to check link+run sanity. I understand the appeal to follow the pattern of different test_progs flavors, but shared doesn't fit. Different flavors are used when tests themselves are compiled differently. Not when test_progs runner is different. imo standalone binary that calls 100% of func from libbpf.map will serve us better in the long run from maintainability pov. That will be an accurate unit test for .so functionality.
