Re: [PATCH nf] ipvs: make destination flags atomic
From: Julian Anastasov <ja@ssi.bg>
Date: 2026-07-07 19:18:47
Also in:
lkml, lvs-devel, netfilter-devel, stable
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 [off-list ref]