[PATCH v2 bpf-next 0/2] Make bpf_endian.h compatible with vmlinux.h

[PATCH v2 bpf-next 0/2] Make bpf_endian.h compatible with vmlinux.h

[Andrii Nakryiko]

Change libbpf's bpf_endian.h header to be compatible when used with system
headers and when using just vmlinux.h. This is a frequent request for users
writing BPF CO-RE applications. Do this by re-implementing byte swap
compile-time macros. Also add simple tests validating correct results both for
byte-swapping built-ins and macros.

Andrii Nakryiko (2):
  libbpf: make bpf_endian co-exist with vmlinux.h
  selftests/bpf: add byte swapping selftest

 tools/lib/bpf/bpf_endian.h                    | 43 ++++++++++++---
 .../testing/selftests/bpf/prog_tests/endian.c | 53 +++++++++++++++++++
 .../testing/selftests/bpf/progs/test_endian.c | 37 +++++++++++++
 3 files changed, 125 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/endian.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_endian.c

-- 
2.24.1

[PATCH v2 bpf-next 1/2] libbpf: make bpf_endian co-exist with vmlinux.h

[Andrii Nakryiko]

Make bpf_endian.h compatible with vmlinux.h. It is a frequent request from
users wanting to use bpf_endian.h in their BPF applications using CO-RE and
vmlinux.h.

To achieve that, re-implement byte swap macros and drop all the header
includes. This way it can be used both with linux header includes, as well as
with a vmlinux.h.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/bpf_endian.h | 43 +++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/tools/lib/bpf/bpf_endian.h b/tools/lib/bpf/bpf_endian.h
index fbe28008450f..ec9db4feca9f 100644
--- a/tools/lib/bpf/bpf_endian.h
+++ b/tools/lib/bpf/bpf_endian.h
@@ -2,8 +2,35 @@
 #ifndef __BPF_ENDIAN__
 #define __BPF_ENDIAN__
 
-#include <linux/stddef.h>
-#include <linux/swab.h>
+/*
+ * Isolate byte #n and put it into byte #m, for __u##b type.
+ * E.g., moving byte #6 (nnnnnnnn) into byte #1 (mmmmmmmm) for __u64:
+ * 1) xxxxxxxx nnnnnnnn xxxxxxxx xxxxxxxx xxxxxxxx xxxxxxxx mmmmmmmm xxxxxxxx
+ * 2) nnnnnnnn xxxxxxxx xxxxxxxx xxxxxxxx xxxxxxxx mmmmmmmm xxxxxxxx 00000000
+ * 3) 00000000 00000000 00000000 00000000 00000000 00000000 00000000 nnnnnnnn
+ * 4) 00000000 00000000 00000000 00000000 00000000 00000000 nnnnnnnn 00000000
+ */
+#define ___bpf_mvb(x, b, n, m) ((__u##b)(x) << (b-(n+1)*8) >> (b-8) << (m*8))
+
+#define ___bpf_swab16(x) ((__u16)(			\
+			  ___bpf_mvb(x, 16, 0, 1) |	\
+			  ___bpf_mvb(x, 16, 1, 0)))
+
+#define ___bpf_swab32(x) ((__u32)(			\
+			  ___bpf_mvb(x, 32, 0, 3) |	\
+			  ___bpf_mvb(x, 32, 1, 2) |	\
+			  ___bpf_mvb(x, 32, 2, 1) |	\
+			  ___bpf_mvb(x, 32, 3, 0)))
+
+#define ___bpf_swab64(x) ((__u64)(			\
+			  ___bpf_mvb(x, 64, 0, 7) |	\
+			  ___bpf_mvb(x, 64, 1, 6) |	\
+			  ___bpf_mvb(x, 64, 2, 5) |	\
+			  ___bpf_mvb(x, 64, 3, 4) |	\
+			  ___bpf_mvb(x, 64, 4, 3) |	\
+			  ___bpf_mvb(x, 64, 5, 2) |	\
+			  ___bpf_mvb(x, 64, 6, 1) |	\
+			  ___bpf_mvb(x, 64, 7, 0)))
 
 /* LLVM's BPF target selects the endianness of the CPU
  * it compiles on, or the user specifies (bpfel/bpfeb),
@@ -23,16 +50,16 @@
 #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
 # define __bpf_ntohs(x)			__builtin_bswap16(x)
 # define __bpf_htons(x)			__builtin_bswap16(x)
-# define __bpf_constant_ntohs(x)	___constant_swab16(x)
-# define __bpf_constant_htons(x)	___constant_swab16(x)
+# define __bpf_constant_ntohs(x)	___bpf_swab16(x)
+# define __bpf_constant_htons(x)	___bpf_swab16(x)
 # define __bpf_ntohl(x)			__builtin_bswap32(x)
 # define __bpf_htonl(x)			__builtin_bswap32(x)
-# define __bpf_constant_ntohl(x)	___constant_swab32(x)
-# define __bpf_constant_htonl(x)	___constant_swab32(x)
+# define __bpf_constant_ntohl(x)	___bpf_swab32(x)
+# define __bpf_constant_htonl(x)	___bpf_swab32(x)
 # define __bpf_be64_to_cpu(x)		__builtin_bswap64(x)
 # define __bpf_cpu_to_be64(x)		__builtin_bswap64(x)
-# define __bpf_constant_be64_to_cpu(x)	___constant_swab64(x)
-# define __bpf_constant_cpu_to_be64(x)	___constant_swab64(x)
+# define __bpf_constant_be64_to_cpu(x)	___bpf_swab64(x)
+# define __bpf_constant_cpu_to_be64(x)	___bpf_swab64(x)
 #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
 # define __bpf_ntohs(x)			(x)
 # define __bpf_htons(x)			(x)
-- 
2.24.1

[PATCH v2 bpf-next 2/2] selftests/bpf: add byte swapping selftest

[Andrii Nakryiko]

Add simple selftest validating byte swap built-ins and compile-time macros.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../testing/selftests/bpf/prog_tests/endian.c | 53 +++++++++++++++++++
 .../testing/selftests/bpf/progs/test_endian.c | 37 +++++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/endian.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_endian.c

diff --git a/tools/testing/selftests/bpf/prog_tests/endian.c b/tools/testing/selftests/bpf/prog_tests/endian.c
new file mode 100644
index 000000000000..1a11612ace6c
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/endian.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+
+#include <test_progs.h>
+#include "test_endian.skel.h"
+
+static int duration;
+
+#define IN16 0x1234
+#define IN32 0x12345678U
+#define IN64 0x123456789abcdef0ULL
+
+#define OUT16 0x3412
+#define OUT32 0x78563412U
+#define OUT64 0xf0debc9a78563412ULL
+
+void test_endian(void)
+{
+	struct test_endian* skel;
+	struct test_endian__bss *bss;
+	int err;
+
+	skel = test_endian__open_and_load();
+	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
+		return;
+	bss = skel->bss;
+
+	bss->in16 = IN16;
+	bss->in32 = IN32;
+	bss->in64 = IN64;
+
+	err = test_endian__attach(skel);
+	if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
+		goto cleanup;
+
+	usleep(1);
+
+	CHECK(bss->out16 != OUT16, "out16", "got 0x%llx != exp 0x%llx\n",
+	      (__u64)bss->out16, (__u64)OUT16);
+	CHECK(bss->out32 != OUT32, "out32", "got 0x%llx != exp 0x%llx\n",
+	      (__u64)bss->out32, (__u64)OUT32);
+	CHECK(bss->out64 != OUT64, "out16", "got 0x%llx != exp 0x%llx\n",
+	      (__u64)bss->out64, (__u64)OUT64);
+
+	CHECK(bss->const16 != OUT16, "const16", "got 0x%llx != exp 0x%llx\n",
+	      (__u64)bss->const16, (__u64)OUT16);
+	CHECK(bss->const32 != OUT32, "const32", "got 0x%llx != exp 0x%llx\n",
+	      (__u64)bss->const32, (__u64)OUT32);
+	CHECK(bss->const64 != OUT64, "const64", "got 0x%llx != exp 0x%llx\n",
+	      (__u64)bss->const64, (__u64)OUT64);
+cleanup:
+	test_endian__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_endian.c b/tools/testing/selftests/bpf/progs/test_endian.c
new file mode 100644
index 000000000000..1d37d93102a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_endian.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
+
+#define IN16 0x1234
+#define IN32 0x12345678U
+#define IN64 0x123456789abcdef0ULL
+
+__u16 in16;
+__u32 in32;
+__u64 in64;
+
+__u16 out16;
+__u32 out32;
+__u64 out64;
+
+__u16 const16;
+__u32 const32;
+__u64 const64;
+
+SEC("raw_tp/sys_enter")
+int sys_enter(const void *ctx)
+{
+	out16 = __builtin_bswap16(in16);
+	out32 = __builtin_bswap32(in32);
+	out64 = __builtin_bswap64(in64);
+	const16 = ___bpf_swab16(IN16);
+	const32 = ___bpf_swab32(IN32);
+	const64 = ___bpf_swab64(IN64);
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.24.1

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add byte swapping selftest

[Daniel Borkmann]

On 6/30/20 8:07 AM, Andrii Nakryiko wrote:
> Add simple selftest validating byte swap built-ins and compile-time macros.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>   .../testing/selftests/bpf/prog_tests/endian.c | 53 +++++++++++++++++++
>   .../testing/selftests/bpf/progs/test_endian.c | 37 +++++++++++++
>   2 files changed, 90 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/endian.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_endian.c

This fails the build for me with:

[...]
   GEN-SKEL [test_progs] tailcall3.skel.h
   GEN-SKEL [test_progs] test_endian.skel.h
libbpf: invalid relo for 'const16' in special section 0xfff2; forgot to initialize global var?..
Error: failed to open BPF object file: 0
Makefile:372: recipe for target '/root/bpf-next/tools/testing/selftests/bpf/test_endian.skel.h' failed
make: *** [/root/bpf-next/tools/testing/selftests/bpf/test_endian.skel.h] Error 255
make: *** Deleting file '/root/bpf-next/tools/testing/selftests/bpf/test_endian.skel.h'

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add byte swapping selftest

[Andrii Nakryiko]

On Tue, Jun 30, 2020 at 7:09 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 6/30/20 8:07 AM, Andrii Nakryiko wrote:
> > Add simple selftest validating byte swap built-ins and compile-time macros.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >   .../testing/selftests/bpf/prog_tests/endian.c | 53 +++++++++++++++++++
> >   .../testing/selftests/bpf/progs/test_endian.c | 37 +++++++++++++
> >   2 files changed, 90 insertions(+)
> >   create mode 100644 tools/testing/selftests/bpf/prog_tests/endian.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/test_endian.c
>
> This fails the build for me with:
>
> [...]
>    GEN-SKEL [test_progs] tailcall3.skel.h
>    GEN-SKEL [test_progs] test_endian.skel.h
> libbpf: invalid relo for 'const16' in special section 0xfff2; forgot to initialize global var?..
> Error: failed to open BPF object file: 0
> Makefile:372: recipe for target '/root/bpf-next/tools/testing/selftests/bpf/test_endian.skel.h' failed
> make: *** [/root/bpf-next/tools/testing/selftests/bpf/test_endian.skel.h] Error 255
> make: *** Deleting file '/root/bpf-next/tools/testing/selftests/bpf/test_endian.skel.h'

Interesting. You must have a bit of an older Clang. I noticed people
submit code without explicit initialization of global variables, which
is ok now, because I think Clang doesn't emit it into the COM section
anymore. I'm surprised you don't get other compilation errors.

But regardless, I'll respin with explicit zero-initialization to fix this.