Re: [PATCH v2 nf-next 4/7] netfilter: conntrack: add gc worker to remove timed-out entries

[Florian Westphal <fw@strlen.de>]

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2016-08-24 at 13:55 +0200, Florian Westphal wrote:
> > Conntrack gc worker to evict stale entries.
> 
> 
> >  static struct nf_conn *
> >  __nf_conntrack_alloc(struct net *net,
> >  		     const struct nf_conntrack_zone *zone,
> > @@ -1527,6 +1597,7 @@ static int untrack_refs(void)
> >  
> >  void nf_conntrack_cleanup_start(void)
> >  {
> > +	conntrack_gc_work.exiting = true;
> >  	RCU_INIT_POINTER(ip_ct_attach, NULL);
> >  }
> >  
> > @@ -1536,6 +1607,9 @@ void nf_conntrack_cleanup_end(void)
> >  	while (untrack_refs() > 0)
> >  		schedule();
> >  
> > +	cancel_delayed_work_sync(&conntrack_gc_work.dwork);
> > +	/* can be re-scheduled once */
> 
> Are you sure ?
> 
> As conntrack_gc_work.exiting = true, I do not see how this can happen ?

nf_conntrack_cleanup_start() sets exiting = true

current cpu blocks in

cancel_delayed_work_sync(&conntrack_gc_work.dwork);

Iff the work queue was running on other cpu but was already past
gc_work->exiting check then when cancel_delayed_work_sync() (first one)
returns it will have re-armed itself via schedule_delayed_work().

So I think the 2nd cancel_delayed_work_sync is needed.

Let me know if you'd like to see a v3 with more verbose
comment about this.

Thanks!