Re: [PATCH nf] ipvs: make destination flags atomic

Hi Julian,


> On Jul 8, 2026, at 03:18, Julian Anastasov <[email protected]> wrote:
> 
> 
> Hello,
> 
> On Tue, 7 Jul 2026, Yizhou Zhao wrote:
> 
>> is_unavailable() in the SH scheduler reads dest->flags from the packet
>> scheduling path while holding only the RCU read lock.  The same word is
>> updated by read-modify-write operations from connection accounting and
>> destination update paths, for example ip_vs_bind_dest(),
>> ip_vs_unbind_dest(), and __ip_vs_update_dest().
>> 
>> The RCU read lock only protects the destination lifetime; it does not
>> serialize accesses to dest->flags.  A racing plain load or RMW update can
>> therefore observe stale state or lose an AVAILABLE/OVERLOAD bit update,
>> which can make the scheduler choose an overloaded destination or report no
>> available destination even though one should be usable.
> 
> While the patch correctly serializes the concurrent
> modifications for the flags, we can not claim that the scheduler
> will not choose an overloaded or unavailable destination.
> The patch does not change the fact that we can work with
> stale data.
> 
> We can compare 3 solutions, from fast to slow:
> 
> 1. atomic_read or test_bit
> - no memory barriers for the readers
> - no memory ordering (=> stale data)
> 
> PRO:
> - serializes RMW operations
> 
> CON:
> - readers can use old values
> - writers may need to synchronize while changing
> the flags, eg. to check the thresholds and update the
> flags in atomic way. We do not do this.
> 
> 2. Use refcount_inc_not_zero(&dest->available) from readers
> 
> - and put the ref immediately or later:
> 
> smp_mb__before_atomic();
> refcount_dec(&dest->available);
> 
> - alternative: RMW such as atomic_fetch_add
> 
> - writers can synchronize by using the IP_VS_DEST_F_AVAILABLE
> flag and then to inc/dec &dest->available when the
> flag changes
> - the same can be done for &dest->not_overloaded and
> IP_VS_DEST_F_OVERLOAD
> - PRO: readers are serialized perfectly with the
> changed value, new packets will detect the changes
> immediately
> - CON:
> - 2 full memory barriers for the readers
> - writers may need to synchronize while changing
> the flags
> 
> 3. read_lock/write_lock
> - PRO: can modify more things under write lock
> - CON: full memory barriers
> 
> With this patch you choose solution 1.
> The other solutions can be expensive for the fast path.
> Lets fix the commit message. Also, it is better to fix the
> scripts/checkpatch.pl warnings about the 'if' conditions,
> even if they are not introduced now.
> 
> Regards
> 
> --
> Julian Anastasov <[email protected]>

Thank you for your suggestions.

We have posted a v2 patch at:
https://lore.kernel.org/netfilter-devel/[email protected]/

The v2 patch updates the commit message with more conservative
wording, and fixes the checkpatch logical-continuation warnings.

Regards,
Yizhou