Download raw body.
make route rt_locks atomic
Hi,
rt_locks is accessed lockless. Setting the bits has to be done
atomically. Loads should be marked as atomic to avoid wrong compiler
optimizations.
The correlation between rt_locks and rt_mtu is very weak. When
RTV_MTU is set, stop modifying rt_mtu. But is is not really necessary
to stop updating rt_mtu immediately when another thread modifies
rt_locks. The events in the network stack changing those values
are not synchronized anyway. So I came to the conclusion that
memory barriers are not necessary.
In route message output, I have left setting the flags in exclusive
net lock as they are modified together with other route attributes.
ok?
bluhm
Index: net/route.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
diff -u -p -r1.438 route.c
--- net/route.c 3 Jan 2025 21:27:40 -0000 1.438
+++ net/route.c 13 Jan 2025 22:50:54 -0000
@@ -557,7 +557,7 @@ rt_setgwroute(struct rtentry *rt, const
* If the MTU of next hop is 0, this will reset the MTU of the
* route to run PMTUD again from scratch.
*/
- if (!ISSET(rt->rt_locks, RTV_MTU)) {
+ if (!ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU)) {
u_int mtu, nhmtu;
mtu = atomic_load_int(&rt->rt_mtu);
Index: net/route.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.h,v
diff -u -p -r1.211 route.h
--- net/route.h 3 Jan 2025 21:27:40 -0000 1.211
+++ net/route.h 13 Jan 2025 22:50:54 -0000
@@ -38,6 +38,7 @@
/*
* Locks used to protect struct members in this file:
* I immutable after creation
+ * a atomic operations
* N net lock
* X exclusive net lock, or shared net lock + kernel lock
* R art (rtable) lock
@@ -60,7 +61,7 @@
struct rt_kmetrics {
u_int64_t rmx_pksent; /* packets sent using this route */
int64_t rmx_expire; /* lifetime for route, e.g. redirect */
- u_int rmx_locks; /* Kernel must leave these values */
+ u_int rmx_locks; /* [a] Kernel must leave these values */
u_int rmx_mtu; /* [a] MTU for this path */
};
#endif
Index: net/rtsock.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtsock.c,v
diff -u -p -r1.377 rtsock.c
--- net/rtsock.c 9 Jan 2025 18:20:29 -0000 1.377
+++ net/rtsock.c 13 Jan 2025 22:50:54 -0000
@@ -1198,8 +1198,9 @@ change:
}
if_group_routechange(info->rti_info[RTAX_DST],
info->rti_info[RTAX_NETMASK]);
- rt->rt_locks &= ~(rtm->rtm_inits);
- rt->rt_locks |= (rtm->rtm_inits & rtm->rtm_rmx.rmx_locks);
+ atomic_clearbits_int(&rt->rt_locks, rtm->rtm_inits);
+ atomic_setbits_int(&rt->rt_locks,
+ rtm->rtm_inits & rtm->rtm_rmx.rmx_locks);
NET_UNLOCK();
break;
case RTM_GET:
@@ -1344,7 +1345,7 @@ route_cleargateway(struct rtentry *rt, v
struct rtentry *nhrt = arg;
if (ISSET(rt->rt_flags, RTF_GATEWAY) && rt->rt_gwroute == nhrt &&
- !ISSET(rt->rt_locks, RTV_MTU))
+ !ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU))
atomic_store_int(&rt->rt_mtu, 0);
return (0);
@@ -1420,7 +1421,7 @@ rtm_getmetrics(const struct rtentry *rt,
}
bzero(out, sizeof(*out));
- out->rmx_locks = in->rmx_locks;
+ out->rmx_locks = atomic_load_int(&in->rmx_locks);
out->rmx_mtu = atomic_load_int(&in->rmx_mtu);
out->rmx_expire = expire;
out->rmx_pksent = in->rmx_pksent;
Index: netinet/ip_icmp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.c,v
diff -u -p -r1.198 ip_icmp.c
--- netinet/ip_icmp.c 3 Jan 2025 21:27:40 -0000 1.198
+++ netinet/ip_icmp.c 13 Jan 2025 22:50:54 -0000
@@ -1054,9 +1054,9 @@ icmp_mtudisc(struct icmp *icp, u_int rta
* on a route. We should be using a separate flag
* for the kernel to indicate this.
*/
- if ((rt->rt_locks & RTV_MTU) == 0) {
+ if (!ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU)) {
if (mtu < 296 || mtu > ifp->if_mtu)
- rt->rt_locks |= RTV_MTU;
+ atomic_setbits_int(&rt->rt_locks, RTV_MTU);
else if (rtmtu > mtu || rtmtu == 0)
atomic_cas_uint(&rt->rt_mtu, rtmtu, mtu);
}
@@ -1090,7 +1090,7 @@ icmp_mtudisc_timeout(struct rtentry *rt,
(*ctlfunc)(PRC_MTUINC, sintosa(&sin),
rtableid, NULL);
} else {
- if ((rt->rt_locks & RTV_MTU) == 0)
+ if (!ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU))
atomic_store_int(&rt->rt_mtu, 0);
}
Index: netinet/ip_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
diff -u -p -r1.402 ip_output.c
--- netinet/ip_output.c 3 Jan 2025 21:27:40 -0000 1.402
+++ netinet/ip_output.c 13 Jan 2025 22:50:54 -0000
@@ -384,7 +384,7 @@ sendit:
* the route's MTU is locked.
*/
if ((flags & IP_MTUDISC) && ro && ro->ro_rt &&
- (ro->ro_rt->rt_locks & RTV_MTU) == 0)
+ (!ISSET(atomic_load_int(&ro->ro_rt->rt_locks), RTV_MTU)))
ip->ip_off |= htons(IP_DF);
#ifdef IPSEC
@@ -471,7 +471,7 @@ sendit:
*/
if (rtisvalid(ro->ro_rt) &&
ISSET(ro->ro_rt->rt_flags, RTF_HOST) &&
- !(ro->ro_rt->rt_locks & RTV_MTU)) {
+ !ISSET(atomic_load_int(&ro->ro_rt->rt_locks), RTV_MTU)) {
u_int rtmtu;
rtmtu = atomic_load_int(&ro->ro_rt->rt_mtu);
Index: netinet/tcp_subr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_subr.c,v
diff -u -p -r1.204 tcp_subr.c
--- netinet/tcp_subr.c 3 Jan 2025 17:23:51 -0000 1.204
+++ netinet/tcp_subr.c 13 Jan 2025 22:50:54 -0000
@@ -868,7 +868,8 @@ tcp_mtudisc(struct inpcb *inp, int errno
rt = in_pcbrtentry(inp);
if (rt != NULL) {
- unsigned int orig_mtulock = (rt->rt_locks & RTV_MTU);
+ unsigned int orig_mtulock =
+ (atomic_load_int(&rt->rt_locks) & RTV_MTU);
/*
* If this was not a host route, remove and realloc.
@@ -878,7 +879,7 @@ tcp_mtudisc(struct inpcb *inp, int errno
if ((rt = in_pcbrtentry(inp)) == NULL)
return;
}
- if (orig_mtulock < (rt->rt_locks & RTV_MTU))
+ if (orig_mtulock < (atomic_load_int(&rt->rt_locks) & RTV_MTU))
change = 1;
}
tcp_mss(tp, -1);
Index: netinet/tcp_timer.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_timer.c,v
diff -u -p -r1.80 tcp_timer.c
--- netinet/tcp_timer.c 5 Jan 2025 12:18:48 -0000 1.80
+++ netinet/tcp_timer.c 13 Jan 2025 22:50:54 -0000
@@ -282,7 +282,7 @@ tcp_timer_rexmt(void *arg)
rt = in_pcbrtentry(inp);
/* Check if path MTU discovery is disabled already */
if (rt && (rt->rt_flags & RTF_HOST) &&
- (rt->rt_locks & RTV_MTU))
+ ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU))
goto leave;
rt = NULL;
@@ -303,8 +303,8 @@ tcp_timer_rexmt(void *arg)
}
if (rt != NULL) {
/* Disable path MTU discovery */
- if ((rt->rt_locks & RTV_MTU) == 0) {
- rt->rt_locks |= RTV_MTU;
+ if (!ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU)) {
+ atomic_setbits_int(&rt->rt_locks, RTV_MTU);
in_rtchange(inp, 0);
}
Index: netinet6/icmp6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
diff -u -p -r1.256 icmp6.c
--- netinet6/icmp6.c 3 Jan 2025 21:27:40 -0000 1.256
+++ netinet6/icmp6.c 13 Jan 2025 22:50:54 -0000
@@ -1016,7 +1016,7 @@ icmp6_mtudisc_update(struct ip6ctlparam
rt = icmp6_mtudisc_clone(&sin6, m->m_pkthdr.ph_rtableid, 0);
if (rt != NULL && ISSET(rt->rt_flags, RTF_HOST) &&
- !(rt->rt_locks & RTV_MTU)) {
+ !ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU)) {
u_int rtmtu;
rtmtu = atomic_load_int(&rt->rt_mtu);
@@ -1851,7 +1851,7 @@ icmp6_mtudisc_timeout(struct rtentry *rt
if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
rtdeletemsg(rt, ifp, rtableid);
} else {
- if (!(rt->rt_locks & RTV_MTU))
+ if (!ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU))
atomic_store_int(&rt->rt_mtu, 0);
}
Index: netinet6/ip6_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_output.c,v
diff -u -p -r1.294 ip6_output.c
--- netinet6/ip6_output.c 3 Jan 2025 21:27:40 -0000 1.294
+++ netinet6/ip6_output.c 13 Jan 2025 22:50:54 -0000
@@ -1047,7 +1047,7 @@ ip6_getpmtu(struct rtentry *rt, struct i
* field isn't locked).
*/
mtu = ifp->if_mtu;
- if (!(rt->rt_locks & RTV_MTU))
+ if (!ISSET(atomic_load_int(&rt->rt_locks), RTV_MTU))
atomic_cas_uint(&rt->rt_mtu, rtmtu, mtu);
}
} else {
make route rt_locks atomic