Re: [PATCH nf-next 1/3] netfilter: nf_tables: add generation mask to table objects

[Patrick McHardy <kaber@trash.net>]

On 06.08, Pablo Neira Ayuso wrote:
> On Wed, Aug 05, 2015 at 11:09:16AM +0200, Patrick McHardy wrote:
> > > > > We shouldn't check if the object is active from the lookup function if
> > > > > we're in the middle of a transaction, since we hold the lock there is
> > > > > no way we can see inactive objects in the list. There's only one
> > > > > transaction at the same time.
> > > > 
> > > > That's not entirely correct. Dump continuations happen asynchronously to
> > > > netlink modifications and commit operations, so the genid may bump in the
> > > > middle. We can get an inconsistent view if we have:
> > > > 
> > > > 			dump set elements from set x table y
> > > > delete table y
> > > > create table y
> > > > create set x
> > > > begin commit
> > > > 			continue dump from new set
> > > 
> > > We catch this from the nfnlhdr->res_id field in the nfnetlink message,
> > > but see below.
> > > 
> > > > commit, send NEWGEN
> > > > 
> > > > Sure, we will get a NEWGEN message, but at that time we might already have
> > > > sent a full message for the new table/set since that message is only send
> > > > after the commit is completed.
> > > 
> > > I agree in that an event message at the beginning of the commit phase
> > > to announce the beginning new generation and another one to indicate
> > > of this transaction.
> > > 
> > > - preparation phase -
> > > delete table y
> > > create table y
> > > create set x
> > > - commit phase -
> > > send NEWGEN, attribute type: begin
> > > delete table y
> > > create table y
> > > create set x
> > > send NEWGEN, attribute type: end
> > > 
> > > Thanks for your feedback!
> > 
> > That might work if the message ordering is then guaranteed. However I think
> > we can fix this case without changing NEWGEN. Let me think about that a bit,
> > for now just taking care of the genid checks correctly seems like a good
> > step forward.
> 
> But we can catch this problem through ->res_id, OK?

I guess we could with a unique res_id per object, but how would this work
with multiple object types? Any change bumps res_id, so we'd invalidate
the full dump for any change.