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

Hello Julian,

Thank you for the detailed proposal. Yes, I am happy to handle the
follow-up conversion of the overload flag to bitops.


> On Jul 8, 2026, at 23:53, Julian Anastasov <[email protected]> wrote:
>=20
>=20
> Hello,
>=20
> On Wed, 8 Jul 2026, Yizhou Zhao wrote:
>=20
>>> On Jul 8, 2026, at 03:18, Julian Anastasov <[email protected]> wrote:
>>>=20
>>> On Tue, 7 Jul 2026, Yizhou Zhao wrote:
>>>=20
>>=20
>> We have posted a v2 patch at:
>> =
https://lore.kernel.org/netfilter-devel/20260708060454.20534-1-zhaoyz24@ma=
ils.tsinghua.edu.cn/
>>=20
>> The v2 patch updates the commit message with more conservative
>> wording, and fixes the checkpatch logical-continuation warnings.
>=20
> After looking again at the code, I think we can
> do it in different way:
>=20
> - IP_VS_DEST_F_AVAILABLE and IP_VS_DEST_F_OVERLOAD are defined
> in include/uapi/linux/ip_vs.h but we never export them to user
> space. So, we are free to change them. We can move them to=20
> include/net/ip_vs.h, see below...
>=20
> - IP_VS_DEST_F_AVAILABLE is changed only under service_mutex,
> so we can keep its usage
>=20
> - IP_VS_DEST_F_OVERLOAD needs different access methods.
> We can add 'unsigned long flags2;', may be after l_threshold.
> And to switch to such usage (F_OVERLOAD -> FL_OVERLOAD):
>=20
> - test_bit(IP_VS_DEST_FL_OVERLOAD, &dest->flags2)
> - set_bit(IP_VS_DEST_FL_OVERLOAD, &dest->flags2)
>=20
> Sometimes if (test_bit()) clear_bit() can avoid
> full memory barrier in ip_vs_dest_update_overload()
>=20
> - clear_bit(IP_VS_DEST_FL_OVERLOAD, &dest->flags2)
> test_bit() guard can help here too
>=20
> As there are other races involved, something like
> this can be a starting point for such change. It tries harder
> to update the overload flag on dest edit/add but it does not
> include the proposed bitops:
>=20
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 49297fec448a..b34631270e24 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -1906,6 +1906,8 @@ static inline void =
ip_vs_dest_put_and_free(struct ip_vs_dest *dest)
> kfree(dest);
> }
>=20
> +void ip_vs_dest_update_overload(struct ip_vs_dest *dest);
> +
> /* IPVS sync daemon data and function prototypes
>  * (from ip_vs_sync.c)
>  */
> diff --git a/net/netfilter/ipvs/ip_vs_conn.c =
b/net/netfilter/ipvs/ip_vs_conn.c
> index d19caf66afeb..3fd221996e6e 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -1087,6 +1087,26 @@ static inline int ip_vs_dest_totalconns(struct =
ip_vs_dest *dest)
> + atomic_read(&dest->inactconns);
> }
>=20
> +__always_inline void ip_vs_dest_update_overload(struct ip_vs_dest =
*dest)
> +{
> + int conns, l, u;
> +
> + u =3D READ_ONCE(dest->u_threshold);
> + if (!u)
> + goto unset;
> + conns =3D ip_vs_dest_totalconns(dest);
> + if (conns >=3D u) {
> + dest->flags |=3D IP_VS_DEST_F_OVERLOAD;
> + return;
> + }
> + l =3D READ_ONCE(dest->l_threshold) ? : (u * 3 / 4);
> + if (conns >=3D l && l)
> + return;
> +

I noticed one integer-rounding detail in the proposed helper. The
existing default lower-threshold check:

ip_vs_dest_totalconns(dest) * 4 < dest->u_threshold * 3

clears OVERLOAD when conns is below ceil(3 * u / 4), assuming the
multiplications do not overflow. In the proposed helper:

l =3D u * 3 / 4;
if (conns >=3D l && l)
return;

the division rounds down, so the boundary is different when u is not a
multiple of four. For example, with u =3D=3D 2 and conns =3D=3D 1, the =
existing
code clears OVERLOAD while the helper keeps it set. Would using

l =3D u - u / 4;

for the default lower threshold preserve the existing behavior while
also avoiding the multiplication overflow?

> +unset:
> + dest->flags &=3D ~IP_VS_DEST_F_OVERLOAD;
> +}
> +
> /*
>  * Bind a connection entry with a virtual service destination
>  * Called just after a new connection entry is created.
> @@ -1161,9 +1181,7 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct =
ip_vs_dest *dest)
> atomic_inc(&dest->persistconns);
> }
>=20
> - if (dest->u_threshold !=3D 0 &&
> -    ip_vs_dest_totalconns(dest) >=3D dest->u_threshold)
> - dest->flags |=3D IP_VS_DEST_F_OVERLOAD;
> + ip_vs_dest_update_overload(dest);
> }
>=20
>=20
> @@ -1257,16 +1275,8 @@ static inline void ip_vs_unbind_dest(struct =
ip_vs_conn *cp)
> atomic_dec(&dest->persistconns);
> }
>=20
> - if (dest->l_threshold !=3D 0) {
> - if (ip_vs_dest_totalconns(dest) < dest->l_threshold)
> - dest->flags &=3D ~IP_VS_DEST_F_OVERLOAD;
> - } else if (dest->u_threshold !=3D 0) {
> - if (ip_vs_dest_totalconns(dest) * 4 < dest->u_threshold * 3)
> - dest->flags &=3D ~IP_VS_DEST_F_OVERLOAD;
> - } else {
> - if (dest->flags & IP_VS_DEST_F_OVERLOAD)
> - dest->flags &=3D ~IP_VS_DEST_F_OVERLOAD;
> - }
> + if (dest->flags & IP_VS_DEST_F_OVERLOAD)
> + ip_vs_dest_update_overload(dest);
>=20
> ip_vs_dest_put(dest);
> }
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c =
b/net/netfilter/ipvs/ip_vs_ctl.c
> index bcf40b8c41cf..2871116e46ec 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1315,6 +1315,7 @@ __ip_vs_update_dest(struct ip_vs_service *svc, =
struct ip_vs_dest *dest,
> struct ip_vs_service *old_svc;
> struct ip_vs_scheduler *sched;
> int conn_flags;
> + bool upd_thresh;
>=20
> /* We cannot modify an address and change the address family */
> BUG_ON(!add && udest->af !=3D dest->af);
> @@ -1370,10 +1371,12 @@ __ip_vs_update_dest(struct ip_vs_service *svc, =
struct ip_vs_dest *dest,
> /* set the dest status flags */
> dest->flags |=3D IP_VS_DEST_F_AVAILABLE;
>=20
> - if (udest->u_threshold =3D=3D 0 || udest->u_threshold > =
dest->u_threshold)
> - dest->flags &=3D ~IP_VS_DEST_F_OVERLOAD;
> - dest->u_threshold =3D udest->u_threshold;
> - dest->l_threshold =3D udest->l_threshold;
> + upd_thresh =3D READ_ONCE(dest->u_threshold) !=3D udest->u_threshold =
||
> +     READ_ONCE(dest->l_threshold) !=3D udest->l_threshold;
> + WRITE_ONCE(dest->u_threshold, udest->u_threshold);
> + WRITE_ONCE(dest->l_threshold, udest->l_threshold);
> + if (upd_thresh)
> + ip_vs_dest_update_overload(dest);
>=20
> dest->af =3D udest->af;
>=20
> @@ -3667,8 +3670,8 @@ __ip_vs_get_dest_entries(struct netns_ipvs =
*ipvs, const struct ip_vs_get_dests *
> entry.port =3D dest->port;
> entry.conn_flags =3D atomic_read(&dest->conn_flags);
> entry.weight =3D atomic_read(&dest->weight);
> - entry.u_threshold =3D dest->u_threshold;
> - entry.l_threshold =3D dest->l_threshold;
> + entry.u_threshold =3D READ_ONCE(dest->u_threshold);
> + entry.l_threshold =3D READ_ONCE(dest->l_threshold);
> entry.activeconns =3D atomic_read(&dest->activeconns);
> entry.inactconns =3D atomic_read(&dest->inactconns);
> entry.persistconns =3D atomic_read(&dest->persistconns);
> @@ -4277,8 +4280,10 @@ static int ip_vs_genl_fill_dest(struct sk_buff =
*skb, struct ip_vs_dest *dest)
> dest->tun_port) ||
>    nla_put_u16(skb, IPVS_DEST_ATTR_TUN_FLAGS,
> dest->tun_flags) ||
> -    nla_put_u32(skb, IPVS_DEST_ATTR_U_THRESH, dest->u_threshold) ||
> -    nla_put_u32(skb, IPVS_DEST_ATTR_L_THRESH, dest->l_threshold) ||
> +    nla_put_u32(skb, IPVS_DEST_ATTR_U_THRESH,
> + READ_ONCE(dest->u_threshold)) ||
> +    nla_put_u32(skb, IPVS_DEST_ATTR_L_THRESH,
> + READ_ONCE(dest->l_threshold)) ||
>    nla_put_u32(skb, IPVS_DEST_ATTR_ACTIVE_CONNS,
> atomic_read(&dest->activeconns)) ||
>    nla_put_u32(skb, IPVS_DEST_ATTR_INACT_CONNS,
>=20
> Regards
>=20
> --
> Julian Anastasov <[email protected]>

Please go ahead with the ip_vs_dest_update_overload() patch. I will base
the follow-up on the posted version and check the resulting struct
ip_vs_dest layout as Vadim suggested.

Regards,
Yizhou