Re: About ip6_dst_destroy()

[Eric Dumazet <eric.dumazet@gmail.com>]

On 4/27/19 8:22 PM, David Ahern wrote:
> On 4/27/19 5:56 PM, Eric Dumazet wrote:
>> David
>>
>> I am staring at ip6_dst_destroy() and the last part makes really no sense to me.
>>
>> How rcu_read_lock()/rcu_read_unnlock() can help in a writer side ???
>>
>> Changlog of a68886a691804d3f6d479ebf6825480fbafb6a00 ("net/ipv6: Make from in rt6_info rcu protected")
>> does not make sense either.
>>
>> <quote>
>>     There is a race window when a FIB entry is deleted and the 'from' on the
>>     pcpu route is dropped and the pcpu route hits a cookie check. Handle
>>     this race using rcu on from.
>> </quote>
>>
> 
> A FIB entry (fib6_info) is deleted, but resources are not cleaned up as
> there are outstanding references to the entry. Specifically, the
> references are the 'from' on pcpu routes. Commit (93531c6743157) added
> code to release those references as otherwise there is nothing that
> forces it. Further testing hit the condition noted in a68886a69180.
> 
> I presume you are asking about ip6_dst_destroy vs all of the other
> 'from' references because of the fib6_info_release - which would result
> in an underflow when it is released twice. I guess something like a
> rmb() / wmb() pair is needed for this case.

I do not see how rmb/wmb pair will help.

Writers need to use a stronger synchronization between themselves. 

This can be some spinlock, a xchg() or cmpxchg()

The problem here is that nothing prevent ip6_dst_destroy() being called concurrently
with another writer like fib6_drop_pcpu_from()

fib6_drop_pcpu_from() uses &table->tb6_lock, which is not held in ip6_dst_destroy()

I will submit a patch switching all writers to xchg()