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

Hi Julian,


> On Jul 8, 2026, at 03:18, Julian Anastasov <[email protected]> wrote:
>=20
>=20
> Hello,
>=20
> On Tue, 7 Jul 2026, Yizhou Zhao wrote:
>=20
>> 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().
>>=20
>> 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.
>=20
> 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.
>=20
> We can compare 3 solutions, from fast to slow:
>=20
> 1. atomic_read or test_bit
> - no memory barriers for the readers
> - no memory ordering (=3D> stale data)
>=20
> PRO:
> - serializes RMW operations
>=20
> 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.
>=20
> 2. Use refcount_inc_not_zero(&dest->available) from readers
>=20
> - and put the ref immediately or later:
>=20
> smp_mb__before_atomic();
> refcount_dec(&dest->available);
>=20
> - alternative: RMW such as atomic_fetch_add
>=20
> - 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
>=20
> 3. read_lock/write_lock
> - PRO: can modify more things under write lock
> - CON: full memory barriers
>=20
> 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.
>=20
> Regards
>=20
> --
> Julian Anastasov <[email protected]>

Thank you for your suggestions.

We have posted a v2 patch at:
=
https://lore.kernel.org/netfilter-devel/20260708060454.20534-1-zhaoyz24@ma=
ils.tsinghua.edu.cn/

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

Regards,
Yizhou=