net: sched: run ingress qdisc without locks
Subsystems (3)
PATCH v2 1/1 net: sched: run ingress qdisc without locks
From: John Fastabend <[email protected]>
TC classifiers/actions were converted to RCU by John in the series:
http://thread.gmane.org/gmane.linux.network/329739/focus=329739
and many follow on patches.
This is the last patch from that series that finally drops ingress spin_lock.
Single cpu ingress+u32 performance goes from 22.9 Mpps to 24.5 Mpps.
In two cpu case when both cores are receiving traffic on the same device and go into the same ingress+u32 the performance jumps from 4.5 + 4.5 Mpps to 23.5 + 23.5 Mpps
---
v1->v2: add From:John tag, Sob, Ack
net/core/dev.c | 2 -- net/sched/sch_ingress.c | 5 +++-- 2 files changed, 3 insertions(+), 4 deletions(-)
net/core/dev.c +0 -2
@@ -3538,10 +3538,8 @@ static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq) q =rcu_dereference (rxq->qdisc);if (q != &noop_qdisc) { -spin_lock (qdisc_lock (q));if (likely (!test_bit (__QDISC_STATE_DEACTIVATED, &q->state))) result =qdisc_enqueue_root (skb, q); -spin_unlock (qdisc_lock (q)); }return result;
net/sched/sch_ingress.c +3 -2
@@ -65,11 +65,11 @@ static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch) result =tc_classify (skb, fl, &res); -qdisc_bstats_update (sch, skb); +qdisc_bstats_update_cpu (sch, skb);switch (result) {case TC_ACT_SHOT : result =TC_ACT_SHOT ; -qdisc_qstats_drop (sch); +qdisc_qstats_drop_cpu (sch);break ;case TC_ACT_STOLEN :case TC_ACT_QUEUED :
@@ -91,6 +91,7 @@ static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)static int ingress_init (struct Qdisc *sch,struct nlattr *opt) {net_inc_ingress_queue (); + sch->flags |=TCQ_F_CPUSTATS ;return 0 ; }
Signature · 1 line
1.7.9.5
Quoted 7 lines · On Fri, 1 May 2015 22:27:28 -0700 Alexei Starovoitov <[email protected]> wrote:
From: John Fastabend <[email protected]>
TC classifiers/actions were converted to RCU by John in the series:
http://thread.gmane.org/gmane.linux.network/329739/focus=329739
and many follow on patches.
This is the last patch from that series that finally drops ingressspin_lock.
I absolutely love this change. It is a huge step for ingress scalability.
Quoted 1 line
Single cpu ingress+u32 performance goes from 22.9 Mpps to 24.5 Mpps.
I was actually expecting to see a higher performance boost.
(processing cost per packet)
(1/(22.9*10^6)*10^9) = 43.67 ns
(1/(24.5*10^6)*10^9) = 40.82 ns
improvement diff = -2.85 ns
The patch is removing two atomic operations, spin_{un,}lock, which I have benchmarked[1] to cost approx 14ns on my system. Your system likely is faster, but not that much (p.s. benchmark your own system with [1])
[1] https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/lib/time_bench_sample.c
Quoted 3 lines
In two cpu case when both cores are receiving traffic on the same device and go into the same ingress+u32 the performance jumps from 4.5 + 4.5 Mpps to 23.5 + 23.5 Mpps
This looks good for scalability :-)))
Quoted 4 lines
Signed-off-by John Fastabend <[email protected]>Signed-off-by Alexei Starovoitov <[email protected]>Signed-off-by Jamal Hadi Salim <[email protected]>Acked-by Daniel Borkmann <[email protected]>
Signature · 5 lines
Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer
Quoted 3 lines · Jesper Dangaard Brouer
I was actually expecting to see a higher performance boost.
improvement diff = -2.85 ns
...
Quoted 6 lines
The patch is removing two atomic operations, spin_{un,}lock, which I have benchmarked[1] to cost approx 14ns on my system. Your system likely is faster, but not that much (p.s. benchmark your own system with [1])
[1] https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/lib/time_bench_sample.c
have tried you tight loop spin_lock test on my box and it showed:time_bench: Type:spin_lock_unlock Per elem: 40 cycles(tsc) 11.070 ns and yet the total single cpu gain from removal of spin_lock/unlock in ingress path is smaller than 11ns. I think this observation is telling us that tight loop benchmarking is inherently flawed. I'm guessing that uops that cmpxchg is broken into can execute in parallel with uops of other insns, so tight loops of the same sequence of uops has more alu dependencies whereas in more normal insn flow these uops can mix and match better. Would be great if intel microarch experts can chime in.
Quoted 12 lines · On Sun, 03 May 2015 22:12:43 -0700 Alexei Starovoitov <[email protected]> wrote:
Quoted 3 lines · Jesper Dangaard Brouer
I was actually expecting to see a higher performance boost.
improvement diff = -2.85 ns...
Quoted 6 lines
The patch is removing two atomic operations, spin_{un,}lock, which I have benchmarked[1] to cost approx 14ns on my system. Your system likely is faster, but not that much (p.s. benchmark your own system with [1])
[1] https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/lib/time_bench_sample.c
Quoted 10 lines
have tried you tight loop
spin_locktest on my box and it showed:time_bench: Type:spin_lock_unlock Per elem: 40 cycles(tsc) 11.070 ns and yet the total single cpu gain from removal of spin_lock/unlock in ingress path is smaller than 11ns. I think this observation is telling us that tight loop benchmarking is inherently flawed. I'm guessing that uops that cmpxchg is broken into can execute in parallel with uops of other insns, so tight loops of the same sequence of uops has more alu dependencies whereas in more normal insn flow these uops can mix and match better. Would be great if intel microarch experts can chime in.
How do you activate the ingress code path?
I'm just doing (is this enough?):
export DEV=eth4
tc qdisc add dev $DEV handle ffff: ingress
I re-ran the experiment, and I can also only show a 2.68ns improvement. This is rather strange, and I cannot explain it.
The lock clearly shows up in perf report[1] with 12.23% raw_spin_lock, and perf report[2] it clearly gone, but we don't see a 12% improvement in performance, but around 4.7%.
Before activating qdisc ingress code : 25.3Mpps (25398057)
Activating qdisc ingress with lock : 16.9Mpps (16989315)
Activating qdisc ingress without lock: 17.8Mpps (17800496)
(1/17800496*10^9)-(1/16989315*10^9) = -2.68 ns
The "cost" of activating the ingress qdisc is also interesting:
(1/25398057*10^9)-(1/16989315*10^9) = -19.49 ns (1/25398057*10^9)-(1/17800496*10^9) = -16.81 ns
Signature · 48 lines
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
My setup
* Tested on top of commit 4749c3ef854
* gcc version 4.4.7 20120313 (Red Hat 4.4.7-11) (GCC)
* CPU E5-2695(ES) @ 2.8GHz
[1] perf report with ingress qlock
Samples: 2K of event 'cycles', Event count (approx.): 1762298819
Overhead Command Shared Object Symbol
+ 35.86% kpktgend_0 [kernel.vmlinux] [k] __netif_receive_skb_core
+ 17.81% kpktgend_0 [kernel.vmlinux] [k] kfree_skb
+ 12.23% kpktgend_0 [kernel.vmlinux] [k] _raw_spin_lock
- _raw_spin_lock
+ 93.54% __netif_receive_skb_core
+ 6.46% __netif_receive_skb
+ 5.45% kpktgend_0 [sch_ingress] [k] ingress_enqueue
+ 4.65% kpktgend_0 [pktgen] [k] pktgen_thread_worker
+ 4.23% kpktgend_0 [kernel.vmlinux] [k] ip_rcv
+ 3.95% kpktgend_0 [kernel.vmlinux] [k] tc_classify_compat
+ 3.71% kpktgend_0 [kernel.vmlinux] [k] tc_classify
+ 3.03% kpktgend_0 [kernel.vmlinux] [k] netif_receive_skb_internal
+ 2.65% kpktgend_0 [kernel.vmlinux] [k] netif_receive_skb_sk
+ 1.97% kpktgend_0 [kernel.vmlinux] [k] __netif_receive_skb
+ 0.71% kpktgend_0 [kernel.vmlinux] [k] __local_bh_enable_ip
+ 0.28% kpktgend_0 [kernel.vmlinux] [k] kthread_should_stop
[2] perf report without ingress qlock
Samples: 2K of event 'cycles', Event count (approx.): 1633499063
Overhead Command Shared Object Symbol
+ 39.29% kpktgend_0 [kernel.vmlinux] [k] __netif_receive_skb_core
+ 19.24% kpktgend_0 [kernel.vmlinux] [k] kfree_skb
+ 11.05% kpktgend_0 [sch_ingress] [k] ingress_enqueue
+ 4.69% kpktgend_0 [kernel.vmlinux] [k] tc_classify
+ 4.48% kpktgend_0 [kernel.vmlinux] [k] ip_rcv
+ 4.43% kpktgend_0 [kernel.vmlinux] [k] tc_classify_compat
+ 4.19% kpktgend_0 [pktgen] [k] pktgen_thread_worker
+ 3.50% kpktgend_0 [kernel.vmlinux] [k] netif_receive_skb_internal
+ 2.61% kpktgend_0 [kernel.vmlinux] [k] netif_receive_skb_sk
+ 2.26% kpktgend_0 [kernel.vmlinux] [k] __netif_receive_skb
+ 0.43% kpktgend_0 [kernel.vmlinux] [k] __local_bh_enable_ip
+ 0.13% swapper [kernel.vmlinux] [k] mwait_idleQuoted 6 lines · Jesper Dangaard Brouer
How do you activate the ingress code path?
I'm just doing (is this enough?):
export DEV=eth4
tc qdisc add dev $DEV handle ffff: ingress
yes. plus my numbers also include u32 classifier.
Quoted 6 lines
I re-ran the experiment, and I can also only show a 2.68ns improvement. This is rather strange, and I cannot explain it.
The lock clearly shows up in perf report[1] with 12.23%
raw_spin_lock, and perf report[2] it clearly gone, but we don't see a 12% improvement in performance, but around 4.7%.
It's indeed puzzling. Hopefully intel experts can chime in.
Quoted 3 lines
The "cost" of activating the ingress qdisc is also interesting:
(1/25398057*10^9)-(1/16989315*10^9) = -19.49 ns (1/25398057*10^9)-(1/17800496*10^9) = -16.81 ns
yep, we're working hard on reducing it.
btw the cost of enabling rps without using it is ~8ns.
Our line rate goal is still a bit far, but hopefully getting closer :)