On Thu, Nov 17, 2022 at 7:17 AM Rong Tao <rtoax@xxxxxxxxxxx> wrote:
>
> From: Rong Tao <rongtao@xxxxxxxx>
>
> commit 472caa69183f("netfilter: nat: un-export nf_nat_used_tuple")
> introduce NF_NAT_MANIP_SRC/DST enum in include/net/netfilter/nf_nat.h,
> and commit b06b45e82b59("selftests/bpf: add tests for bpf_ct_set_nat_info
> kfunc") use NF_NAT_MANIP_SRC/DST in test_bpf_nf.c.
>
> In bpf kself-test config (tools/testing/selftests/bpf/config) nf_nat
> is compiled as built-in, this issue occurs just if it is compiled as
> module. We could use BPF CO-RE and ___suffix rule to avoid this.
>
> How to reproduce the error:
>
> $ make -C tools/testing/selftests/bpf/
> ...
> CLNG-BPF [test_maps] test_bpf_nf.bpf.o
> error: use of undeclared identifier 'NF_NAT_MANIP_SRC'
> bpf_ct_set_nat_info(ct, &saddr, sport, NF_NAT_MANIP_SRC);
> ^
> error: use of undeclared identifier 'NF_NAT_MANIP_DST'
> bpf_ct_set_nat_info(ct, &daddr, dport, NF_NAT_MANIP_DST);
> ^
> 2 errors generated.
>
> Signed-off-by: Rong Tao <rongtao@xxxxxxxx>
> ---
> v2: use BPF CO-RE and ___suffix rule to avoid this error.
> v1: https://lore.kernel.org/lkml/tencent_29D7ABD1744417031AA1B52C914B61158E07@xxxxxx/
> ---
> .../testing/selftests/bpf/progs/test_bpf_nf.c | 30 +++++++++++++++++--
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
> index 227e85e85dda..1706984e1a6a 100644
> --- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c
> +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
> @@ -2,6 +2,7 @@
> #include <vmlinux.h>
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_endian.h>
> +#include <bpf/bpf_core_read.h>
>
> #define EAFNOSUPPORT 97
> #define EPROTO 71
> @@ -11,6 +12,11 @@
>
> extern unsigned long CONFIG_HZ __kconfig;
>
> +enum nf_nat_manip_type___x {
> + NF_NAT_MANIP_SRC___x,
> + NF_NAT_MANIP_DST___x,
> +};
> +
> int test_einval_bpf_tuple = 0;
> int test_einval_reserved = 0;
> int test_einval_netns_id = 0;
> @@ -58,7 +64,7 @@ int bpf_ct_change_timeout(struct nf_conn *, u32) __ksym;
> int bpf_ct_set_status(struct nf_conn *, u32) __ksym;
> int bpf_ct_change_status(struct nf_conn *, u32) __ksym;
> int bpf_ct_set_nat_info(struct nf_conn *, union nf_inet_addr *,
> - int port, enum nf_nat_manip_type) __ksym;
> + int port, int type) __ksym;
>
> static __always_inline void
> nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32,
> @@ -151,16 +157,34 @@ nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32,
> union nf_inet_addr saddr = {};
> union nf_inet_addr daddr = {};
> struct nf_conn *ct_ins;
> + int manip_src;
> + int manip_dst;
> + enum nf_nat_manip_type___x mapip_type_x;
> +
> + if (!bpf_core_type_exists(enum nf_nat_manip_type)) {
> + bpf_printk("enum nf_nat_manip_type not exist.\n");
> + return;
> + }
> +
> + if (bpf_core_enum_value_exists(mapip_type_x, NF_NAT_MANIP_SRC___x))
> + manip_src = bpf_core_enum_value(mapip_type_x, NF_NAT_MANIP_SRC___x);
> + else
> + return;
> +
> + if (bpf_core_enum_value_exists(mapip_type_x, NF_NAT_MANIP_DST___x))
> + manip_dst = bpf_core_enum_value(mapip_type_x, NF_NAT_MANIP_DST___x);
> + else
> + return;
>
> bpf_ct_set_timeout(ct, 10000);
> ct->mark = 77;
>
> /* snat */
> saddr.ip = bpf_get_prandom_u32();
> - bpf_ct_set_nat_info(ct, &saddr, sport, NF_NAT_MANIP_SRC);
> + bpf_ct_set_nat_info(ct, &saddr, sport, manip_src);
I'm not sure these co-re checks are helpful. Can we just hardcode 1/0
here and below?
bpf_ct_set_nat_info(ct, &saddr, sport, 0 /*NF_NAT_MANIP_SRC*/);
bpf_ct_set_nat_info(ct, &daddr, dport, 1 /*NF_NAT_MANIP_DST*/);
But I'm also overall not sure we need to make this test flexible; we
have a lot of tests that depend on tools/testing/selftests/bpf/config;
at some point I was trying to make the tests more tolerant to
different environments, but it went nowhere..
> /* dnat */
> daddr.ip = bpf_get_prandom_u32();
> - bpf_ct_set_nat_info(ct, &daddr, dport, NF_NAT_MANIP_DST);
> + bpf_ct_set_nat_info(ct, &daddr, dport, manip_dst);
>
> ct_ins = bpf_ct_insert_entry(ct);
> if (ct_ins) {
> --
> 2.31.1
>
