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 >> >> >> >> On 02/24/2019 05:58 PM, Li RongQing wrote: >>> basechain->stats is rcu protected data, cannot assure that >>> twice accesses have the same result, so dereference it once. >>> >>> basechain->stats is allocated by percpu allocater, if it is not NULL, >>> its percpu variable does not need to be checked with NULL >>> >>> Signed-off-by: Zhang Yu <zhangyu31@xxxxxxxxx> >>> Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx> >>> --- >>> net/netfilter/nf_tables_core.c | 18 +++++++++--------- >>> 1 file changed, 9 insertions(+), 9 deletions(-) >>> >>> diff --git a/net/netfilter/nf_tables_core.c >>> b/net/netfilter/nf_tables_core.c index 2a00aef7b6d4..9be622c76a62 >>> 100644 >>> --- a/net/netfilter/nf_tables_core.c >>> +++ b/net/netfilter/nf_tables_core.c >>> @@ -98,20 +98,20 @@ 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, *pstat; >>> >>> base_chain = nft_base_chain(chain); >>> - if (!rcu_access_pointer(base_chain->stats)) >>> + >>> + stats = rcu_dereference(base_chain->stats); >> >> This looks bogus to me. >> >> Where is the needed rcu_read_lock() before rcu_dereference() ? >> > > Ok, I will check it carefully. > >> This rcu_access_pointer() test is fine, and avoids a >> local_bh_disable()/local_bh_enable() >> if they are not needed. > > > > But it can not assure that rcu_dereference(base_chain->stats) returns NULL since nft_chain_stats_replace, should we check it again, other than check the returning of this_cpu_ptr? > Sorry I do not understand your concern.
