summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* tests/shell: generate and add ".nft" dump files for existing testsThomas Haller2023-09-09161-0/+2546
| | | | | | | | | | | | | | | | | | | | | | | | | | | Several tests didn't have a ".nft" dump file committed. Generate one and commit it to git. While not all tests have a stable ruleset to compare, many have. Commit the .nft files for the tests where the output appears to be stable. This was generated by running `./tests/shell/run-tests.sh -g` twice, and commit the files that were identical both times. Note that 7 tests on my machine fail, so those are skipped. Also skip the files tests/shell/testcases/maps/dumps/0004interval_map_create_once_0.nft tests/shell/testcases/nft-f/dumps/0011manydefines_0.nft tests/shell/testcases/sets/dumps/0011add_many_elements_0.nft tests/shell/testcases/sets/dumps/0030add_many_elements_interval_0.nft tests/shell/testcases/sets/dumps/0068interval_stack_overflow_0.nft Those files are larger than 100KB, and I don't think we want to blow up the git repository this way. Even if they are only text files and compress well. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: honor .nodump file for tests without nft dumpsThomas Haller2023-09-091-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | For some tests, the dump is not stable or useful to test. For example, if they have an "expires" timestamps. Those tests don't have a .nft file in the dumps directory, and don't have it checked. DUMPGEN=y generates a new dump file, if the "dumps/" directory exists. Omitting that directory is a way to prevent the generation of the file. However, many such tests share their directory with tests that do have dumps. When running tests with DUMPGEN=y, new files for old tests are generated. Those files are not meant to be compared or committed to git because it's known to not work. Whether a test has a dump file, is part of the test. The absence of the dump file should also be recorded and committed to git. Add a way to opt-out from such generating such dumps by having .nodump files instead of the .nft dump. Later we should add unit tests that checks that no test has both a .nft and a .nodump file in git, that the .nodump file is always empty, and that every .nft/.nodump file has a corresponding test committed to git. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* datatype: rename "dtype_clone()" to datatype_clone()Thomas Haller2023-09-084-8/+8
| | | | | | | | | | | | | The struct is called "datatype" and related functions have the fitting "datatype_" prefix. Rename. Also rename the internal "dtype_alloc()" to "datatype_alloc()". This is a follow up to commit 01a13882bb59 ('src: add reference counter for dynamic datatypes'), which started adding "datatype_*()" functions. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* cache: avoid accessing uninitialized varible in implicit_chain_cache()Thomas Haller2023-09-081-2/+4
| | | | | | | | | | | | | | | | | | | | | $ ./tests/shell/run-tests.sh -V tests/shell/testcases/cache/0010_implicit_chain_0 Gives: ==59== Conditional jump or move depends on uninitialised value(s) ==59== at 0x48A6A6B: mnl_nft_rule_dump (mnl.c:695) ==59== by 0x48778EA: rule_cache_dump (cache.c:664) ==59== by 0x487797D: rule_init_cache (cache.c:997) ==59== by 0x4877ABF: implicit_chain_cache.isra.0 (cache.c:1032) ==59== by 0x48785C9: cache_init_objects (cache.c:1132) ==59== by 0x48785C9: nft_cache_init (cache.c:1166) ==59== by 0x48785C9: nft_cache_update (cache.c:1224) ==59== by 0x48ADBE1: nft_evaluate (libnftables.c:530) ==59== by 0x48AEC29: nft_run_cmd_from_buffer (libnftables.c:596) ==59== by 0x402983: main (main.c:535) Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: set NFT_TEST_JOBS based on $(nproc)Thomas Haller2023-09-081-3/+7
| | | | | | | | Choose 150% of $(nproc) for the default vlaue of NFT_TEST_JOBS (rounded up). The minimal value chosen by default is 2. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: record wall time of test run in result dataThomas Haller2023-09-081-0/+16
| | | | | | | | | | | | | | | | | | | | | When running tests, it's useful to see how long it took. Keep track if the timestamps in a "times" file. Try: ( \ for d in /tmp/nft-test.latest.*/test-*/ ; do \ printf '%10.2f %s\n' \ "$(sed '1!d' "$d/times")" \ "$(cat "$d/name")" ; \ done \ | sort -n \ | awk '{print $0; s+=$1} END{printf("%10.2f\n", s)}' ; \ printf '%10.2f wall time\n' "$(sed '1!d' /tmp/nft-test.latest.*/times)" \ ) Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: no longer enable verbose output when selecting a testThomas Haller2023-09-081-4/+1
| | | | | | | | | | | | | | | | | | | | Previously, when selecting a test on the command line, it would also enable verbose output (except if the "--" separator was used). This convenience feature seems not great because the output from the test badly clutters the "run-test.sh" output. Now that the test results are all on disk, you can search them after the run with great flexibility (grep). Additionally, in previous versions, command line argument parsing was more restrictive, requiring that "-v" always be placed first. Now, the order does not matter, so it's easy to edit the command prompt and append a "-v", if that is what you want. Or if you really like verbose output, then `export VERBOSE=y`. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: print "kernel is tainted" separate from test resultThomas Haller2023-09-081-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Once the kernel is tainted, it stays until reboot. It would not be useful to fail the entire test run based on that (and we don't do that). But then, it seems odd to print this in the same style as the test results, because a [FAILED] of a test counts as an overall failure. Instead, print this warning in a different style. Previously: $ ./tests/shell/run-tests.sh -- /usr/bin/true ... W: [FAILED] kernel is tainted I: [OK] /usr/bin/true I: results: [OK] 1 [SKIPPED] 0 [FAILED] 0 [TOTAL] 1 Now: $ ./tests/shell/run-tests.sh -- /usr/bin/true ... W: kernel is tainted I: [OK] /usr/bin/true I: results: [OK] 1 [SKIPPED] 0 [FAILED] 0 [TOTAL] 1 Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: redirect output of test script to file tooThomas Haller2023-09-081-0/+2
| | | | | | | | | | | It's useful to keep around for later. Redirect to the temporary directory. Note that the file content may be colorized too. `less -R` helps with that. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: don't redirect error/warning messages to stderrThomas Haller2023-09-081-2/+2
| | | | | | | | | | | | | | Writing some messages to stderr and some to stdout is not helpful. Once they are written to separate streams, it's hard to be sure about their relative order. Use grep to filter messages. Also, next we will redirect the entire output also to a file. There the output is also not split in two files. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: print the NFT setting with the VALGRIND=y wrapperThomas Haller2023-09-081-4/+5
| | | | | | | | | With this we see in the info output I: info: NFT=./tests/shell/helpers/nft-valgrind-wrapper.sh Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: fix handling failures with VALGRIND=yThomas Haller2023-09-083-6/+26
| | | | | | | | | | | | | | | | | | With VALGRIND=y, on memleaks the tests did not fail. Fix that by passing "--error-exitcode=122" to valgrind. But just returning 122 from $NFT command may not correctly fail the test. Instead, ensure to write a "rc-failed-valrind" file, which is picked up by "test-wrapper.sh" to properly handle the valgrind failure (and fail with error code 122 itself). Also, accept NFT_TEST_VALGRIND_OPTS variable to a pass additional arguments to valgrind. For example a "--suppressions" file. Also show the special error code [VALGRIND] in "run-test.sh". Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: colorize terminal output with test resultThomas Haller2023-09-081-10/+61
| | | | | | | | | | Colors help to see what is important. It honors the common NO_COLOR=<anything> to disable coloring. It also does not colorize, if [ -t 1 ] indicates that stdout is not a terminal. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: cleanup print_test_result() and show TAINTED error codeThomas Haller2023-09-081-8/+10
| | | | | | | | | | | We will add more special error codes (122 for VALGRIND). Minor refactor of print_test_result() to make it easier to extend for that. Also, we will soon colorize the output. This preparation patch makes that easier too. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: cleanup result handling in "test-wrapper.sh"Thomas Haller2023-09-081-24/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | The previous code was mostly correct, but hard to understand. Rework it. Also, on failure now always write "rc-failed-exit", which is the exit code that "test-wrapper.sh" reports to "run-test.sh". Note that this error code may not be the same as the one returned by the TEST binary. The latter you can find in one of the files "rc-{ok,skipped,failed}". In general, you can search the directory with test results for those "rc-*" files. If you find a "rc-failed" file, it was counted as failure. There might be other "rc-failed-*" files, depending on whether the diff failed or kernel got tainted. Also, reserve all the error codes 118 - 124 for the "test-wrapper.sh". For example, 124 means a dump difference and 123 means kernel got tainted. In the future, 122 will mean a valgrind error. Other numbers are not reserved. If a test command fails with such an reserved code, "test-wrapper.sh" modifies it to 125, so that "run-test.sh" does not get the wrong idea about the failure reason. This is not new in this patch, except that the reserved range was extended for future additions. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: return 77/skip for tests that fail to create dummy deviceThomas Haller2023-09-083-5/+5
| | | | | | | | | | | | | There are some existing tests, that skip operation when they fail to create a dummy interface. Use the new exit code 77 to indicate "SKIPPED". I wonder why creating a dummy device would ever fail and why we don't just fail the test altogether in that case. But the patch does not change that. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: set TMPDIR for tests in "test-wrapper.sh"Thomas Haller2023-09-071-0/+2
| | | | | | | | | | | | | | Various tests create additional temporary files. They really should just use "$NFT_TEST_TESTTMPDIR" for that. However, they mostly use `mktemp`. The scripts are supposed to cleanup those files afterwards. However, often that does not work correctly and /tmp gets full of left over temporary files. Export "TMPDIR" so that they use the test-specific temporary directory. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: fix "0003includepath_0" for different TMPDIRThomas Haller2023-09-071-2/+2
| | | | | | | | We are going to set $TMPDIR to another location. The previous code made assumptions that the generated path would always be in /tmp. Fix that. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: record the test duration (wall time) in the result dataThomas Haller2023-09-071-0/+6
| | | | | | | | | | | | | | | | | Runtimes are important. Add a way to find out how long tests took. $ ./tests/shell/run-tests.sh ... $ for d in /tmp/nft-test.latest.*/test-*/ ; do \ printf '%10.2f %s\n' \ "$(sed '1!d' "$d/times")" \ "$(cat "$d/name")" ; \ done \ | sort -n \ | awk '{print $0; s+=$1} END{printf("%10.2f\n", s)}' Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: skip test in rootless that hit socket buffer size limitThomas Haller2023-09-0710-5/+167
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The socket buffer limits like /proc/sys/net/core/{rmem_max,wmem_max} can cause tests to fail, when running rootless. That's because real-root can override those limits, rootless cannot. Add an environment variable NFT_TEST_HAS_SOCKET_LIMITS=*|n which is automatically set by "run-tests.sh". Certain tests will check for [ "$NFT_TEST_HAS_SOCKET_LIMITS" = y ] and skip the test. The user may manually bump those limits (requires root), and set NFT_TEST_HAS_SOCKET_LIMITS=n to get the tests to pass even as rootless. For example, the test passes with root: sudo ./tests/shell/run-tests.sh -- tests/shell/testcases/sets/automerge_0 Without root, it would fail. Skip it instead: ./tests/shell/run-tests.sh -- tests/shell/testcases/sets/automerge_0 ... I: [SKIPPED] tests/shell/testcases/sets/automerge_0 Or bump the limit: $ echo 3000000 | sudo tee /proc/sys/net/core/wmem_max $ NFT_TEST_HAS_SOCKET_LIMITS=n ./tests/shell/run-tests.sh -- tests/shell/testcases/sets/automerge_0 ... I: [OK] tests/shell/testcases/sets/automerge_0 Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: bind mount private /var/run/netns in test containerThomas Haller2023-09-072-4/+51
| | | | | | | | | | | | | | | | | | | | | | Some tests want to run `ip netns add`, which requires write permissions to /var/run/netns. Also, /var/run/netns would be a systemwide mount path, and shared between the tests. We would want to isolate that. Fix that by bind mount a tmpfs inside the test wrapper, if we appear to have a private mount namespace. Fixes $ ./tests/shell/run-tests.sh -- tests/shell/testcases/netns/0001nft-f_0 Optimally, `ip netns add` would allow to specify a private location for those bind mounts. It seems that iproute2 is build with /var/run/netns, instead the more common /run/netns. Hence, handle /var/run instead of /run. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: support running tests in parallelThomas Haller2023-09-071-13/+73
| | | | | | | | | | | | | | | Add option to enable running jobs in parallel. The purpose is to speed up the run time of the tests. The global cleanup (removal of kernel modules) interferes with parallel jobs (or even with, unrelated jobs on the system). By setting NFT_TEST_JOBS= to a positive number, that cleanup is skipped. This option is too good to miss. Hence parallel execution is enabled by default, and you have to opt-out from it. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: move valgrind wrapper script to separate scriptThomas Haller2023-09-072-34/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, in valgrind mode we would generate one script, which had "$NFT" variable and the temp directory hard coded. Soon, we will run jobs in parallel, so they would need at least different temp directories. Also, we want to put the valgrind results are inside "$NFT_TEST_TESTTMPDIR", along the test data. Extract the wrapper script to a separate script. It does not need to be generated ad-hoc, instead it uses the environment variables "$NFT_REAL" and "$NFT_TEST_TESTTMPDIR", as "run-tests.sh" prepares them. Also, add a "$NFT_REAL" variable for the actual NFT binary. We wrap the "$NFT" variable with VALGRIND=y or the user may pass "NFT='valgrind nft'". We should have access to the real binary. That might be useful for example to call `ldd "$NFT_REAL" | grep libjansson` to check for JSON support. Also, we use libtool. So quite possible the nft binary is actually a shell script. Calling valgrind on that script results in a lot of leak reports from shell (and slows down the command). Instead, use `libtool --mode=execute`. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: move taint check to "test-wrapper.sh"Thomas Haller2023-09-072-15/+19
| | | | | | | | | We will run tests in parallel. That means, we have multiple tests data and results in fly. That becomes simpler, if we move more result data to the test-wrapper and out of "run-tests.sh". Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: rework printing of test resultsThomas Haller2023-09-072-40/+81
| | | | | | | | | | | | | | | | | | | | - "test-wrapper.sh" no longer will print the test output to its stdout. Instead, it only writes the testout.log file. - rework the loop "run-tests.sh" for printing the test results. It no longer captures the output of the test, as the wrapper is expected to be silent. Instead, they get the output from the result directory. The benefit is, that there is no duplication in what we print and the captured data in the result directory. The verbose mode is only for convenience, to safe looking at the test data. It's not essential otherwise. - also move the evaluation of the test result (and printing of the information) to a separate function. Later we want to run tests in parallel, so the steps need to be clearly separated. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: move the dump diff handling inside "test-wrapper.sh"Thomas Haller2023-09-072-35/+66
| | | | | | | | | | | | | | | | | | | | | | | | | | | This fits there better. At this point, we are still inside the unshared namespace and right after the test. The test-wrapper.sh should compare (and generate) the dumps. Also change behavior for DUMPGEN=y. - Previously it would only rewrite the dump if the dumpfile didn't exist yet. Now instead, always rewrite the file with DUMPGEN=y. The mode of operation is anyway, that the developer afterwards checks `git diff|status` to pick up the changes. There should be no changes to existing files (as existing tests are supposed to pass). So a diff there either means something went wrong (and we should see it) or it just means the dumps correctly should be regenerated. - also, only generate the file if the "dumps/" directory exists. This allows to write tests that don't have a dump file and don't get it automatically generated. The test wrapper will return a special error code 124 to indicate that the test passed, but the dumps file differed. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: support --keep-logs option (NFT_TEST_KEEP_LOGS=y) to preserve ↵Thomas Haller2023-09-071-3/+19
| | | | | | | | | | | test output The test output is now all collected in the temporary directory. On success, that directory is deleted. Add an option to always preserve that directory. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: interpret an exit code of 77 from scripts as "skipped"Thomas Haller2023-09-072-1/+12
| | | | | | | | | | | | Allow scripts to indicate that a test could not run by exiting 77. "77" is chosen as exit code from automake's testsuites ([1]). Compare to git-bisect which chooses 125 to indicate skipped. [1] https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: run each test in separate namespace and allow rootlessThomas Haller2023-09-072-17/+130
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Don't unshare the entire shell script. Instead, call unshare each test separately. That means, all tests use now a different sandbox and will also allow (with further changes) to run them in parallel. Also, allow to run rootless/unprivileged. The script first tries to run a separate PID+USER+NET namespace. If that fails, it downgrades to USER+NET. If that fails, it downgrades to a separate NET namespace. If unshare still fails, the script fails entirely. That differs from before, where the script would proceed without sandboxing. The script will now always require that unsharing works, unless the user opts-out. If the user cannot unshare, they can set NFT_TEST_UNSHARE_CMD to the command used for unsharing. It may be empty for no unshare. The command line arguments -U/--no-unshare are a shortcut for setting NFT_TEST_UNSHARE_CMD="". If we are able to create a separate USER namespace, then this mode allows to run the test as rootless/unprivileged. We no longer require [ `id -u` = 0 ]. Some tests may not work as rootless. For example, the socket buffers is limited by /proc/sys/net/core/{wmem_max,rmem_max} which real-root can override, but rootless tests cannot. Such tests should check for [ "$NFT_TEST_HAS_REALROOT" != y ] and skip gracefully. Usually, the user doesn't need to tell the script whether they have real-root. The script will autodetect it via [ `id -u` = 0 ]. But that won't work when run inside a rootless container already. In that case, the user would want to tell the script that there is no real-root. They can do so via the -R/--without-root option or NFT_TEST_HAS_REALROOT=n. If tests wish, the can know whether they run inside "unshare" environment by checking for [ "$NFT_TEST_HAS_UNSHARED" = y ]. When setting NFT_TEST_UNSHARE_CMD to override the unshare command, users may want to also set NFT_TEST_HAS_UNSHARED= and NFT_TEST_HAS_REALROOT= correctly. As we run each test in a separate unshare environment, we need a wrapper "tests/shell/helpers/test-wrapper.sh" around the test, which executes inside the tested environment. Also, each test gets its own temp directory prepared in NFT_TEST_TESTTMPDIR. This is also the place, where test artifacts and results will be collected. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: print test configurationThomas Haller2023-09-071-5/+10
| | | | | | | | | As the script can be configured via environment variables or command line option, it's useful to show the environment variables that we received or set during the test setup. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: normalize boolean configuration in environment variablesThomas Haller2023-09-071-4/+15
| | | | | | | | | | | | Previously, we would honor "y" as opt-in, and all other values meant false. - accept alternatives to "y", like "1" or "true". - normalize the value, to either be "y" or "n". Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: export NFT_TEST_BASEDIR and NFT_TEST_TMPDIR for testsThomas Haller2023-09-071-24/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | Let the test wrapper prepare and export two environment variables for the test: - "$NFT_TEST_BASEDIR" is just the top directory where the test scripts lie. - "$NFT_TEST_TMPDIR" is a `mktemp` directory created by "run-tests.sh" and removed at the end. Tests may use that to leave data there. This directory will be used for various things, like the "nft" wrapper in valgrind mode, the results of the tests and possibly as cache for feature detection. The "$NFT_TEST_TMPDIR" was already used before with the "VALGRIND=y" mode. It's only renamed and got an extended purpose. Also drop the unnecessary first detection of "$DIFF" and the "$SRC_NFT" variable. Also, note that the mktemp creates the temporary directory under /tmp. Which is commonly a tempfs. The user can override that by exporting TMPDIR. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: check test names before start and support directoriesThomas Haller2023-09-071-0/+12
| | | | | | | | | | | Check for valid test names early. That's useful because we treat any unrecognized options as test names. We should detect a mistake early. While at it, also support specifying directory names instead of executable files. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: rework finding tests and add "--list-tests" optionThomas Haller2023-09-071-28/+30
| | | | | | | | | | | | | | Cleanup finding the test files. Also add a "--list-tests" option to see which tests are found and would run. Also get rid of the FIND="$(which find)" detection. Which system doesn't have a working find? Also, we can just fail when we try to use find, and don't need a check first. This is still after "unshare", which will be addressed next. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: rework command line parsing in "run-tests.sh"Thomas Haller2023-09-071-30/+68
| | | | | | | | | | | | Parse the arguments in a loop, so that their order does not matter. Also, soon more command line arguments will be added, and this way of parsing seems more maintainable and flexible. Currently this is still after the is-root check and after unshare. That will be addressed later. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: shell: 0043concatenated_ranges_0: re-enable all testsFlorian Westphal2023-09-061-6/+1
| | | | | | | | This script suppressed a few tests when ran via run-tests.sh, don't do that, it would have caught the previous 'get' bug years ago. Signed-off-by: Florian Westphal <fw@strlen.de>
* evaluate: fix get element for concatenated setFlorian Westphal2023-09-061-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | given: table ip filter { set test { type ipv4_addr . ether_addr . mark flags interval elements = { 198.51.100.0/25 . 00:0b:0c:ca:cc:10-c1:a0:c1:cc:10:00 . 0x0000006f, } } } We get lookup failure: nft get element ip filter test { 198.51.100.1 . 00:0b:0c:ca:cc:10 . 0x6f } Error: Could not process rule: No such file or directory Its possible to work around this via dummy range somewhere in the key, e.g. nft get element ip filter test { 198.51.100.1 . 00:0b:0c:ca:cc:10 . 0x6f-0x6f } but that shouldn't be needed, so make sure the INTERVAL flag is enabled for the queried element if the set is of interval type. Signed-off-by: Florian Westphal <fw@strlen.de>
* doc: describe behaviour of {ip,ip6} lengthPablo Neira Ayuso2023-09-031-0/+16
| | | | | | | | This field exposes internal kernel GRO/GSO packet aggregation implementation details to userspace, provide a hint to the user to understand better when matching on this field. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: revisit anonymous set with single element optimizationPablo Neira Ayuso2023-09-021-20/+40
| | | | | | | | | | | | | | | | | | | | | This patch reworks it to perform this optimization from the evaluation step of the relational expression. Hence, when optimizing for protocol flags, use OP_EQ instead of OP_IMPLICIT, that is: tcp flags { syn } becomes (to represent an exact match): tcp flags == syn given OP_IMPLICIT and OP_EQ are not equivalent for flags. 01167c393a12 ("evaluate: do not remove anonymous set with protocol flags and single element") disabled this optimization, which is enabled again after this patch. Fixes: 01167c393a12 ("evaluate: do not remove anonymous set with protocol flags and single element") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: place byteorder conversion after numgen for IP address datatypesJorge Ortiz2023-09-015-0/+77
| | | | | | | | | | | | | | | | | | | | | | | | The numgen extension generates numbers in little-endian. This can be very tricky when trying to combine it with IP addresses, which use big endian. This change adds a new byteorder operation to convert data type endianness. Before this patch: $ sudo nft -d netlink add rule nat snat_chain snat to numgen inc mod 7 offset 0x0a000001 ip nat snat_chain [ numgen reg 1 = inc mod 7 offset 167772161 ] [ nat snat ip addr_min reg 1 ] After this patch: $ sudo nft -d netlink add rule nat snat_chain snat to numgen inc mod 7 offset 0x0a000001 ip nat snat_chain [ numgen reg 1 = inc mod 7 offset 167772161 ] [ byteorder reg 1 = hton(reg 1, 4, 4) ] [ nat snat ip addr_min reg 1 ] Regression tests have been modified to include these new cases. Signed-off-by: Jorge Ortiz Escribano <jorge.ortiz.escribano@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: set internal_location for table and chainPablo Neira Ayuso2023-08-311-0/+2
| | | | | | | JSON parser does not seem to set on this, better provide a default location. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: simplify chain_alloc()Pablo Neira Ayuso2023-08-316-9/+9
| | | | | | | Remove parameter to set the chain name which is only used from netlink path. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: remove check for NULL before calling expr_free()Pablo Neira Ayuso2023-08-312-4/+3
| | | | | | expr_free() already handles NULL pointer, remove redundant check. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: use internal_location for unspecified location at allocation timePablo Neira Ayuso2023-08-313-12/+19
| | | | | | | Set location to internal_location instead of NULL to ensure this is always set. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* xt: avoid "-Wmissing-field-initializers" for "original_opts"Thomas Haller2023-08-301-1/+1
| | | | | | | | | | | | | | | | | | Avoid this warning with clang: CC src/xt.lo src/xt.c:353:9: error: missing field 'has_arg' initializer [-Werror,-Wmissing-field-initializers] { NULL }, ^ The warning seems not very useful, because it's well understood that specifying only some initializers leaves the remaining fields initialized with the default. However, as this warning is only hit once in the code base, it doesn't seem that we violate this style frequently. Hence, fix it instead of disabling the warning. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: silence "implicit-fallthrough" warningsThomas Haller2023-08-302-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | Gcc with "-Wextra" warns: CC segtree.lo segtree.c: In function 'get_set_interval_find': segtree.c:129:28: error: this statement may fall through [-Werror=implicit-fallthrough=] 129 | if (expr_basetype(i->key)->type != TYPE_STRING) | ^ segtree.c:134:17: note: here 134 | case EXPR_PREFIX: | ^~~~ CC optimize.lo optimize.c: In function 'rule_collect_stmts': optimize.c:396:28: error: this statement may fall through [-Werror=implicit-fallthrough=] 396 | if (stmt->expr->left->etype == EXPR_CONCAT) { | ^ optimize.c:400:17: note: here 400 | case STMT_VERDICT: | ^~~~ Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* utils: call abort() after BUG() macroThomas Haller2023-08-301-1/+2
| | | | | | | | | | | | | | | | | | | | | | | Otherwise, we get spurious warnings. The compiler should be aware that there is no return from BUG(). Call abort() there, which is marked as __attribute__ ((__noreturn__)). In file included from ./include/nftables.h:6, from ./include/rule.h:4, from src/payload.c:26: src/payload.c: In function 'icmp_dep_to_type': ./include/utils.h:39:34: error: this statement may fall through [-Werror=implicit-fallthrough=] 39 | #define BUG(fmt, arg...) ({ fprintf(stderr, "BUG: " fmt, ##arg); assert(0); }) | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/payload.c:791:17: note: in expansion of macro 'BUG' 791 | BUG("Invalid map for simple dependency"); | ^~~ src/payload.c:792:9: note: here 792 | case PROTO_ICMP_ECHO: return ICMP_ECHO; | ^~~~ Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: fix "const static" declarationThomas Haller2023-08-301-2/+2
| | | | | | | | | | | | | | Gcc warns against this with "-Wextra": src/rule.c:869:1: error: static is not at beginning of declaration [-Werror=old-style-declaration] 869 | const static struct prio_tag std_prios[] = { | ^~~~~ src/rule.c:878:1: error: static is not at beginning of declaration [-Werror=old-style-declaration] 878 | const static struct prio_tag bridge_std_prios[] = { | ^~~~~ Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: py: debloat frag.t.payload.netdevPablo Neira Ayuso2023-08-301-1990/+36
| | | | | | This bytecode output file contains many duplicated entries, remove them. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: py: extend ip frag-off coveragePablo Neira Ayuso2023-08-306-0/+156
| | | | | | Cover matching on DF and MF bits and fragments. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>