[PATCH nf] ipvs: fix more races around the overload flag

[PATCH nf] ipvs: fix more races around the overload flag

[Julian Anastasov]

Sashiko warns that we still have races when the dest overload
thresholds are updated. If we narrow the range between
thresholds the lockless readers (ip_vs_bind_dest and
ip_vs_unbind_dest) may use old threshold values and
as result to skip the required flag change.

To solve the problem ip_vs_bind_dest() and ip_vs_unbind_dest()
will have the chance as before to change the overload
state not only at the exact threshold positions but
at any conn count below the lower and above the upper thresholds.
To make it work without entering the locked region for
every connection, we introduce l_threshold_val. Together with
u_threshold_val we will switch between two states:

1. not overloaded: where l_threshold_val is set to 0
to close the door for more ip_vs_unbind_dest() calls
to clear the overload state.

2. overloaded: where u_threshold_val is set to INT_MAX
to close the door for more ip_vs_bind_dest() calls
to set the overload state.

If a race occurs, multiple CPUs can repeat the state change
but this should be for short time window.

With the introduction of l_threshold_val the
IP_VS_DEST_F_OVERLOAD flag and the 'flags' field are not
needed anymore. We will use the ip_vs_dest_is_overloaded()
helper for all places that read the overload state.

Do not check IP_VS_DEST_CF_AVAILABLE in ip_vs_dh.c because
we do not hold reference to dest after it is removed and
rely on RCU to access the available dests.

To reduce the cacheline accesses while selecting destination,
keep the mostly used fields (weight and l_threshold_val)
at the first read-mostly cacheline together with the hash
nodes.

Fixes: 8f843441c4e7 ("ipvs: properly update the overload flag on dest edit")
Link: https://sashiko.dev/#/patchset/20260810190621.894119-1-pablo%40netfilter.org
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
 include/net/ip_vs.h              | 50 ++++++++++++++++++++------------
 net/netfilter/ipvs/ip_vs_conn.c  |  8 ++---
 net/netfilter/ipvs/ip_vs_ctl.c   | 38 ++++++++++++------------
 net/netfilter/ipvs/ip_vs_dh.c    | 16 ++--------
 net/netfilter/ipvs/ip_vs_fo.c    |  2 +-
 net/netfilter/ipvs/ip_vs_lblc.c  |  4 +--
 net/netfilter/ipvs/ip_vs_lblcr.c |  8 ++---
 net/netfilter/ipvs/ip_vs_lc.c    |  2 +-
 net/netfilter/ipvs/ip_vs_mh.c    |  2 +-
 net/netfilter/ipvs/ip_vs_nq.c    |  2 +-
 net/netfilter/ipvs/ip_vs_ovf.c   |  2 +-
 net/netfilter/ipvs/ip_vs_rr.c    |  2 +-
 net/netfilter/ipvs/ip_vs_sed.c   |  4 +--
 net/netfilter/ipvs/ip_vs_sh.c    |  2 +-
 net/netfilter/ipvs/ip_vs_twos.c  |  4 +--
 net/netfilter/ipvs/ip_vs_wlc.c   |  4 +--
 net/netfilter/ipvs/ip_vs_wrr.c   |  2 +-
 17 files changed, 76 insertions(+), 76 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index be3a6617adf4..e09b9598a476 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -36,9 +36,6 @@
 #define IP_VS_HDR_INVERSE	1
 #define IP_VS_HDR_ICMP		2
 
-/* Destination Server Flags */
-#define IP_VS_DEST_F_OVERLOAD	0x0002		/* server is overloaded */
-
 /* Destination Server Config Flags */
 #define IP_VS_DEST_CF_AVAILABLE	0x0001		/* server is available */
 
@@ -973,32 +970,43 @@ struct ip_vs_dest_dst {
  * and so on.
  */
 struct ip_vs_dest {
+	/* Cacheline for hash table nodes - read-mostly */
+
 	struct list_head	n_list;   /* for the dests in the service */
 	struct hlist_node	d_list;   /* for table with all the dests */
 
-	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		conn_flags;	/* flags to copy to conn */
 	atomic_t		weight;		/* server weight */
-	unsigned long		cflags;		/* config flags */
 	atomic_t		last_weight;	/* server latest weight */
+
+	/* connection thresholds */
+	u32			l_threshold_val;/* used lower threshold */
+	u32			u_threshold_val;/* used upper threshold */
+	/* 32/48 */
+	u32			l_threshold;	/* lower threshold */
+	u32			u_threshold;	/* upper threshold */
+
+	unsigned long		cflags;		/* config flags */
+
+	/* 44/64 */
+	atomic_t		conn_flags;	/* flags to copy to conn */
+
 	__u16			tun_type;	/* tunnel type */
 	__be16			tun_port;	/* tunnel port */
 	__u16			tun_flags;	/* tunnel flags */
 
-	refcount_t		refcnt;		/* reference counter */
-	struct ip_vs_stats      stats;          /* statistics */
-	unsigned long		idle_start;	/* start time, jiffies */
+	u16			af;		/* address family */
+	__be16			port;		/* port number of the server */
+	/* 60/80 */
+	union nf_inet_addr	addr;		/* IP address of the server */
 
-	/* connection counters and thresholds */
-	atomic_t		activeconns;	/* active connections */
+	/* connection counters */
 	atomic_t		totalconns;	/* total connections */
+	atomic_t		activeconns;	/* active connections */
 	atomic_t		persistconns;	/* persistent connections */
-	__u32			u_threshold;	/* upper threshold */
-	__u32			l_threshold;	/* lower threshold */
-	__u32			l_threshold_val;/* used lower threshold */
+
+	refcount_t		refcnt;		/* reference counter */
+	struct ip_vs_stats      stats;          /* statistics */
+	unsigned long		idle_start;	/* start time, jiffies */
 
 	/* for destination cache */
 	spinlock_t		dst_lock;	/* lock of dst_cache */
@@ -1915,7 +1923,13 @@ 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, int mode);
+void ip_vs_dest_update_overload(struct ip_vs_dest *dest, bool overload);
+
+/* Check if dest is in overloaded state */
+static inline bool ip_vs_dest_is_overloaded(const struct ip_vs_dest *dest)
+{
+	return READ_ONCE(dest->l_threshold_val);
+}
 
 /* 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 1ff8cad39ab4..ce0883f259cb 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -1150,8 +1150,8 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
 		if (!(flags & IP_VS_CONN_F_INACTIVE))
 			atomic_inc(&dest->activeconns);
 		tc = atomic_inc_return(&dest->totalconns);
-		if (tc == READ_ONCE(dest->u_threshold))
-			ip_vs_dest_update_overload(dest, 1);
+		if (tc >= READ_ONCE(dest->u_threshold_val))
+			ip_vs_dest_update_overload(dest, true);
 	} else {
 		/* It is a persistent connection/template, so increase
 		   the persistent connection counter */
@@ -1243,8 +1243,8 @@ static inline void ip_vs_unbind_dest(struct ip_vs_conn *cp)
 		if (!(cp->flags & IP_VS_CONN_F_INACTIVE))
 			atomic_dec(&dest->activeconns);
 		tc = atomic_fetch_dec(&dest->totalconns);
-		if (tc == READ_ONCE(dest->l_threshold_val))
-			ip_vs_dest_update_overload(dest, -1);
+		if (tc <= READ_ONCE(dest->l_threshold_val))
+			ip_vs_dest_update_overload(dest, false);
 	} else {
 		/* It is a persistent connection/template, so decrease
 		   the persistent connection counter */
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 8f9a8e491ad6..5de69404a5e0 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1309,32 +1309,32 @@ void ip_vs_stats_free(struct ip_vs_stats *stats)
  * - conns reach u_threshold and exceed it: set the flag
  * - conns go below l_threshold (or 75% of u_threshold): clear the flag
  */
-static void __ip_vs_dest_update_overload(struct ip_vs_dest *dest, int mode)
+static void __ip_vs_dest_update_overload(struct ip_vs_dest *dest, bool overload)
 {
 	int conns;
 	u32 l, u;
 
 	lockdep_assert_held(&dest->dst_lock);
 	u = READ_ONCE(dest->u_threshold);
-	if (!u)
-		goto unset;
-	l = READ_ONCE(dest->l_threshold_val);
-	conns = atomic_read(&dest->totalconns);
-	if (conns >= (mode > 0 ? l : u)) {
-		dest->flags |= IP_VS_DEST_F_OVERLOAD;
-		return;
+	if (u) {
+		/* Low threshold defaults to 75% of upper threshold */
+		l = READ_ONCE(dest->l_threshold) ? : (u - (u >> 2));
+		conns = atomic_read(&dest->totalconns);
+		if (conns >= (overload ? l : u)) {
+			WRITE_ONCE(dest->u_threshold_val, INT_MAX);
+			WRITE_ONCE(dest->l_threshold_val, l);
+			return;
+		}
 	}
-	if (conns >= (mode < 0 ? u : l))
-		return;
 
-unset:
-	dest->flags &= ~IP_VS_DEST_F_OVERLOAD;
+	WRITE_ONCE(dest->u_threshold_val, u ? : INT_MAX);
+	WRITE_ONCE(dest->l_threshold_val, 0);
 }
 
-void ip_vs_dest_update_overload(struct ip_vs_dest *dest, int mode)
+void ip_vs_dest_update_overload(struct ip_vs_dest *dest, bool overload)
 {
 	spin_lock_bh(&dest->dst_lock);
-	__ip_vs_dest_update_overload(dest, mode);
+	__ip_vs_dest_update_overload(dest, overload);
 	spin_unlock_bh(&dest->dst_lock);
 }
 
@@ -1406,15 +1406,13 @@ __ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
 
 	if (READ_ONCE(dest->u_threshold) != udest->u_threshold ||
 	    READ_ONCE(dest->l_threshold) != udest->l_threshold) {
+		bool overload;
+
 		spin_lock_bh(&dest->dst_lock);
 		WRITE_ONCE(dest->u_threshold, udest->u_threshold);
 		WRITE_ONCE(dest->l_threshold, udest->l_threshold);
-		/* Low threshold defaults to 75% of upper threshold */
-		WRITE_ONCE(dest->l_threshold_val,
-			   udest->l_threshold ? :
-			   (udest->u_threshold -
-			    (udest->u_threshold >> 2)));
-		__ip_vs_dest_update_overload(dest, 0);
+		overload = ip_vs_dest_is_overloaded(dest);
+		__ip_vs_dest_update_overload(dest, overload);
 		spin_unlock_bh(&dest->dst_lock);
 	}
 
diff --git a/net/netfilter/ipvs/ip_vs_dh.c b/net/netfilter/ipvs/ip_vs_dh.c
index 43abed7a26a6..95e52950adbb 100644
--- a/net/netfilter/ipvs/ip_vs_dh.c
+++ b/net/netfilter/ipvs/ip_vs_dh.c
@@ -195,16 +195,6 @@ static int ip_vs_dh_dest_changed(struct ip_vs_service *svc,
 }
 
 
-/*
- *      If the dest flags is set with IP_VS_DEST_F_OVERLOAD,
- *      consider that the server is overloaded here.
- */
-static inline int is_overloaded(struct ip_vs_dest *dest)
-{
-	return dest->flags & IP_VS_DEST_F_OVERLOAD;
-}
-
-
 /*
  *      Destination hashing scheduling
  */
@@ -219,10 +209,8 @@ ip_vs_dh_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 
 	s = (struct ip_vs_dh_state *) svc->sched_data;
 	dest = ip_vs_dh_get(svc->af, s, &iph->daddr);
-	if (!dest ||
-	    !(dest->cflags & IP_VS_DEST_CF_AVAILABLE)
-	    || atomic_read(&dest->weight) <= 0
-	    || is_overloaded(dest)) {
+	if (!dest || atomic_read(&dest->weight) <= 0 ||
+	    ip_vs_dest_is_overloaded(dest)) {
 		ip_vs_scheduler_err(svc, "no destination available");
 		return NULL;
 	}
diff --git a/net/netfilter/ipvs/ip_vs_fo.c b/net/netfilter/ipvs/ip_vs_fo.c
index d657b47c6511..e07fa33f6d52 100644
--- a/net/netfilter/ipvs/ip_vs_fo.c
+++ b/net/netfilter/ipvs/ip_vs_fo.c
@@ -29,7 +29,7 @@ ip_vs_fo_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 	 * Find virtual server with highest weight and send it traffic
 	 */
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
-		if (!(dest->flags & IP_VS_DEST_F_OVERLOAD) &&
+		if (!ip_vs_dest_is_overloaded(dest) &&
 		    atomic_read(&dest->weight) > hw) {
 			hweight = dest;
 			hw = atomic_read(&dest->weight);
diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index 693bcc82ccb7..bff109c1c959 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -414,7 +414,7 @@ __ip_vs_lblc_schedule(struct ip_vs_service *svc)
 	 * new connection.
 	 */
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(dest))
 			continue;
 		if (atomic_read(&dest->weight) > 0) {
 			least = dest;
@@ -429,7 +429,7 @@ __ip_vs_lblc_schedule(struct ip_vs_service *svc)
 	 */
   nextstage:
 	list_for_each_entry_continue_rcu(dest, &svc->destinations, n_list) {
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(dest))
 			continue;
 
 		doh = ip_vs_dest_conn_overhead(dest);
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index f53f05ceea36..c2853e07e787 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -166,7 +166,7 @@ static inline struct ip_vs_dest *ip_vs_dest_set_min(struct ip_vs_dest_set *set)
 	/* select the first destination server, whose weight > 0 */
 	list_for_each_entry_rcu(e, &set->list, list) {
 		least = e->dest;
-		if (least->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(least))
 			continue;
 
 		if ((atomic_read(&least->weight) > 0) &&
@@ -181,7 +181,7 @@ static inline struct ip_vs_dest *ip_vs_dest_set_min(struct ip_vs_dest_set *set)
   nextstage:
 	list_for_each_entry_continue_rcu(e, &set->list, list) {
 		dest = e->dest;
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(dest))
 			continue;
 
 		doh = ip_vs_dest_conn_overhead(dest);
@@ -577,7 +577,7 @@ __ip_vs_lblcr_schedule(struct ip_vs_service *svc)
 	 * new connection.
 	 */
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(dest))
 			continue;
 
 		if (atomic_read(&dest->weight) > 0) {
@@ -593,7 +593,7 @@ __ip_vs_lblcr_schedule(struct ip_vs_service *svc)
 	 */
   nextstage:
 	list_for_each_entry_continue_rcu(dest, &svc->destinations, n_list) {
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(dest))
 			continue;
 
 		doh = ip_vs_dest_conn_overhead(dest);
diff --git a/net/netfilter/ipvs/ip_vs_lc.c b/net/netfilter/ipvs/ip_vs_lc.c
index 18b976a7c4d2..9002f491bff6 100644
--- a/net/netfilter/ipvs/ip_vs_lc.c
+++ b/net/netfilter/ipvs/ip_vs_lc.c
@@ -38,7 +38,7 @@ ip_vs_lc_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 	 */
 
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
-		if ((dest->flags & IP_VS_DEST_F_OVERLOAD) ||
+		if (ip_vs_dest_is_overloaded(dest) ||
 		    atomic_read(&dest->weight) == 0)
 			continue;
 		doh = ip_vs_dest_conn_overhead(dest);
diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c
index 020863047562..d70b23aec5fe 100644
--- a/net/netfilter/ipvs/ip_vs_mh.c
+++ b/net/netfilter/ipvs/ip_vs_mh.c
@@ -80,7 +80,7 @@ static inline void generate_hash_secret(hsiphash_key_t *hash1,
 static inline bool is_unavailable(struct ip_vs_dest *dest)
 {
 	return atomic_read(&dest->weight) <= 0 ||
-	       dest->flags & IP_VS_DEST_F_OVERLOAD;
+	       ip_vs_dest_is_overloaded(dest);
 }
 
 /* Returns hash value for IPVS MH entry */
diff --git a/net/netfilter/ipvs/ip_vs_nq.c b/net/netfilter/ipvs/ip_vs_nq.c
index ada158c610ce..7cc28902086e 100644
--- a/net/netfilter/ipvs/ip_vs_nq.c
+++ b/net/netfilter/ipvs/ip_vs_nq.c
@@ -72,7 +72,7 @@ ip_vs_nq_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
 
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD ||
+		if (ip_vs_dest_is_overloaded(dest) ||
 		    !atomic_read(&dest->weight))
 			continue;
 
diff --git a/net/netfilter/ipvs/ip_vs_ovf.c b/net/netfilter/ipvs/ip_vs_ovf.c
index c5c67df80a0b..eaa3a7ae6efa 100644
--- a/net/netfilter/ipvs/ip_vs_ovf.c
+++ b/net/netfilter/ipvs/ip_vs_ovf.c
@@ -33,7 +33,7 @@ ip_vs_ovf_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 	*/
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
 		w = atomic_read(&dest->weight);
-		if ((dest->flags & IP_VS_DEST_F_OVERLOAD) ||
+		if (ip_vs_dest_is_overloaded(dest) ||
 		    atomic_read(&dest->activeconns) > w ||
 		    w == 0)
 			continue;
diff --git a/net/netfilter/ipvs/ip_vs_rr.c b/net/netfilter/ipvs/ip_vs_rr.c
index 4125ee561cdc..bd17cdd092f5 100644
--- a/net/netfilter/ipvs/ip_vs_rr.c
+++ b/net/netfilter/ipvs/ip_vs_rr.c
@@ -66,7 +66,7 @@ ip_vs_rr_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 		list_for_each_entry_continue_rcu(dest,
 						 &svc->destinations,
 						 n_list) {
-			if (!(dest->flags & IP_VS_DEST_F_OVERLOAD) &&
+			if (!ip_vs_dest_is_overloaded(dest) &&
 			    atomic_read(&dest->weight) > 0)
 				/* HIT */
 				goto out;
diff --git a/net/netfilter/ipvs/ip_vs_sed.c b/net/netfilter/ipvs/ip_vs_sed.c
index 245a323c84cd..7925f4d28fce 100644
--- a/net/netfilter/ipvs/ip_vs_sed.c
+++ b/net/netfilter/ipvs/ip_vs_sed.c
@@ -75,7 +75,7 @@ ip_vs_sed_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 	 */
 
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
-		if (!(dest->flags & IP_VS_DEST_F_OVERLOAD) &&
+		if (!ip_vs_dest_is_overloaded(dest) &&
 		    atomic_read(&dest->weight) > 0) {
 			least = dest;
 			loh = ip_vs_sed_dest_overhead(least);
@@ -90,7 +90,7 @@ ip_vs_sed_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 	 */
   nextstage:
 	list_for_each_entry_continue_rcu(dest, &svc->destinations, n_list) {
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(dest))
 			continue;
 		doh = ip_vs_sed_dest_overhead(dest);
 		if ((__s64)loh * atomic_read(&dest->weight) >
diff --git a/net/netfilter/ipvs/ip_vs_sh.c b/net/netfilter/ipvs/ip_vs_sh.c
index cd67066e3b26..c24dd5cb7e47 100644
--- a/net/netfilter/ipvs/ip_vs_sh.c
+++ b/net/netfilter/ipvs/ip_vs_sh.c
@@ -73,7 +73,7 @@ struct ip_vs_sh_state {
 static inline bool is_unavailable(struct ip_vs_dest *dest)
 {
 	return atomic_read(&dest->weight) <= 0 ||
-	       dest->flags & IP_VS_DEST_F_OVERLOAD;
+	       ip_vs_dest_is_overloaded(dest);
 }
 
 /*
diff --git a/net/netfilter/ipvs/ip_vs_twos.c b/net/netfilter/ipvs/ip_vs_twos.c
index dbb7f5fd4688..86186122a1f0 100644
--- a/net/netfilter/ipvs/ip_vs_twos.c
+++ b/net/netfilter/ipvs/ip_vs_twos.c
@@ -52,7 +52,7 @@ static struct ip_vs_dest *ip_vs_twos_schedule(struct ip_vs_service *svc,
 
 	/* Generate a random weight between [0,sum of all weights) */
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
-		if (!(dest->flags & IP_VS_DEST_F_OVERLOAD)) {
+		if (!ip_vs_dest_is_overloaded(dest)) {
 			weight = atomic_read(&dest->weight);
 			if (weight > 0) {
 				total_weight += weight;
@@ -75,7 +75,7 @@ static struct ip_vs_dest *ip_vs_twos_schedule(struct ip_vs_service *svc,
 
 	/* Pick two weighted servers */
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(dest))
 			continue;
 
 		weight = atomic_read(&dest->weight);
diff --git a/net/netfilter/ipvs/ip_vs_wlc.c b/net/netfilter/ipvs/ip_vs_wlc.c
index 9da445ca09a1..08e9e22f72bf 100644
--- a/net/netfilter/ipvs/ip_vs_wlc.c
+++ b/net/netfilter/ipvs/ip_vs_wlc.c
@@ -47,7 +47,7 @@ ip_vs_wlc_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 	 */
 
 	list_for_each_entry_rcu(dest, &svc->destinations, n_list) {
-		if (!(dest->flags & IP_VS_DEST_F_OVERLOAD) &&
+		if (!ip_vs_dest_is_overloaded(dest) &&
 		    atomic_read(&dest->weight) > 0) {
 			least = dest;
 			loh = ip_vs_dest_conn_overhead(least);
@@ -62,7 +62,7 @@ ip_vs_wlc_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 	 */
   nextstage:
 	list_for_each_entry_continue_rcu(dest, &svc->destinations, n_list) {
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
+		if (ip_vs_dest_is_overloaded(dest))
 			continue;
 		doh = ip_vs_dest_conn_overhead(dest);
 		if ((__s64)loh * atomic_read(&dest->weight) >
diff --git a/net/netfilter/ipvs/ip_vs_wrr.c b/net/netfilter/ipvs/ip_vs_wrr.c
index 2dcff1040da5..fad69bae5a22 100644
--- a/net/netfilter/ipvs/ip_vs_wrr.c
+++ b/net/netfilter/ipvs/ip_vs_wrr.c
@@ -176,7 +176,7 @@ ip_vs_wrr_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 		list_for_each_entry_continue_rcu(dest,
 						 &svc->destinations,
 						 n_list) {
-			if (!(dest->flags & IP_VS_DEST_F_OVERLOAD) &&
+			if (!ip_vs_dest_is_overloaded(dest) &&
 			    atomic_read(&dest->weight) >= mark->cw)
 				goto found;
 			if (dest == stop)
-- 
2.55.0

Re: [PATCH nf] ipvs: fix more races around the overload flag

[Julian Anastasov]

	Hello,

On Mon, 7 Sep 2026, Julian Anastasov wrote:

> Sashiko warns that we still have races when the dest overload
> thresholds are updated. If we narrow the range between
> thresholds the lockless readers (ip_vs_bind_dest and
> ip_vs_unbind_dest) may use old threshold values and
> as result to skip the required flag change.
> 
> To solve the problem ip_vs_bind_dest() and ip_vs_unbind_dest()
> will have the chance as before to change the overload
> state not only at the exact threshold positions but
> at any conn count below the lower and above the upper thresholds.
> To make it work without entering the locked region for
> every connection, we introduce l_threshold_val. Together with
> u_threshold_val we will switch between two states:
> 
> 1. not overloaded: where l_threshold_val is set to 0
> to close the door for more ip_vs_unbind_dest() calls
> to clear the overload state.
> 
> 2. overloaded: where u_threshold_val is set to INT_MAX
> to close the door for more ip_vs_bind_dest() calls
> to set the overload state.
> 
> If a race occurs, multiple CPUs can repeat the state change
> but this should be for short time window.
> 
> With the introduction of l_threshold_val the
> IP_VS_DEST_F_OVERLOAD flag and the 'flags' field are not
> needed anymore. We will use the ip_vs_dest_is_overloaded()
> helper for all places that read the overload state.
> 
> Do not check IP_VS_DEST_CF_AVAILABLE in ip_vs_dh.c because
> we do not hold reference to dest after it is removed and
> rely on RCU to access the available dests.
> 
> To reduce the cacheline accesses while selecting destination,
> keep the mostly used fields (weight and l_threshold_val)
> at the first read-mostly cacheline together with the hash
> nodes.
> 
> Fixes: 8f843441c4e7 ("ipvs: properly update the overload flag on dest edit")
> Link: https://sashiko.dev/#/patchset/20260810190621.894119-1-pablo%40netfilter.org
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> ---
>  include/net/ip_vs.h              | 50 ++++++++++++++++++++------------
>  net/netfilter/ipvs/ip_vs_conn.c  |  8 ++---
>  net/netfilter/ipvs/ip_vs_ctl.c   | 38 ++++++++++++------------
>  net/netfilter/ipvs/ip_vs_dh.c    | 16 ++--------
>  net/netfilter/ipvs/ip_vs_fo.c    |  2 +-
>  net/netfilter/ipvs/ip_vs_lblc.c  |  4 +--
>  net/netfilter/ipvs/ip_vs_lblcr.c |  8 ++---
>  net/netfilter/ipvs/ip_vs_lc.c    |  2 +-
>  net/netfilter/ipvs/ip_vs_mh.c    |  2 +-
>  net/netfilter/ipvs/ip_vs_nq.c    |  2 +-
>  net/netfilter/ipvs/ip_vs_ovf.c   |  2 +-
>  net/netfilter/ipvs/ip_vs_rr.c    |  2 +-
>  net/netfilter/ipvs/ip_vs_sed.c   |  4 +--
>  net/netfilter/ipvs/ip_vs_sh.c    |  2 +-
>  net/netfilter/ipvs/ip_vs_twos.c  |  4 +--
>  net/netfilter/ipvs/ip_vs_wlc.c   |  4 +--
>  net/netfilter/ipvs/ip_vs_wrr.c   |  2 +-
>  17 files changed, 76 insertions(+), 76 deletions(-)
> 

> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 8f9a8e491ad6..5de69404a5e0 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1309,32 +1309,32 @@ void ip_vs_stats_free(struct ip_vs_stats *stats)
>   * - conns reach u_threshold and exceed it: set the flag
>   * - conns go below l_threshold (or 75% of u_threshold): clear the flag
>   */
> -static void __ip_vs_dest_update_overload(struct ip_vs_dest *dest, int mode)
> +static void __ip_vs_dest_update_overload(struct ip_vs_dest *dest, bool overload)
>  {
>  	int conns;
>  	u32 l, u;
>  
>  	lockdep_assert_held(&dest->dst_lock);
>  	u = READ_ONCE(dest->u_threshold);
> -	if (!u)
> -		goto unset;
> -	l = READ_ONCE(dest->l_threshold_val);
> -	conns = atomic_read(&dest->totalconns);
> -	if (conns >= (mode > 0 ? l : u)) {
> -		dest->flags |= IP_VS_DEST_F_OVERLOAD;
> -		return;
> +	if (u) {
> +		/* Low threshold defaults to 75% of upper threshold */
> +		l = READ_ONCE(dest->l_threshold) ? : (u - (u >> 2));
> +		conns = atomic_read(&dest->totalconns);
> +		if (conns >= (overload ? l : u)) {
> +			WRITE_ONCE(dest->u_threshold_val, INT_MAX);
> +			WRITE_ONCE(dest->l_threshold_val, l);
> +			return;
> +		}
>  	}

	Comments about the Sashiko review here:

https://sashiko.dev/#/patchset/20260907183453.112559-1-ja%40ssi.bg

Sashiko said:

> Can a lockless time-of-check to time-of-use race here permanently brick 
> an IPVS destination?
> It seems there is a race window between reading the connections and 
> updating
> l_threshold_val. Because l_threshold_val acts as a dynamic gate for
> ip_vs_unbind_dest(), concurrent unbinds might fail to synchronize with 
> this thread.
> 
> If we have the following sequence:
> CPU1 in ip_vs_bind_dest():
>   - increments totalconns to u_threshold
>   - evaluates tc >= u_threshold_val and calls 
> ip_vs_dest_update_overload()
>   - acquires dst_lock in __ip_vs_dest_update_overload()
>   - reads conns = atomic_read(&dest->totalconns)
> CPU2 in ip_vs_unbind_dest():
>   - decrements totalconns, returning the old value
>   - evaluates tc <= READ_ONCE(dest->l_threshold_val)
>   - CPU1 hasn't updated l_threshold_val yet, so it is still 0
>   - the condition is false, so CPU2 bypasses the lock and does not
>     clear the overload state
> CPU1 resumes:
>   - evaluates conns >= l (using its stale read)
>   - writes INT_MAX to u_threshold_val and l to l_threshold_val
>   - returns, leaving the destination in an overloaded state
> 
> Because the destination is marked overloaded, it receives no new 
> traffic.
> And because it has no traffic, unbind will never be called to clear the
> overloaded state. Does this leave the destination permanently bricked 
> with 0 active connections?

	The destination is not permanently blocked because for
this connection ip_vs_unbind_dest() should be called after
its timer expires. But it is true that the dest can be blocked
for long time.

	My opinion is that we still should apply this patch because
it improves the handling from the previously applied patch. And we
are still searching for solution that does not hurt the performance.

	The problem is that the CPU that is about to mark the dest
as overloaded can be interrupted while at the same time timers
expired on other CPUs can reduce the total conns down to 1
leaving the only connection attached to a blocked dest
where the schedulers do not select this dest until the
connection expires and clears the overload state.

	May be followup patches should allow the schedulers
to re-evaluate the overload state if not done "recently".
Or some other mechanism that will unblock the dest.

> -	if (conns >= (mode < 0 ? u : l))
> -		return;
>  
> -unset:
> -	dest->flags &= ~IP_VS_DEST_F_OVERLOAD;
> +	WRITE_ONCE(dest->u_threshold_val, u ? : INT_MAX);
> +	WRITE_ONCE(dest->l_threshold_val, 0);
>  }
>  
> -void ip_vs_dest_update_overload(struct ip_vs_dest *dest, int mode)
> +void ip_vs_dest_update_overload(struct ip_vs_dest *dest, bool overload)
>  {
>  	spin_lock_bh(&dest->dst_lock);
> -	__ip_vs_dest_update_overload(dest, mode);
> +	__ip_vs_dest_update_overload(dest, overload);
>  	spin_unlock_bh(&dest->dst_lock);
>  }
>  
> @@ -1406,15 +1406,13 @@ __ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
>  
>  	if (READ_ONCE(dest->u_threshold) != udest->u_threshold ||
>  	    READ_ONCE(dest->l_threshold) != udest->l_threshold) {
> +		bool overload;
> +
>  		spin_lock_bh(&dest->dst_lock);

	As for the uninitialized u_threshold_val, the initialization
will happen later, so it is deferred, which is unintentionally better
from configuration point of view.

>  		WRITE_ONCE(dest->u_threshold, udest->u_threshold);
>  		WRITE_ONCE(dest->l_threshold, udest->l_threshold);
> -		/* Low threshold defaults to 75% of upper threshold */
> -		WRITE_ONCE(dest->l_threshold_val,
> -			   udest->l_threshold ? :
> -			   (udest->u_threshold -
> -			    (udest->u_threshold >> 2)));
> -		__ip_vs_dest_update_overload(dest, 0);
> +		overload = ip_vs_dest_is_overloaded(dest);
> +		__ip_vs_dest_update_overload(dest, overload);
>  		spin_unlock_bh(&dest->dst_lock);
>  	}

Regards

--
Julian Anastasov <ja@ssi.bg>