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). It is indeed a corner-case issue, though. Cheers, Phil
