From: Alexandr Nedvedicky <sashan@fastmail.net>
Subject: Re: Error when at startup more than 512 anchors are loaded from pf.conf
To: Rafa?? Ramocki <rafal.ramocki@eo.pl>
Cc: tech <tech@openbsd.org>
Date: Sat, 26 Apr 2025 20:28:10 +0200

Hello,

I'm sorry I've missed your first email on Apr 17th.

Below is a diff which should solve the issue for you. It also
illustrates the problem which you already understand very well.

Indeed the new limit value is set after all rules are loaded
to transaction and pf(4) is doing a commit. So it is futile
to attempt increase the limit pf.conf(5) which attempts to
load the rules/anchors where achnor limit is being exceeded.

to workaround it you need to do it in two steps now:
    use file (pf-limits.conf) and load it first (pfctl -f...)
    and then load the file rules (pf-rules.conf)

diff below changes the DIOCSETLIMIT ioctl to set the limit
immediately when ioctl is being handled. We still need to
enable pfctl(8) parser to issue the ioctl as soon as rule
is accepted. The current one-liner tweak is a small-hack
to verify the idea.

The not so intended effect of this change is that if
pfctl(8) parser fails to load/accept all rules then
the limits changed by command are left behind.

The change which will also revert the limit settings
if something will go wrong with transaction will require
some more thought.

I think it's worth to consider the proposed change
which leaves changed limits behind if error happens.
I still need to make pfctl side more tidy if there will
be a general agreement on this.

To be honest I can not think of any scenario where changing
the limits like in presented diff can be problematic.

thanks and
regards
sashan

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 825bd4cf04c..816d7644b97 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1878,6 +1878,8 @@ pfctl_set_limit(struct pfctl *pf, const char *opt, unsigned int limit)
 	if (pf->opts & PF_OPT_VERBOSE)
 		printf("set limit %s %d\n", opt, limit);
 
+	pfctl_load_options(pf);
+
 	return (0);
 }
 
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index e7591ab4d40..16f8719386b 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -2140,8 +2140,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			goto fail;
 		}
 
-		pf_pool_limits[pl->index].limit_new = pl->limit;
-		pl->limit = pf_pool_limits[pl->index].limit;
+		error = pool_sethardlimit(pf_pool_limits[pl->index].pp,
+		    pl->limit, NULL, 0);
+		if (error == 0) {
+			pf_pool_limits[pl->index].limit_new = pl->limit;
+			pf_pool_limits[pl->index].limit = pl->limit;
+		}
 		PF_UNLOCK();
 		break;
 	}
@@ -2702,21 +2706,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		NET_LOCK();
 		PF_LOCK();
 
-		/*
-		 * Checked already in DIOCSETLIMIT, but check again as the
-		 * situation might have changed.
-		 */
-		for (i = 0; i < PF_LIMIT_MAX; i++) {
-			if (((struct pool *)pf_pool_limits[i].pp)->pr_nout >
-			    pf_pool_limits[i].limit_new) {
-				PF_UNLOCK();
-				NET_UNLOCK();
-				free(table, M_PF, sizeof(*table));
-				free(ioe, M_PF, sizeof(*ioe));
-				error = EBUSY;
-				goto fail;
-			}
-		}
 		/* now do the commit - no errors should happen here */
 		for (i = 0; i < io->size; i++) {
 			PF_UNLOCK();
@@ -2769,20 +2758,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 				goto fail; /* really bad */
 			}
 		}
-		for (i = 0; i < PF_LIMIT_MAX; i++) {
-			if (pf_pool_limits[i].limit_new !=
-			    pf_pool_limits[i].limit &&
-			    pool_sethardlimit(pf_pool_limits[i].pp,
-			    pf_pool_limits[i].limit_new, NULL, 0) != 0) {
-				PF_UNLOCK();
-				NET_UNLOCK();
-				free(table, M_PF, sizeof(*table));
-				free(ioe, M_PF, sizeof(*ioe));
-				error = EBUSY;
-				goto fail; /* really bad */
-			}
-			pf_pool_limits[i].limit = pf_pool_limits[i].limit_new;
-		}
 		for (i = 0; i < PFTM_MAX; i++) {
 			int old = pf_default_rule.timeout[i];