Re: [PATCH nf v4] netfilter: ipset: remove need to allocate memory on delete operations operations

Hi Florian,

On Fri, 7 Aug 2026, Florian Westphal wrote:

> Florian Westphal <[email protected]> wrote:
> > +		write_seqcount_end(&nets->seq);
> > +		spin_unlock_bh(&set->lock);
> >  	}
> >  #endif
> >  }
> > @@ -1253,31 +1310,41 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
> >  #if IPSET_NET_COUNT == 2
> >  	struct net_prefixes *nets1;
> >  	struct mtype_elem orig = *d;
> > +	unsigned int seq1;
> >  	int ret, i, j, k;
> >  #else
> >  	int ret, i, j;
> >  #endif
> > -	u32 key, multi = 0;
> > +	unsigned int seq0;
> > +	u32 key, multi;
> >  	u8 pos;
> >  
> >  	pr_debug("test by nets\n");
> >  	rcu_read_lock_bh();
> > +retry:
> > +	multi = 0;
> >  	nets0 = rcu_dereference_bh(h->rnets[0]);
> > +	seq0 = read_seqcount_begin(&nets0->seq);
> [..]
> >  unlock:
> > +	if (read_seqcount_retry(&nets0->seq, seq0))
> > +		goto retry;
> 
> Sashiko now says this retry loop causes double-count on match, yet v3 
> said not doing this causes possible miss of a nomatch entry.
> 
> I think missing nomatch entry is worse and doublecount is ok.
> 
> The restart is rare enough anyway because write_seqcount_begin() is only 
> called when set is flushed or a element deletion brings a cidr count 
> value down to 0.

Yes, exactly. [Somehow flush seems to "ask" to create a new empty set, 
switch to it and release the old one after, but then again, we might be 
out of memory to allocate.]
 
> Another option is to prellocate all possible CIDRs but I'd like to avoid 
> it, this means checking up to 128 possible CIDRs in ipv6 case.
> 
> Pre-sampling all values into local on-stack array is a non-starter too 
> due to stack cost.
> 
> What one COULD do is to use pcpu stash area for this, needs local bh / 
> local_lock.

I was thinking on pcpu as well: keep preallocated max size "struct 
net_prefixes" per cpu, signal end of lookup by the zero count value of 
"struct net_prefix". But then we'd just keep a lot of stash areas. But the 
arrays are not huge - I refrained to use linked lists both because struct 
net_prefix is small and also to count on the prefetching of the array 
elements.

> But I think its a bit "too much" for this problem.

Best regards,
Jozsef
-- 
E-mail : [email protected], [email protected], [email protected]
Address: Wigner Research Centre for Physics
         H-1525 Budapest 114, POB. 49, Hungary