Re: [PATCH nft 0/5] relax cache requirements, speed up incremental updates

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Aug 15, 2024 at 03:10:13PM +0200, Phil Sutter wrote:
> On Thu, Aug 15, 2024 at 02:46:02PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Aug 15, 2024 at 02:25:15PM +0200, Phil Sutter wrote:
> > > On Thu, Aug 15, 2024 at 01:37:07PM +0200, Pablo Neira Ayuso wrote:
> > > > Hi,
> > > > 
> > > > The following patchset relaxes cache requirements, this is based on the
> > > > observation that objects are fetched to report errors and provide hints.
> > > 
> > > This is nice as it applies to error path only, though the second cache
> > > fetch is prone to race conditions.
> > 
> > The call to nft_cache_update() ensures cache is consistent, old cache
> > is dropped and a new consistent cache is obtained. The hint could be
> > misleading (worst case) though since the cache could have different
> > generation ID that the transaction itself, but it is just a hint.
> > 
> > > Did you consider retrying the whole transaction with beefed-up cache
> > > in error case?
> > 
> > Why retry? I am assuming a batch where the user made a mistake, retry
> > will fail again.
> > 
> > > I was about to mention how it nicely integrates with transaction
> > > refresh in ERESTART case, but then realized this is iptables code
> > > and nft doesn't retry in that case?!
> > 
> > I think you are talking about different scenario, that is, userspace
> > sends an update but generation ID mismatches, kernel reports ERESTART
> > and nftables revamps, this is to catch an interference with another
> > process, that needs to be done in nft, but it is a different issue.
> 
> Yes, I had incorrect error reporting in mind: Kernel reports ENOENT for
> a chain which another process creates concurrently. The error path cache
> update fetches the newly created chain and error reporting suggests to
> use the exact chain user specified (I assume).

IIRC, the fuzzy match code skips exact matches, worst case can be a
very hint.

> It is indeed a corner-case issue, though.

ERESTART handling can be useful for your rule index feature, where
consistency is fundamental to ensure that rule is added where the user
really wants.