Re: [PATCH bpf-next v3 7/9] net/netfilter: Add unstable CT lookup helpers for XDP and TC-BPF

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Pablo Neira Ayuso <pablo@netfilter.org> writes:

> On Sat, Dec 11, 2021 at 07:35:58PM +0100, Toke Høiland-Jørgensen wrote:
>> Pablo Neira Ayuso <pablo@netfilter.org> writes:
>> 
>> > On Fri, Dec 10, 2021 at 09:01:29PM +0530, Kumar Kartikeya Dwivedi wrote:
>> >> On Fri, Dec 10, 2021 at 08:39:14PM IST, Pablo Neira Ayuso wrote:
>> >> > On Fri, Dec 10, 2021 at 06:32:28PM +0530, Kumar Kartikeya Dwivedi wrote:
>> >> > [...]
>> >> > >  net/netfilter/nf_conntrack_core.c | 252 ++++++++++++++++++++++++++++++
>> >> > >  7 files changed, 497 insertions(+), 1 deletion(-)
>> >> > >
>> >> > [...]
>> >> > > diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
>> >> > > index 770a63103c7a..85042cb6f82e 100644
>> >> > > --- a/net/netfilter/nf_conntrack_core.c
>> >> > > +++ b/net/netfilter/nf_conntrack_core.c
>> >> >
>> >> > Please, keep this new code away from net/netfilter/nf_conntrack_core.c
>> >> 
>> >> Ok. Can it be a new file under net/netfilter, or should it live elsewhere?
>> >
>> > IPVS and OVS use conntrack for already quite a bit of time and they
>> > keep their code in their respective folders.
>> 
>> Those are users, though.
>
> OK, I see this as a yet user of the conntrack infrastructure.

The users are the BPF programs; this series adds the exports. I.e., the
code defines an API that BPF programs can hook into, and implements the
validation and lifetime enforcement that is necessary for the particular
data structures being exposed. This is very much something that the
module doing the exports should be concerned with, so from that
perspective it makes sense to keep it in the nf_conntrack kmod.

>> This is adding a different set of exported functions, like a BPF
>> version of EXPORT_SYMBOL(). We don't put those outside the module
>> where the code lives either...
>
> OVS and IPVS uses Kconfig to enable the conntrack module as a
> dependency. Then, add module that is loaded when conntrack is used.

BPF can't do that, though: all the core BPF code is always built into
the kernel, so we can't have any dependencies on module code. Until now,
this has meant that hooking into modules has been out of scope for BPF
entirely. With kfuncs and the module BTF support this is now possible,
but it's a bit "weird" (i.e., different) compared to what we're used to
with kernel modules.

This series represents the first instance of actually implementing BPF
hooks into a module, BTW, so opinions on how to do it best are
absolutely welcome. But I have a hard time seeing how this could be done
without introducing *any* new code into the conntrack module...

-Toke