diff mbox series

[v3,2/3] Split off executable code from bpf/bpf_common.h

Message ID 20200217141622.26912-2-mdoucha@suse.cz
State Superseded
Headers show
Series [v3,1/3] Redesign TST_RETRY_FUNC() | expand

Commit Message

Martin Doucha Feb. 17, 2020, 2:16 p.m. UTC
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

Changes since v1: This patch was split off from the v1 BPF fix. Code cleanup
to prevent future bugs and make the common code more readable.

Changes since v2: None.

 testcases/kernel/syscalls/bpf/Makefile     |  3 ++
 testcases/kernel/syscalls/bpf/bpf_common.c | 45 ++++++++++++++++++++++
 testcases/kernel/syscalls/bpf/bpf_common.h | 39 ++-----------------
 3 files changed, 51 insertions(+), 36 deletions(-)
 create mode 100644 testcases/kernel/syscalls/bpf/bpf_common.c

Comments

Li Wang Feb. 18, 2020, 8:15 a.m. UTC | #1
On Mon, Feb 17, 2020 at 10:16 PM Martin Doucha <mdoucha@suse.cz> wrote:

> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>
> Changes since v1: This patch was split off from the v1 BPF fix. Code
> cleanup
> to prevent future bugs and make the common code more readable.
>
> Changes since v2: None.
>
>  testcases/kernel/syscalls/bpf/Makefile     |  3 ++
>  testcases/kernel/syscalls/bpf/bpf_common.c | 45 ++++++++++++++++++++++
>  testcases/kernel/syscalls/bpf/bpf_common.h | 39 ++-----------------
>  3 files changed, 51 insertions(+), 36 deletions(-)
>  create mode 100644 testcases/kernel/syscalls/bpf/bpf_common.c
>
> diff --git a/testcases/kernel/syscalls/bpf/Makefile
> b/testcases/kernel/syscalls/bpf/Makefile
> index 990c8c83c..2241bce9b 100644
> --- a/testcases/kernel/syscalls/bpf/Makefile
> +++ b/testcases/kernel/syscalls/bpf/Makefile
> @@ -5,6 +5,9 @@ top_srcdir              ?= ../../../..
>
>  include $(top_srcdir)/include/mk/testcases.mk
>
> +FILTER_OUT_MAKE_TARGETS        := bpf_common
>  CFLAGS                 += -D_GNU_SOURCE
>
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> +
> +$(MAKE_TARGETS): %: %.o bpf_common.o
>

May I ask why we need to reserve these *.o binary files in the compiling?

# ls
bpf_common.c  bpf_common.o  bpf_map01.c  bpf_prog01    bpf_prog01.o
 bpf_prog02.c  bpf_prog03    bpf_prog03.o
bpf_common.h  bpf_map01     bpf_map01.o  bpf_prog01.c  bpf_prog02
 bpf_prog02.o  bpf_prog03.c  Makefile


Otherwise, the patchset looks good.
    Acked-by: Li Wang <liwang@redhat.com>
Martin Doucha Feb. 18, 2020, 8:44 a.m. UTC | #2
On 2/18/20 9:15 AM, Li Wang wrote:
>     diff --git a/testcases/kernel/syscalls/bpf/Makefile
>     b/testcases/kernel/syscalls/bpf/Makefile
>     index 990c8c83c..2241bce9b 100644
>     --- a/testcases/kernel/syscalls/bpf/Makefile
>     +++ b/testcases/kernel/syscalls/bpf/Makefile
>     @@ -5,6 +5,9 @@ top_srcdir              ?= ../../../..
> 
>      include $(top_srcdir)/include/mk/testcases.mk <http://testcases.mk>
> 
>     +FILTER_OUT_MAKE_TARGETS        := bpf_common
>      CFLAGS                 += -D_GNU_SOURCE
> 
>      include $(top_srcdir)/include/mk/generic_leaf_target.mk
>     <http://generic_leaf_target.mk>
>     +
>     +$(MAKE_TARGETS): %: %.o bpf_common.o
> 
> 
> May I ask why we need to reserve these *.o binary files in the compiling?

Sorry, I don't understand the question.
Li Wang Feb. 18, 2020, 8:49 a.m. UTC | #3
On Tue, Feb 18, 2020 at 4:44 PM Martin Doucha <mdoucha@suse.cz> wrote:

> On 2/18/20 9:15 AM, Li Wang wrote:
> >     diff --git a/testcases/kernel/syscalls/bpf/Makefile
> >     b/testcases/kernel/syscalls/bpf/Makefile
> >     index 990c8c83c..2241bce9b 100644
> >     --- a/testcases/kernel/syscalls/bpf/Makefile
> >     +++ b/testcases/kernel/syscalls/bpf/Makefile
> >     @@ -5,6 +5,9 @@ top_srcdir              ?= ../../../..
> >
> >      include $(top_srcdir)/include/mk/testcases.mk <http://testcases.mk>
> >
> >     +FILTER_OUT_MAKE_TARGETS        := bpf_common
> >      CFLAGS                 += -D_GNU_SOURCE
> >
> >      include $(top_srcdir)/include/mk/generic_leaf_target.mk
> >     <http://generic_leaf_target.mk>
> >     +
> >     +$(MAKE_TARGETS): %: %.o bpf_common.o
> >
> >
> > May I ask why we need to reserve these *.o binary files in the compiling?
>
> Sorry, I don't understand the question.
>

Sorry for the unclear question. I mean can we modify the Makefile as:

--- a/testcases/kernel/syscalls/bpf/Makefile
+++ b/testcases/kernel/syscalls/bpf/Makefile
@@ -10,4 +10,4 @@ CFLAGS                        += -D_GNU_SOURCE

 include $(top_srcdir)/include/mk/generic_leaf_target.mk

-$(MAKE_TARGETS): %: %.o bpf_common.o
+$(MAKE_TARGETS): %: bpf_common.o
Petr Vorel Feb. 18, 2020, 8:53 a.m. UTC | #4
Hi,

> Sorry for the unclear question. I mean can we modify the Makefile as:

> --- a/testcases/kernel/syscalls/bpf/Makefile
> +++ b/testcases/kernel/syscalls/bpf/Makefile
> @@ -10,4 +10,4 @@ CFLAGS                        += -D_GNU_SOURCE

>  include $(top_srcdir)/include/mk/generic_leaf_target.mk

> -$(MAKE_TARGETS): %: %.o bpf_common.o
> +$(MAKE_TARGETS): %: bpf_common.o
+1 that removes object files.


Kind regards,
Petr
Petr Vorel Feb. 18, 2020, 8:55 a.m. UTC | #5
Hi,

> May I ask why we need to reserve these *.o binary files in the compiling?

> # ls
> bpf_common.c  bpf_common.o  bpf_map01.c  bpf_prog01    bpf_prog01.o
>  bpf_prog02.c  bpf_prog03    bpf_prog03.o
> bpf_common.h  bpf_map01     bpf_map01.o  bpf_prog01.c  bpf_prog02
>  bpf_prog02.o  bpf_prog03.c  Makefile


> Otherwise, the patchset looks good.
>     Acked-by: Li Wang <liwang@redhat.com>

Reviewed-by: Petr Vorel <pvorel@suse.cz>
(+1 for Li change to Makefile).

Kind regards,
Petr
Martin Doucha Feb. 18, 2020, 9:43 a.m. UTC | #6
On 2/18/20 9:49 AM, Li Wang wrote:
> Sorry for the unclear question. I mean can we modify the Makefile as:
> 
> --- a/testcases/kernel/syscalls/bpf/Makefile
> +++ b/testcases/kernel/syscalls/bpf/Makefile
> @@ -10,4 +10,4 @@ CFLAGS                        += -D_GNU_SOURCE
>  
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
>  
> -$(MAKE_TARGETS): %: %.o bpf_common.o
> +$(MAKE_TARGETS): %: bpf_common.o

Ah, OK. I'll fix that.
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/bpf/Makefile b/testcases/kernel/syscalls/bpf/Makefile
index 990c8c83c..2241bce9b 100644
--- a/testcases/kernel/syscalls/bpf/Makefile
+++ b/testcases/kernel/syscalls/bpf/Makefile
@@ -5,6 +5,9 @@  top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
 
+FILTER_OUT_MAKE_TARGETS	:= bpf_common
 CFLAGS			+= -D_GNU_SOURCE
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
+
+$(MAKE_TARGETS): %: %.o bpf_common.o
diff --git a/testcases/kernel/syscalls/bpf/bpf_common.c b/testcases/kernel/syscalls/bpf/bpf_common.c
new file mode 100644
index 000000000..fce364af8
--- /dev/null
+++ b/testcases/kernel/syscalls/bpf/bpf_common.c
@@ -0,0 +1,45 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2019-2020 Linux Test Project
+ */
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "lapi/bpf.h"
+#include "bpf_common.h"
+
+void rlimit_bump_memlock(void)
+{
+	struct rlimit memlock_r;
+
+	SAFE_GETRLIMIT(RLIMIT_MEMLOCK, &memlock_r);
+	memlock_r.rlim_cur += BPF_MEMLOCK_ADD;
+	tst_res(TINFO, "Raising RLIMIT_MEMLOCK to %ld",
+		(long)memlock_r.rlim_cur);
+
+	if (memlock_r.rlim_cur <= memlock_r.rlim_max) {
+		SAFE_SETRLIMIT(RLIMIT_MEMLOCK, &memlock_r);
+	} else if ((geteuid() == 0)) {
+		memlock_r.rlim_max += BPF_MEMLOCK_ADD;
+		SAFE_SETRLIMIT(RLIMIT_MEMLOCK, &memlock_r);
+	} else {
+		tst_res(TINFO, "Can't raise RLIMIT_MEMLOCK, test may fail "
+			"due to lack of max locked memory");
+	}
+}
+
+int bpf_map_create(union bpf_attr *attr)
+{
+	TEST(bpf(BPF_MAP_CREATE, attr, sizeof(*attr)));
+	if (TST_RET == -1) {
+		if (TST_ERR == EPERM) {
+			tst_res(TCONF, "Hint: check also /proc/sys/kernel/unprivileged_bpf_disabled");
+			tst_brk(TCONF | TTERRNO,
+				"bpf() requires CAP_SYS_ADMIN on this system");
+		} else {
+			tst_brk(TBROK | TTERRNO, "Failed to create array map");
+		}
+	}
+
+	return TST_RET;
+}
diff --git a/testcases/kernel/syscalls/bpf/bpf_common.h b/testcases/kernel/syscalls/bpf/bpf_common.h
index f700bede2..e46a519eb 100644
--- a/testcases/kernel/syscalls/bpf/bpf_common.h
+++ b/testcases/kernel/syscalls/bpf/bpf_common.h
@@ -1,6 +1,6 @@ 
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
- * Copyright (c) 2019 Linux Test Project
+ * Copyright (c) 2019-2020 Linux Test Project
  */
 
 #ifndef LTP_BPF_COMMON_H
@@ -8,40 +8,7 @@ 
 
 #define BPF_MEMLOCK_ADD (256*1024)
 
-void rlimit_bump_memlock(void)
-{
-	struct rlimit memlock_r;
-
-	SAFE_GETRLIMIT(RLIMIT_MEMLOCK, &memlock_r);
-	memlock_r.rlim_cur += BPF_MEMLOCK_ADD;
-	tst_res(TINFO, "Raising RLIMIT_MEMLOCK to %ld",
-		(long)memlock_r.rlim_cur);
-
-	if (memlock_r.rlim_cur <= memlock_r.rlim_max) {
-		SAFE_SETRLIMIT(RLIMIT_MEMLOCK, &memlock_r);
-	} else if ((geteuid() == 0)) {
-		memlock_r.rlim_max += BPF_MEMLOCK_ADD;
-		SAFE_SETRLIMIT(RLIMIT_MEMLOCK, &memlock_r);
-	} else {
-		tst_res(TINFO, "Can't raise RLIMIT_MEMLOCK, test may fail "
-			"due to lack of max locked memory");
-	}
-}
-
-int bpf_map_create(union bpf_attr *attr)
-{
-	TEST(bpf(BPF_MAP_CREATE, attr, sizeof(*attr)));
-	if (TST_RET == -1) {
-		if (TST_ERR == EPERM) {
-			tst_res(TCONF, "Hint: check also /proc/sys/kernel/unprivileged_bpf_disabled");
-			tst_brk(TCONF | TTERRNO,
-				"bpf() requires CAP_SYS_ADMIN on this system");
-		} else {
-			tst_brk(TBROK | TTERRNO, "Failed to create array map");
-		}
-	}
-
-	return TST_RET;
-}
+void rlimit_bump_memlock(void);
+int bpf_map_create(union bpf_attr *attr);
 
 #endif