netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Daniel Borkmann <daniel@iogearbox.net>
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: Sun, 15 Mar 2020 14:28:36 +0100	[thread overview]
Message-ID: <20200315132836.cj36ape6rpw33iqb@salvia> (raw)
In-Reply-To: <1bd50836-33c4-da44-5771-654bfb0348cc@iogearbox.net>

Hello Daniel,

On Sat, Mar 14, 2020 at 01:12:02AM +0100, Daniel Borkmann wrote:
> On 3/13/20 3:55 PM, Pablo Neira Ayuso wrote:
[...]
> > 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?

Those hooks are not coming at the end of the IP processing. There is
very relevant IP code after those hooks that cannot be bypassed such
as fragmentation, tunneling and neighbour output. Such transformation
needs to happen after the IP processing, exactly from where Lukas is
proposing.

[...]
> > 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

Look: ip6tables, arptables and ebtables are a copy and paste from the
original iptables.

At that time, the only way one way to add support for ingress/egress
classification in netfilter: add "devtables", yet another copy and
past from iptables, that was a no-go.

This is not a problem anymore since there is a consolidated netfilter
framework to achieve ingress/egress classification.

> > 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.

I remember one of your collegues used this same argument against new
hooks back in 2015 [0], and the introduction of this hook was proven
to be negligible. This patchset introduces code that looks very much
the same.

I can make a list of recent updates to the output path, several of
them very are targeted to very specific usecases. You did not care at
all about performance impact of those at all, however, you care about
netfilter for some unknown reason.

In my opinion, your original feedback has been addressed, it's time to
move on.

Thank you.

[0] https://lwn.net/Articles/642414/

  reply	other threads:[~2020-03-15 13:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-11 11:59 [PATCH nf-next 0/3] Netfilter egress hook Lukas Wunner
2020-03-11 11:59 ` [PATCH nf-next 1/3] netfilter: Rename ingress hook include file Lukas Wunner
2020-03-11 11:59 ` [PATCH nf-next 2/3] netfilter: Generalize ingress hook Lukas Wunner
2020-03-11 11:59 ` [PATCH nf-next 3/3] netfilter: Introduce egress hook Lukas Wunner
2020-03-11 14:05   ` Daniel Borkmann
2020-03-11 15:54     ` Lukas Wunner
2020-03-12 22:40       ` Daniel Borkmann
2020-03-13 14:55     ` Pablo Neira Ayuso
2020-03-14  0:12       ` Daniel Borkmann
2020-03-15 13:28         ` Pablo Neira Ayuso [this message]
2020-04-23 14:44           ` Laura Garcia
2020-04-23 16:05             ` Lukas Wunner
2020-04-27 23:44               ` Pablo Neira Ayuso
2020-04-28 20:11               ` Daniel Borkmann
2020-08-20 10:37                 ` Lukas Wunner
2020-08-20 16:35                   ` Lukas Wunner
2020-03-18  0:21 ` [PATCH nf-next 0/3] Netfilter " Pablo Neira Ayuso

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=20200315132836.cj36ape6rpw33iqb@salvia \
    --to=pablo@netfilter.org \
    --cc=0x7f454c46@gmail.com \
    --cc=ast@kernel.org \
    --cc=coreteam@netfilter.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=fw@strlen.de \
    --cc=kadlec@netfilter.org \
    --cc=lukas@wunner.de \
    --cc=mj@ucw.cz \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=tgraf@suug.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).