Page MenuHomeFreeBSD

NAT basic test for pf, ipfw (both in-kernel and userspace) and ipf
ClosedPublic

Authored by ahsanb on Aug 9 2019, 5:03 PM.

Details

Summary

Add tests for basic nat in which it is tested that two clients behind the nat are able to reach a common host.

Diff Detail

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

Event Timeline

ahsanb created this revision.Aug 9 2019, 5:03 PM
ahsanb retitled this revision from NAT basic test for pf, ipfw (noth in-kernel and userspace) and ipf to NAT basic test for pf, ipfw (both in-kernel and userspace) and ipf.
thj added inline comments.Aug 10 2019, 1:39 PM
tests/sys/netpfil/common/nat.sh
151 ↗(On Diff #60616)

The setup test lines should align based on the first character

154 ↗(On Diff #60616)

I do think 'userspace_nat' is descriptie enough, I can't tell from this line which firewall is being tested. Is there a different name that could be used?

155 ↗(On Diff #60616)

ipfw is in the list twice, should this be ipf?

ahsanb added inline comments.Aug 11 2019, 3:28 PM
tests/sys/netpfil/common/nat.sh
151 ↗(On Diff #60616)

I don't know why after git push 4-spaced tabs are being converted to 8-spaced tabs.

154 ↗(On Diff #60616)

The "userspace_nat" is the name of the test case, same as "basic". The "basic" is tested on ipf, ipfw and pf. while "userspace_nat" is tested on ipfw. This is also the reason why ipfw appears twice.

kp added a comment.Aug 11 2019, 5:01 PM

I seem to run into issues running the ipfw_basic test:

Part of the run output (with 'set -x' added to the test):

+ [ 4 -gt 0 ]
+ is_firewall ipfw -q add 1000 nat 123 all from any to any
+ [ ipfw '=' pf -o ipfw '=' ipfw -o ipfw '=' ipf -o ipfw '=' ipfnat ]
+ echo 1
+ [ 1 -eq 1 ]
+ current_fw='ipfw -q add 1000 nat 123 all from any to any'
+ shift
+ filename='ipfw -q add 1000 nat 123 all from any to any.rule'
+ pwd
+ cwd=/tmp/kyua.plyoOz/2/work
+ [ -f ipfw -q add 1000 nat 123 all from any to any.rule ]
[: ipfw: unexpected operator

It seems to take the ipfw rule for a filename for some reason.

ahsanb updated this revision to Diff 60654.Aug 11 2019, 6:58 PM

fixed the ipfw rule loading issue

kp added a comment.EditedAug 12 2019, 2:41 PM

And this is wrong, or at least very confusing, in firewall_init():

elif [ ${firewall} == "ipfnat" ]; then
        if ! kldstat -q -m ipfw_nat; then
                atf_skip "This test requires ipfw_nat"
        fi
else
tests/sys/netpfil/common/utils.subr
111 ↗(On Diff #60654)

You're checking for ipfw_nat, but report needing ipfw here.

ahsanb updated this revision to Diff 60712.Aug 13 2019, 6:56 AM

Fix wrong module name check for ipfw (ipfw_nat instead of ipfw)

ahsanb updated this revision to Diff 60779.Aug 14 2019, 11:18 AM
  • Fixed indentation for setup_tests() in nat.sh
  • Fixed indentation in common/Makefile
thj accepted this revision.Aug 14 2019, 12:17 PM
This revision is now accepted and ready to land.Aug 14 2019, 12:17 PM
kp accepted this revision.Aug 14 2019, 12:21 PM
This revision was automatically updated to reflect the committed changes.