On 02/25/2019 06:53 PM, Li,Rongqing wrote:
>
>
>> -----邮件原件-----
>> 发件人: Eric Dumazet [mailto:[email protected]]
>> 发送时间: 2019年2月25日 23:56
>> 收件人: Li,Rongqing <[email protected]>; [email protected]
>> 主题: 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:[email protected]]
>>>> 发送时间: 2019年2月25日 12:11
>>>> 收件人: Li,Rongqing <[email protected]>;
>>>> [email protected]
>>>> 主题: 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:[email protected]]
>>>>>> 发送时间: 2019年2月25日 11:50
>>>>>> 收件人: Li,Rongqing <[email protected]>;
>>>>>> [email protected]
>>>>>> 主题: 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 {
