Re: [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Phil Sutter <phil@nwl.cc>]

On Thu, Apr 30, 2020 at 05:48:34PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Apr 30, 2020 at 05:44:40PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Apr 30, 2020 at 05:26:07PM +0200, Phil Sutter wrote:
> > [...]
> > > > BTW, this cache consistency check
> > > > 
> > > > commit 200bc399651499f502ac0de45f4d4aa4c9d37ab6
> > > > Author: Phil Sutter <phil@nwl.cc>
> > > > Date:   Fri Mar 13 13:02:12 2020 +0100
> > > > 
> > > >     nft: cache: Fix iptables-save segfault under stress
> > > > 
> > > > is already restored in this series, right?
> > > 
> > > Yes, IIRC this was the reason why I got a merge conflict upon rebase.
> > > But the problem shouldn't exist with the new logic: We fetch cache just
> > > once, so there is no cache update (and potential cache free) happening
> > > while iterating through chain lists or anything.
> > 
> > Still another process might be competing to update the ruleset, right?
> 
> I mean this case:
> 
> -       mnl_genid_get(h, &genid_stop);
> -       if (genid_start != genid_stop) {
> -               flush_chain_cache(h, NULL);
> -               goto retry;
> -       }
> 
> if the cache is inconsistent (another process stepped in and updated
> the ruleset), the discard this cache and fetch it again.

Ah, I understand your point now. Let me try to reenable this so
__nft_build_cache() returns a cache in which all entries "match"
regarding genid. I think this shouldn't cause problems anymore, even if
cache is freed in between attempts.

Thanks, Phil