Commit d5e032fc authored by Julian Anastasov's avatar Julian Anastasov Committed by Pablo Neira Ayuso

ipvs: fix stats update from local clients

Local clients are not properly synchronized on 32-bit CPUs when
updating stats (3.10+). Now it is possible estimation_timer (timer),
a stats reader, to interrupt the local client in the middle of
write_seqcount_{begin,end} sequence leading to loop (DEADLOCK).
The same interrupt can happen from received packet (SoftIRQ)
which updates the same per-CPU stats.

Fix it by disabling BH while updating stats.

Found with debug:

WARNING: inconsistent lock state
4.17.0-rc2-00105-g35cb6d7-dirty #2 Not tainted
--------------------------------
inconsistent {IN-SOFTIRQ-R} -> {SOFTIRQ-ON-W} usage.
ftp/2545 [HC0[0]:SC0[0]:HE1:SE1] takes:
86845479 (&syncp->seq#6){+.+-}, at: ip_vs_schedule+0x1c5/0x59e [ip_vs]
{IN-SOFTIRQ-R} state was registered at:
 lock_acquire+0x44/0x5b
 estimation_timer+0x1b3/0x341 [ip_vs]
 call_timer_fn+0x54/0xcd
 run_timer_softirq+0x10c/0x12b
 __do_softirq+0xc1/0x1a9
 do_softirq_own_stack+0x1d/0x23
 irq_exit+0x4a/0x64
 smp_apic_timer_interrupt+0x63/0x71
 apic_timer_interrupt+0x3a/0x40
 default_idle+0xa/0xc
 arch_cpu_idle+0x9/0xb
 default_idle_call+0x21/0x23
 do_idle+0xa0/0x167
 cpu_startup_entry+0x19/0x1b
 start_secondary+0x133/0x182
 startup_32_smp+0x164/0x168
irq event stamp: 42213

other info that might help us debug this:
Possible unsafe locking scenario:

      CPU0
      ----
 lock(&syncp->seq#6);
 <Interrupt>
   lock(&syncp->seq#6);

*** DEADLOCK ***

Fixes: ac69269a ("ipvs: do not disable bh for long time")
Signed-off-by: default avatarJulian Anastasov <ja@ssi.bg>
Acked-by: default avatarSimon Horman <horms@verge.net.au>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent a050d345
...@@ -119,6 +119,8 @@ ip_vs_in_stats(struct ip_vs_conn *cp, struct sk_buff *skb) ...@@ -119,6 +119,8 @@ ip_vs_in_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
struct ip_vs_cpu_stats *s; struct ip_vs_cpu_stats *s;
struct ip_vs_service *svc; struct ip_vs_service *svc;
local_bh_disable();
s = this_cpu_ptr(dest->stats.cpustats); s = this_cpu_ptr(dest->stats.cpustats);
u64_stats_update_begin(&s->syncp); u64_stats_update_begin(&s->syncp);
s->cnt.inpkts++; s->cnt.inpkts++;
...@@ -137,6 +139,8 @@ ip_vs_in_stats(struct ip_vs_conn *cp, struct sk_buff *skb) ...@@ -137,6 +139,8 @@ ip_vs_in_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
s->cnt.inpkts++; s->cnt.inpkts++;
s->cnt.inbytes += skb->len; s->cnt.inbytes += skb->len;
u64_stats_update_end(&s->syncp); u64_stats_update_end(&s->syncp);
local_bh_enable();
} }
} }
...@@ -151,6 +155,8 @@ ip_vs_out_stats(struct ip_vs_conn *cp, struct sk_buff *skb) ...@@ -151,6 +155,8 @@ ip_vs_out_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
struct ip_vs_cpu_stats *s; struct ip_vs_cpu_stats *s;
struct ip_vs_service *svc; struct ip_vs_service *svc;
local_bh_disable();
s = this_cpu_ptr(dest->stats.cpustats); s = this_cpu_ptr(dest->stats.cpustats);
u64_stats_update_begin(&s->syncp); u64_stats_update_begin(&s->syncp);
s->cnt.outpkts++; s->cnt.outpkts++;
...@@ -169,6 +175,8 @@ ip_vs_out_stats(struct ip_vs_conn *cp, struct sk_buff *skb) ...@@ -169,6 +175,8 @@ ip_vs_out_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
s->cnt.outpkts++; s->cnt.outpkts++;
s->cnt.outbytes += skb->len; s->cnt.outbytes += skb->len;
u64_stats_update_end(&s->syncp); u64_stats_update_end(&s->syncp);
local_bh_enable();
} }
} }
...@@ -179,6 +187,8 @@ ip_vs_conn_stats(struct ip_vs_conn *cp, struct ip_vs_service *svc) ...@@ -179,6 +187,8 @@ ip_vs_conn_stats(struct ip_vs_conn *cp, struct ip_vs_service *svc)
struct netns_ipvs *ipvs = svc->ipvs; struct netns_ipvs *ipvs = svc->ipvs;
struct ip_vs_cpu_stats *s; struct ip_vs_cpu_stats *s;
local_bh_disable();
s = this_cpu_ptr(cp->dest->stats.cpustats); s = this_cpu_ptr(cp->dest->stats.cpustats);
u64_stats_update_begin(&s->syncp); u64_stats_update_begin(&s->syncp);
s->cnt.conns++; s->cnt.conns++;
...@@ -193,6 +203,8 @@ ip_vs_conn_stats(struct ip_vs_conn *cp, struct ip_vs_service *svc) ...@@ -193,6 +203,8 @@ ip_vs_conn_stats(struct ip_vs_conn *cp, struct ip_vs_service *svc)
u64_stats_update_begin(&s->syncp); u64_stats_update_begin(&s->syncp);
s->cnt.conns++; s->cnt.conns++;
u64_stats_update_end(&s->syncp); u64_stats_update_end(&s->syncp);
local_bh_enable();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment