Re: [PATCH bpf-next 2/6] libbpf: move bpf_helpers.h, bpf_endian.h into libbpf

From: Song Liu <songliubraving@fb.com>
To: Andrii Nakryiko <andriin@fb.com>
Cc: "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>,
	"andrii.nakryiko@gmail.com" <andrii.nakryiko@gmail.com>,
	Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 2/6] libbpf: move bpf_helpers.h, bpf_endian.h into libbpf
Date: Tue, 1 Oct 2019 21:19:19 +0000	[thread overview]
Message-ID: <E0A2B793-7660-481B-9ADB-6B544518865A@fb.com> (raw)
In-Reply-To: <20190930185855.4115372-3-andriin@fb.com>

> On Sep 30, 2019, at 11:58 AM, Andrii Nakryiko <andriin@fb.com> wrote:
> 
> Make bpf_helpers.h and bpf_endian.h official part of libbpf. Ensure they
> are installed along the other libbpf headers.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/Makefile      |   4 +-
> tools/lib/bpf/bpf_endian.h  |  72 +++++
> tools/lib/bpf/bpf_helpers.h | 527 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 602 insertions(+), 1 deletion(-)
> create mode 100644 tools/lib/bpf/bpf_endian.h
> create mode 100644 tools/lib/bpf/bpf_helpers.h
> 
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index c6f94cffe06e..2ff345981803 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -240,7 +240,9 @@ install_headers:
> 		$(call do_install,libbpf.h,$(prefix)/include/bpf,644); \
> 		$(call do_install,btf.h,$(prefix)/include/bpf,644); \
> 		$(call do_install,libbpf_util.h,$(prefix)/include/bpf,644); \
> -		$(call do_install,xsk.h,$(prefix)/include/bpf,644);
> +		$(call do_install,xsk.h,$(prefix)/include/bpf,644); \
> +		$(call do_install,bpf_helpers.h,$(prefix)/include/bpf,644); \
> +		$(call do_install,bpf_endian.h,$(prefix)/include/bpf,644);
> 
> install_pkgconfig: $(PC_FILE)
> 	$(call QUIET_INSTALL, $(PC_FILE)) \
> diff --git a/tools/lib/bpf/bpf_endian.h b/tools/lib/bpf/bpf_endian.h
> new file mode 100644
> index 000000000000..fbe28008450f
> --- /dev/null
> +++ b/tools/lib/bpf/bpf_endian.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> +#ifndef __BPF_ENDIAN__
> +#define __BPF_ENDIAN__
> +
> +#include <linux/stddef.h>
> +#include <linux/swab.h>
> +
> +/* LLVM's BPF target selects the endianness of the CPU
> + * it compiles on, or the user specifies (bpfel/bpfeb),
> + * respectively. The used __BYTE_ORDER__ is defined by
> + * the compiler, we cannot rely on __BYTE_ORDER from
> + * libc headers, since it doesn't reflect the actual
> + * requested byte order.
> + *
> + * Note, LLVM's BPF target has different __builtin_bswapX()
> + * semantics. It does map to BPF_ALU | BPF_END | BPF_TO_BE
> + * in bpfel and bpfeb case, which means below, that we map
> + * to cpu_to_be16(). We could use it unconditionally in BPF
> + * case, but better not rely on it, so that this header here
> + * can be used from application and BPF program side, which
> + * use different targets.
> + */
> +#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_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_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)
> +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> +# define __bpf_ntohs(x)			(x)
> +# define __bpf_htons(x)			(x)
> +# define __bpf_constant_ntohs(x)	(x)
> +# define __bpf_constant_htons(x)	(x)
> +# define __bpf_ntohl(x)			(x)
> +# define __bpf_htonl(x)			(x)
> +# define __bpf_constant_ntohl(x)	(x)
> +# define __bpf_constant_htonl(x)	(x)
> +# define __bpf_be64_to_cpu(x)		(x)
> +# define __bpf_cpu_to_be64(x)		(x)
> +# define __bpf_constant_be64_to_cpu(x)  (x)
> +# define __bpf_constant_cpu_to_be64(x)  (x)
> +#else
> +# error "Fix your compiler's __BYTE_ORDER__?!"
> +#endif
> +
> +#define bpf_htons(x)				\
> +	(__builtin_constant_p(x) ?		\
> +	 __bpf_constant_htons(x) : __bpf_htons(x))
> +#define bpf_ntohs(x)				\
> +	(__builtin_constant_p(x) ?		\
> +	 __bpf_constant_ntohs(x) : __bpf_ntohs(x))
> +#define bpf_htonl(x)				\
> +	(__builtin_constant_p(x) ?		\
> +	 __bpf_constant_htonl(x) : __bpf_htonl(x))
> +#define bpf_ntohl(x)				\
> +	(__builtin_constant_p(x) ?		\
> +	 __bpf_constant_ntohl(x) : __bpf_ntohl(x))
> +#define bpf_cpu_to_be64(x)			\
> +	(__builtin_constant_p(x) ?		\
> +	 __bpf_constant_cpu_to_be64(x) : __bpf_cpu_to_be64(x))
> +#define bpf_be64_to_cpu(x)			\
> +	(__builtin_constant_p(x) ?		\
> +	 __bpf_constant_be64_to_cpu(x) : __bpf_be64_to_cpu(x))
> +
> +#endif /* __BPF_ENDIAN__ */
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> new file mode 100644
> index 000000000000..a1d9b97b8e15
> --- /dev/null
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -0,0 +1,527 @@
> +/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> +#ifndef __BPF_HELPERS__
> +#define __BPF_HELPERS__
> +
> +#define __uint(name, val) int (*name)[val]
> +#define __type(name, val) val *name

Similar to the concern with 4/6, maybe we should rename/prefix/postfix
these two macros?

Thanks,
Song