On Fri, 15 Apr 2022 22:00:35 +0100
Stuart Henderson <[email protected]> wrote:
> jturner had a problem with this, here's a diff on top of what was
> committed.
>
> - I think the first line is superfluous as /etc/rc.d/rc.subr has some
> special case for this (and I think this maybe responsible for a
> problem which jturner ran into)
You're right, the same spamlogd start conditions are checked in both
_rc_quirks() in rc.d/rc.subr and rc_pre() in rc.d/spamlogd.
However, isn't it better to have this spamlogd quirk in rc.d/spamlogd
and remove it from rc.d/rc.subr?
The _rc_quirks() function (including the spamlogd pre-start checks) was
added to rc.d/rc.subr,v1.73 (2014). I'm guessing it was to fix spamlogd
because it was broken since the error introduced in rc.d/spamlogd,v1.2
(2011), where the start conditions were no longer checked properly
in rc_pre(). I think the fix went in the wrong file back then, unless
I'm mistaken as to how the rc system functions.
>
> - look for the pflog interface passed in flags and init that
This is an improvement.
In fact, this change could be applied to rc.d/pflogd too. For example,
if pflog1 is specified in pflogd_flags, it will not be created via
rc_pre(), which only creates pflog0 (see -i in pflogd(8)).
Again, if my thinking is correct, it seems the "special care" for
pflogd startup in _rc_quirks() (i.e. check if pf is enabled) should be
moved to rc_pre() in rc.d/pflogd. Correct?
> I think this is probably correct within the bounds of how spamlogd
> currently works but some tests would be appreciated (as would
> any improvements to the chicken scratches)
>
> Index: spamlogd
> ===================================================================
> RCS file: /cvs/src/etc/rc.d/spamlogd,v
> retrieving revision 1.5
> diff -u -p -r1.5 spamlogd
> --- spamlogd 11 Apr 2022 09:32:20 -0000 1.5
> +++ spamlogd 15 Apr 2022 21:00:10 -0000
> @@ -9,11 +9,13 @@ daemon="/usr/libexec/spamlogd"
> rc_reload=NO
>
> rc_pre() {
> - [[ ${spamd_flags} != NO && ${spamd_black} == NO ]] && return
> 1
> + pflog=$(echo $daemon_flags | sed -En 's/.*-l
> *(pflog[0-9]+).*/\1/p')
> + pflog=${pflog:-pflog0}
> +
> if pfctl -si | grep -q Enabled; then
> - ifconfig pflog0 create
> - if ifconfig pflog0; then
> - ifconfig pflog0 up
> + ifconfig $pflog create
> + if ifconfig $pflog; then
> + ifconfig $pflog up
> else
> return 1
> fi
>
