| Message ID | 20190315200414.32346-1-ivecera@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | f6827526279d75f0b1c1605b1bf560024bd7696f |
| Headers | show |
| Series | [net-next] selftests: bpf: modify urandom_read and link it non-statically | expand |
On 03/15, Ivan Vecera wrote: > After some experiences I found that urandom_read does not need to be > linked statically. When the 'read' syscall call is moved to separate > non-inlined function then bpf_get_stackid() is able to find > the executable in stack trace and extract its build_id from it. But why? Do you have some problems with it being linked statically? > > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > tools/testing/selftests/bpf/Makefile | 2 +- > tools/testing/selftests/bpf/urandom_read.c | 15 +++++++++++---- > 2 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 2aed37ea61a4..c33900a8fec0 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -69,7 +69,7 @@ TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read > all: $(TEST_CUSTOM_PROGS) > > $(OUTPUT)/urandom_read: $(OUTPUT)/%: %.c > - $(CC) -o $@ -static $< -Wl,--build-id > + $(CC) -o $@ $< -Wl,--build-id > > BPFOBJ := $(OUTPUT)/libbpf.a > > diff --git a/tools/testing/selftests/bpf/urandom_read.c b/tools/testing/selftests/bpf/urandom_read.c > index 9de8b7cb4e6d..db781052758d 100644 > --- a/tools/testing/selftests/bpf/urandom_read.c > +++ b/tools/testing/selftests/bpf/urandom_read.c > @@ -7,11 +7,19 @@ > > #define BUF_SIZE 256 > > +static __attribute__((noinline)) > +void urandom_read(int fd, int count) > +{ > + char buf[BUF_SIZE]; > + int i; > + > + for (i = 0; i < count; ++i) > + read(fd, buf, BUF_SIZE); > +} > + > int main(int argc, char *argv[]) > { > int fd = open("/dev/urandom", O_RDONLY); > - int i; > - char buf[BUF_SIZE]; > int count = 4; > > if (fd < 0) > @@ -20,8 +28,7 @@ int main(int argc, char *argv[]) > if (argc == 2) > count = atoi(argv[1]); > > - for (i = 0; i < count; ++i) > - read(fd, buf, BUF_SIZE); > + urandom_read(fd, count); > > close(fd); > return 0; > -- > 2.19.2 >
On 15. 03. 19 21:08, Stanislav Fomichev wrote: > On 03/15, Ivan Vecera wrote: >> After some experiences I found that urandom_read does not need to be >> linked statically. When the 'read' syscall call is moved to separate >> non-inlined function then bpf_get_stackid() is able to find >> the executable in stack trace and extract its build_id from it. > But why? Do you have some problems with it being linked statically? > Dependency... you don't need to install static glibc to compile the bpf samples. Shared libc is available everytime. Ivan
On 03/15, Ivan Vecera wrote: > On 15. 03. 19 21:08, Stanislav Fomichev wrote: > > On 03/15, Ivan Vecera wrote: > > > After some experiences I found that urandom_read does not need to be > > > linked statically. When the 'read' syscall call is moved to separate > > > non-inlined function then bpf_get_stackid() is able to find > > > the executable in stack trace and extract its build_id from it. > > But why? Do you have some problems with it being linked statically? > > > Dependency... you don't need to install static glibc to compile the bpf > samples. Shared libc is available everytime. Oh, the distros that do -devel _and_ -static packages :-) So your patch essentially adds a call, that leaves a trace on the stack with our build-id. I guess that works as well. > > Ivan
----- Stanislav Fomichev <sdf@fomichev.me> wrote: > On 03/15, Ivan Vecera wrote: > > On 15. 03. 19 21:08, Stanislav Fomichev wrote: > > > On 03/15, Ivan Vecera wrote: > > > > After some experiences I found that urandom_read does not need to be > > > > linked statically. When the 'read' syscall call is moved to separate > > > > non-inlined function then bpf_get_stackid() is able to find > > > > the executable in stack trace and extract its build_id from it. > > > But why? Do you have some problems with it being linked statically? > > > > > Dependency... you don't need to install static glibc to compile the bpf > > samples. Shared libc is available everytime. > Oh, the distros that do -devel _and_ -static packages :-) > > So your patch essentially adds a call, that leaves a trace on the stack > with our build-id. I guess that works as well. Without that additional call this does not work and build_id selftest fails. I.
On 03/15, Ivan Vecera wrote: > > ----- Stanislav Fomichev <sdf@fomichev.me> wrote: > > On 03/15, Ivan Vecera wrote: > > > On 15. 03. 19 21:08, Stanislav Fomichev wrote: > > > > On 03/15, Ivan Vecera wrote: > > > > > After some experiences I found that urandom_read does not need to be > > > > > linked statically. When the 'read' syscall call is moved to separate > > > > > non-inlined function then bpf_get_stackid() is able to find > > > > > the executable in stack trace and extract its build_id from it. > > > > But why? Do you have some problems with it being linked statically? > > > > > > > Dependency... you don't need to install static glibc to compile the bpf > > > samples. Shared libc is available everytime. > > Oh, the distros that do -devel _and_ -static packages :-) > > > > So your patch essentially adds a call, that leaves a trace on the stack > > with our build-id. I guess that works as well. > > Without that additional call this does not work and build_id selftest fails. Oh, yeah, I was just trying to clarify why it fails without -static and why your patch makes it work for non-static :-) You can put more details in the commit message; you'd have to resubmit whenever net-next/bpf-next opens anyway. > > I.
On Fri, Mar 15, 2019 at 1:04 PM Ivan Vecera <ivecera@redhat.com> wrote: > > After some experiences I found that urandom_read does not need to be > linked statically. When the 'read' syscall call is moved to separate > non-inlined function then bpf_get_stackid() is able to find > the executable in stack trace and extract its build_id from it. > > Signed-off-by: Ivan Vecera <ivecera@redhat.com> Applied. Thanks
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 2aed37ea61a4..c33900a8fec0 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -69,7 +69,7 @@ TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read all: $(TEST_CUSTOM_PROGS) $(OUTPUT)/urandom_read: $(OUTPUT)/%: %.c - $(CC) -o $@ -static $< -Wl,--build-id + $(CC) -o $@ $< -Wl,--build-id BPFOBJ := $(OUTPUT)/libbpf.a diff --git a/tools/testing/selftests/bpf/urandom_read.c b/tools/testing/selftests/bpf/urandom_read.c index 9de8b7cb4e6d..db781052758d 100644 --- a/tools/testing/selftests/bpf/urandom_read.c +++ b/tools/testing/selftests/bpf/urandom_read.c @@ -7,11 +7,19 @@ #define BUF_SIZE 256 +static __attribute__((noinline)) +void urandom_read(int fd, int count) +{ + char buf[BUF_SIZE]; + int i; + + for (i = 0; i < count; ++i) + read(fd, buf, BUF_SIZE); +} + int main(int argc, char *argv[]) { int fd = open("/dev/urandom", O_RDONLY); - int i; - char buf[BUF_SIZE]; int count = 4; if (fd < 0) @@ -20,8 +28,7 @@ int main(int argc, char *argv[]) if (argc == 2) count = atoi(argv[1]); - for (i = 0; i < count; ++i) - read(fd, buf, BUF_SIZE); + urandom_read(fd, count); close(fd); return 0;
After some experiences I found that urandom_read does not need to be linked statically. When the 'read' syscall call is moved to separate non-inlined function then bpf_get_stackid() is able to find the executable in stack trace and extract its build_id from it. Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- tools/testing/selftests/bpf/Makefile | 2 +- tools/testing/selftests/bpf/urandom_read.c | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-)