From: kn <kn@openbsd.org>
Date: Wed, 3 May 2023 10:32:47 +0000 (+0000)
Subject: Remove net lock from DIOCGETRULESET and DIOCGETRULESETS
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=3e519755f71129251aeb31dcbfd658dd1775863b;p=openbsd

Remove net lock from DIOCGETRULESET and DIOCGETRULESETS

Both walk the list of rulesets aka. anchors, to yield a total count and
specific anchor name, respectively.  Same access, different copy out.

pf_anchor_global are contained within pf_ioctl.c and pf_ruleset.c and
fully protected by the pf lock, as is pf_main_ruleset and its pf.c usage.

Rely on and assert for pf lock alone.  'pfctl -sr' on 60k unique rules gets
noticably faster, around 2.1s instead of 3.5s.

OK sashan
---

diff --git a/sys/net/pf.c b/sys/net/pf.c
index 8af5155866a..56739a2e8fc 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.1174 2023/04/28 14:08:34 phessler Exp $ */
+/*	$OpenBSD: pf.c,v 1.1175 2023/05/03 10:32:47 kn Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1370,6 +1370,8 @@ pf_state_import(const struct pfsync_state *sp, int flags)
 	int error = ENOMEM;
 	int n = 0;
 
+	PF_ASSERT_LOCKED();
+
 	if (sp->creatorid == 0) {
 		DPFPRINTF(LOG_NOTICE, "%s: invalid creator id: %08x", __func__,
 		    ntohl(sp->creatorid));
@@ -4270,6 +4272,8 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm,
 	struct pf_test_ctx	 ctx;
 	int			 rv;
 
+	PF_ASSERT_LOCKED();
+
 	memset(&ctx, 0, sizeof(ctx));
 	ctx.pd = pd;
 	ctx.rm = rm;
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 61a1660766d..14c377d5ef6 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.402 2023/04/29 10:25:32 kn Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.403 2023/05/03 10:32:48 kn Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -858,6 +858,8 @@ pf_commit_rules(u_int32_t version, char *anchor)
 	struct pf_rulequeue	*old_rules;
 	u_int32_t		 old_rcount;
 
+	PF_ASSERT_LOCKED();
+
 	rs = pf_find_ruleset(anchor);
 	if (rs == NULL || !rs->rules.inactive.open ||
 	    version != rs->rules.inactive.version)
@@ -2151,13 +2153,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_ruleset	*ruleset;
 		struct pf_anchor	*anchor;
 
-		NET_LOCK();
 		PF_LOCK();
 		pr->path[sizeof(pr->path) - 1] = '\0';
 		if ((ruleset = pf_find_ruleset(pr->path)) == NULL) {
 			error = EINVAL;
 			PF_UNLOCK();
-			NET_UNLOCK();
 			goto fail;
 		}
 		pr->nr = 0;
@@ -2172,7 +2172,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				pr->nr++;
 		}
 		PF_UNLOCK();
-		NET_UNLOCK();
 		break;
 	}
 
@@ -2182,13 +2181,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		struct pf_anchor	*anchor;
 		u_int32_t		 nr = 0;
 
-		NET_LOCK();
 		PF_LOCK();
 		pr->path[sizeof(pr->path) - 1] = '\0';
 		if ((ruleset = pf_find_ruleset(pr->path)) == NULL) {
 			error = EINVAL;
 			PF_UNLOCK();
-			NET_UNLOCK();
 			goto fail;
 		}
 		pr->name[0] = '\0';
@@ -2210,7 +2207,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				}
 		}
 		PF_UNLOCK();
-		NET_UNLOCK();
 		if (!pr->name[0])
 			error = EBUSY;
 		break;