Page MenuHomeFreeBSD

pf: Basic automated test using VIMAGE
ClosedPublic

Authored by kp on Oct 3 2017, 8:26 PM.

Details

Summary

If VIMAGE is present we can start jails with their own pf instance. This
makes it fairly easy to run tests.
For example, this basic test verifies that drop/pass and icmp
classification works. It's a basic sanity test for pf, and hopefully an
example on how to write more pf tests.

The tests are skipped if VIMAGE is not enabled.

This work is inspired by the GSoC work of Panagiotes Mousikides.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Consider using one of the TEST-NET-* IPv4 prefixes (see RFC 5737) instead of using 172.16.42.0/24.

tests/sys/netpfil/pf/pass_block.sh
69 ↗(On Diff #33662)

Ugh... Is 1 second long enough when you're spinning up lots of jails at the same time? Might be worth wrapping this in a loop that checks if the address actually got assigned to avoid surprising test failures.

asomers added inline comments.
tests/sys/netpfil/pf/pass_block.sh
5 ↗(On Diff #33662)

Conventionally, this function goes at the end of the file.

23 ↗(On Diff #33662)

This could potentially conflict with a real interface. For other tests, I've used the 192.0.2.0/24 network, which is reserved for documentation per RFC5737.

69 ↗(On Diff #33662)

Try passing no_dad to ifconfig. That might eliminate the need for the sleep.

tests/sys/netpfil/pf/utils.subr
7 ↗(On Diff #33662)

ATF tests aren't supposed to modify the host system any more than necessary. I would recommend not loading a kld here. Instead, check if it's already loaded, and skip if it isn't.

14 ↗(On Diff #33662)

In the unlikely event that your test crashes during setup, the cleanup must still succeed. So I wouldn't bother creating these files during setup. Instead, handle their potential nonexistence during cleanup.

This should address all of the review remarks.

It's safe to disable DAD here, because the epair interfaces are not connected to anything which could have a conflicting address.

kp marked 5 inline comments as done.Oct 5 2017, 6:09 PM
kp added inline comments.
tests/sys/netpfil/pf/utils.subr
7 ↗(On Diff #33662)

Once this goes in I will see about getting the ci scripts to load pf (and install scapy) in the test-VM so these tests get executed as well.

Are you planning to change the cleanup as I suggested, or do you disagree?

kp marked an inline comment as done.

Sorry, I missed that remark.

kp marked an inline comment as done.Oct 6 2017, 7:37 PM
This revision is now accepted and ready to land.Oct 6 2017, 7:45 PM
This revision was automatically updated to reflect the committed changes.