Re: [Patch net-next] fib: move fib_rules_cleanup_ops() under rtnl lock

[Alexander Duyck <alexander.h.duyck@redhat.com>]

On 03/30/2015 05:12 PM, Cong Wang wrote:
> On Mon, Mar 30, 2015 at 5:02 PM, Alexander Duyck
> <alexander.h.duyck@redhat.com> wrote:
>> On 03/30/2015 04:47 PM, Cong Wang wrote:
>>> As long as we agree rtnl lock should be taken, you already take my point
>>> here ($subject says so).
>>
>> Yes, I agree lock can be held.  For fib4 it was already holding the RTNL
>> lock when it made that call.  You can update the other users of
>> fib_rules_unregister so that they call it with the RTNL lock held as well.
>>
>>> It is just API change to move rtnl_lock up to caller or whatever
>>> appropriate.
>>
>> Right, so like I said for fib4 this is resolved.  That just leaves ipmr,
>> ip6mr, fib6, and dn_rules that need to be updated so that they correctly
>> handle the RTNL locking in their exit/cleanup paths. Since you already have
>> some related patches out for these I will let you take them otherwise I
>> might try to go through and clean them up next week.
>>
> Ok, then we are finally on the same page. We need two patches:
>
> 1) move unregister under rtnl lock (as what this patch intended to do)

Yes I think the only disagreement was on how to do it.  Your original 
patch placed it in fib_rules_cleanup_ops, and the preference of myself 
and Thomas was to hold the lock before you even call 
fib_rules_unregister.  So the IPv4 code is fine with the patch I 
submitted, it is the other callers of fib_rules_unregister which must be 
updated.

> 2) remove the unnecessary rules_mod_lock
>
> Thanks.

Please define "unnecessary" as we have had a bit of back and forth on 
how our views can differ there.  As far as I know it still has to be 
held for the fib_rule_ops list manipulation, specifically the call to 
list_del_rcu.  However, it doesn't need to be held when we call 
fib_rules_cleanup_ops.

- Alex