Re: PR 291981: authpf under netlink, and a pfctl adoption flag

From: Avinash H. Duduskar <[email protected]>

> The way to fix this is to just call libpfctl from change_filter() [...]
> if libpfctl is missing functions I'll happily add them.

Glad to drop the flag and go the libpfctl way.  But I want to be sure I
have read the scope right before I start, because I audited it and it
came out a lot bigger than a missing function.

The table, the addresses and the state kill authpf already does through
libpfctl.  What still forks is the ruleset load, and that part is the
pf.conf parser: change_filter hands pfctl a file with macros, and
turning that text into rules is parse.y, which is not in libpfctl.
parse.y is about 8,400 lines of OpenBSD-derived yacc.  I mapped every
point on the load path where it would kill a library caller: 154
err()/exit() sites.  113 of the 154 are allocation failures OpenBSD
also leaves as err(); keep those and the real conversion is the other
41, the ones that fire on a bad ruleset rather than a failed malloc.
Convert the 113 as well and it is all 154, and a standing divergence
from the OpenBSD tree you resync.

Here is one of the 154, and the shape of all of them, from port_item:

	$$ = calloc(1, sizeof(struct node_port));
	if ($$ == NULL)
		err(1, "port_item: calloc");	/* exits the caller */
	...
	if (validate_range(...)) {
		yyerror("invalid port range");
		YYERROR;			/* returns instead */
	}

The return path is already in the grammar: the err() lines just do not
take the YYERROR the range check below already uses.  Routing each one
through it, and cleaning up the parser state each YYERROR leaves behind,
is the conversion.

End to end it is several pieces, not a patch: lifting the load path out
of main()'s translation unit, the exit-to-return conversion, a teardown
clean enough for a long-lived process to parse once, the change_filter()
rewrite, and authpf's first tests to catch a regression in any of it,
since it has none today.  A multi-PR series against code you track
upstream, not a weekend's work.

So before I pick up a keyboard: is that really the scope you have in
mind, or did you have a different design for how authpf gets its rules
in without the fork?  You know parse.y far better than I do, so I would
rather understand the design you are picturing than guess at one.

Best,
Avinash