Cc'ing Eric Biederman. On Mon, May 16, 2016 at 09:38:53PM -0700, Joe Stringer wrote: > On 6 May 2016 at 04:03, Pablo Neira Ayuso <pa...@netfilter.org> wrote: > > Hi Joe, > > > > On Thu, May 05, 2016 at 03:50:37PM -0700, Joe Stringer wrote: > >> diff --git a/net/netfilter/nf_conntrack_helper.c > >> b/net/netfilter/nf_conntrack_helper.c > >> index 3b40ec575cd5..6860b19be406 100644 > >> --- a/net/netfilter/nf_conntrack_helper.c > >> +++ b/net/netfilter/nf_conntrack_helper.c > >> @@ -449,10 +449,10 @@ void nf_conntrack_helper_unregister(struct > >> nf_conntrack_helper *me) > >> */ > >> synchronize_rcu(); > >> > >> - rtnl_lock(); > >> + mutex_lock(&net_mutex); > >> for_each_net(net) > >> __nf_conntrack_helper_unregister(me, net); > >> - rtnl_unlock(); > >> + mutex_unlock(&net_mutex); > > > > This simple solution works because we have no .exit callbacks in any > > of our helpers. Otherwise, the helper code may be already gone by when > > the worker has a chance to run to release the netns. > > > > If so, probably I can append this as comment to this function so we > > don't forget. If we ever have .exit callbacks (I don't expect so), we > > would need to wait for worker completion. > > Hi Pablo, > > Did you want me to re-spin this patch or look into another approach?
Last time I looked into this, my impression was that we should register each conntrack helper via register_pernet_subsys(). Then, from the exit path I was missing something like netns_barrier() [ note that I didn't seem to find anything similar] to ensure netns workqueue completion before we leave from the module exit path so that the ->exit() netns callback code doesn't go away. That would also avoid the rtnl_lock() dependency.
