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

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 08/07/2026 07:04, Yizhou Zhao wrote:
> IPVS destination schedulers read dest->flags from packet processing paths
> while holding only the RCU read lock.  The same word is updated by plain
> 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 protects the destination lifetime, but it does not
> serialize accesses to dest->flags.  A plain load can therefore race with a
> plain write, and concurrent plain read-modify-write updates can lose an
> AVAILABLE or OVERLOAD bit update.
> 
> KCSAN reports the race with a standard IPVS configuration using the SH
> scheduler and a destination with u_threshold set:
> 
>    BUG: KCSAN: data-race in __ip_vs_update_dest / ip_vs_sh_schedule
>    write to ... of 4 bytes by task ipvs_cfg:
>      __ip_vs_update_dest
>      ip_vs_edit_dest
>      do_ip_vs_set_ctl
>      __x64_sys_setsockopt
>    read to ... of 4 bytes by task ipvs_churn:
>      ip_vs_sh_schedule
>      ip_vs_schedule
>      tcp_conn_schedule
>      ip_vs_in_hook
>      tcp_connect
>      __x64_sys_connect
>    value changed: 0x00000003 -> 0x00000001
> 
> Convert dest->flags to atomic_t and use atomic_read(), atomic_or(), and
> atomic_and() for all destination flag tests and updates.  This preserves
> the existing 32-bit field size while making the flag updates atomic RMW
> operations and making readers use atomic accesses.  Valid minimum-sized
> IPVS configuration and scheduling paths are unchanged; only the
> synchronization of the destination status flags changes.
> 
> This is limited to synchronizing the flags word itself.  It does not add
> ordering for readers, and it does not make scheduler decisions operate on a
> fresh snapshot of all destination state; readers may still observe stale
> state in the usual IPVS fast path. This keeps the packet fast path free
> of additional barriers or locks.
> 
> Fixes: eba3b5a78799d ("ipvs: SH fallback and L4 hashing")
> Cc: stable@vger.kernel.org
> Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
> Reported-by: Ao Wang <wangao@seu.edu.cn>
> Reported-by: Xuewei Feng <fengxw06@126.com>
> Reported-by: Qi Li <qli01@tsinghua.edu.cn>
> Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
> Assisted-by: Claude-Code:GLM-5.2
> Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> ---
> Changes in v2:
> - Clarify that the patch fixes the flags data race and RMW lost updates,
>    but does not prevent readers from observing stale scheduling state.
> - Fix checkpatch logical-continuation warnings.
> - Suggested by Julian Anastasov.
> - Link to v1: https://lore.kernel.org/netfilter-devel/20260707085706.96322-1-zhaoyz24@mails.tsinghua.edu.cn/
> ---
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 49297fec448a..bb969738ed73 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -972,7 +972,7 @@ struct ip_vs_dest {
>   	u16			af;		/* address family */
>   	__be16			port;		/* port number of the server */
>   	union nf_inet_addr	addr;		/* IP address of the server */
> -	volatile unsigned int	flags;		/* dest status flags */
> +	atomic_t		flags;		/* dest status flags */
>   	atomic_t		conn_flags;	/* flags to copy to conn */
>   	atomic_t		weight;		/* server weight */
>   	atomic_t		last_weight;	/* server latest weight */

It would be quite interesting to look at pahole output of ip_vs_dest
structure after the modification, it may have some "areas to improve"
for the performance