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()
