Re: [PATCH nf-next v3 3/3] netfilter: Introduce egress hook

From: John Fastabend <john.fastabend@gmail.com>
To: "Laura García Liébana" <nevola@gmail.com>,
	"John Fastabend" <john.fastabend@gmail.com>
Cc: Lukas Wunner <lukas@wunner.de>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Jozsef Kadlecsik <kadlec@netfilter.org>,
	Florian Westphal <fw@strlen.de>,
	Netfilter Development Mailing list 
	<netfilter-devel@vger.kernel.org>,
	coreteam@netfilter.org, netdev@vger.kernel.org,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Eric Dumazet <edumazet@google.com>, Thomas Graf <tgraf@suug.ch>,
	David Miller <davem@davemloft.net>
Subject: Re: [PATCH nf-next v3 3/3] netfilter: Introduce egress hook
Date: Fri, 04 Sep 2020 08:46:27 -0700	[thread overview]
Message-ID: <5f5261535a32a_1932208c8@john-XPS-13-9370.notmuch> (raw)
In-Reply-To: <CAF90-WgMiJkFsZaGBJQVVrmQz9==cq22NErpcWuE7z-Q+A8PzQ@mail.gmail.com>

Laura García Liébana wrote:
> Hi,
> 
> On Thu, Sep 3, 2020 at 7:00 AM John Fastabend <john.fastabend@gmail.com> wrote:
> >
> [...]
> >
> > I don't think it actualy improves performance at least I didn't observe
> > that. From the code its not clear why this would be the case either. As
> > a nit I would prefer that line removed from the commit message.
> >
> 
> It hasn't been proven to be untrue either.

huh? Its stated in the commit message with no reason for why it might
be the case and I can't reproduce it. Also the numbers posted show such a
slight increase (~1%) its likely just random system noise.

Sorry maybe that was a joke? Just poured some coffee so might be missing it.

> 
> 
> [...]
> >
> > Do you have plans to address the performance degradation? Otherwise
> > if I was building some new components its unclear why we would
> > choose the slower option over the tc hook. The two suggested
> > use cases security policy and DSR sound like new features, any
> > reason to not just use existing infrastructure?
> >
> 
> Unfortunately, tc is not an option as it is required to interact with
> nft objects (sets, maps, chains, etc), more complex than just a drop.
> Also, when building new features we try to maintain the application
> stack as simple as possible, not trying to do ugly integrations.

We have code that interacts with iptables as well. How I read the
above is in your case you have a bunch of existing software and you
want something slightly faster. Even if its not as fast the 10%
overhead is OK in your case and/or you believe the overhead of all
the other components is much higher so it will be lost in the noise.

> I understand that you measure performance with a drop, but using this
> hook we reduce the datapath consistently for these use cases and
> hence, improving traffic performance.

I measured drops because it was the benchmark provided in the patch
series. Also it likely looks a lot like any DDOS that might be put
there. You mentioned security policies which should probably include
DDOS so I would expect drop performance to be at least a useful
metric even if its not the only or most important in your case.

Lets post a selftest that represents the use case so folks like
myself can understand and benchmark correctly. This gives the extra
benefit of ensuring we don't regress going forward and can add it
to CI.

> 
> Thank you for your time!