From: Daniel Borkmann <daniel@iogearbox.net>
To: Alexei Starovoitov <ast@plumgrid.com>,
Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netdev@vger.kernel.org, davem@davemloft.net, jhs@mojatatu.com
Subject: Re: [PATCH 2/2 net-next] net: move qdisc ingress filtering code where it belongs
Date: Mon, 11 May 2015 14:58:03 +0200 [thread overview]
Message-ID: <5550A75B.3010003@iogearbox.net> (raw)
In-Reply-To: <555044D8.3080606@plumgrid.com>
On 05/11/2015 07:57 AM, Alexei Starovoitov wrote:
...
> It sounds like you're saying that code that not even being executed
> is somehow affecting the speed? How is that possible ?
> What kind of benchmark do you run? I really want to get to the bottom
> of it.
I'm also curious and would like to get it clarified, so I've added the
below silly test patch.
Since ingress path is *not* executed by the static key, it just bloats up
the handle_ing code, which is the whole purpose of this test.
The claim is : since handle_ing() is inlined it adds up on run time
performance *even* if static keys prevents execution of it.
From the disassembly, I've verified that the below printk's are not
optimized out and that handle_ing() is inlined.
The *disabled* code path gets added below the normal execution path of
__netif_receive_skb_core(), in other words, after the return location
of 'expected' execution.
text data bss dec hex filename
57036 2204 2858 62098 f292 net/core/dev.o.before-printk
60180 2204 2858 65242 feda net/core/dev.o.after-printk
Before printk change:
Samples: 50K of event 'cycles:k', Event count (approx.): 45068616248
+ 40.51% kpktgend_0 [kernel.kallsyms] [k] __netif_receive_skb_core
+ 31.28% kpktgend_0 [kernel.kallsyms] [k] kfree_skb
+ 6.79% kpktgend_0 [kernel.kallsyms] [k] netif_receive_skb_internal
+ 6.65% kpktgend_0 [pktgen] [k] pktgen_thread_worker
+ 6.62% kpktgend_0 [kernel.kallsyms] [k] ip_rcv
+ 3.41% kpktgend_0 [kernel.kallsyms] [k] __netif_receive_skb
+ 3.21% kpktgend_0 [kernel.kallsyms] [k] netif_receive_skb_sk
+ 0.95% kpktgend_0 [kernel.kallsyms] [k] __local_bh_enable_ip
+ 0.40% kpktgend_0 [kernel.kallsyms] [k] kthread_should_stop
+ 0.02% kpktgend_0 [kernel.kallsyms] [k] _cond_resched
After printk-change:
Samples: 50K of event 'cycles:k', Event count (approx.): 45200368743
+ 40.11% kpktgend_0 [kernel.kallsyms] [k] __netif_receive_skb_core
+ 31.09% kpktgend_0 [kernel.kallsyms] [k] kfree_skb
+ 6.93% kpktgend_0 [pktgen] [k] pktgen_thread_worker
+ 6.75% kpktgend_0 [kernel.kallsyms] [k] ip_rcv
+ 6.58% kpktgend_0 [kernel.kallsyms] [k] netif_receive_skb_internal
+ 3.49% kpktgend_0 [kernel.kallsyms] [k] __netif_receive_skb
+ 3.39% kpktgend_0 [kernel.kallsyms] [k] netif_receive_skb_sk
+ 0.96% kpktgend_0 [kernel.kallsyms] [k] __local_bh_enable_ip
+ 0.49% kpktgend_0 [kernel.kallsyms] [k] kthread_should_stop
+ 0.04% kpktgend_0 [kernel.kallsyms] [k] _cond_resched
I would consider the stuff after comma as noise.
After your patch set:
Samples: 50K of event 'cycles:k', Event count (approx.): 45160667741
+ 40.49% kpktgend_0 [kernel.kallsyms] [k] __netif_receive_skb_core
+ 31.21% kpktgend_0 [kernel.kallsyms] [k] kfree_skb
+ 6.94% kpktgend_0 [pktgen] [k] pktgen_thread_worker
+ 6.63% kpktgend_0 [kernel.kallsyms] [k] netif_receive_skb_internal
+ 6.63% kpktgend_0 [kernel.kallsyms] [k] ip_rcv
+ 3.30% kpktgend_0 [kernel.kallsyms] [k] netif_receive_skb_sk
+ 3.30% kpktgend_0 [kernel.kallsyms] [k] __netif_receive_skb
+ 0.96% kpktgend_0 [kernel.kallsyms] [k] __local_bh_enable_ip
+ 0.37% kpktgend_0 [kernel.kallsyms] [k] kthread_should_stop
+ 0.03% kpktgend_0 [kernel.kallsyms] [k] _cond_resched
For *all* three, I reliably get ~40.0 Mpps with the benchmark.
In your perf report I see in addition ...
18.46% kpktgend_0 [kernel.kallsyms] [k] atomic_dec_and_test
15.87% kpktgend_0 [kernel.kallsyms] [k] deliver_ptype_list_skb
... so I have no idea how/where that possibly interferes to your before/after
numbers.
Best & thanks,
Daniel
diff --git a/net/core/dev.c b/net/core/dev.c
index 862875e..7ec18fd 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3545,11 +3545,178 @@ static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
return result;
}
-static inline struct sk_buff *handle_ing(struct sk_buff *skb,
+static __always_inline struct sk_buff *handle_ing(struct sk_buff *skb,
struct packet_type **pt_prev,
int *ret, struct net_device *orig_dev)
{
struct netdev_queue *rxq = rcu_dereference(skb->dev->ingress_queue);
+ int i = 0;
+
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
+ printk("XXX %d\n", i++);
if (!rxq || rcu_access_pointer(rxq->qdisc) == &noop_qdisc)
return skb;
next prev parent reply other threads:[~2015-05-11 12:58 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-10 16:59 [PATCH 0/2 net-next] critical ingress path performance improvements Pablo Neira Ayuso
2015-05-10 16:59 ` [PATCH 1/2 net-next] net: kill useless net_*_ingress_queue() definitions when NET_CLS_ACT is unset Pablo Neira Ayuso
2015-05-10 16:58 ` Sergei Shtylyov
2015-05-10 16:59 ` [PATCH 2/2 net-next] net: move qdisc ingress filtering code where it belongs Pablo Neira Ayuso
2015-05-10 17:25 ` Eric Dumazet
2015-05-10 17:45 ` Alexei Starovoitov
2015-05-10 17:59 ` Pablo Neira Ayuso
2015-05-10 18:05 ` Alexei Starovoitov
2015-05-10 18:24 ` Pablo Neira Ayuso
2015-05-10 18:47 ` Alexei Starovoitov
2015-05-10 19:00 ` Pablo Neira Ayuso
2015-05-10 19:06 ` Alexei Starovoitov
2015-05-10 19:20 ` Pablo Neira Ayuso
2015-05-10 19:37 ` Alexei Starovoitov
2015-05-10 19:50 ` Pablo Neira Ayuso
2015-05-10 21:31 ` Daniel Borkmann
2015-05-10 21:44 ` Daniel Borkmann
2015-05-10 23:43 ` Pablo Neira Ayuso
2015-05-11 5:57 ` Alexei Starovoitov
2015-05-11 12:49 ` Jamal Hadi Salim
2015-05-11 12:58 ` Daniel Borkmann [this message]
2015-05-11 17:15 ` Alexei Starovoitov
2015-05-11 13:32 ` Pablo Neira Ayuso
2015-05-11 14:35 ` Eric Dumazet
2015-05-11 23:02 ` Alexei Starovoitov
2015-05-11 23:30 ` Eric Dumazet
2015-05-12 3:21 ` Alexei Starovoitov
2015-05-12 12:55 ` Pablo Neira Ayuso
2015-05-12 13:27 ` Daniel Borkmann
2015-05-12 21:22 ` Alexei Starovoitov
2015-05-12 21:43 ` Daniel Borkmann
2015-05-10 20:40 ` Daniel Borkmann
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=5550A75B.3010003@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=ast@plumgrid.com \
--cc=davem@davemloft.net \
--cc=jhs@mojatatu.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.