netpfil tests: do not run ipfw tests if net.inet.ip.fw.default_to_accept is not set

[kp]

Summary

The tests all assume that they can pass traffic unless
ipfw is configured otherwise. That's only true if
net.inet.ip.fw.default_to_accept is set (as is the case on
ci.freebsd.org).

Reported by: rscheff
MFC after: 1 week

[rscheff]

Maybe just print a warning instead;
I'd prefer adding the appropriate rules in the tests is IMHO the better approach (prior to the sanity checks) - to let the tests pass without any non-default configuration (such as the loader.conf net.inet.ip.fw.default_to_accept=1)

But this is indeed the setting preventing the tests to pass, as no accept-all rule is added prior to the sanity checks in the existing ipfw / pf tests.

[zlei]

tests/sys/netpfil/common/utils.subr

90

Currently this MIB net.inet.ip.fw.default_to_accept is not per-vnet one.

@kp How about converting it to per-vnet one and enabling it while running tests ?

[zlei]

tests/sys/netpfil/common/utils.subr

90

Emm, net.inet.ip.fw.default_to_accept default is 0. Why

ipfw tests assume net.inet.ip.fw.default_to_accept=1

?

[rscheff]

tests/sys/netpfil/common/utils.subr

90

Yes exactly; I'd rather opt for changing the existing tests to include a catch-all allow prior to the sanity checks (see the tests in D42980) than asking for a loader-time major config adjustment prior of running these tests.

Apart from that, the setting could also be changed to a per-VNET one, but again, modifying existing tests to simply add the catch-all allow any should suffice IMHO.

[kp]

In D43078#982089, @rscheff wrote:

Maybe just print a warning instead;
I'd prefer adding the appropriate rules in the tests is IMHO the better approach (prior to the sanity checks) - to let the tests pass without any non-default configuration (such as the loader.conf net.inet.ip.fw.default_to_accept=1)

But this is indeed the setting preventing the tests to pass, as no accept-all rule is added prior to the sanity checks in the existing ipfw / pf tests.

In principle you are correct that it'd be better to assume the default (i.e. block) behaviour from ipfw.

I expect it'd be fairly straightforward to adjust the existing tests to match that assumption. My main concern with that is that we do change the default for ci.freebsd.org (presumably at least in part because otherwise we'd have to configure ipfw rules on the test host/VM), and that'd mean that the assumed test environment wouldn't match the actual environment.
I also run with default accept (for ipfw), because otherwise running the tests becomes a pain (because as soon as I load ipfw I lose connection to the box, due to the block by default behaviour).

I'm going to cc lwhsu here, as he tends to take care of the CI tests.

[rscheff]

In D43078#982176, @kp wrote:

I expect it'd be fairly straightforward to adjust the existing tests to match that assumption. My main concern with that is that we do change the default for ci.freebsd.org (presumably at least in part because otherwise we'd have to configure ipfw rules on the test host/VM), and that'd mean that the assumed test environment wouldn't match the actual environment.
I also run with default accept (for ipfw), because otherwise running the tests becomes a pain (because as soon as I load ipfw I lose connection to the box, due to the block by default behaviour).

Maybe the middle ground would be to have a printf instead of the atf_skip in that clause, to inform about the possibility of tests failing, but running them nevertheless...

Subsequently, the VNET config in the various test cases could add the defaul permit all, to allow executing the tests typically on a system without rebooting...

[kp]

In D43078#982205, @rscheff wrote:

Maybe the middle ground would be to have a printf instead of the atf_skip in that clause, to inform about the possibility of tests failing, but running them nevertheless...

As far as I know there's not really any infrastructure in atf-sh to allow for warnings like this. We could printf it, but that'd only be visible in the test debug output which is not displayed by default.

Subsequently, the VNET config in the various test cases could add the defaul permit all, to allow executing the tests typically on a system without rebooting...

I've taken a quick look at changing the existing tests to cope with net.inet.ip.fw.default_to_accept being 0, and it ends up adding a fair bit of boilerplate to the tests. Not insurmountably so, but I have a strong desire to keep the tests as simple as possible.
I've wound up with this, and that only fixes the dummynet tests. forward, fragments, pass_block and tos tests are still broken:

dummynet.sh |  154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------
utils.subr  |   18 +++++++++++++++---
2 files changed, 122 insertions(+), 50 deletions(-)

tests/sys/netpfil/common/utils.subr

90

Currently this MIB net.inet.ip.fw.default_to_accept is not per-vnet one.

@kp How about converting it to per-vnet one and enabling it while running tests ?

That would probably be a good idea, although it's not by itself sufficient to fix the existing tests.
I'm not volunteering for that though.