Hi,

I've noticed a few issues with pfctl that are caused by this change:

https://github.com/openbsd/src/commit/85baac77515140239632c5e733ba5c896915fadc

'pfctl -F Reset' no longer resets limits back to their default because they are
initialized with the currently active limit in pfctl_init_options now.

Because of that initialization limits are also "sticky" now. If you load a
configuration that changes a limit and then load another configuration that does
not specify that limit the old limit will be kept. This can lead to a
configuration suddenly breaking after a reboot when the old limit is gone.

If we want to set limits as soon as we parse them we should use pfctl_load_limit
instead of pfctl_load_options in pfctl_set_limit. This avoids side effects from
other limits that may or may not yet have been parsed at that point. It also
allows us to initialize options with their default again in pfctl_init_options.
Unless I am missing something that should fix both issues I've mentioned while
not causing a regression for the issue that the original change addressed.

diff --git sbin/pfctl/pfctl.c sbin/pfctl/pfctl.c
index be1b4bf497c..92cb653cd56 100644
--- sbin/pfctl/pfctl.c
+++ sbin/pfctl/pfctl.c
@@ -2210,30 +2210,18 @@ pfctl_init_options(struct pfctl *pf)
 	pf->syncookieswat[0] = PF_SYNCOOKIES_LOWATPCT;
 	pf->syncookieswat[1] = PF_SYNCOOKIES_HIWATPCT;
 
-	/*
-	 * limit_curr is populated by pfctl_read_limits() after main() opens
-	 * /dev/pf.
-	 */
 	mib[0] = CTL_KERN;
 	mib[1] = KERN_MAXCLUSTERS;
 	size = sizeof(mcl);
 	if (sysctl(mib, 2, &mcl, &size, NULL, 0) == -1)
 		err(1, "sysctl");
-	pf->limit[PF_LIMIT_FRAGS] = (limit_curr[PF_LIMIT_FRAGS] == 0) ?
-	    mcl / 4 : limit_curr[PF_LIMIT_FRAGS];
-
-	pf->limit[PF_LIMIT_SRC_NODES] = (limit_curr[PF_LIMIT_SRC_NODES] == 0) ?
-	    PFSNODE_HIWAT : limit_curr[PF_LIMIT_SRC_NODES];
-	pf->limit[PF_LIMIT_TABLES] = (limit_curr[PF_LIMIT_TABLES] == 0) ?
-	    PFR_KTABLE_HIWAT : limit_curr[PF_LIMIT_TABLES];
-	pf->limit[PF_LIMIT_TABLE_ENTRIES] =
-	    (limit_curr[PF_LIMIT_TABLE_ENTRIES] == 0) ?
-		PFR_KENTRY_HIWAT : limit_curr[PF_LIMIT_TABLE_ENTRIES];
-	pf->limit[PF_LIMIT_PKTDELAY_PKTS] =
-	    (limit_curr[PF_LIMIT_PKTDELAY_PKTS] == 0) ?
-		PF_PKTDELAY_MAXPKTS : limit_curr[PF_LIMIT_PKTDELAY_PKTS];
-	pf->limit[PF_LIMIT_ANCHORS] = (limit_curr[PF_LIMIT_ANCHORS] == 0) ?
-	    PF_ANCHOR_HIWAT : limit_curr[PF_LIMIT_ANCHORS];
+	pf->limit[PF_LIMIT_FRAGS] = mcl / 4;
+
+	pf->limit[PF_LIMIT_SRC_NODES] = PFSNODE_HIWAT;
+	pf->limit[PF_LIMIT_TABLES] = PFR_KTABLE_HIWAT;
+	pf->limit[PF_LIMIT_TABLE_ENTRIES] = PFR_KENTRY_HIWAT;
+	pf->limit[PF_LIMIT_PKTDELAY_PKTS] = PF_PKTDELAY_MAXPKTS;
+	pf->limit[PF_LIMIT_ANCHORS] = PF_ANCHOR_HIWAT;
 
 	mib[0] = CTL_HW;
 	mib[1] = HW_PHYSMEM64;
@@ -2339,7 +2327,7 @@ pfctl_set_limit(struct pfctl *pf, const char *opt, unsigned int limit)
 		printf("set limit %s %d\n", opt, limit);
 
 	if ((pf->opts & PF_OPT_NOACTION) == 0)
-		pfctl_load_options(pf);
+		pfctl_load_limit(pf, pf_limits[i].index, limit);
 
 	return (0);
 }

• smime.p7s (5K)