2020-08-01 08:52:35

by Song Liu

[permalink] [raw]
Subject: [PATCH bpf-next 4/5] selftests/bpf: move two functions to test_progs.c

Move time_get_ns() and get_base_addr() to test_progs.c, so they can be
used in other tests.

Signed-off-by: Song Liu <[email protected]>
---
.../selftests/bpf/prog_tests/attach_probe.c | 21 -------------
.../selftests/bpf/prog_tests/test_overhead.c | 8 -----
tools/testing/selftests/bpf/test_progs.c | 30 +++++++++++++++++++
tools/testing/selftests/bpf/test_progs.h | 2 ++
4 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index a0ee87c8e1ea7..3bda8acbbafb5 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -2,27 +2,6 @@
#include <test_progs.h>
#include "test_attach_probe.skel.h"

-ssize_t get_base_addr() {
- size_t start, offset;
- char buf[256];
- FILE *f;
-
- f = fopen("/proc/self/maps", "r");
- if (!f)
- return -errno;
-
- while (fscanf(f, "%zx-%*x %s %zx %*[^\n]\n",
- &start, buf, &offset) == 3) {
- if (strcmp(buf, "r-xp") == 0) {
- fclose(f);
- return start - offset;
- }
- }
-
- fclose(f);
- return -EINVAL;
-}
-
void test_attach_probe(void)
{
int duration = 0;
diff --git a/tools/testing/selftests/bpf/prog_tests/test_overhead.c b/tools/testing/selftests/bpf/prog_tests/test_overhead.c
index 2702df2b23433..3fe32e9357c4b 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_overhead.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_overhead.c
@@ -7,14 +7,6 @@

#define MAX_CNT 100000

-static __u64 time_get_ns(void)
-{
- struct timespec ts;
-
- clock_gettime(CLOCK_MONOTONIC, &ts);
- return ts.tv_sec * 1000000000ull + ts.tv_nsec;
-}
-
static int test_task_rename(const char *prog)
{
int i, fd, duration = 0, err;
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index b1e4dadacd9b4..c9e6a5ad5b9a4 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -622,6 +622,36 @@ int cd_flavor_subdir(const char *exec_name)
return chdir(flavor);
}

+__u64 time_get_ns(void)
+{
+ struct timespec ts;
+
+ clock_gettime(CLOCK_MONOTONIC, &ts);
+ return ts.tv_sec * 1000000000ull + ts.tv_nsec;
+}
+
+ssize_t get_base_addr(void)
+{
+ size_t start, offset;
+ char buf[256];
+ FILE *f;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!f)
+ return -errno;
+
+ while (fscanf(f, "%zx-%*x %s %zx %*[^\n]\n",
+ &start, buf, &offset) == 3) {
+ if (strcmp(buf, "r-xp") == 0) {
+ fclose(f);
+ return start - offset;
+ }
+ }
+
+ fclose(f);
+ return -EINVAL;
+}
+
#define MAX_BACKTRACE_SZ 128
void crash_handler(int signum)
{
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 6e09bf738473e..2d617201024bf 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -91,6 +91,8 @@ extern bool test__start_subtest(const char *name);
extern void test__skip(void);
extern void test__fail(void);
extern int test__join_cgroup(const char *path);
+extern __u64 time_get_ns(void);
+extern ssize_t get_base_addr();

#define PRINT_FAIL(format...) \
({ \
--
2.24.1


2020-08-03 01:47:59

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH bpf-next 4/5] selftests/bpf: move two functions to test_progs.c

On Sat, Aug 1, 2020 at 1:50 AM Song Liu <[email protected]> wrote:
>
> Move time_get_ns() and get_base_addr() to test_progs.c, so they can be
> used in other tests.
>
> Signed-off-by: Song Liu <[email protected]>
> ---
> .../selftests/bpf/prog_tests/attach_probe.c | 21 -------------
> .../selftests/bpf/prog_tests/test_overhead.c | 8 -----
> tools/testing/selftests/bpf/test_progs.c | 30 +++++++++++++++++++
> tools/testing/selftests/bpf/test_progs.h | 2 ++
> 4 files changed, 32 insertions(+), 29 deletions(-)
>

[...]

> static int test_task_rename(const char *prog)
> {
> int i, fd, duration = 0, err;
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index b1e4dadacd9b4..c9e6a5ad5b9a4 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -622,6 +622,36 @@ int cd_flavor_subdir(const char *exec_name)
> return chdir(flavor);
> }
>
> +__u64 time_get_ns(void)
> +{

I'd try to avoid adding stuff to test_progs.c. There is generic
testing_helpers.c, maybe let's put this there?

> + struct timespec ts;
> +
> + clock_gettime(CLOCK_MONOTONIC, &ts);
> + return ts.tv_sec * 1000000000ull + ts.tv_nsec;
> +}
> +
> +ssize_t get_base_addr(void)
> +{

This would definitely be better in trace_helpers.c, though.

> + size_t start, offset;
> + char buf[256];
> + FILE *f;
> +

[...]

2020-08-03 04:35:51

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next 4/5] selftests/bpf: move two functions to test_progs.c



> On Aug 2, 2020, at 6:46 PM, Andrii Nakryiko <[email protected]> wrote:
>
> On Sat, Aug 1, 2020 at 1:50 AM Song Liu <[email protected]> wrote:
>>
>> Move time_get_ns() and get_base_addr() to test_progs.c, so they can be
>> used in other tests.
>>
>> Signed-off-by: Song Liu <[email protected]>
>> ---
>> .../selftests/bpf/prog_tests/attach_probe.c | 21 -------------
>> .../selftests/bpf/prog_tests/test_overhead.c | 8 -----
>> tools/testing/selftests/bpf/test_progs.c | 30 +++++++++++++++++++
>> tools/testing/selftests/bpf/test_progs.h | 2 ++
>> 4 files changed, 32 insertions(+), 29 deletions(-)
>>
>
> [...]
>
>> static int test_task_rename(const char *prog)
>> {
>> int i, fd, duration = 0, err;
>> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
>> index b1e4dadacd9b4..c9e6a5ad5b9a4 100644
>> --- a/tools/testing/selftests/bpf/test_progs.c
>> +++ b/tools/testing/selftests/bpf/test_progs.c
>> @@ -622,6 +622,36 @@ int cd_flavor_subdir(const char *exec_name)
>> return chdir(flavor);
>> }
>>
>> +__u64 time_get_ns(void)
>> +{
>
> I'd try to avoid adding stuff to test_progs.c. There is generic
> testing_helpers.c, maybe let's put this there?
>
>> + struct timespec ts;
>> +
>> + clock_gettime(CLOCK_MONOTONIC, &ts);
>> + return ts.tv_sec * 1000000000ull + ts.tv_nsec;
>> +}
>> +
>> +ssize_t get_base_addr(void)
>> +{
>
> This would definitely be better in trace_helpers.c, though.

Will update.

Thanks,
Song