netdev - Re: [PATCH 0/4] Netfilter ingress support (v3)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150504161956.GK22481@breakpoint.cc>
Date:	Mon, 4 May 2015 18:19:56 +0200
From:	Florian Westphal <fw@...len.de>
To:	Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:	Pablo Neira Ayuso <pablo@...filter.org>,
	netfilter-devel@...r.kernel.org, davem@...emloft.net,
	netdev@...r.kernel.org, kaber@...sh.net, jhs@...atatu.com
Subject: Re: [PATCH 0/4] Netfilter ingress support (v3)

Alexei Starovoitov <alexei.starovoitov@...il.com> wrote:
> > Another round of the patchset to add Netfilter ingress support. This new
> > patchset introduces the necessary updates in 2 steps:
> > 
> > 1) Add minismalistic ingress hook infrastructure that allows to register one
> >    client at a time, so you hit -EBUSY in case the hook is in use. Basically,
> >    we have a function pointer that is rcu-protected to invoke the corresponding
> >    filter framework which has minimal performance impact in the critical ingress
> >    path and avoid more pollution in it. This patch also ports the ingress qdisc
> >    on top of this.
> ...
> > In summary, this provides the facility to keep both tc and netfilter in place,
> > while the user can select what they prefer to filter from ingress.
> 
> wow, I have to say I'm impressed. That's the most genius way to
> really kill TC.
> Patch 1 looks good, patch 2,3,4 are nicely building on top...
> until somebody starts asking how patch 5 will look.
> In the future netfilter ingress module will be loaded along with
> other iptables modules just like conntrack is today and users
> who would want to use ingress tc would have to _unload_
> netfilter_ingress module, but if it has interesting dependencies
> it may mean to unload iptables and the rest.

FWIW while I think this is a valid concern, I believe its unfounded.

netfilter_ingress must not force run-time
dependencies like 'oh, you want tc, too bad, no conntrack for you)'.

(and i don't see any need for such a dependency).

> So at the end the users will have a binary choice either to use
> iptables/nft or use tc, because they won't be able to co-exist
> because ingress_hook is the only one.
> I don't understand this 'tc hate'. Why go out of the way to
> make TC more difficult to use ?

I don't see this having anything to do with 'TC hate'.

Lets face it, tc and netfilter are horrid when it comes to play
nice with each other, and functionality provided by both partially
overlaps.

Just look at all the hacks we have in tc to use netfilter (iptables,
conntrack) functionality, or even iptables CLASSIFY target (aka
"tc filters are too hard for me").

In case there is later some really good reason why you need both
tc ingress and nft ingress simultaneously then we can always revisit this
and add a second hook for coexistence, at the expense of more
complexity.

> Just add _new_ hook for netfilter ingress and both subsystems
> can happily co-exist.

I don't see a use case where you'd need both tc ingress and nft
ingress at the same time.  Can you elaborate?

At the moment I think having two hooks provides no advantage, it only
complicates code.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html