Download raw body.
Error when at startup more than 512 anchors are loaded from pf.conf
Error when at startup more than 512 anchors are loaded from pf.conf
Error when at startup more than 512 anchors are loaded from pf.conf
Hello,
In this order, with your diff it will get loaded. But if you will have:
----8<-------8<-------8<-------8<-----------8<----
set limit states 700000
set limit src-nodes 500000
set limit tables 2000
set limit anchors 2000
anchor test_0
anchor test_1
anchor test_2
----8<-------8<-------8<-------8<-----------8<----
you will have three errors for three limits set before anchors.
Regarding strange state when rule load is interrupted... I've do some tests
to narrow this issue down. And I do not confirm your hypotesis. Error
is triggered in "PF_TRANS_RULESET" branch in second "for" loop where
"no errors should happen". I've figured out that error is returing function
pf_commit_rules(). Then I've narrowed down it a bit further and found out
that the error is proxied from function pf_commit_queues() that is called at the
verry end of pf_commit_rules(). Then I've found out that error is actualy
triggered from pf_create_queues(). And it looks like EIVAL comes from
error = qif->pfqops->pfq_addqueue(qif->disc, q);
It looks like it is verry low level error.
I also found out that this strange state is triggered when rules are loaded
but there were no commit triggered. I the configuration I have verry few queues.
Only three actualy.
queue dl_XXXXX_limit on $if_XXXXX_ipsec bandwidth "120Mb"
queue XXXXX_limit parent dl_XXXXX_limit bandwidth "50Mb"
queue dl_XXXXXX_limit_default parent dl_XXXXX_limit bandwidth "40Mb" default
pass in quick on $if_XXXXX_ipsec proto tcp from { X.X.X.X Y.Y.Y.Y } to Z.Z.Z.Z port { AAAA } rdr-to X.X.X.X port AAAA queue XXXXX_limit
----- Original Message -----
From: "Alexandr Nedvedicky" <sashan@fastmail.net>
To: "Rafał Ramocki" <rafal.ramocki@eo.pl>
Cc: "tech" <tech@openbsd.org>
Sent: Tuesday, April 29, 2025 4:01:16 PM
Subject: Re: Error when at startup more than 512 anchors are loaded from pf.conf
Hello,
I did have some more time to re-test the diff.
this is my testing rule-set I'm using:
----8<-------8<-------8<-------8<-----------8<----
set limit anchors 2048
set limit states 60000
anchor test_0
anchor test_1
anchor test_2
....
anchor test_1023
----8<-------8<-------8<-------8<-----------8<----
I did verify it gets loaded.
src# ./pfctl -sr |wc -l
1024
Also using pfctl -sm confirms the limit for anchors got changed:
src# ./pfctl -sm
states hard limit 60000
src-nodes hard limit 10000
frags hard limit 65536
tables hard limit 1000
table-entries hard limit 200000
pktdelay-pkts hard limit 10000
anchors hard limit 2048
So the fix seems to work when running without '-n'. I agree diff does
not work when '-n' is used, more work is needed. I forgot to test my
change with -n option. I will try to get to it over the weekend.
> I think I've also hit another error. After execution this way:
> "pfctl -vv -f /etc/pf.conf | less" and quitting before end of the
> output I've got DIOCXCOMMIT error for any further loads:
>
> # pfctl -f /etc/pf.conf
> pfctl: Current pool size exceeds requested anchors limit 512
> pfctl: Current pool size exceeds requested anchors limit 512
> pfctl: Current pool size exceeds requested anchors limit 512
> pfctl: DIOCXCOMMIT: Invalid argument
I could not reproduce it yet. If I understand code right, then this error
must coming from default branch where in switch:
switch (ioe->type) {
case PF_TRANS_TABLE:
memset(table, 0, sizeof(*table));
strlcpy(table->pfrt_anchor, ioe->anchor,
sizeof(table->pfrt_anchor));
if ((error = pfr_ina_commit(table, ioe->ticket,
NULL, NULL, 0))) {
PF_UNLOCK();
NET_UNLOCK();
free(table, M_PF, sizeof(*table));
free(ioe, M_PF, sizeof(*ioe));
goto fail; /* really bad */
}
break;
case PF_TRANS_RULESET:
if ((error = pf_commit_rules(ioe->ticket,
ioe->anchor))) {
PF_UNLOCK();
NET_UNLOCK();
free(table, M_PF, sizeof(*table));
free(ioe, M_PF, sizeof(*ioe));
goto fail; /* really bad */
}
break;
default:
PF_UNLOCK();
NET_UNLOCK();
free(table, M_PF, sizeof(*table));
free(ioe, M_PF, sizeof(*ioe));
error = EINVAL;
goto fail; /* really bad */
The EINVAL here indicates there is some binary incompatibility
between pfctl and pf. And you should be hitting it always
regardless hitting CTRL+C when loading the rules or not.
I would expect to see EBUSY error which is eventually
returned before we arrive to the switch statement shown
above. The EBUSY indicates there is mismatch between transaction
id. This may happen when two processes attempt to change the
same ruleset and it should be recoverable (temporal).
thanks and
regards
sashan
On Mon, Apr 28, 2025 at 07:13:59PM +0200, Rafa?? Ramocki wrote:
> Hello,
>
> I've rebuilded kernel and pfctl with that change have proposed.
> It when rules were loaded it popped up three warnings as bellow:
>
> # pfctl -f /etc/pf.conf
> pfctl: Current pool size exceeds requested anchors limit 512
> pfctl: Current pool size exceeds requested anchors limit 512
> pfctl: Current pool size exceeds requested anchors limit 512
>
> I've gathered some more information and it looks like pfctl_set_limit
> is actually caled four times in my case. One time for every
> "set limit" in pfctl.conf. And since set limit anchors was the forth limit
> only forth execution passed without this error. When I rearanged
> order of limits placing limit of anchors before all other limits loading
> rules was possible without this message of exceeded anchors.
>
> I think I've hit another problem with that change too.
> Adding "-n" to pfctl gives error messages too:
>
> # pfctl -n -f /etc/pf.conf
> pfctl: Cannot set states limit to 700000
> pfctl: Cannot set src-nodes limit to 10000
> pfctl: Cannot set frags limit to 65536
> pfctl: Cannot set tables limit to 1000
> pfctl: Cannot set table-entries limit to 200000
> pfctl: Cannot set pktdelay-pkts limit to 10000
> pfctl: Cannot set anchors limit to 512
> pfctl: DIOCSETTIMEOUT
> (...cut...)
> pfctl: DIOCSETTIMEOUT
> pfctl: DIOCSETDEBUG
> pfctl: DIOCSETSTATUSIF
>
> (and this is was displayed three times). Following version is preventing
> this:
>
>
> diff -u -p -r1.390 pfctl.c
> --- pfctl.c 6 Jan 2023 17:44:33 -0000 1.390
> +++ pfctl.c 28 Apr 2025 17:11:57 -0000
> @@ -1843,6 +1843,9 @@ pfctl_set_limit(struct pfctl *pf, const
> if (pf->opts & PF_OPT_VERBOSE)
> printf("set limit %s %d\n", opt, limit);
>
> + if ((pf->opts & PF_OPT_NOACTION) == 0)
> + pfctl_load_options(pf);
> +
> return (0);
> }
>
>
> I think I've also hit another error. After execution this way:
> "pfctl -vv -f /etc/pf.conf | less" and quitting before end of the
> output I've got DIOCXCOMMIT error for any further loads:
>
> # pfctl -f /etc/pf.conf
> pfctl: Current pool size exceeds requested anchors limit 512
> pfctl: Current pool size exceeds requested anchors limit 512
> pfctl: Current pool size exceeds requested anchors limit 512
> pfctl: DIOCXCOMMIT: Invalid argument
>
> After no changes in pf.conf and reboot there is no DIOCXCOMMIT error
> any more. It looks like interruption of transaction prevents for any
> future transactions.
>
>
>
> ----- Original Message -----
> From: "Alexandr Nedvedicky" <sashan@fastmail.net>
> To: "Rafa?? Ramocki" <rafal.ramocki@eo.pl>
> Cc: "tech" <tech@openbsd.org>
> Sent: Saturday, April 26, 2025 8:28:10 PM
> Subject: Re: Error when at startup more than 512 anchors are loaded from pf.conf
>
> 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];
Error when at startup more than 512 anchors are loaded from pf.conf
Error when at startup more than 512 anchors are loaded from pf.conf
Error when at startup more than 512 anchors are loaded from pf.conf