From: Daniel Borkmann <daniel@iogearbox.net>
To: Florian Westphal <fw@strlen.de>
Cc: davem@davemloft.net, netdev@vger.kernel.org,
Pablo Neira Ayuso <pablo@netfilter.org>,
Alexei Starovoitov <ast@kernel.org>
Subject: Re: [PATCH net-next] netfilter: revert introduction of egress hook
Date: Wed, 18 Mar 2020 11:50:56 +0100 [thread overview]
Message-ID: <c7c6fb40-06f9-8078-6f76-5dc75a094e25@iogearbox.net> (raw)
In-Reply-To: <20200318100227.GE979@breakpoint.cc>
On 3/18/20 11:02 AM, Florian Westphal wrote:
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>> This reverts the following commits:
>>
>> 8537f78647c0 ("netfilter: Introduce egress hook")
>> 5418d3881e1f ("netfilter: Generalize ingress hook")
>> b030f194aed2 ("netfilter: Rename ingress hook include file")
>>
>> From the discussion in [0], the author's main motivation to add a hook
>> in fast path is for an out of tree kernel module, which is a red flag
>> to begin with.
>
> The author did post patches for nftables, i.e. you can hook up rulesets to
> this new hook point.
>
>> is on future extensions w/o concrete code in the tree yet. Revert as
>> suggested [1] given the weak justification to add more hooks to critical
>> fast-path.
>
> Do you have an alternative suggestion on how to expose this?
Yeah, I think we should not plaster the stack with same/similar hooks that
achieve the same functionality next to each other over and over at the cost
of performance for users .. ideally there should just be a single entry point
that is very lightweight/efficient when not used and can otherwise patch to
a direct call when in use. Recent work from KP Singh goes into this direction
with the fmodify_return work [0], so we would have a single static key which
wraps an empty function call entry which can then be patched by the kernel at
runtime. Inside that trampoline we can still keep the ordering intact, but
imho this would overall better reduce overhead when functionality is not used
compared to the practice of duplication today.
Thanks,
Daniel
[0] https://lore.kernel.org/bpf/20200304191853.1529-1-kpsingh@chromium.org/
next prev parent reply other threads:[~2020-03-18 10:51 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-18 9:33 [PATCH net-next] netfilter: revert introduction of egress hook Daniel Borkmann
2020-03-18 9:36 ` Pablo Neira Ayuso
2020-03-18 9:41 ` Daniel Borkmann
2020-03-18 9:51 ` Pablo Neira Ayuso
2020-03-18 10:02 ` Florian Westphal
2020-03-18 10:50 ` Daniel Borkmann [this message]
2020-03-18 12:33 ` Florian Westphal
2020-03-18 14:22 ` Daniel Borkmann
2020-03-19 1:45 ` David Miller
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=c7c6fb40-06f9-8078-6f76-5dc75a094e25@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=ast@kernel.org \
--cc=davem@davemloft.net \
--cc=fw@strlen.de \
--cc=netdev@vger.kernel.org \
--cc=pablo@netfilter.org \
/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).