Hi Julian,

On Jul 8, 2026, at 03:18, Julian Anastasov [off-list ref] wrote:


Hello,

On Tue, 7 Jul 2026, Yizhou Zhao wrote:

quoted

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]

Thank you for your suggestions.

We have posted a v2 patch at:
https://lore.kernel.org/netfilter-devel/20260708060454.20534-1-zhaoyz24@mails.tsinghua.edu.cn/ (local)

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

Regards,
Yizhou