From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pablo Neira Ayuso Subject: Re: [PATCH nf-next 1/3] netfilter: nf_tables: add generation mask to table objects Date: Thu, 6 Aug 2015 12:21:05 +0200 Message-ID: <20150806102105.GB18683@salvia> References: <1438679128-4146-1-git-send-email-pablo@netfilter.org> <20150804090917.GA6033@acer.localdomain> <20150804182154.GA737@salvia> <20150805084128.GB13187@acer.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netfilter-devel@vger.kernel.org To: Patrick McHardy Return-path: Received: from mail.us.es ([193.147.175.20]:58890 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753368AbbHFKPA (ORCPT ); Thu, 6 Aug 2015 06:15:00 -0400 Content-Disposition: inline In-Reply-To: <20150805084128.GB13187@acer.localdomain> Sender: netfilter-devel-owner@vger.kernel.org List-ID: On Wed, Aug 05, 2015 at 10:41:28AM +0200, Patrick McHardy wrote: > On 04.08, Pablo Neira Ayuso wrote: > > On Tue, Aug 04, 2015 at 11:09:17AM +0200, Patrick McHardy wrote: > > > On 04.08, Pablo Neira Ayuso wrote: > > > > The dumping of table objects can be inconsistent when interfering with the > > > > preparation phase of our 2-phase commit protocol because: > > > > > > > > 1) We remove objects from the lists during the preparation phase, that can be > > > > added re-added from the abort step. Thus, we may miss objects that are still > > > > active. > > > > > > > > 2) We add new objects to the lists during the preparation phase, so we may get > > > > objects that are not yet active with an internal flag set. > > > > > > > > We can resolve this problem with generation masks, as we already do for rules > > > > when we expose them to the packet path. > > > > > > > > After this change, we always obtain a consistent list as long as we stay in the > > > > same generation. The userspace side can detect interferences through the > > > > generation counter. If so, it needs to restart. > > > > > > > > As a result, we can get rid of the internal NFT_TABLE_INACTIVE flag. > > > > > > I have a similar patch queued up, however there seems to be something missing > > > in this patch. The lookup functions need to take the genmask into account. > > > Otherwise you can not delete and add a new table in the same batch. The same > > > holds for all other object types. > > > > I got what you meant, we have to skip the delete table when iterating > > over the list. > > Exactly. I'd propose to simply pass in the requested genid, this has the added > benefit of not having to sprinkle those checks throughout the code. If that simplifies the patchset, I think it's a good idea. Thanks!