Message ID | 20180126003930.10573-1-mic@digikod.net |
---|---|
State | Accepted, archived |
Delegated to: | BPF Maintainers |
Headers | show |
Series | [net-next,v1] samples/bpf: Partially fixes the bpf.o build | expand |
On Fri, Jan 26, 2018 at 01:39:30AM +0100, Mickaël Salaün wrote: > Do not build lib/bpf/bpf.o with this Makefile but use the one from the > library directory. This avoid making a buggy bpf.o file (e.g. missing > symbols). could you provide an example? What symbols will be missing? I don't think there is an issue with existing Makefile. > This patch is useful if some code (e.g. Landlock tests) needs both the > bpf.o (from tools/lib/bpf) and the bpf_load.o (from samples/bpf). is that some future patches? we're trying to move everything form samples/bpf/ into selftests/bpf/ and convert to use libbpf.a instead of obsolete bpf_load.c Please use this approach for landlock as well. > Signed-off-by: Mickaël Salaün <mic@digikod.net> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > --- > > This is not a complet fix because the call to multi_depend with > $(host-cmulti) from scripts/Makefile.host force the build of bpf.o > anyway. I'm not sure how to completely avoid this automatic build > though. > --- > samples/bpf/Makefile | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 7f61a3d57fa7..64335bb94f9f 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -201,13 +201,16 @@ CLANG_ARCH_ARGS = -target $(ARCH) > endif > > # Trick to allow make to be run from this directory > -all: > +all: $(LIBBPF) > $(MAKE) -C ../../ $(CURDIR)/ > > clean: > $(MAKE) -C ../../ M=$(CURDIR) clean > @rm -f *~ > > +$(LIBBPF): FORCE > + $(MAKE) -C $(dir $@) $(notdir $@) > + > $(obj)/syscall_nrs.s: $(src)/syscall_nrs.c > $(call if_changed_dep,cc_s_c) > > -- > 2.15.1 >
On 26/01/2018 03:16, Alexei Starovoitov wrote: > On Fri, Jan 26, 2018 at 01:39:30AM +0100, Mickaël Salaün wrote: >> Do not build lib/bpf/bpf.o with this Makefile but use the one from the >> library directory. This avoid making a buggy bpf.o file (e.g. missing >> symbols). > > could you provide an example? > What symbols will be missing? > I don't think there is an issue with existing Makefile. You can run this commands: make -C samples/bpf; nm tools/lib/bpf/bpf.o > a; make -C tools/lib/bpf; nm tools/lib/bpf/bpf.o > b; diff -u a b Symbols like bzero and sys_bpf are missing with the samples/bpf Makefile, which makes the bpf.o shrink from 25K to 7K. > >> This patch is useful if some code (e.g. Landlock tests) needs both the >> bpf.o (from tools/lib/bpf) and the bpf_load.o (from samples/bpf). > > is that some future patches? Yes, I'll send them next week. > > we're trying to move everything form samples/bpf/ into selftests/bpf/ > and convert to use libbpf.a instead of obsolete bpf_load.c > Please use this approach for landlock as well. Ok, it should be better with this lib. > >> Signed-off-by: Mickaël Salaün <mic@digikod.net> >> Cc: Alexei Starovoitov <ast@kernel.org> >> Cc: Daniel Borkmann <daniel@iogearbox.net> >> --- >> >> This is not a complet fix because the call to multi_depend with >> $(host-cmulti) from scripts/Makefile.host force the build of bpf.o >> anyway. I'm not sure how to completely avoid this automatic build >> though. >> --- >> samples/bpf/Makefile | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile >> index 7f61a3d57fa7..64335bb94f9f 100644 >> --- a/samples/bpf/Makefile >> +++ b/samples/bpf/Makefile >> @@ -201,13 +201,16 @@ CLANG_ARCH_ARGS = -target $(ARCH) >> endif >> >> # Trick to allow make to be run from this directory >> -all: >> +all: $(LIBBPF) >> $(MAKE) -C ../../ $(CURDIR)/ >> >> clean: >> $(MAKE) -C ../../ M=$(CURDIR) clean >> @rm -f *~ >> >> +$(LIBBPF): FORCE >> + $(MAKE) -C $(dir $@) $(notdir $@) >> + >> $(obj)/syscall_nrs.s: $(src)/syscall_nrs.c >> $(call if_changed_dep,cc_s_c) >> >> -- >> 2.15.1 >> >
On 01/26/2018 09:30 AM, Mickaël Salaün wrote: > On 26/01/2018 03:16, Alexei Starovoitov wrote: >> On Fri, Jan 26, 2018 at 01:39:30AM +0100, Mickaël Salaün wrote: >>> Do not build lib/bpf/bpf.o with this Makefile but use the one from the >>> library directory. This avoid making a buggy bpf.o file (e.g. missing >>> symbols). >> >> could you provide an example? >> What symbols will be missing? >> I don't think there is an issue with existing Makefile. > > You can run this commands: > make -C samples/bpf; nm tools/lib/bpf/bpf.o > a; make -C tools/lib/bpf; > nm tools/lib/bpf/bpf.o > b; diff -u a b > > Symbols like bzero and sys_bpf are missing with the samples/bpf > Makefile, which makes the bpf.o shrink from 25K to 7K. I've applied it to bpf-next, thanks Mickaël!
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 7f61a3d57fa7..64335bb94f9f 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -201,13 +201,16 @@ CLANG_ARCH_ARGS = -target $(ARCH) endif # Trick to allow make to be run from this directory -all: +all: $(LIBBPF) $(MAKE) -C ../../ $(CURDIR)/ clean: $(MAKE) -C ../../ M=$(CURDIR) clean @rm -f *~ +$(LIBBPF): FORCE + $(MAKE) -C $(dir $@) $(notdir $@) + $(obj)/syscall_nrs.s: $(src)/syscall_nrs.c $(call if_changed_dep,cc_s_c)
Do not build lib/bpf/bpf.o with this Makefile but use the one from the library directory. This avoid making a buggy bpf.o file (e.g. missing symbols). This patch is useful if some code (e.g. Landlock tests) needs both the bpf.o (from tools/lib/bpf) and the bpf_load.o (from samples/bpf). Signed-off-by: Mickaël Salaün <mic@digikod.net> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> --- This is not a complet fix because the call to multi_depend with $(host-cmulti) from scripts/Makefile.host force the build of bpf.o anyway. I'm not sure how to completely avoid this automatic build though. --- samples/bpf/Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)