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@xxxxxx>
> > > > 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
