Download raw body.
udp, sysctl: unlock UDPCTL_ROOTONLY and UDPCTL_BADDYNAMIC
sysctl(2) is not the hot path, but I want to remove kernel lock with
vslock() and relax netlock pressure here.
Use temporary buffer to exchange data with the userland. So we need to
take exclusive netlock only if we modify values. The shared netlock is
still required even if we read values, but it doesn't stop packets
processing in the most paths.
I have more complicated version of this diff which introduces the mutex
for `baddynamicports' protection, so the netlock could be avoided here.
However this requires to take this mutex in the in_pcbbind() path and I
see no reason to do this now. sysctl(2) is not the hot path, the most
common read-only case doesn't stop the netstack and doesn't take kernel
lock, I'm fine with it. I like to use producer/consumer locking here
instead of mutexes.
The TCP related `rootonlyports' and `baddynamicports' could be
refactored in the same way.
Index: sys/netinet/udp_usrreq.c
===================================================================
RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.336
diff -u -p -r1.336 udp_usrreq.c
--- sys/netinet/udp_usrreq.c 11 Mar 2025 15:31:03 -0000 1.336
+++ sys/netinet/udp_usrreq.c 10 May 2025 02:23:08 -0000
@@ -174,7 +174,6 @@ void udp_sbappend(struct inpcb *, struct
u_int32_t, struct netstack *);
int udp_output(struct inpcb *, struct mbuf *, struct mbuf *, struct mbuf *);
void udp_notify(struct inpcb *, int);
-int udp_sysctl_locked(int *, u_int, void *, size_t *, void *, size_t);
int udp_sysctl_udpstat(void *, size_t *, void *);
#ifndef UDB_INITIAL_HASH_SIZE
@@ -1278,16 +1277,37 @@ udp_sysctl(int *name, u_int namelen, voi
return (ENOTDIR);
switch (name[0]) {
- case UDPCTL_BADDYNAMIC:
- case UDPCTL_ROOTONLY: {
- size_t savelen = *oldlenp;
+ case UDPCTL_ROOTONLY:
+ if (newp && (int)atomic_load_int(&securelevel) > 0)
+ return (EPERM);
+ /* FALLTHROUGH */
+ case UDPCTL_BADDYNAMIC: {
+ struct baddynamicports *ports = (name[0] == UDPCTL_ROOTONLY ?
+ &rootonlyports : &baddynamicports);
+ const size_t bufitems = DP_MAPSIZE;
+ const size_t buflen = bufitems * sizeof(u_int32_t);
+ size_t i;
+ uint32_t *buf;
int error;
- if ((error = sysctl_vslock(oldp, savelen)))
- return (error);
- error = udp_sysctl_locked(name, namelen, oldp, oldlenp,
- newp, newlen);
- sysctl_vsunlock(oldp, savelen);
+ buf = malloc(buflen, M_SYSCTL, M_WAITOK | M_ZERO);
+
+ NET_LOCK_SHARED();
+ for (i = 0; i < bufitems; ++i)
+ buf[i] = ports->udp[i];
+ NET_UNLOCK_SHARED();
+
+ error = sysctl_struct(oldp, oldlenp, newp, newlen,
+ buf, buflen);
+
+ if ((error == 0) && newp) {
+ NET_LOCK();
+ for (i = 0; i < bufitems; ++i)
+ ports->udp[i] = buf[i];
+ NET_UNLOCK();
+ }
+
+ free(buf, M_SYSCTL, buflen);
return (error);
}
@@ -1302,33 +1322,6 @@ udp_sysctl(int *name, u_int namelen, voi
name, namelen, oldp, oldlenp, newp, newlen));
}
/* NOTREACHED */
-}
-
-int
-udp_sysctl_locked(int *name, u_int namelen, void *oldp, size_t *oldlenp,
- void *newp, size_t newlen)
-{
- int error = ENOPROTOOPT;
-
- switch (name[0]) {
- case UDPCTL_BADDYNAMIC:
- NET_LOCK();
- error = sysctl_struct(oldp, oldlenp, newp, newlen,
- baddynamicports.udp, sizeof(baddynamicports.udp));
- NET_UNLOCK();
- break;
-
- case UDPCTL_ROOTONLY:
- if (newp && securelevel > 0)
- return (EPERM);
- NET_LOCK();
- error = sysctl_struct(oldp, oldlenp, newp, newlen,
- rootonlyports.udp, sizeof(rootonlyports.udp));
- NET_UNLOCK();
- break;
- }
-
- return (error);
}
int
udp, sysctl: unlock UDPCTL_ROOTONLY and UDPCTL_BADDYNAMIC