Re: CVS commit: src/etc/rc.d

[Stephen Borrill <sborrill%NetBSD.org@localhost>]

On 26/11/2021 17:52, Robert Elz wrote:
>      Date:        Fri, 26 Nov 2021 13:11:36 +0000
>      From:        "Stephen Borrill" <sborrill%netbsd.org@localhost>
>      Message-ID:  <20211126131136.63FABFAEC%cvs.NetBSD.org@localhost>
>
>    | Load rc configuration based on rcvar, not name, so that correct settings
>    | in /etc/rc.conf.d are loaded.
>
> This looks wrong to me (and a pullup request so soon after making a
> change, before it has had any time for testing in HEAD is a *really*
> bad idea).
>
>    | Usually this does not matter as rcvar and name are set to the same value.
>    | For pf_boot and npf_boot, rcvar is set to pf and npf respectively.
>    |
>    | Prior to the change, if:
>    | rc.conf contains npf=YES
>    | rc.conf.d/npf does not exist
>
> Nor should it, that's not the file that is supposed to be used.
>
> In rc.conf(5):
>
>
>      rc.d(8) scripts that use load_rc_config from rc.subr(8) also support
>      sourcing an optional end-user provided per-script override file
>      /etc/rc.conf.d/service, (where service is the contents of the name
>      variable in the rc.d(8) script).
>
> That is, what should happen to make this work...
>
>    | If:
>    | rc.conf contains npf=NO (or is not set)
>    | rc.conf.d/npf contains npf=YES
>
> is that rc.conf.d/npf_boot should contain npf=YES
>
> The rc.conf.d files have the same names as the rc.d/script files in
> general, for good reason, as while they often only contain this
> rcvar setting, they can contain overrides to anything in the script.
> Further, if there is more than one rcvar in a script (which I think
> has happened once or twice) the settings for both of them would go
> in the same file, not one file for each of them.
>
>    | This means that in the latter case, at boot time the npfctl start command
>    | is never run and the firewall is not operational.
>
> Because of user error.
>
> Please revert this change, and request the pullup be undone as well.
I don't think it is as simple as this in the case of npf and pf. What 
has happened here is that the functionality of /etc/rc.d/npf has 
effectively been split into two parts (one to run early, the other 
late). It does not make sense to run one without the other. It's not 
like nfs which is a stack of mountd, rpcbind, nfsd, etc. which may well 
want different flags for each.

In rc.conf, npf=YES is sufficient, but you are advocating the setting 
needs to be duplicated if put into rc.conf.d. When upgrading from 8 to 
9, this is a breaking change.

I propose an alternative fix which is to change /etc/rc.d/npf_boot to read:

load_rc_config $name
load_rc_config_var npf npf

--
Stephen