|
Hello,
On Wed, 8 Jul 2026, Yizhou Zhao wrote:
> > On Jul 8, 2026, at 03:18, Julian Anastasov <ja@xxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx/
>
> 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@xxxxxx>
|
