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]>
