[PATCH nf] ipvs: make destination flags atomic

[PATCH nf] ipvs: make destination flags atomic

[Yizhou Zhao]

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().

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.

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.

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>
---
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 */
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index cb36641f8d1c..539f603f38b7 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -1055,7 +1055,7 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
 
 	if (dest->u_threshold != 0 &&
 	    ip_vs_dest_totalconns(dest) >= dest->u_threshold)
-		dest->flags |= IP_VS_DEST_F_OVERLOAD;
+		atomic_or(IP_VS_DEST_F_OVERLOAD, &dest->flags);
 }
 
 
@@ -1151,13 +1151,13 @@ static inline void ip_vs_unbind_dest(struct ip_vs_conn *cp)
 
 	if (dest->l_threshold != 0) {
 		if (ip_vs_dest_totalconns(dest) < dest->l_threshold)
-			dest->flags &= ~IP_VS_DEST_F_OVERLOAD;
+			atomic_and(~IP_VS_DEST_F_OVERLOAD, &dest->flags);
 	} else if (dest->u_threshold != 0) {
 		if (ip_vs_dest_totalconns(dest) * 4 < dest->u_threshold * 3)
-			dest->flags &= ~IP_VS_DEST_F_OVERLOAD;
+			atomic_and(~IP_VS_DEST_F_OVERLOAD, &dest->flags);
 	} else {
-		if (dest->flags & IP_VS_DEST_F_OVERLOAD)
-			dest->flags &= ~IP_VS_DEST_F_OVERLOAD;
+		if (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
+			atomic_and(~IP_VS_DEST_F_OVERLOAD, &dest->flags);
 	}
 
 	ip_vs_dest_put(dest);
@@ -1188,7 +1188,7 @@ int ip_vs_check_template(struct ip_vs_conn *ct, struct ip_vs_dest *cdest)
 	 * Checking the dest server status.
 	 */
 	if ((dest == NULL) ||
-	    !(dest->flags & IP_VS_DEST_F_AVAILABLE) ||
+	    !(atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE) ||
 	    expire_quiescent_template(ipvs, dest) ||
 	    (cdest && (dest != cdest))) {
 		IP_VS_DBG_BUF(9, "check_template: dest not available for "
@@ -1929,7 +1929,7 @@ void ip_vs_expire_nodest_conn_flush(struct netns_ipvs *ipvs)
 			cp = ip_vs_hn0_to_conn(hn);
 			resched_score++;
 			dest = cp->dest;
-			if (!dest || (dest->flags & IP_VS_DEST_F_AVAILABLE))
+			if (!dest || (atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE))
 				continue;
 
 			if (atomic_read(&cp->n_control))
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index d40b404c1bf6..ca778937facf 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -302,7 +302,7 @@ ip_vs_in_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
 	struct ip_vs_dest *dest = cp->dest;
 	struct netns_ipvs *ipvs = cp->ipvs;
 
-	if (dest && (dest->flags & IP_VS_DEST_F_AVAILABLE)) {
+	if (dest && (atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE)) {
 		struct ip_vs_cpu_stats *s;
 		struct ip_vs_service *svc;
 
@@ -338,7 +338,7 @@ ip_vs_out_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
 	struct ip_vs_dest *dest = cp->dest;
 	struct netns_ipvs *ipvs = cp->ipvs;
 
-	if (dest && (dest->flags & IP_VS_DEST_F_AVAILABLE)) {
+	if (dest && (atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE)) {
 		struct ip_vs_cpu_stats *s;
 		struct ip_vs_service *svc;
 
@@ -2204,7 +2204,7 @@ ip_vs_in_hook(void *priv, struct sk_buff *skb, const struct nf_hook_state *state
 	}
 
 	/* Check the server status */
-	if (cp && cp->dest && !(cp->dest->flags & IP_VS_DEST_F_AVAILABLE)) {
+	if (cp && cp->dest && !(atomic_read(&cp->dest->flags) & IP_VS_DEST_F_AVAILABLE)) {
 		/* the destination server is not available */
 		if (sysctl_expire_nodest_conn(ipvs)) {
 			bool old_ct = ip_vs_conn_uses_old_conntrack(cp, skb);
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index bcf40b8c41cf..685b2675b6e0 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1368,10 +1368,10 @@ __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;
+	atomic_or(IP_VS_DEST_F_AVAILABLE, &dest->flags);
 
 	if (udest->u_threshold == 0 || udest->u_threshold > dest->u_threshold)
-		dest->flags &= ~IP_VS_DEST_F_OVERLOAD;
+		atomic_and(~IP_VS_DEST_F_OVERLOAD, &dest->flags);
 	dest->u_threshold = udest->u_threshold;
 	dest->l_threshold = udest->l_threshold;
 
@@ -1613,7 +1613,7 @@ static void __ip_vs_unlink_dest(struct ip_vs_service *svc,
 				struct ip_vs_dest *dest,
 				int svcupd)
 {
-	dest->flags &= ~IP_VS_DEST_F_AVAILABLE;
+	atomic_and(~IP_VS_DEST_F_AVAILABLE, &dest->flags);
 
 	spin_lock_bh(&dest->dst_lock);
 	__ip_vs_dst_cache_reset(dest);
diff --git a/net/netfilter/ipvs/ip_vs_dh.c b/net/netfilter/ipvs/ip_vs_dh.c
index e1f62f6b25e2..82492e824f02 100644
--- a/net/netfilter/ipvs/ip_vs_dh.c
+++ b/net/netfilter/ipvs/ip_vs_dh.c
@@ -201,7 +201,7 @@ static int ip_vs_dh_dest_changed(struct ip_vs_service *svc,
  */
 static inline int is_overloaded(struct ip_vs_dest *dest)
 {
-	return dest->flags & IP_VS_DEST_F_OVERLOAD;
+	return atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD;
 }
 
 
@@ -219,10 +219,10 @@ 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->flags & IP_VS_DEST_F_AVAILABLE)
-	    || atomic_read(&dest->weight) <= 0
-	    || is_overloaded(dest)) {
+	if (!dest
+	    || !(atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE)
+	    || atomic_read(&dest->weight) <= 0
+	    || 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..5231e518c07c 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 (!(atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD) &&
 		    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 15ccb2b2fa1f..d2eb5dda5b68 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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
 			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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
 			continue;
 
 		doh = ip_vs_dest_conn_overhead(dest);
@@ -502,7 +502,7 @@ ip_vs_lblc_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
 		 */
 
 		dest = en->dest;
-		if ((dest->flags & IP_VS_DEST_F_AVAILABLE) &&
+		if ((atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE) &&
 		    atomic_read(&dest->weight) > 0 && !is_overloaded(dest, svc))
 			goto out;
 	}
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index c90ea897c3f7..48f02453a5be 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -166,11 +166,11 @@ 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 (atomic_read(&least->flags) & IP_VS_DEST_F_OVERLOAD)
 			continue;
 
-		if ((atomic_read(&least->weight) > 0)
-		    && (least->flags & IP_VS_DEST_F_AVAILABLE)) {
+		if ((atomic_read(&least->weight) > 0)
+		    && (atomic_read(&least->flags) & IP_VS_DEST_F_AVAILABLE)) {
 			loh = ip_vs_dest_conn_overhead(least);
 			goto nextstage;
 		}
@@ -181,13 +181,13 @@ 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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
 			continue;
 
 		doh = ip_vs_dest_conn_overhead(dest);
 		if (((__s64)loh * atomic_read(&dest->weight) >
-		     (__s64)doh * atomic_read(&least->weight))
-		    && (dest->flags & IP_VS_DEST_F_AVAILABLE)) {
+		     (__s64)doh * atomic_read(&least->weight))
+		    && (atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE)) {
 			least = dest;
 			loh = doh;
 		}
@@ -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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
 			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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
 			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 38cc38c5d8bb..6acb3c904af5 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 ((atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD) ||
 		    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..c322ed1754b7 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;
+	       atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD;
 }
 
 /* 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..ffa4bfeb21d9 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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD ||
 		    !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..f7f17dddbb05 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 ((atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD) ||
 		    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..98453d205d6f 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 (!(atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD) &&
 			    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..0249062d1360 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 (!(atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD) &&
 		    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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
 			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..343780b82c95 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;
+	       atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD;
 }
 
 /*
diff --git a/net/netfilter/ipvs/ip_vs_twos.c b/net/netfilter/ipvs/ip_vs_twos.c
index dbb7f5fd4688..35fa4c6dc5cf 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 (!(atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)) {
 			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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
 			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..c2d09ac96fe8 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 (!(atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD) &&
 		    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 (atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD)
 			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..f21a75284971 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 (!(atomic_read(&dest->flags) & IP_VS_DEST_F_OVERLOAD) &&
 			    atomic_read(&dest->weight) >= mark->cw)
 				goto found;
 			if (dest == stop)
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index ce542ed4b013..37b9671e4960 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -351,7 +351,7 @@ __ip_vs_get_out_rt(struct netns_ipvs *ipvs, int skb_af, struct sk_buff *skb,
 			 * stored in dest_trash.
 			 */
 			if (!rt_dev_is_down(dst_dev_rcu(&rt->dst)) &&
-			    dest->flags & IP_VS_DEST_F_AVAILABLE)
+			    atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE)
 				__ip_vs_dst_set(dest, dest_dst, &rt->dst, 0);
 			else
 				noref = 0;
@@ -530,7 +530,7 @@ __ip_vs_get_out_rt_v6(struct netns_ipvs *ipvs, int skb_af, struct sk_buff *skb,
 			 * stored in dest_trash.
 			 */
 			if (!rt_dev_is_down(dst_dev_rcu(&rt->dst)) &&
-			    dest->flags & IP_VS_DEST_F_AVAILABLE)
+			    atomic_read(&dest->flags) & IP_VS_DEST_F_AVAILABLE)
 				__ip_vs_dst_set(dest, dest_dst, &rt->dst, cookie);
 			else
 				noref = 0;

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

[Julian Anastasov]

	Hello,

On Tue, 7 Jul 2026, Yizhou Zhao wrote:

> 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().
> 
> 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.

	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.

	We can compare 3 solutions, from fast to slow:

1. atomic_read or test_bit
	- no memory barriers for the readers
	- no memory ordering (=> stale data)

	PRO:
	- serializes RMW operations

	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.

2. Use refcount_inc_not_zero(&dest->available) from readers

	- and put the ref immediately or later:

	smp_mb__before_atomic();
	refcount_dec(&dest->available);

	- alternative: RMW such as atomic_fetch_add

	- 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

3. read_lock/write_lock
	- PRO: can modify more things under write lock
	- CON: full memory barriers

	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.

Regards

--
Julian Anastasov <ja@ssi.bg>

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

[Yizhou Zhao]

Hi Julian,


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

Thank you for your suggestions.

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

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

Regards,
Yizhou

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

[Julian Anastasov]

	Hello,

On Wed, 8 Jul 2026, Yizhou Zhao wrote:

> > On Jul 8, 2026, at 03:18, Julian Anastasov <ja@ssi.bg> 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/
> 
> 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 <ja@ssi.bg>

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

[Florian Westphal]

Julian Anastasov <ja@ssi.bg> wrote:
> 	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

Who is supposed to do what?

I.e., are you going to submit this officially as replacement
for the v2 of this patch or do you expect the sumbitters of
this patch to rework their v2 along these lines?

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

[Julian Anastasov]

	Hello,

On Thu, 9 Jul 2026, Florian Westphal wrote:

> Julian Anastasov <ja@ssi.bg> wrote:
> > 	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
> 
> Who is supposed to do what?
> 
> I.e., are you going to submit this officially as replacement
> for the v2 of this patch or do you expect the sumbitters of
> this patch to rework their v2 along these lines?

	I can sumbit the ip_vs_dest_update_overload()
part as separate patch which can be followed by patch(es)
that convert the overload flag to bitops.

	I'll wait for comments from Yizhou Zhao, he should
tell me if he is willing to convert the overload flag as
additional patch.

Regards

--
Julian Anastasov <ja@ssi.bg>