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
