Re: [PATCH nf] ipvs: make destination flags atomic
From: Julian Anastasov <ja@ssi.bg>
Date: 2026-07-08 15:53:35
Also in:
lkml, lvs-devel, netfilter-devel, stable
Subsystem:
ipvs, netfilter, networking [general], networking [ipv4/ipv6], the rest · Maintainers:
Simon Horman, Julian Anastasov, Pablo Neira Ayuso, Florian Westphal, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Ido Schimmel, Linus Torvalds
Hello, On Wed, 8 Jul 2026, Yizhou Zhao wrote:
quoted
On Jul 8, 2026, at 03:18, Julian Anastasov [off-list ref] wrote: On Tue, 7 Jul 2026, Yizhou Zhao wrote: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.
After looking again at the code, I think we can do it in different way: - 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 include/net/ip_vs.h, see below... - IP_VS_DEST_F_AVAILABLE is changed only under service_mutex, so we can keep its usage - 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): - test_bit(IP_VS_DEST_FL_OVERLOAD, &dest->flags2) - set_bit(IP_VS_DEST_FL_OVERLOAD, &dest->flags2) Sometimes if (test_bit()) clear_bit() can avoid full memory barrier in ip_vs_dest_update_overload() - clear_bit(IP_VS_DEST_FL_OVERLOAD, &dest->flags2) test_bit() guard can help here too 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:
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); } +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); } +__always_inline void ip_vs_dest_update_overload(struct ip_vs_dest *dest) +{ + int conns, l, u; + + u = READ_ONCE(dest->u_threshold); + if (!u) + goto unset; + conns = ip_vs_dest_totalconns(dest); + if (conns >= u) { + dest->flags |= IP_VS_DEST_F_OVERLOAD; + return; + } + l = READ_ONCE(dest->l_threshold) ? : (u * 3 / 4); + if (conns >= l && l) + return; + +unset: + dest->flags &= ~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); } - if (dest->u_threshold != 0 && - ip_vs_dest_totalconns(dest) >= dest->u_threshold) - dest->flags |= IP_VS_DEST_F_OVERLOAD; + ip_vs_dest_update_overload(dest); }
@@ -1257,16 +1275,8 @@ static inline void ip_vs_unbind_dest(struct ip_vs_conn *cp) atomic_dec(&dest->persistconns); } - if (dest->l_threshold != 0) { - if (ip_vs_dest_totalconns(dest) < dest->l_threshold) - dest->flags &= ~IP_VS_DEST_F_OVERLOAD; - } else if (dest->u_threshold != 0) { - if (ip_vs_dest_totalconns(dest) * 4 < dest->u_threshold * 3) - dest->flags &= ~IP_VS_DEST_F_OVERLOAD; - } else { - if (dest->flags & IP_VS_DEST_F_OVERLOAD) - dest->flags &= ~IP_VS_DEST_F_OVERLOAD; - } + if (dest->flags & IP_VS_DEST_F_OVERLOAD) + ip_vs_dest_update_overload(dest); 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; /* We cannot modify an address and change the address family */ BUG_ON(!add && udest->af != 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 |= IP_VS_DEST_F_AVAILABLE; - if (udest->u_threshold == 0 || udest->u_threshold > dest->u_threshold) - dest->flags &= ~IP_VS_DEST_F_OVERLOAD; - dest->u_threshold = udest->u_threshold; - dest->l_threshold = udest->l_threshold; + upd_thresh = READ_ONCE(dest->u_threshold) != udest->u_threshold || + READ_ONCE(dest->l_threshold) != 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); dest->af = udest->af;
@@ -3667,8 +3670,8 @@ __ip_vs_get_dest_entries(struct netns_ipvs *ipvs, const struct ip_vs_get_dests * entry.port = dest->port; entry.conn_flags = atomic_read(&dest->conn_flags); entry.weight = atomic_read(&dest->weight); - entry.u_threshold = dest->u_threshold; - entry.l_threshold = dest->l_threshold; + entry.u_threshold = READ_ONCE(dest->u_threshold); + entry.l_threshold = READ_ONCE(dest->l_threshold); entry.activeconns = atomic_read(&dest->activeconns); entry.inactconns = atomic_read(&dest->inactconns); entry.persistconns = 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,
Regards -- Julian Anastasov [off-list ref]