linux/net/sched
Vlad Buslov 653cd284a8 net: sched: always disable bh when taking tcf_lock
Recently, ops->init() and ops->dump() of all actions were modified to
always obtain tcf_lock when accessing private action state. Actions that
don't depend on tcf_lock for synchronization with their data path use
non-bh locking API. However, tcf_lock is also used to protect rate
estimator stats in softirq context by timer callback.

Change ops->init() and ops->dump() of all actions to disable bh when using
tcf_lock to prevent deadlock reported by following lockdep warning:

[  105.470398] ================================
[  105.475014] WARNING: inconsistent lock state
[  105.479628] 4.18.0-rc8+ #664 Not tainted
[  105.483897] --------------------------------
[  105.488511] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
[  105.494871] swapper/16/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
[  105.500449] 00000000f86c012e (&(&p->tcfa_lock)->rlock){+.?.}, at: est_fetch_counters+0x3c/0xa0
[  105.509696] {SOFTIRQ-ON-W} state was registered at:
[  105.514925]   _raw_spin_lock+0x2c/0x40
[  105.519022]   tcf_bpf_init+0x579/0x820 [act_bpf]
[  105.523990]   tcf_action_init_1+0x4e4/0x660
[  105.528518]   tcf_action_init+0x1ce/0x2d0
[  105.532880]   tcf_exts_validate+0x1d8/0x200
[  105.537416]   fl_change+0x55a/0x268b [cls_flower]
[  105.542469]   tc_new_tfilter+0x748/0xa20
[  105.546738]   rtnetlink_rcv_msg+0x56a/0x6d0
[  105.551268]   netlink_rcv_skb+0x18d/0x200
[  105.555628]   netlink_unicast+0x2d0/0x370
[  105.559990]   netlink_sendmsg+0x3b9/0x6a0
[  105.564349]   sock_sendmsg+0x6b/0x80
[  105.568271]   ___sys_sendmsg+0x4a1/0x520
[  105.572547]   __sys_sendmsg+0xd7/0x150
[  105.576655]   do_syscall_64+0x72/0x2c0
[  105.580757]   entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  105.586243] irq event stamp: 489296
[  105.590084] hardirqs last  enabled at (489296): [<ffffffffb507e639>] _raw_spin_unlock_irq+0x29/0x40
[  105.599765] hardirqs last disabled at (489295): [<ffffffffb507e745>] _raw_spin_lock_irq+0x15/0x50
[  105.609277] softirqs last  enabled at (489292): [<ffffffffb413a6a3>] irq_enter+0x83/0xa0
[  105.618001] softirqs last disabled at (489293): [<ffffffffb413a800>] irq_exit+0x140/0x190
[  105.626813]
               other info that might help us debug this:
[  105.633976]  Possible unsafe locking scenario:

[  105.640526]        CPU0
[  105.643325]        ----
[  105.646125]   lock(&(&p->tcfa_lock)->rlock);
[  105.650747]   <Interrupt>
[  105.653717]     lock(&(&p->tcfa_lock)->rlock);
[  105.658514]
                *** DEADLOCK ***

[  105.665349] 1 lock held by swapper/16/0:
[  105.669629]  #0: 00000000a640ad99 ((&est->timer)){+.-.}, at: call_timer_fn+0x10b/0x550
[  105.678200]
               stack backtrace:
[  105.683194] CPU: 16 PID: 0 Comm: swapper/16 Not tainted 4.18.0-rc8+ #664
[  105.690249] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[  105.698626] Call Trace:
[  105.701421]  <IRQ>
[  105.703791]  dump_stack+0x92/0xeb
[  105.707461]  print_usage_bug+0x336/0x34c
[  105.711744]  mark_lock+0x7c9/0x980
[  105.715500]  ? print_shortest_lock_dependencies+0x2e0/0x2e0
[  105.721424]  ? check_usage_forwards+0x230/0x230
[  105.726315]  __lock_acquire+0x923/0x26f0
[  105.730597]  ? debug_show_all_locks+0x240/0x240
[  105.735478]  ? mark_lock+0x493/0x980
[  105.739412]  ? check_chain_key+0x140/0x1f0
[  105.743861]  ? __lock_acquire+0x836/0x26f0
[  105.748323]  ? lock_acquire+0x12e/0x290
[  105.752516]  lock_acquire+0x12e/0x290
[  105.756539]  ? est_fetch_counters+0x3c/0xa0
[  105.761084]  _raw_spin_lock+0x2c/0x40
[  105.765099]  ? est_fetch_counters+0x3c/0xa0
[  105.769633]  est_fetch_counters+0x3c/0xa0
[  105.773995]  est_timer+0x87/0x390
[  105.777670]  ? est_fetch_counters+0xa0/0xa0
[  105.782210]  ? lock_acquire+0x12e/0x290
[  105.786410]  call_timer_fn+0x161/0x550
[  105.790512]  ? est_fetch_counters+0xa0/0xa0
[  105.795055]  ? del_timer_sync+0xd0/0xd0
[  105.799249]  ? __lock_is_held+0x93/0x110
[  105.803531]  ? mark_held_locks+0x20/0xe0
[  105.807813]  ? _raw_spin_unlock_irq+0x29/0x40
[  105.812525]  ? est_fetch_counters+0xa0/0xa0
[  105.817069]  ? est_fetch_counters+0xa0/0xa0
[  105.821610]  run_timer_softirq+0x3c4/0x9f0
[  105.826064]  ? lock_acquire+0x12e/0x290
[  105.830257]  ? __bpf_trace_timer_class+0x10/0x10
[  105.835237]  ? __lock_is_held+0x25/0x110
[  105.839517]  __do_softirq+0x11d/0x7bf
[  105.843542]  irq_exit+0x140/0x190
[  105.847208]  smp_apic_timer_interrupt+0xac/0x3b0
[  105.852182]  apic_timer_interrupt+0xf/0x20
[  105.856628]  </IRQ>
[  105.859081] RIP: 0010:cpuidle_enter_state+0xd8/0x4d0
[  105.864395] Code: 46 ff 48 89 44 24 08 0f 1f 44 00 00 31 ff e8 cf ec 46 ff 80 7c 24 07 00 0f 85 1d 02 00 00 e8 9f 90 4b ff fb 66 0f 1f 44 00 00 <4c> 8b 6c 24 08 4d 29 fd 0f 80 36 03 00 00 4c 89 e8 48 ba cf f7 53
[  105.884288] RSP: 0018:ffff8803ad94fd20 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
[  105.892494] RAX: 0000000000000000 RBX: ffffe8fb300829c0 RCX: ffffffffb41e19e1
[  105.899988] RDX: 0000000000000007 RSI: dffffc0000000000 RDI: ffff8803ad9358ac
[  105.907503] RBP: ffffffffb6636300 R08: 0000000000000004 R09: 0000000000000000
[  105.914997] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000004
[  105.922487] R13: ffffffffb6636140 R14: ffffffffb66362d8 R15: 000000188d36091b
[  105.929988]  ? trace_hardirqs_on_caller+0x141/0x2d0
[  105.935232]  do_idle+0x28e/0x320
[  105.938817]  ? arch_cpu_idle_exit+0x40/0x40
[  105.943361]  ? mark_lock+0x8c1/0x980
[  105.947295]  ? _raw_spin_unlock_irqrestore+0x32/0x60
[  105.952619]  cpu_startup_entry+0xc2/0xd0
[  105.956900]  ? cpu_in_idle+0x20/0x20
[  105.960830]  ? _raw_spin_unlock_irqrestore+0x32/0x60
[  105.966146]  ? trace_hardirqs_on_caller+0x141/0x2d0
[  105.971391]  start_secondary+0x2b5/0x360
[  105.975669]  ? set_cpu_sibling_map+0x1330/0x1330
[  105.980654]  secondary_startup_64+0xa5/0xb0

Taking tcf_lock in sample action with bh disabled causes lockdep to issue a
warning regarding possible irq lock inversion dependency between tcf_lock,
and psample_groups_lock that is taken when holding tcf_lock in sample init:

[  162.108959]  Possible interrupt unsafe locking scenario:

[  162.116386]        CPU0                    CPU1
[  162.121277]        ----                    ----
[  162.126162]   lock(psample_groups_lock);
[  162.130447]                                local_irq_disable();
[  162.136772]                                lock(&(&p->tcfa_lock)->rlock);
[  162.143957]                                lock(psample_groups_lock);
[  162.150813]   <Interrupt>
[  162.153808]     lock(&(&p->tcfa_lock)->rlock);
[  162.158608]
                *** DEADLOCK ***

In order to prevent potential lock inversion dependency between tcf_lock
and psample_groups_lock, extract call to psample_group_get() from tcf_lock
protected section in sample action init function.

Fixes: 4e232818bd ("net: sched: act_mirred: remove dependency on rtnl lock")
Fixes: 764e9a2448 ("net: sched: act_vlan: remove dependency on rtnl lock")
Fixes: 729e012609 ("net: sched: act_tunnel_key: remove dependency on rtnl lock")
Fixes: d772849566 ("net: sched: act_sample: remove dependency on rtnl lock")
Fixes: e8917f4370 ("net: sched: act_gact: remove dependency on rtnl lock")
Fixes: b6a2b971c0 ("net: sched: act_csum: remove dependency on rtnl lock")
Fixes: 2142236b45 ("net: sched: act_bpf: remove dependency on rtnl lock")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2018-08-19 10:46:21 -07:00
..
act_api.c net/sched: user-space can't set unknown tcfa_action values 2018-07-30 09:31:13 -07:00
act_bpf.c net: sched: always disable bh when taking tcf_lock 2018-08-19 10:46:21 -07:00
act_connmark.c net: sched: act_connmark method rename for grep-ability and consistency 2018-08-13 09:06:16 -07:00
act_csum.c net: sched: always disable bh when taking tcf_lock 2018-08-19 10:46:21 -07:00
act_gact.c net: sched: always disable bh when taking tcf_lock 2018-08-19 10:46:21 -07:00
act_ife.c net: sched: act_ife: always release ife action on init error 2018-08-16 12:12:12 -07:00
act_ipt.c net: sched: act_ipt method rename for grep-ability and consistency 2018-08-13 09:06:16 -07:00
act_meta_mark.c
act_meta_skbprio.c
act_meta_skbtcindex.c
act_mirred.c net: sched: always disable bh when taking tcf_lock 2018-08-19 10:46:21 -07:00
act_nat.c net: sched: act_nat method rename for grep-ability and consistency 2018-08-13 09:06:16 -07:00
act_pedit.c net: sched: act_pedit method rename for grep-ability and consistency 2018-08-13 09:06:17 -07:00
act_police.c net: sched: act_police method rename for grep-ability and consistency 2018-08-13 09:06:17 -07:00
act_sample.c net: sched: always disable bh when taking tcf_lock 2018-08-19 10:46:21 -07:00
act_simple.c net: sched: act_simple method rename for grep-ability and consistency 2018-08-13 09:06:17 -07:00
act_skbedit.c net: sched: act_skbedit method rename for grep-ability and consistency 2018-08-13 09:06:17 -07:00
act_skbmod.c net: sched: act_skbmod method rename for grep-ability and consistency 2018-08-13 09:06:17 -07:00
act_tunnel_key.c net: sched: always disable bh when taking tcf_lock 2018-08-19 10:46:21 -07:00
act_vlan.c net: sched: always disable bh when taking tcf_lock 2018-08-19 10:46:21 -07:00
cls_api.c net: sched: extend action ops with put_dev callback 2018-08-11 12:37:10 -07:00
cls_basic.c sched: fix trailing whitespace 2018-07-24 14:10:42 -07:00
cls_bpf.c cls_bpf: Use kmemdup instead of duplicating it in cls_bpf_prog_from_ops 2018-07-29 13:19:49 -07:00
cls_cgroup.c
cls_flow.c
cls_flower.c net: sched: cls_flower: set correct offload data in fl_reoffload 2018-08-07 12:35:17 -07:00
cls_fw.c
cls_matchall.c cls_matchall: fix tcf_unbind_filter missing 2018-08-16 12:08:26 -07:00
cls_route.c
cls_rsvp6.c
cls_rsvp.c
cls_rsvp.h
cls_tcindex.c net_sched: Fix missing res info when create new tc_index filter 2018-08-13 19:37:42 -07:00
cls_u32.c net: sched: cls_u32: implement offload tcf_proto_op 2018-06-26 23:21:33 +09:00
em_canid.c
em_cmp.c
em_ipset.c
em_ipt.c
em_meta.c
em_nbyte.c
em_text.c
em_u32.c
ematch.c
Kconfig net/sched: add skbprio scheduler 2018-07-24 14:44:00 -07:00
Makefile net/sched: add skbprio scheduler 2018-07-24 14:44:00 -07:00
sch_api.c net/sched: Allow creating a Qdisc watchdog with other clocks 2018-07-04 22:30:27 +09:00
sch_atm.c
sch_blackhole.c
sch_cake.c sch_cake: Make gso-splitting configurable from userspace 2018-07-27 13:38:20 -07:00
sch_cbq.c
sch_cbs.c cbs: Add support for the graft function 2018-07-26 13:58:30 -07:00
sch_choke.c
sch_codel.c
sch_drr.c
sch_dsmark.c
sch_etf.c net/sched: Make etf report drops on error_queue 2018-07-04 22:30:28 +09:00
sch_fifo.c
sch_fq_codel.c sch_fq_codel: zero q->flows_cnt when fq_codel_init fails 2018-07-12 12:32:09 -07:00
sch_fq.c
sch_generic.c
sch_gred.c
sch_hfsc.c net_sched: remove a bogus warning in hfsc 2018-06-23 10:58:46 +09:00
sch_hhf.c
sch_htb.c net_sched: remove unused htb drop_list 2018-06-24 16:42:46 +09:00
sch_ingress.c
sch_mq.c
sch_mqprio.c
sch_multiq.c
sch_netem.c netem: slotting with non-uniform distribution 2018-06-28 22:06:24 +09:00
sch_pie.c
sch_plug.c
sch_prio.c
sch_qfq.c
sch_red.c
sch_sfb.c
sch_sfq.c
sch_skbprio.c net/sched: add skbprio scheduler 2018-07-24 14:44:00 -07:00
sch_tbf.c
sch_teql.c