From: "Toke Høiland-Jørgensen" <toke@redhat•com>
To: Andrii Nakryiko <andrii.nakryiko@gmail•com>
Cc: Alexei Starovoitov <ast@kernel•org>,
Daniel Borkmann <daniel@iogearbox•net>,
Andrii Nakryiko <andrii@kernel•org>,
Martin KaFai Lau <martin.lau@linux•dev>,
Song Liu <song@kernel•org>, Yonghong Song <yhs@fb•com>,
John Fastabend <john.fastabend@gmail•com>,
KP Singh <kpsingh@kernel•org>,
Stanislav Fomichev <sdf@google•com>, Hao Luo <haoluo@google•com>,
Jiri Olsa <jolsa@kernel•org>,
"David S. Miller" <davem@davemloft•net>,
Jakub Kicinski <kuba@kernel•org>,
Jesper Dangaard Brouer <hawk@kernel•org>,
Lorenzo Bianconi <lorenzo@kernel•org>,
Kumar Kartikeya Dwivedi <memxor@gmail•com>,
Jiri Benc <jbenc@redhat•com>, Eric Dumazet <edumazet@google•com>,
Paolo Abeni <pabeni@redhat•com>,
bpf@vger•kernel.org, netdev@vger•kernel.org
Subject: Re: [PATCH bpf 1/2] bpf: Add dummy type reference to nf_conn___init to fix type deduplication
Date: Thu, 01 Dec 2022 12:06:17 +0100 [thread overview]
Message-ID: <87fsdzxu1i.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4BzafebzBTVqtTHotRcySXPafF5JK11Svpirtnvz7c9O7uQ@mail.gmail.com>
Andrii Nakryiko <andrii.nakryiko@gmail•com> writes:
> On Wed, Nov 30, 2022 at 6:42 AM Toke Høiland-Jørgensen <toke@redhat•com> 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@mail.gmail.com/
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@toke.dk/
>>
>> 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@redhat•com>
>> ---
>> 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
prev parent reply other threads:[~2022-12-01 11:11 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-30 14:42 [PATCH bpf 1/2] bpf: Add dummy type reference to nf_conn___init to fix type deduplication Toke Høiland-Jørgensen
2022-12-01 1:16 ` Yonghong Song
2022-12-01 1:16 ` Andrii Nakryiko
2022-12-01 11:06 ` Toke Høiland-Jørgensen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87fsdzxu1i.fsf@toke.dk \
--to=toke@redhat$(echo .)com \
--cc=andrii.nakryiko@gmail$(echo .)com \
--cc=andrii@kernel$(echo .)org \
--cc=ast@kernel$(echo .)org \
--cc=bpf@vger$(echo .)kernel.org \
--cc=daniel@iogearbox$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=haoluo@google$(echo .)com \
--cc=hawk@kernel$(echo .)org \
--cc=jbenc@redhat$(echo .)com \
--cc=john.fastabend@gmail$(echo .)com \
--cc=jolsa@kernel$(echo .)org \
--cc=kpsingh@kernel$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=lorenzo@kernel$(echo .)org \
--cc=martin.lau@linux$(echo .)dev \
--cc=memxor@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=sdf@google$(echo .)com \
--cc=song@kernel$(echo .)org \
--cc=yhs@fb$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox