* [PATCH bpf-next 0/4] Generate NULL in vmlinux.h
@ 2021-03-17 3:03 Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 1/4] bpftool: generate NULL definition " Andrii Nakryiko
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-03-17 3:03 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Generate NULL definition as part of vmlinux.h. This is a pretty common and
unfortunate annoyance that most users of vmlinux.h have to deal with. Patch #2
drops such custom NULL definition in one of the selftests. Patches #3 and #4
make bpftool and selftests compilations stricter by treating warnings as
errors.
Andrii Nakryiko (4):
bpftool: generate NULL definition in vmlinux.h
selftests/bpf: drop custom NULL #define in skb_pkt_end selftest
selftests/bpf: treat compilation warnings as errors
bpftool: treat compilation warnings as errors
tools/bpf/bpftool/Makefile | 3 ++-
tools/bpf/bpftool/btf.c | 2 ++
tools/bpf/bpftool/jit_disasm.c | 3 +++
tools/testing/selftests/bpf/Makefile | 4 ++--
tools/testing/selftests/bpf/progs/skb_pkt_end.c | 1 -
5 files changed, 9 insertions(+), 4 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 1/4] bpftool: generate NULL definition in vmlinux.h
2021-03-17 3:03 [PATCH bpf-next 0/4] Generate NULL in vmlinux.h Andrii Nakryiko
@ 2021-03-17 3:03 ` Andrii Nakryiko
2021-03-17 3:08 ` Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 2/4] selftests/bpf: drop custom NULL #define in skb_pkt_end selftest Andrii Nakryiko
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2021-03-17 3:03 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Given that vmlinux.h is not compatible with headers like stdint.h, NULL poses
an annoying problem: it is defined as #define, so is not captured in BTF, so
is not emitted into vmlinux.h. This leads to users either sticking to explicit
0, or defining their own NULL (as progs/skb_pkt_end.c does).
It's pretty trivial for bpftool to generate NULL definition, though, so let's
just do that. This might cause compilation warning for existing BPF
applications:
progs/skb_pkt_end.c:7:9: warning: 'NULL' macro redefined [-Wmacro-redefined]
progs/skb_pkt_end.c:7:9: error: 'NULL' macro redefined [-Werror,-Wmacro-redefined]
#define NULL 0
^
/tmp/linux/tools/testing/selftests/bpf/tools/include/vmlinux.h:4:9: note: previous definition is here
#define NULL ((void *)0)
^
It is trivial to fix, though, so long-term benefits outweight temporary
inconveniences.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/bpftool/btf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 62953bbf68b4..ff6a76632873 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -405,6 +405,8 @@ static int dump_btf_c(const struct btf *btf,
printf("#ifndef __VMLINUX_H__\n");
printf("#define __VMLINUX_H__\n");
printf("\n");
+ printf("#define NULL ((void *)0)\n");
+ printf("\n");
printf("#ifndef BPF_NO_PRESERVE_ACCESS_INDEX\n");
printf("#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)\n");
printf("#endif\n\n");
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/4] selftests/bpf: drop custom NULL #define in skb_pkt_end selftest
2021-03-17 3:03 [PATCH bpf-next 0/4] Generate NULL in vmlinux.h Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 1/4] bpftool: generate NULL definition " Andrii Nakryiko
@ 2021-03-17 3:03 ` Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 3/4] selftests/bpf: treat compilation warnings as errors Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 4/4] bpftool: " Andrii Nakryiko
3 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-03-17 3:03 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Now that bpftool generates NULL definition as part of vmlinux.h, drop custom
NULL definition in skb_pkt_end.c.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/progs/skb_pkt_end.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/progs/skb_pkt_end.c b/tools/testing/selftests/bpf/progs/skb_pkt_end.c
index cf6823f42e80..7f2eaa2f89f8 100644
--- a/tools/testing/selftests/bpf/progs/skb_pkt_end.c
+++ b/tools/testing/selftests/bpf/progs/skb_pkt_end.c
@@ -4,7 +4,6 @@
#include <bpf/bpf_core_read.h>
#include <bpf/bpf_helpers.h>
-#define NULL 0
#define INLINE __always_inline
#define skb_shorter(skb, len) ((void *)(long)(skb)->data + (len) > (void *)(long)skb->data_end)
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 3/4] selftests/bpf: treat compilation warnings as errors
2021-03-17 3:03 [PATCH bpf-next 0/4] Generate NULL in vmlinux.h Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 1/4] bpftool: generate NULL definition " Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 2/4] selftests/bpf: drop custom NULL #define in skb_pkt_end selftest Andrii Nakryiko
@ 2021-03-17 3:03 ` Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 4/4] bpftool: " Andrii Nakryiko
3 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-03-17 3:03 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Make selftests/bpf compilation more strict by treating warnings as errors. We
are generally pretty good at keeping compilation clean, but it's easy to miss
minor warnings. So let's not trust people and just employ stricter compiler
enforcement.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index d0db2b673c6f..7e18f3e66e61 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -21,7 +21,7 @@ endif
BPF_GCC ?= $(shell command -v bpf-gcc;)
SAN_CFLAGS ?=
-CFLAGS += -g -Og -rdynamic -Wall $(GENFLAGS) $(SAN_CFLAGS) \
+CFLAGS += -g -Og -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \
-I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
-I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) \
-Dbpf_prog_load=bpf_prog_test_load \
@@ -270,7 +270,7 @@ IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \
MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
-BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
+BPF_CFLAGS = -g -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
-I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
-I$(abspath $(OUTPUT)/../usr/include)
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 4/4] bpftool: treat compilation warnings as errors
2021-03-17 3:03 [PATCH bpf-next 0/4] Generate NULL in vmlinux.h Andrii Nakryiko
` (2 preceding siblings ...)
2021-03-17 3:03 ` [PATCH bpf-next 3/4] selftests/bpf: treat compilation warnings as errors Andrii Nakryiko
@ 2021-03-17 3:03 ` Andrii Nakryiko
3 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-03-17 3:03 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
Make bpftool compilation stricter and treat all compilation warnigs as errors.
Depending on libbfd version on the system, jit_disasm.c might trigger the
following compilation warning-turned-error:
jit_disasm.c: In function ‘disasm_print_insn’:
jit_disasm.c:121:29: error: assignment discards ‘const’ qualifier from pointer
target type [-Werror=discarded-qualifiers]
info.disassembler_options = disassembler_options;
^
This was fixed in libbfd, but older versions of the library are still widely
used. So disable -Wdiscarded-qualifiers for that particular line of code.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/bpftool/Makefile | 3 ++-
tools/bpf/bpftool/jit_disasm.c | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index b3073ae84018..59de954faaf5 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -56,7 +56,8 @@ prefix ?= /usr/local
bash_compdir ?= /usr/share/bash-completion/completions
CFLAGS += -O2
-CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers
+CFLAGS += -W -Wall -Wextra -Werror
+CFLAGS += -Wno-unused-parameter -Wno-missing-field-initializers
CFLAGS += $(filter-out -Wswitch-enum -Wnested-externs,$(EXTRA_WARNINGS))
CFLAGS += -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ \
-I$(if $(OUTPUT),$(OUTPUT),.) \
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index e7e7eee9f172..48bc7f7a542f 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -118,7 +118,10 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
info.arch = bfd_get_arch(bfdf);
info.mach = bfd_get_mach(bfdf);
if (disassembler_options)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdiscarded-qualifiers"
info.disassembler_options = disassembler_options;
+#pragma GCC diagnostic push
info.buffer = image;
info.buffer_length = len;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/4] bpftool: generate NULL definition in vmlinux.h
2021-03-17 3:03 ` [PATCH bpf-next 1/4] bpftool: generate NULL definition " Andrii Nakryiko
@ 2021-03-17 3:08 ` Andrii Nakryiko
0 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-03-17 3:08 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Tue, Mar 16, 2021 at 8:03 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Given that vmlinux.h is not compatible with headers like stdint.h, NULL poses
> an annoying problem: it is defined as #define, so is not captured in BTF, so
> is not emitted into vmlinux.h. This leads to users either sticking to explicit
> 0, or defining their own NULL (as progs/skb_pkt_end.c does).
>
> It's pretty trivial for bpftool to generate NULL definition, though, so let's
> just do that. This might cause compilation warning for existing BPF
> applications:
>
> progs/skb_pkt_end.c:7:9: warning: 'NULL' macro redefined [-Wmacro-redefined]
> progs/skb_pkt_end.c:7:9: error: 'NULL' macro redefined [-Werror,-Wmacro-redefined]
oops, this shouldn't have been copy/pasted. This is how the line above
looks like if -Werror is specified in Makefile.
> #define NULL 0
> ^
> /tmp/linux/tools/testing/selftests/bpf/tools/include/vmlinux.h:4:9: note: previous definition is here
> #define NULL ((void *)0)
> ^
>
> It is trivial to fix, though, so long-term benefits outweight temporary
> inconveniences.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> tools/bpf/bpftool/btf.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 62953bbf68b4..ff6a76632873 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -405,6 +405,8 @@ static int dump_btf_c(const struct btf *btf,
> printf("#ifndef __VMLINUX_H__\n");
> printf("#define __VMLINUX_H__\n");
> printf("\n");
> + printf("#define NULL ((void *)0)\n");
> + printf("\n");
> printf("#ifndef BPF_NO_PRESERVE_ACCESS_INDEX\n");
> printf("#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)\n");
> printf("#endif\n\n");
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-17 3:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-17 3:03 [PATCH bpf-next 0/4] Generate NULL in vmlinux.h Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 1/4] bpftool: generate NULL definition " Andrii Nakryiko
2021-03-17 3:08 ` Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 2/4] selftests/bpf: drop custom NULL #define in skb_pkt_end selftest Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 3/4] selftests/bpf: treat compilation warnings as errors Andrii Nakryiko
2021-03-17 3:03 ` [PATCH bpf-next 4/4] bpftool: " 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).