Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Prepare bpf_sysctl() for net_sysctl() unlocking
To:
tech@openbsd.org
Date:
Tue, 6 Aug 2024 15:30:01 +0300

Download raw body.

Thread
bluhm@ reworked sysctl_int_bounded() to deal with atomically accessed
integer variables. bpf(4) sysctl has two such variables, NET_BPF_BUFSIZE
and NET_BPF_MAXBUFSIZE respectively, so rework bpf_sysctl() to be ready
for upcoming net_sysctl() unlocking.

Please note, bpf_movein() obviously require to store `bpf_maxbufsize'
to local variable, but bpf_sysctl() accesses it only once, so
atomic_load_int() is unnecessary here. This is also true for
`bpf_bufsize' which is accessed only once in bpfopen().

ok?

Index: sys/net/bpf.c
===================================================================
RCS file: /cvs/src/sys/net/bpf.c,v
diff -u -p -r1.223 bpf.c
--- sys/net/bpf.c	5 Aug 2024 23:56:10 -0000	1.223
+++ sys/net/bpf.c	6 Aug 2024 12:02:40 -0000
@@ -117,8 +117,6 @@ int	filt_bpfread(struct knote *, long);
 int	filt_bpfreadmodify(struct kevent *, struct knote *);
 int	filt_bpfreadprocess(struct knote *, struct kevent *);
 
-int	bpf_sysctl_locked(int *, u_int, void *, size_t *, void *, size_t);
-
 struct bpf_d *bpfilter_lookup(int);
 
 /*
@@ -137,9 +135,6 @@ void	bpf_d_smr(void *);
 void	bpf_get(struct bpf_d *);
 void	bpf_put(struct bpf_d *);
 
-
-struct rwlock bpf_sysctl_lk = RWLOCK_INITIALIZER("bpfsz");
-
 int
 bpf_movein(struct uio *uio, struct bpf_d *d, struct mbuf **mp,
     struct sockaddr *sockp)
@@ -853,9 +848,11 @@ bpfioctl(dev_t dev, u_long cmd, caddr_t 
 			error = EINVAL;
 		else {
 			u_int size = *(u_int *)addr;
+			int bpf_maxbufsize_local =
+			    atomic_load_int(&bpf_maxbufsize);
 
-			if (size > bpf_maxbufsize)
-				*(u_int *)addr = size = bpf_maxbufsize;
+			if (size > bpf_maxbufsize_local)
+				*(u_int *)addr = size = bpf_maxbufsize_local;
 			else if (size < BPF_MINBUFSIZE)
 				*(u_int *)addr = size = BPF_MINBUFSIZE;
 			mtx_enter(&d->bd_mtx);
@@ -1815,9 +1812,12 @@ bpfsdetach(void *p)
 }
 
 int
-bpf_sysctl_locked(int *name, u_int namelen, void *oldp, size_t *oldlenp,
-    void *newp, size_t newlen)
+bpf_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
+    size_t newlen)
 {
+	if (namelen != 1)
+		return (ENOTDIR);
+
 	switch (name[0]) {
 	case NET_BPF_BUFSIZE:
 		return sysctl_int_bounded(oldp, oldlenp, newp, newlen,
@@ -1828,29 +1828,8 @@ bpf_sysctl_locked(int *name, u_int namel
 	default:
 		return (EOPNOTSUPP);
 	}
-}
 
-int
-bpf_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
-    size_t newlen)
-{
-	int flags = RW_INTR;
-	int error;
-
-	if (namelen != 1)
-		return (ENOTDIR);
-
-	flags |= (newp == NULL) ? RW_READ : RW_WRITE;
-
-	error = rw_enter(&bpf_sysctl_lk, flags);
-	if (error != 0)
-		return (error);
-
-	error = bpf_sysctl_locked(name, namelen, oldp, oldlenp, newp, newlen);
-
-	rw_exit(&bpf_sysctl_lk);
-
-	return (error);
+	/* NOTREACHED */
 }
 
 struct bpf_d *