linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next] selftests/bpf: fix building bpf selftests statically
@ 2022-05-13  0:41 Yosry Ahmed
  2022-05-13 22:57 ` Andrii Nakryiko
  0 siblings, 1 reply; 4+ messages in thread
From: Yosry Ahmed @ 2022-05-13  0:41 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song
  Cc: John Fastabend, KP Singh, Hao Luo, netdev, bpf, linux-kernel,
	Yosry Ahmed

bpf selftests can no longer be built with CFLAGS=-static with
liburandom_read.so and its dependent target.

Filter out -static for liburandom_read.so and its dependent target.

Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
---
 tools/testing/selftests/bpf/Makefile | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 6bbc03161544..4eaefc187d5b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -168,14 +168,17 @@ $(OUTPUT)/%:%.c
 	$(call msg,BINARY,,$@)
 	$(Q)$(LINK.c) $^ $(LDLIBS) -o $@
 
+# If the tests are being built statically, exclude dynamic libraries defined
+# in this Makefile and their dependencies.
+DYNAMIC_CFLAGS := $(filter-out -static,$(CFLAGS))
 $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
 	$(call msg,LIB,,$@)
-	$(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
+	$(Q)$(CC) $(DYNAMIC_CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
 
 $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so
 	$(call msg,BINARY,,$@)
-	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^)			       \
-		  liburandom_read.so $(LDLIBS)	       			       \
+	$(Q)$(CC) $(DYNAMIC_CFLAGS) $(LDFLAGS) $(filter %.c,$^)			\
+		  liburandom_read.so $(LDLIBS)					\
 		  -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
 
 $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
-- 
2.36.0.550.gb090851708-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: fix building bpf selftests statically
  2022-05-13  0:41 [PATCH bpf-next] selftests/bpf: fix building bpf selftests statically Yosry Ahmed
@ 2022-05-13 22:57 ` Andrii Nakryiko
  2022-05-13 23:05   ` Yosry Ahmed
  0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2022-05-13 22:57 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Hao Luo, Networking, bpf, open list

On Thu, May 12, 2022 at 5:41 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> bpf selftests can no longer be built with CFLAGS=-static with
> liburandom_read.so and its dependent target.
>
> Filter out -static for liburandom_read.so and its dependent target.
>
> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> ---
>  tools/testing/selftests/bpf/Makefile | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 6bbc03161544..4eaefc187d5b 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -168,14 +168,17 @@ $(OUTPUT)/%:%.c
>         $(call msg,BINARY,,$@)
>         $(Q)$(LINK.c) $^ $(LDLIBS) -o $@
>
> +# If the tests are being built statically, exclude dynamic libraries defined
> +# in this Makefile and their dependencies.
> +DYNAMIC_CFLAGS := $(filter-out -static,$(CFLAGS))

I don't particularly like yet another CFLAGS global variable, but also
you are not filtering out -static from LDFLAGS, which would be
problematic if you try to do

make SAN_FLAGS=-static

which otherwise would work (and is probably better than overriding all of CFLAGS

How about something like this

diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 6bbc03161544..2e8eddf240af 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -170,11 +170,11 @@ $(OUTPUT)/%:%.c

 $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
        $(call msg,LIB,,$@)
-       $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
+       $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $^
$(LDLIBS) -fPIC -shared -o $@

 $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c
$(OUTPUT)/liburandom_read.so
        $(call msg,BINARY,,$@)
-       $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^)                        \
+       $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $(filter %.c,$^)  \
                  liburandom_read.so $(LDLIBS)                                 \
                  -Wl,-rpath=. -Wl,--build-id=sha1 -o $@

?


But I also have a question, this leaves urandom_read relying on
system-wide shared libraries, isn't that still a problem for you? Or
you intend to just ignore urandom_read-related tests?


$ ldd urandom_read
        linux-vdso.so.1 (0x00007ffd0d5e5000)
        liburandom_read.so (0x00007fc7f7d76000)
        libelf.so.1 => /lib64/libelf.so.1 (0x00007fc7f7937000)
        libz.so.1 => /lib64/libz.so.1 (0x00007fc7f7720000)
        librt.so.1 => /lib64/librt.so.1 (0x00007fc7f7518000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fc7f72f8000)
        libc.so.6 => /lib64/libc.so.6 (0x00007fc7f6f33000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fc7f7b50000)




>  $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
>         $(call msg,LIB,,$@)
> -       $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
> +       $(Q)$(CC) $(DYNAMIC_CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
>
>  $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so
>         $(call msg,BINARY,,$@)
> -       $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^)                        \
> -                 liburandom_read.so $(LDLIBS)                                 \
> +       $(Q)$(CC) $(DYNAMIC_CFLAGS) $(LDFLAGS) $(filter %.c,$^)                 \
> +                 liburandom_read.so $(LDLIBS)                                  \
>                   -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
>
>  $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
> --
> 2.36.0.550.gb090851708-goog
>

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: fix building bpf selftests statically
  2022-05-13 22:57 ` Andrii Nakryiko
@ 2022-05-13 23:05   ` Yosry Ahmed
  2022-05-13 23:11     ` Andrii Nakryiko
  0 siblings, 1 reply; 4+ messages in thread
From: Yosry Ahmed @ 2022-05-13 23:05 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Hao Luo, Networking, bpf, open list

On Fri, May 13, 2022 at 3:57 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, May 12, 2022 at 5:41 PM Yosry Ahmed <yosryahmed@google.com> wrote:
> >
> > bpf selftests can no longer be built with CFLAGS=-static with
> > liburandom_read.so and its dependent target.
> >
> > Filter out -static for liburandom_read.so and its dependent target.
> >
> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> > ---
> >  tools/testing/selftests/bpf/Makefile | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 6bbc03161544..4eaefc187d5b 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -168,14 +168,17 @@ $(OUTPUT)/%:%.c
> >         $(call msg,BINARY,,$@)
> >         $(Q)$(LINK.c) $^ $(LDLIBS) -o $@
> >
> > +# If the tests are being built statically, exclude dynamic libraries defined
> > +# in this Makefile and their dependencies.
> > +DYNAMIC_CFLAGS := $(filter-out -static,$(CFLAGS))
>
> I don't particularly like yet another CFLAGS global variable, but also
> you are not filtering out -static from LDFLAGS, which would be
> problematic if you try to do
>
> make SAN_FLAGS=-static
>
> which otherwise would work (and is probably better than overriding all of CFLAGS
>
> How about something like this

Yeah this looks good, thanks for pointing out the problem with LDFLAGS.

>
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 6bbc03161544..2e8eddf240af 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -170,11 +170,11 @@ $(OUTPUT)/%:%.c
>
>  $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
>         $(call msg,LIB,,$@)
> -       $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
> +       $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $^
> $(LDLIBS) -fPIC -shared -o $@
>
>  $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c
> $(OUTPUT)/liburandom_read.so
>         $(call msg,BINARY,,$@)
> -       $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^)                        \
> +       $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $(filter %.c,$^)  \
>                   liburandom_read.so $(LDLIBS)                                 \
>                   -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
>
> ?
>
>
> But I also have a question, this leaves urandom_read relying on
> system-wide shared libraries, isn't that still a problem for you? Or
> you intend to just ignore urandom_read-related tests?
>

I wasn't running those tests, and I thought having most tests compile
statically is better than nothing. Maybe this can be fixed by defining
a static target for liburandom_read? I am honestly not sure, I have
little experience with Makefiles/compilation.

>
> $ ldd urandom_read
>         linux-vdso.so.1 (0x00007ffd0d5e5000)
>         liburandom_read.so (0x00007fc7f7d76000)
>         libelf.so.1 => /lib64/libelf.so.1 (0x00007fc7f7937000)
>         libz.so.1 => /lib64/libz.so.1 (0x00007fc7f7720000)
>         librt.so.1 => /lib64/librt.so.1 (0x00007fc7f7518000)
>         libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fc7f72f8000)
>         libc.so.6 => /lib64/libc.so.6 (0x00007fc7f6f33000)
>         /lib64/ld-linux-x86-64.so.2 (0x00007fc7f7b50000)
>
>
>
>
> >  $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
> >         $(call msg,LIB,,$@)
> > -       $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
> > +       $(Q)$(CC) $(DYNAMIC_CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
> >
> >  $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so
> >         $(call msg,BINARY,,$@)
> > -       $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^)                        \
> > -                 liburandom_read.so $(LDLIBS)                                 \
> > +       $(Q)$(CC) $(DYNAMIC_CFLAGS) $(LDFLAGS) $(filter %.c,$^)                 \
> > +                 liburandom_read.so $(LDLIBS)                                  \
> >                   -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
> >
> >  $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
> > --
> > 2.36.0.550.gb090851708-goog
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: fix building bpf selftests statically
  2022-05-13 23:05   ` Yosry Ahmed
@ 2022-05-13 23:11     ` Andrii Nakryiko
  0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2022-05-13 23:11 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Hao Luo, Networking, bpf, open list

On Fri, May 13, 2022 at 4:05 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Fri, May 13, 2022 at 3:57 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, May 12, 2022 at 5:41 PM Yosry Ahmed <yosryahmed@google.com> wrote:
> > >
> > > bpf selftests can no longer be built with CFLAGS=-static with
> > > liburandom_read.so and its dependent target.
> > >
> > > Filter out -static for liburandom_read.so and its dependent target.
> > >
> > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> > > ---
> > >  tools/testing/selftests/bpf/Makefile | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 6bbc03161544..4eaefc187d5b 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -168,14 +168,17 @@ $(OUTPUT)/%:%.c
> > >         $(call msg,BINARY,,$@)
> > >         $(Q)$(LINK.c) $^ $(LDLIBS) -o $@
> > >
> > > +# If the tests are being built statically, exclude dynamic libraries defined
> > > +# in this Makefile and their dependencies.
> > > +DYNAMIC_CFLAGS := $(filter-out -static,$(CFLAGS))
> >
> > I don't particularly like yet another CFLAGS global variable, but also
> > you are not filtering out -static from LDFLAGS, which would be
> > problematic if you try to do
> >
> > make SAN_FLAGS=-static
> >
> > which otherwise would work (and is probably better than overriding all of CFLAGS
> >
> > How about something like this
>
> Yeah this looks good, thanks for pointing out the problem with LDFLAGS.

ok, cool, please incorporate into v2 then

>
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile
> > b/tools/testing/selftests/bpf/Makefile
> > index 6bbc03161544..2e8eddf240af 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -170,11 +170,11 @@ $(OUTPUT)/%:%.c
> >
> >  $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
> >         $(call msg,LIB,,$@)
> > -       $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
> > +       $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $^
> > $(LDLIBS) -fPIC -shared -o $@
> >
> >  $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c
> > $(OUTPUT)/liburandom_read.so
> >         $(call msg,BINARY,,$@)
> > -       $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^)                        \
> > +       $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $(filter %.c,$^)  \
> >                   liburandom_read.so $(LDLIBS)                                 \
> >                   -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
> >
> > ?
> >
> >
> > But I also have a question, this leaves urandom_read relying on
> > system-wide shared libraries, isn't that still a problem for you? Or
> > you intend to just ignore urandom_read-related tests?
> >
>
> I wasn't running those tests, and I thought having most tests compile
> statically is better than nothing. Maybe this can be fixed by defining
> a static target for liburandom_read? I am honestly not sure, I have
> little experience with Makefiles/compilation.

usdt selftest explicitly tests working with shared libraries, so we
have to keep liburandom_read.so as a shared library. But yes, at least
you can skip the test at runtime.

>
> >
> > $ ldd urandom_read
> >         linux-vdso.so.1 (0x00007ffd0d5e5000)
> >         liburandom_read.so (0x00007fc7f7d76000)
> >         libelf.so.1 => /lib64/libelf.so.1 (0x00007fc7f7937000)
> >         libz.so.1 => /lib64/libz.so.1 (0x00007fc7f7720000)
> >         librt.so.1 => /lib64/librt.so.1 (0x00007fc7f7518000)
> >         libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fc7f72f8000)
> >         libc.so.6 => /lib64/libc.so.6 (0x00007fc7f6f33000)
> >         /lib64/ld-linux-x86-64.so.2 (0x00007fc7f7b50000)
> >
> >
> >
> >
> > >  $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
> > >         $(call msg,LIB,,$@)
> > > -       $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
> > > +       $(Q)$(CC) $(DYNAMIC_CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
> > >
> > >  $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so
> > >         $(call msg,BINARY,,$@)
> > > -       $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^)                        \
> > > -                 liburandom_read.so $(LDLIBS)                                 \
> > > +       $(Q)$(CC) $(DYNAMIC_CFLAGS) $(LDFLAGS) $(filter %.c,$^)                 \
> > > +                 liburandom_read.so $(LDLIBS)                                  \
> > >                   -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
> > >
> > >  $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
> > > --
> > > 2.36.0.550.gb090851708-goog
> > >

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-05-13 23:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13  0:41 [PATCH bpf-next] selftests/bpf: fix building bpf selftests statically Yosry Ahmed
2022-05-13 22:57 ` Andrii Nakryiko
2022-05-13 23:05   ` Yosry Ahmed
2022-05-13 23:11     ` Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).