Message-ID: <20160923131739.GA15828@salvia>
Date:   Fri, 23 Sep 2016 15:17:39 +0200
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Thomas Graf <tgraf@...g.ch>, Daniel Mack <daniel@...que.org>,
        htejun@...com, ast@...com, davem@...emloft.net, kafai@...com,
        fw@...len.de, harald@...hat.com, netdev@...r.kernel.org,
        sargun@...gun.me, cgroups@...r.kernel.org
Subject: Re: [PATCH v6 5/6] net: ipv4, ipv6: run cgroup ebpf egress programs

On Thu, Sep 22, 2016 at 05:12:57PM +0200, Daniel Borkmann wrote:
> On 09/22/2016 02:05 PM, Pablo Neira Ayuso wrote:
[...]
> >Have a look at net/ipv4/netfilter/nft_chain_route_ipv4.c for instance.
> >In your case, you have to add a new chain type:
> >
> >static const struct nf_chain_type nft_chain_bpf = {
> >         .name           = "bpf",
> >         .type           = NFT_CHAIN_T_bpf,
> >         ...
> >         .hooks          = {
> >                 [NF_INET_LOCAL_IN]      = nft_do_bpf,
> >                 [NF_INET_LOCAL_OUT]     = nft_do_bpf,
> >                 [NF_INET_FORWARD]       = nft_do_bpf,
> >                 [NF_INET_PRE_ROUTING]   = nft_do_bpf,
> >                 [NF_INET_POST_ROUTING]  = nft_do_bpf,
> >         },
> >};
> >
> >nft_do_bpf() is the raw netfilter hook that you register, this hook
> >will just execute to iterate over the list of bpf filters and run
> >them.
> >
> >This new chain is created on demand, so no overhead if not needed, eg.
> >
> >nft add table bpf
> >nft add chain bpf input { type bpf hook output priority 0\; }
> >
> >Then, you add a rule for each bpf program you want to run, just like
> >tc+bpf.
> 
> But from a high-level point of view, this sounds like a huge hack to me,
> in the sense that nft as a bytecode engine (from whole architecture I
> mean) calls into another bytecode engine such as bpf as an extension.

nft is not only bytecode engine, it provides a netlink socket
interface to register hooks (from user perspective, these are called
basechain). It is providing the infrastructure that you're lacking
indeed and addressing the concerns I mentioned about the visibility of
the global policy that you want to apply on the packet path.

As I explained you can potentially add any basechain type with
specific semantics. Proposed semantics for this bpf chain would be:

1) You can use any of the existing netfilter hooks.
2) You can only run bpf program from there. No chance for the user
   can mix nftables with bpf VM.

> And bpf code from there isn't using any of the features from nft
> besides being invoked from the hook

I think there's a misunderstading here.

You will not run nft_do_chain(), you don't waste cycles to run what is
specific to nftables. You will just run nft_do_bpf() which will just
do what you want to run for each packet. Thus, you have control on
what nft_do_bpf() does and decide on what that function spend cycles
on.

> [...] I was hoping that nft would try to avoid some of those exotic
> modules we have from xt, I would consider xt_bpf (no offense ;))

This has nothing to do with it. In xt_bpf you waste cycles running
code that is specific to iptables, what I propose would not, just the
generic hook code and then your code.

[...]
> >Benefits are, rewording previous email:
> >
> >* You get access to all of the existing netfilter hooks in one go
> >   to run bpf programs. No need for specific redundant hooks. This
> >   provides raw access to the netfilter hook, you define the little
> >   code that your hook runs before you bpf run invocation. So there
> >   is *no need to bloat the stack with more hooks, we use what we
> >   have.*
> 
> But also this doesn't really address the fundamental underlying problem
> that is discussed here. nft doesn't even have cgroups v2 support.

You don't need native cgroups v2 support in nft, you just run bpf
programs from the native bpf basechain type. So whatever bpf supports,
you can do it.

Instead, if you take this approach, you will get access to all of the
existing hooks to run bpf programs, this includes arp, bridge and
potentially run filters for both ip and ip6 through our inet family.

[...]
> Or would the idea be that the current netfilter hooks would be redone in
> a way that they are generic enough so that any other user could make use
> of it independent of netfilter?

Redone? Why? What do you need, a rename?

Dependencies are very few: CONFIG_NETFILTER for the hooks,
CONFIG_NF_TABLES to obtain the netlink interface to load the bpf
programs and CONFIG_NF_TABLES_BPF to define the bpf basechain type
semantics to run bpf programs from there. It's actually very little
boilerplate code.

Other than that, I can predict where you're going: You will end up
adding a hook just before/after every of the existing netfilter hooks,
and that is really nonsense to me. Why bloat the stack with more
hooks? Use what it is already available.

Powered by blists - more mailing lists