On 17/01/17 17:07, David Ahern wrote:
On 1/17/17 3:04 AM, Robert Shearman wrote:
Is it safe to release the rtnl lock here? E.g. neither fib_get_nhs nor
fib_create_info take a reference on the device and further up inet_rtm_newroute
has a pointer to the struct fib_table without taking a reference.
fib tables can not be deleted so that reference is safe.
Looks like we can free a table on unmerging. Admittedly this is the
local table so is unlikely to be one that lwtunnels are used on, but
does it not need to be safe against races anyway?
You are right about the dev reference in the IPv4 path but the thing is
build_state does not need the device reference.
Roopa: do you recall why dev is passed to build_state? Nothing about the encap
should require a device reference and none of the current encaps use it.
Unfortunately, I think more invasive changes are required to solve this. I'm
happy to work on solving this.
I have a patch that removes passing dev to build_state. Walking the remainder
of the code I do not see any more references that would be affected by dropping
rtnl here on the add path. Still looking at the delete path.
Ok, I'll continue looking too and let you know if there's anything else
that pops up.
Having said that, even if we eliminate all the unreferenced objects in
the current code, what are the chances that we'll be able to keep it
this way going forward? Perhaps it would be safer to retry the insert
operation from as close to the start as possible?
Thanks,
Rob
