netdev - Re: [PATCH 6/6] net: move qdisc ingress filtering on top of netfilter ingress hooks

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: <20150430011633.GA12674@Alexeis-MBP.westell.com>
Date:	Wed, 29 Apr 2015 18:16:35 -0700
From:	Alexei Starovoitov <alexei.starovoitov@...il.com>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	Daniel Borkmann <daniel@...earbox.net>,
	Pablo Neira Ayuso <pablo@...filter.org>,
	netfilter-devel@...r.kernel.org, davem@...emloft.net,
	netdev@...r.kernel.org, jhs@...atatu.com
Subject: Re: [PATCH 6/6] net: move qdisc ingress filtering on top of
 netfilter ingress hooks

On Thu, Apr 30, 2015 at 02:48:39AM +0200, Patrick McHardy wrote:
> 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.

Unfortunately the numbers that Pablo shown are not measuring
the right thing.

> 840203pps 403Mb/sec

this is 20 times less than what they should be.
Something else were measured together with netif_receive_skb.

I've applied these patches and see the following
for eth0 + ingress + u32:

18.0 Mpps
  21.43%  kpktgend_0   [kernel.vmlinux]  [k] __netif_receive_skb_core
   9.88%  kpktgend_0   [kernel.vmlinux]  [k] kfree_skb
   9.79%  kpktgend_0   [cls_u32]         [k] u32_classify
   9.16%  kpktgend_0   [kernel.vmlinux]  [k] _raw_spin_lock
   8.16%  kpktgend_0   [kernel.vmlinux]  [k] nf_iterate
   5.28%  kpktgend_0   [sch_ingress]     [k] handle_ing
   4.51%  kpktgend_0   [sch_ingress]     [k] ingress_enqueue
   4.42%  kpktgend_0   [kernel.vmlinux]  [k] tc_classify_compat
   3.16%  kpktgend_0   [kernel.vmlinux]  [k] nf_hook_slow
   3.01%  kpktgend_0   [kernel.vmlinux]  [k] ip_rcv
   2.70%  kpktgend_0   [kernel.vmlinux]  [k] tc_classify

without these patches:

22.4 Mpps
  25.89%  kpktgend_0   [kernel.vmlinux]  [k] __netif_receive_skb_core
  14.41%  kpktgend_0   [kernel.vmlinux]  [k] kfree_skb
  14.05%  kpktgend_0   [kernel.vmlinux]  [k] _raw_spin_lock
  11.75%  kpktgend_0   [cls_u32]         [k] u32_classify
   6.48%  kpktgend_0   [sch_ingress]     [k] ingress_enqueue
   6.06%  kpktgend_0   [kernel.vmlinux]  [k] tc_classify_compat
   4.16%  kpktgend_0   [kernel.vmlinux]  [k] tc_classify
   3.77%  kpktgend_0   [kernel.vmlinux]  [k] ip_rcv

clearly nf_iterate/nf_hook_slow are slowing things down.

I've spent more than a week trying to speedup ingress qdisc
and, so far, got from 22.4 Mpps to 27.2 Mpps,
so this 'generalization' is totally not acceptable to me.

You're right that for 10 years no one cared about performance
of ingress qdisc, but that doesn't mean it's a wrong abstraction
and wrong architecture. Now I care about its performance and
I hope other people will do too.
So please leave ingress qdisc alone, this 'generalization'
is too costly.

That doesn't mean that netfilter shouldn't have its own hook
on ingress. Without patch 6, the set looks good.

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