Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> writes: > On Wed, Nov 30, 2022 at 6:42 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: >> >> The bpf_ct_set_nat_info() kfunc is defined in the nf_nat.ko module, and >> takes as a parameter the nf_conn___init struct, which is allocated through >> the bpf_xdp_ct_alloc() helper defined in the nf_conntrack.ko module. >> However, because kernel modules can't deduplicate BTF types between each >> other, and the nf_conn___init struct is not referenced anywhere in vmlinux >> BTF, this leads to two distinct BTF IDs for the same type (one in each >> module). This confuses the verifier, as described here: >> > > Argh, shouldn't have wasted writing [1], but oh well. > > [1] https://lore.kernel.org/bpf/CAEf4Bza2xDZ45kxxa3dg1C_RWE=UB5UFYEuFp6rbXgX=LRHv-A@xxxxxxxxxxxxxx/ Ah, yeah, crossed streams; as you can see I came to the same conclusion wrt types being conceptually independent. >> https://lore.kernel.org/all/87leoh372s.fsf@xxxxxxx/ >> >> As a workaround, add a dummy pointer to the type in net/filter.c, so the >> type definition gets included in vmlinux BTF. This way, both modules can >> refer to the same type ID (as they both build on top of vmlinux BTF), and >> the verifier is no longer confused. >> >> Fixes: 820dc0523e05 ("net: netfilter: move bpf_ct_set_nat_info kfunc in nf_nat_bpf.c") >> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx> >> --- >> net/core/filter.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/net/core/filter.c b/net/core/filter.c >> index bb0136e7a8e4..1bdf9efe8593 100644 >> --- a/net/core/filter.c >> +++ b/net/core/filter.c >> @@ -80,6 +80,7 @@ >> #include <net/tls.h> >> #include <net/xdp.h> >> #include <net/mptcp.h> >> +#include <net/netfilter/nf_conntrack_bpf.h> >> >> static const struct bpf_func_proto * >> bpf_sk_base_func_proto(enum bpf_func_id func_id); >> @@ -11531,3 +11532,17 @@ bpf_sk_base_func_proto(enum bpf_func_id func_id) >> >> return func; >> } >> + >> +#if IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) >> +/* The nf_conn___init type is used in the NF_CONNTRACK kfuncs. The kfuncs are >> + * defined in two different modules, and we want to be able to use them >> + * interchangably with the same BTF type ID. Because modules can't de-duplicate >> + * BTF IDs between each other, we need the type to be referenced in the vmlinux >> + * BTF or the verifier will get confused about the different types. So we add >> + * this dummy pointer to serve as a type reference which will be included in >> + * vmlinux BTF, allowing both modules to refer to the same type ID. >> + * >> + * We use a pointer as that is smaller than an instance of the struct. >> + */ >> +const struct nf_conn___init *ctinit; >> +#endif > > Use BTF_TYPE_EMIT() instead maybe? Ah, TIL about that macro; thanks, will fix! -Toke
