diff mbox series

[bpf-next] selftests/bpf: support building selftests when CONFIG_NF_CONNTRACK=m

Message ID 1655982614-13571-1-git-send-email-alan.maguire@oracle.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series [bpf-next] selftests/bpf: support building selftests when CONFIG_NF_CONNTRACK=m | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR pending PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for Kernel LATEST on ubuntu-latest with gcc
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Kernel LATEST on ubuntu-latest with llvm-15
bpf/vmtest-bpf-next-VM_Test-3 pending Logs for Kernel LATEST on z15 with gcc
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 3 maintainers not CCed: netdev@vger.kernel.org linux-kselftest@vger.kernel.org shuah@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 37 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Alan Maguire June 23, 2022, 11:10 a.m. UTC
when CONFIG_NF_CONNTRACK=m, vmlinux BTF does not contain
BPF_F_CURRENT_NETNS or bpf_ct_opts; they are both found in nf_conntrack
BTF; for example:

bpftool btf dump file /sys/kernel/btf/nf_conntrack|grep ct_opts
[114754] STRUCT 'bpf_ct_opts' size=12 vlen=5

This causes compilation errors as follows:

  CLNG-BPF [test_maps] xdp_synproxy_kern.o
progs/xdp_synproxy_kern.c:83:14: error: declaration of 'struct bpf_ct_opts' will not be visible outside of this function [-Werror,-Wvisibility]
                                         struct bpf_ct_opts *opts,
                                                ^
progs/xdp_synproxy_kern.c:89:14: error: declaration of 'struct bpf_ct_opts' will not be visible outside of this function [-Werror,-Wvisibility]
                                         struct bpf_ct_opts *opts,
                                                ^
progs/xdp_synproxy_kern.c:397:15: error: use of undeclared identifier 'BPF_F_CURRENT_NETNS'; did you mean 'BPF_F_CURRENT_CPU'?
                .netns_id = BPF_F_CURRENT_NETNS,
                            ^~~~~~~~~~~~~~~~~~~
                            BPF_F_CURRENT_CPU
tools/testing/selftests/bpf/tools/include/vmlinux.h:43115:2: note: 'BPF_F_CURRENT_CPU' declared here
        BPF_F_CURRENT_CPU = 4294967295,

While tools/testing/selftests/bpf/config does specify
CONFIG_NF_CONNTRACK=y, it would be good to use this case to show
how we can generate a module header file via split BTF.

In the selftests Makefile, we define NF_CONNTRACK BTF via VMLINUX_BTF
(thus gaining the path determination logic it uses).  If the nf_conntrack
BTF file exists (which means it is built as a module), we run
"bpftool btf dump" to generate module BTF, and if not we simply copy
vmlinux.h to nf_conntrack.h; this allows us to avoid having to pass
a #define or deal with CONFIG variables in the program.

With these changes the test builds and passes:

Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/testing/selftests/bpf/Makefile                  | 11 +++++++++++
 tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Andrii Nakryiko June 23, 2022, 5:56 p.m. UTC | #1
On Thu, Jun 23, 2022 at 4:10 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> when CONFIG_NF_CONNTRACK=m, vmlinux BTF does not contain
> BPF_F_CURRENT_NETNS or bpf_ct_opts; they are both found in nf_conntrack
> BTF; for example:
>
> bpftool btf dump file /sys/kernel/btf/nf_conntrack|grep ct_opts
> [114754] STRUCT 'bpf_ct_opts' size=12 vlen=5
>
> This causes compilation errors as follows:
>
>   CLNG-BPF [test_maps] xdp_synproxy_kern.o
> progs/xdp_synproxy_kern.c:83:14: error: declaration of 'struct bpf_ct_opts' will not be visible outside of this function [-Werror,-Wvisibility]
>                                          struct bpf_ct_opts *opts,
>                                                 ^
> progs/xdp_synproxy_kern.c:89:14: error: declaration of 'struct bpf_ct_opts' will not be visible outside of this function [-Werror,-Wvisibility]
>                                          struct bpf_ct_opts *opts,
>                                                 ^
> progs/xdp_synproxy_kern.c:397:15: error: use of undeclared identifier 'BPF_F_CURRENT_NETNS'; did you mean 'BPF_F_CURRENT_CPU'?
>                 .netns_id = BPF_F_CURRENT_NETNS,
>                             ^~~~~~~~~~~~~~~~~~~
>                             BPF_F_CURRENT_CPU
> tools/testing/selftests/bpf/tools/include/vmlinux.h:43115:2: note: 'BPF_F_CURRENT_CPU' declared here
>         BPF_F_CURRENT_CPU = 4294967295,
>
> While tools/testing/selftests/bpf/config does specify
> CONFIG_NF_CONNTRACK=y, it would be good to use this case to show
> how we can generate a module header file via split BTF.
>
> In the selftests Makefile, we define NF_CONNTRACK BTF via VMLINUX_BTF
> (thus gaining the path determination logic it uses).  If the nf_conntrack
> BTF file exists (which means it is built as a module), we run
> "bpftool btf dump" to generate module BTF, and if not we simply copy
> vmlinux.h to nf_conntrack.h; this allows us to avoid having to pass
> a #define or deal with CONFIG variables in the program.
>
> With these changes the test builds and passes:
>
> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---

Why not just define expected types locally (doesn't have to be a full
definition)? Adding extra rule and generating header for each
potential module seems like a huge overkill.


>  tools/testing/selftests/bpf/Makefile                  | 11 +++++++++++
>  tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c |  2 +-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index cb8e552..a5fa636 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -141,6 +141,8 @@ VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
>  ifeq ($(VMLINUX_BTF),)
>  $(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
>  endif
> +# If nf_conntrack is a module, need BTF for it also
> +NF_CONNTRACK_BTF ?= $(shell dirname $(VMLINUX_BTF))/nf_conntrack
>
>  # Define simple and short `make test_progs`, `make test_sysctl`, etc targets
>  # to build individual tests.
> @@ -280,6 +282,14 @@ else
>         $(Q)cp "$(VMLINUX_H)" $@
>  endif
>
> +$(INCLUDE_DIR)/nf_conntrack.h: $(INCLUDE_DIR)/vmlinux.h
> +ifneq ("$(wildcard $(NF_CONNTRACK_BTF))","")
> +       $(call msg,GEN,,$@)
> +       $(BPFTOOL) btf dump file $(NF_CONNTRACK_BTF) format c > $@
> +else
> +       $(Q)cp $(INCLUDE_DIR)/vmlinux.h $@
> +endif
> +
>  $(RESOLVE_BTFIDS): $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/resolve_btfids   \
>                        $(TOOLSDIR)/bpf/resolve_btfids/main.c    \
>                        $(TOOLSDIR)/lib/rbtree.c                 \
> @@ -417,6 +427,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.o:                         \
>                      $(TRUNNER_BPF_PROGS_DIR)/%.c                       \
>                      $(TRUNNER_BPF_PROGS_DIR)/*.h                       \
>                      $$(INCLUDE_DIR)/vmlinux.h                          \
> +                    $$(INCLUDE_DIR)/nf_conntrack.h                     \
>                      $(wildcard $(BPFDIR)/bpf_*.h)                      \
>                      $(wildcard $(BPFDIR)/*.bpf.h)                      \
>                      | $(TRUNNER_OUTPUT) $$(BPFOBJ)
> diff --git a/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c b/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
> index 9fd62e9..8c5f46e 100644
> --- a/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
> +++ b/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: LGPL-2.1 OR BSD-2-Clause
>  /* Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
>
> -#include "vmlinux.h"
> +#include "nf_conntrack.h"
>
>  #include <bpf/bpf_helpers.h>
>  #include <bpf/bpf_endian.h>
> --
> 1.8.3.1
>
Alan Maguire June 24, 2022, 9:45 p.m. UTC | #2
On 23/06/2022 18:56, Andrii Nakryiko wrote:
> On Thu, Jun 23, 2022 at 4:10 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> when CONFIG_NF_CONNTRACK=m, vmlinux BTF does not contain
>> BPF_F_CURRENT_NETNS or bpf_ct_opts; they are both found in nf_conntrack
>> BTF; for example:
>>
>> bpftool btf dump file /sys/kernel/btf/nf_conntrack|grep ct_opts
>> [114754] STRUCT 'bpf_ct_opts' size=12 vlen=5
>>
>> This causes compilation errors as follows:
>>
>>   CLNG-BPF [test_maps] xdp_synproxy_kern.o
>> progs/xdp_synproxy_kern.c:83:14: error: declaration of 'struct bpf_ct_opts' will not be visible outside of this function [-Werror,-Wvisibility]
>>                                          struct bpf_ct_opts *opts,
>>                                                 ^
>> progs/xdp_synproxy_kern.c:89:14: error: declaration of 'struct bpf_ct_opts' will not be visible outside of this function [-Werror,-Wvisibility]
>>                                          struct bpf_ct_opts *opts,
>>                                                 ^
>> progs/xdp_synproxy_kern.c:397:15: error: use of undeclared identifier 'BPF_F_CURRENT_NETNS'; did you mean 'BPF_F_CURRENT_CPU'?
>>                 .netns_id = BPF_F_CURRENT_NETNS,
>>                             ^~~~~~~~~~~~~~~~~~~
>>                             BPF_F_CURRENT_CPU
>> tools/testing/selftests/bpf/tools/include/vmlinux.h:43115:2: note: 'BPF_F_CURRENT_CPU' declared here
>>         BPF_F_CURRENT_CPU = 4294967295,
>>
>> While tools/testing/selftests/bpf/config does specify
>> CONFIG_NF_CONNTRACK=y, it would be good to use this case to show
>> how we can generate a module header file via split BTF.
>>
>> In the selftests Makefile, we define NF_CONNTRACK BTF via VMLINUX_BTF
>> (thus gaining the path determination logic it uses).  If the nf_conntrack
>> BTF file exists (which means it is built as a module), we run
>> "bpftool btf dump" to generate module BTF, and if not we simply copy
>> vmlinux.h to nf_conntrack.h; this allows us to avoid having to pass
>> a #define or deal with CONFIG variables in the program.
>>
>> With these changes the test builds and passes:
>>
>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
>>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> ---
> 
> Why not just define expected types locally (doesn't have to be a full
> definition)? Adding extra rule and generating header for each
> potential module seems like a huge overkill.
>

True. I also forgot that if we use vmlinux in the kernel tree as the
source for vmlinux BTF, this approach won't work since it assumes it
will find nf_conntrack in the same directory. I'll figure out a simpler 
approach. Thanks for taking a look!

Alan
Andrii Nakryiko June 24, 2022, 9:48 p.m. UTC | #3
On Fri, Jun 24, 2022 at 2:46 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 23/06/2022 18:56, Andrii Nakryiko wrote:
> > On Thu, Jun 23, 2022 at 4:10 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >>
> >> when CONFIG_NF_CONNTRACK=m, vmlinux BTF does not contain
> >> BPF_F_CURRENT_NETNS or bpf_ct_opts; they are both found in nf_conntrack
> >> BTF; for example:
> >>
> >> bpftool btf dump file /sys/kernel/btf/nf_conntrack|grep ct_opts
> >> [114754] STRUCT 'bpf_ct_opts' size=12 vlen=5
> >>
> >> This causes compilation errors as follows:
> >>
> >>   CLNG-BPF [test_maps] xdp_synproxy_kern.o
> >> progs/xdp_synproxy_kern.c:83:14: error: declaration of 'struct bpf_ct_opts' will not be visible outside of this function [-Werror,-Wvisibility]
> >>                                          struct bpf_ct_opts *opts,
> >>                                                 ^
> >> progs/xdp_synproxy_kern.c:89:14: error: declaration of 'struct bpf_ct_opts' will not be visible outside of this function [-Werror,-Wvisibility]
> >>                                          struct bpf_ct_opts *opts,
> >>                                                 ^
> >> progs/xdp_synproxy_kern.c:397:15: error: use of undeclared identifier 'BPF_F_CURRENT_NETNS'; did you mean 'BPF_F_CURRENT_CPU'?
> >>                 .netns_id = BPF_F_CURRENT_NETNS,
> >>                             ^~~~~~~~~~~~~~~~~~~
> >>                             BPF_F_CURRENT_CPU
> >> tools/testing/selftests/bpf/tools/include/vmlinux.h:43115:2: note: 'BPF_F_CURRENT_CPU' declared here
> >>         BPF_F_CURRENT_CPU = 4294967295,
> >>
> >> While tools/testing/selftests/bpf/config does specify
> >> CONFIG_NF_CONNTRACK=y, it would be good to use this case to show
> >> how we can generate a module header file via split BTF.
> >>
> >> In the selftests Makefile, we define NF_CONNTRACK BTF via VMLINUX_BTF
> >> (thus gaining the path determination logic it uses).  If the nf_conntrack
> >> BTF file exists (which means it is built as a module), we run
> >> "bpftool btf dump" to generate module BTF, and if not we simply copy
> >> vmlinux.h to nf_conntrack.h; this allows us to avoid having to pass
> >> a #define or deal with CONFIG variables in the program.
> >>
> >> With these changes the test builds and passes:
> >>
> >> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> >>
> >> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> >> ---
> >
> > Why not just define expected types locally (doesn't have to be a full
> > definition)? Adding extra rule and generating header for each
> > potential module seems like a huge overkill.
> >
>
> True. I also forgot that if we use vmlinux in the kernel tree as the
> source for vmlinux BTF, this approach won't work since it assumes it
> will find nf_conntrack in the same directory. I'll figure out a simpler
> approach. Thanks for taking a look!

Just define local minimal local definitions of relevant types with
___local suffix, e.g.:

struct nf_conn___local {
    long unsigned int status;
    /* and whatever else we are using */
};

Keep in mind that with CO-RE triple underscore suffix is ignored,
which can be used to avoid type conflicts

>
> Alan
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index cb8e552..a5fa636 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -141,6 +141,8 @@  VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
 ifeq ($(VMLINUX_BTF),)
 $(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
 endif
+# If nf_conntrack is a module, need BTF for it also
+NF_CONNTRACK_BTF ?= $(shell dirname $(VMLINUX_BTF))/nf_conntrack
 
 # Define simple and short `make test_progs`, `make test_sysctl`, etc targets
 # to build individual tests.
@@ -280,6 +282,14 @@  else
 	$(Q)cp "$(VMLINUX_H)" $@
 endif
 
+$(INCLUDE_DIR)/nf_conntrack.h: $(INCLUDE_DIR)/vmlinux.h
+ifneq ("$(wildcard $(NF_CONNTRACK_BTF))","")
+	$(call msg,GEN,,$@)
+	$(BPFTOOL) btf dump file $(NF_CONNTRACK_BTF) format c > $@
+else
+	$(Q)cp $(INCLUDE_DIR)/vmlinux.h $@
+endif
+
 $(RESOLVE_BTFIDS): $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/resolve_btfids	\
 		       $(TOOLSDIR)/bpf/resolve_btfids/main.c	\
 		       $(TOOLSDIR)/lib/rbtree.c			\
@@ -417,6 +427,7 @@  $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.o:				\
 		     $(TRUNNER_BPF_PROGS_DIR)/%.c			\
 		     $(TRUNNER_BPF_PROGS_DIR)/*.h			\
 		     $$(INCLUDE_DIR)/vmlinux.h				\
+		     $$(INCLUDE_DIR)/nf_conntrack.h			\
 		     $(wildcard $(BPFDIR)/bpf_*.h)			\
 		     $(wildcard $(BPFDIR)/*.bpf.h)			\
 		     | $(TRUNNER_OUTPUT) $$(BPFOBJ)
diff --git a/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c b/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
index 9fd62e9..8c5f46e 100644
--- a/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
+++ b/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
@@ -1,7 +1,7 @@ 
 // SPDX-License-Identifier: LGPL-2.1 OR BSD-2-Clause
 /* Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
 
-#include "vmlinux.h"
+#include "nf_conntrack.h"
 
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_endian.h>