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

From: Daniel Borkmann <daniel@iogearbox.net>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Lukas Wunner <lukas@wunner.de>,
	Jozsef Kadlecsik <kadlec@netfilter.org>,
	Florian Westphal <fw@strlen.de>,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	netdev@vger.kernel.org, Martin Mares <mj@ucw.cz>,
	Dmitry Safonov <0x7f454c46@gmail.com>,
	Thomas Graf <tgraf@suug.ch>, Alexei Starovoitov <ast@kernel.org>,
	David Miller <davem@davemloft.net>
Subject: Re: [PATCH nf-next 3/3] netfilter: Introduce egress hook
Date: Sat, 14 Mar 2020 01:12:02 +0100	[thread overview]
Message-ID: <1bd50836-33c4-da44-5771-654bfb0348cc@iogearbox.net> (raw)
In-Reply-To: <20200313145526.ikovaalfuy7rnkdl@salvia>

On 3/13/20 3:55 PM, Pablo Neira Ayuso wrote:
> On Wed, Mar 11, 2020 at 03:05:16PM +0100, Daniel Borkmann wrote:
>> On 3/11/20 12:59 PM, Lukas Wunner wrote:
>>> Commit e687ad60af09 ("netfilter: add netfilter ingress hook after
>>> handle_ing() under unique static key") introduced the ability to
>>> classify packets on ingress.
>>>
>>> Allow the same on egress.  Position the hook immediately before a packet
>>> is handed to tc and then sent out on an interface, thereby mirroring the
>>> ingress order.  This order allows marking packets in the netfilter
>>> egress hook and subsequently using the mark in tc.  Another benefit of
>>> this order is consistency with a lot of existing documentation which
>>> says that egress tc is performed after netfilter hooks.
>>>
>>> Egress hooks already exist for the most common protocols, such as
>>> NF_INET_LOCAL_OUT or NF_ARP_OUT, and those are to be preferred because
>>> they are executed earlier during packet processing.  However for more
>>> exotic protocols, there is currently no provision to apply netfilter on
>>> egress.  A common workaround is to enslave the interface to a bridge and
>>
>> Sorry for late reply, but still NAK.
> 
> I agree Lukas use-case is very specific.
> 
> However, this is useful.
> 
> We have plans to support for NAT64 and NAT46, this is the right spot
> to do this mangling. There is already support for the tunneling

But why is existing local-out or post-routing hook _not_ sufficient for
NAT64 given it being IP based?

> infrastructure in netfilter from ingress, this spot from egress will
> allow us to perform the tunneling from here. There is also no way to
> drop traffic generated by dhclient, this also allow for filtering such
> locally generated traffic. And many more.

This is a known fact for ~17 years [0] or probably more by now and noone
from netfilter folks cared to address it in all the years, so I presume
it cannot be important enough, and these days it can be filtered through
other means already. Tbh, it's a bit laughable that you bring this up as
an argument ...

   [0] https://www.spinics.net/lists/netfilter/msg19488.html

> Performance impact is negligible, Lukas already provided what you
> asked for.

Sure, and the claimed result was "as said the fast-path gets faster, not
slower" without any explanation or digging into details on why this might
be, especially since it appears counter-intuitive as was stated by the
author ... and later demonstrated w/ measurements that show the opposite.