On 02/25/2019 06:53 PM, Li,Rongqing wrote: > > >> -----邮件原件----- >> 发件人: Eric Dumazet [mailto:eric.dumazet@xxxxxxxxx] >> 发送时间: 2019年2月25日 23:56 >> 收件人: Li,Rongqing <lirongqing@xxxxxxxxx>; netfilter-devel@xxxxxxxxxxxxxxx >> 主题: Re: 答复: 答复: [PATCH] netfilter: force access of RCU protected data in >> nft_update_chain_stats >> >> >> >> On 02/24/2019 10:00 PM, Li,Rongqing wrote: >>> >>> >>>> -----邮件原件----- >>>> 发件人: Eric Dumazet [mailto:eric.dumazet@xxxxxxxxx] >>>> 发送时间: 2019年2月25日 12:11 >>>> 收件人: Li,Rongqing <lirongqing@xxxxxxxxx>; >>>> netfilter-devel@xxxxxxxxxxxxxxx >>>> 主题: Re: 答复: [PATCH] netfilter: force access of RCU protected data in >>>> nft_update_chain_stats >>>> >>>> >>>> >>>> On 02/24/2019 08:03 PM, Li,Rongqing wrote: >>>>> >>>>> >>>>>> -----邮件原件----- >>>>>> 发件人: Eric Dumazet [mailto:eric.dumazet@xxxxxxxxx] >>>>>> 发送时间: 2019年2月25日 11:50 >>>>>> 收件人: Li,Rongqing <lirongqing@xxxxxxxxx>; >>>>>> netfilter-devel@xxxxxxxxxxxxxxx >>>>>> 主题: Re: [PATCH] netfilter: force access of RCU protected data in >>>>>> nft_update_chain_stats >>>>>> >>>>>> >>>>>> >>> >>> >>> [RFC] netfilter: check the result of dereferencing >>> base_chain->stats >>> >>> check the result of dereferencing base_chain->stats, instead of >>> result of this_cpu_ptr >>> >>> base_chain->stats maybe change to NULL when a chain is updated, >>> a NULL counter can be attached >>> >>> and we do not need to check returning of this_cpu_ptr since >>> base_chain->stats is allocated by percpu allocator if it is >>> non-NULL, this_cpu_ptr returns a valid value >>> >>> diff --git a/net/netfilter/nf_tables_core.c >>> b/net/netfilter/nf_tables_core.c index 2a00aef7b6d4..ed8a382d1012 >>> 100644 >>> --- a/net/netfilter/nf_tables_core.c >>> +++ b/net/netfilter/nf_tables_core.c >>> @@ -98,15 +98,16 @@ static noinline void nft_update_chain_stats(const >> struct nft_chain *chain, >>> const struct >> nft_pktinfo >>> *pkt) { >>> struct nft_base_chain *base_chain; >>> - struct nft_stats *stats; >>> + struct nft_stats *stats, *pstats; >> >> OK but then make sure to add proper sparse annotations ( aka __percpu ) >> > > I test this and found , there is always sparse error whether add __percpu or not > > Like: > struct nft_stats __percpu *pstats; > > pstats = rcu_dereference(base_chain->stats); I would suggest : diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c index 2a00aef7b6d4aaabfc538f24b0c36b50f76ce2b4..c29a8ce2c6bcdd683400f0b03dcc0c4440ab8fc8 100644 --- a/net/netfilter/nf_tables_core.c +++ b/net/netfilter/nf_tables_core.c @@ -98,21 +98,22 @@ static noinline void nft_update_chain_stats(const struct nft_chain *chain, const struct nft_pktinfo *pkt) { struct nft_base_chain *base_chain; + struct nft_stats __percpu *pstats; struct nft_stats *stats; base_chain = nft_base_chain(chain); - if (!rcu_access_pointer(base_chain->stats)) - return; - - local_bh_disable(); - stats = this_cpu_ptr(rcu_dereference(base_chain->stats)); - if (stats) { + rcu_read_lock(); + pstats = READ_ONCE(base_chain->stats); + if (pstats) { + local_bh_disable(); + stats = this_cpu_ptr(pstats); u64_stats_update_begin(&stats->syncp); stats->pkts++; stats->bytes += pkt->skb->len; u64_stats_update_end(&stats->syncp); + local_bh_enable(); } - local_bh_enable(); + rcu_read_unlock(); } struct nft_jumpstack {
