Re: [PATCH nf-next 2/6] netfilter: conntrack: get rid of conntrack timer

[Florian Westphal <fw@strlen.de>]

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2016-08-19 at 18:04 +0200, Florian Westphal wrote:
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > On Fri, 2016-08-19 at 17:16 +0200, Florian Westphal wrote:
> > > 
> > > > Hmm, ____nf_conntrack_find caller needs to hold rcu_read_lock,
> > > > in case object is free'd SLAB_DESTROY_BY_RCU should delay actual release
> > > > of the page.
> > > 
> > > Well, point is that SLAB_DESTROY_BY_RCU means that we have no grace
> > > period, and object can be immediately reused and recycled.
> > > 
> > > @next pointer can definitely be overwritten.
> > 
> > I see.  Isn't that detected by the nulls magic (to restart
> > lookup if entry was moved to other chain due to overwritten next pointer)?
> 
> Well, you did not add the nulls magic in your code ;)

Oh.  Right, its indeed mising in the gc code.

> It might be fine, since it should be a rare event, and garbage
> collection is best effort, so you might add a comment in gc_worker() why
> it is probably overkill to restart the loop in this unlikely event.

Seems like a good idea, I will add it.

> BTW, maybe nf_conntrack_tuple_taken() should get the nulls magic check,
> as it is currently missing.

Good point, I will investigate.

Thanks Eric!