netdev - Re: [PATCH bpf-next 1/2] selftests: bpf: use KHDR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAADnVQ+JBq8-6Rhi_LHX470uj2_2xxJAhgdUfg_abUxEDqpdJQ@mail.gmail.com>
Date: Sat, 17 Aug 2024 09:22:02 +0200
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: "Matthieu Baerts (NGI0)" <matttbe@...nel.org>
Cc: MPTCP Upstream <mptcp@...ts.linux.dev>, Andrii Nakryiko <andrii@...nel.org>, 
	Eduard Zingerman <eddyz87@...il.com>, Mykola Lysenko <mykolal@...com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, Shuah Khan <shuah@...nel.org>, 
	"David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Jesper Dangaard Brouer <hawk@...nel.org>, bpf <bpf@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] selftests: bpf: use KHDR_INCLUDES for the
 UAPI headers

On Fri, Aug 16, 2024 at 7:56 PM Matthieu Baerts (NGI0)
<matttbe@...nel.org> wrote:
>
> Instead of duplicating UAPI header files in 'tools/include/uapi', the
> BPF selftests can also look at the header files inside the kernel
> source.
>
> To do that, the kernel selftests infrastructure provides the
> 'KHDR_INCLUDES' variable. This is what is being used in most selftests,
> because it is what is recommended in the documentation [1]. If the
> selftests are not executed from the kernel sources, it is possible to
> override the variable, e.g.
>
>   make KHDR_INCLUDES="-I${HDR_DIR}/include" -C "${KSFT_DIR}"
>
> ... where ${HDR_DIR} has been generated by this command:
>
>   make headers_install INSTALL_HDR_PATH="${HDR_DIR}"
>
> Thanks to 'KHDR_INCLUDES', it is no longer needed to duplicate header
> files for userspace test programs, and these programs can include UAPI
> header files without the 'uapi' prefix.
>
> Note that it is still required to use 'tools/include/uapi' -- APIDIR,
> which corresponds to TOOLS_INCLUDES from lib.mk -- for the BPF programs,
> not to conflict with what is already defined in vmlinux.h.
>
> Link: https://docs.kernel.org/dev-tools/kselftest.html#contributing-new-tests-details [1]
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@...nel.org>
> ---
>  tools/testing/selftests/bpf/Makefile                       | 2 +-
>  tools/testing/selftests/bpf/prog_tests/assign_reuse.c      | 2 +-
>  tools/testing/selftests/bpf/prog_tests/tc_links.c          | 4 ++--
>  tools/testing/selftests/bpf/prog_tests/tc_netkit.c         | 2 +-
>  tools/testing/selftests/bpf/prog_tests/tc_opts.c           | 2 +-
>  tools/testing/selftests/bpf/prog_tests/user_ringbuf.c      | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_bonding.c       | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c   | 2 +-
>  tools/testing/selftests/bpf/prog_tests/xdp_link.c          | 2 +-
>  tools/testing/selftests/bpf/xdp_features.c                 | 4 ++--
>  12 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 4eceb491a8ae..6a7aeae7e206 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -37,7 +37,7 @@ CFLAGS += -g $(OPT_FLAGS) -rdynamic                                   \
>           -Wall -Werror -fno-omit-frame-pointer                         \
>           $(GENFLAGS) $(SAN_CFLAGS) $(LIBELF_CFLAGS)                    \
>           -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR)          \
> -         -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
> +         -I$(TOOLSINCDIR) $(KHDR_INCLUDES) -I$(OUTPUT)
>  LDFLAGS += $(SAN_LDFLAGS)
>  LDLIBS += $(LIBELF_LIBS) -lz -lrt -lpthread
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/assign_reuse.c b/tools/testing/selftests/bpf/prog_tests/assign_reuse.c
> index 989ee4d9785b..3d06bf5a1ba4 100644
> --- a/tools/testing/selftests/bpf/prog_tests/assign_reuse.c
> +++ b/tools/testing/selftests/bpf/prog_tests/assign_reuse.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright (c) 2023 Isovalent */
> -#include <uapi/linux/if_link.h>
> +#include <linux/if_link.h>

No. This is not an option.
User space shouldn't include kernel headers like this.
Long ago tools/include directory was specifically
created to break such dependency.
Back then it was done for perf.

pw-bot: cr