public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>,
	netfilter-devel@vger.kernel.org, davem@davemloft.net,
	netdev@vger.kernel.org, jhs@mojatatu.com
Subject: Re: [PATCH 6/6] net: move qdisc ingress filtering on top of netfilter ingress hooks
Date: Thu, 30 Apr 2015 02:48:39 +0200	[thread overview]
Message-ID: <20150430004839.GG7025@acer.localdomain> (raw)
In-Reply-To: <55417A3A.50405@iogearbox.net>

On 30.04, Daniel Borkmann wrote:
> On 04/30/2015 02:30 AM, Patrick McHardy wrote:
> >On 30.04, Daniel Borkmann wrote:
> >>>
> >>>I can also see there were also intentions to support userspace
> >>>queueing at some point since TC_ACT_QUEUED has been there since the
> >>>beginning.  That should be possible at some point using this
> >>>infrastructure (once there are no further concerns on the
> >>>netif_receive_core_finish() patch as soon as gcc 4.9 and follow up
> >>>versions keep inlining this new function).
> >>
> >>Wrt the other mail, just thinking out loud ... do you see a longer-term
> >>possibility of further generalizing the gen hooks infrastructure, so that
> >>actually classifier from tc could attach (disregarding the nf_* naming
> >>scheme for now) ...
> >>
> >>     `-> nf_hook_slow()
> >>      `-> [for each entry in hook list]   <-- here as an entry
> >>       `-> nf_iterate()
> >>        `-> (*elemp)->hook()
> >>
> >>... as well?
> >
> >Jumping in there since I'm probably the one thinking the TC ingress
> >abstraction is wrong the strongest - yes, it's an interesting idea.
> >Unlike egress qdiscs, ingress only has a single classifier chain
> >anyways, so there is no qdisc internal classification structure to
> 
> Yes, exactly.
> 
> >be observed. It should be possible to skip the ingress invocation
> >for classification purposes completely and only use it to expose
> >it to userspace for management purposes, while invoking the chain
> >directly.
> 
> That would also meet our goals to only have a single classifier
> chain, eventually. ;)

For TC, yes. What Pablo is trying to achieve (and I agree that it's
worthwhile) is multiple things:

* have people use TC ingress as before, obviously
* have people use netfilter, if they prefer
* have them combine both if for some unfortunate reason they require it

Netfilter is based on hook chains. The cost when only using a single hook
is minimal (as Pablo showed in his numbers), but even if only using
TC and a single netfilter classifier chain, there has to be some relative
ordering and the hooks provide this in a generic way.

So I'm all in favour of reduing costs for single or for multi-hook
cases, but it wouldn't be usable for us anymore when doing away with
multiple hooks completely.

One more point to consider is - this is generic infrastructure - the
next person requiring a hook will not have to touch the ingress path
at all. He can register with the hooks, having zero additional costs
for people not using it.

  reply	other threads:[~2015-04-30  0:48 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-29 18:53 [PATCH 0/6 RFC] Netfilter ingress support (v2) Pablo Neira Ayuso
2015-04-29 18:53 ` [PATCH 1/6] netfilter: cleanup struct nf_hook_ops indentation Pablo Neira Ayuso
2015-04-29 18:53 ` [PATCH 2/6] netfilter: add hook list to nf_hook_state Pablo Neira Ayuso
2015-04-29 18:53 ` [PATCH 3/6] netfilter: add nf_hook_list_active() Pablo Neira Ayuso
2015-04-29 18:53 ` [PATCH 4/6] netfilter: move generic hook infrastructure into net/core/hooks.c Pablo Neira Ayuso
2015-04-29 23:59   ` Patrick McHardy
2015-04-29 18:53 ` [PATCH 5/6] net: add netfilter ingress hook Pablo Neira Ayuso
2015-04-29 18:53 ` [PATCH 6/6] net: move qdisc ingress filtering on top of netfilter ingress hooks Pablo Neira Ayuso
2015-04-29 20:27   ` Daniel Borkmann
2015-04-29 23:32     ` Pablo Neira Ayuso
2015-04-30  0:10       ` Daniel Borkmann
2015-04-30  0:20       ` Daniel Borkmann
2015-04-30  0:30         ` Patrick McHardy
2015-04-30  0:41           ` Daniel Borkmann
2015-04-30  0:48             ` Patrick McHardy [this message]
2015-04-30  1:16               ` Alexei Starovoitov
2015-04-30  1:34                 ` Patrick McHardy
2015-04-30  2:22                   ` Jamal Hadi Salim
2015-04-30  3:11                     ` Patrick McHardy
2015-04-30 11:55                       ` Jamal Hadi Salim
2015-04-30 15:33                         ` Pablo Neira Ayuso
2015-04-30 16:09                           ` Daniel Borkmann
2015-04-30 16:36                             ` Pablo Neira Ayuso
2015-04-30 19:16                               ` Daniel Borkmann
2015-04-30 23:01                                 ` Daniel Borkmann
2015-05-01  1:15                           ` Jamal Hadi Salim
2015-04-30 10:12                 ` Pablo Neira Ayuso
2015-04-30 19:05                   ` Alexei Starovoitov
2015-04-30  0:37       ` Patrick McHardy
2015-04-30  1:04         ` Daniel Borkmann
2015-04-30  1:43           ` Patrick McHardy
2015-04-30  2:35             ` Jamal Hadi Salim
2015-04-30  3:29               ` Patrick McHardy
2015-04-30  4:05                 ` Patrick McHardy
2015-04-30  6:02                   ` Alexei Starovoitov
2015-04-30  9:24                     ` Daniel Borkmann
2015-04-30 10:28                       ` Pablo Neira Ayuso
2015-04-29 23:36     ` Patrick McHardy
2015-04-30  0:00       ` Daniel Borkmann
2015-04-30  0:15         ` Patrick McHardy
2015-04-29 21:53   ` Cong Wang
2015-04-29 23:37     ` Patrick McHardy
2015-04-29 23:42     ` 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=20150430004839.GG7025@acer.localdomain \
    --to=kaber@trash.net \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jhs@mojatatu.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@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