prefer
binop_with_different_basetype_assert:3:29-35: Error: Binary operation (<<) with different base types (string vs integer) is not supported
oifname set ip9dscp << 26 | 0x10
^^^^^^^~~~~~~
to assertion failure.
Thomas Haller [Fri, 24 Nov 2023 12:45:53 +0000 (13:45 +0100)]
tests/shell: use generated ruleset for `nft --check`
The command `nft [-j] list ruleset | nft [-j] --check -f -` should never
fail. "test-wrapper.sh" already checks for that.
However, previously, we would run check against the .nft/.json-nft
files. In most cases, the generated ruleset and the files in git are
identical. However, when they are not, we (also) want to run the check
against the generated one.
This means, we can also run this check every time, regardless whether a
.nft/.json-nft file exists.
If the .nft/.json-nft file is different from the generated one, (because
a test was skipped or because there is a bug), then also check those
files. But this time, any output is ignored as failures are expected
to happen. We still run the check, to get additional coverage for
valgrind or santizers.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Sat, 2 Dec 2023 10:10:34 +0000 (11:10 +0100)]
main: Refer to nft_options in nft_options_check()
Consult the array when determining whether a given option is followed by
an argument or not instead of hard-coding those that do. The array holds
both short and long option name, so one extra pitfall removed.
Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Florian Westphal <fw@strlen.de>
Thomas Haller [Mon, 27 Nov 2023 19:15:35 +0000 (20:15 +0100)]
tests/shell: workaround lack of `wait -p` before bash 5.1
Before bash 5.1, `wait -p` is not supported. So we cannot know which
child process completed.
As workaround, explicitly wait for the next PID. That works, but it
significantly reduces parallel execution, because a long running job
blocks the queue.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Nov 2023 15:02:14 +0000 (16:02 +0100)]
tests: shell: Fix sets/reset_command_0 for current kernels
Since kernel commit 4c90bba60c26 ("netfilter: nf_tables: do not refresh
timeout when resetting element"), element reset won't touch expiry
anymore. Invert the one check to make sure it remains unaltered, drop
the other testing behaviour for per-element timeouts.
Thomas Haller [Wed, 22 Nov 2023 11:19:40 +0000 (12:19 +0100)]
tests/shell: prettify JSON in test output and add helper
- add helper script "json-pretty.sh" for prettify/format JSON.
It uses either `jq` or a `python` fallback. In my tests, they
produce the same output, but the output is not guaranteed to be
stable. This is mainly for informational purpose.
- add a "json-diff-pretty.sh" which prettifies two JSON inputs and
shows a diff of them.
- in "test-wrapper.sh", after the check for a .json-nft dump fails, also
call "json-diff-pretty.sh" and write the output to "ruleset-diff.json.pretty".
This is beside "ruleset-diff.json", which contains the original diff.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
evaluate: bogus error when adding devices to flowtable
Bail out if flowtable declaration is missing and no devices are
specified.
Otherwise, this reports a bogus error when adding new devices to an
existing flowtable.
# nft -v
nftables v1.0.9 (Old Doc Yak #3)
# ip link add dummy1 type dummy
# ip link set dummy1 up
# nft 'create flowtable inet filter f1 { hook ingress priority 0; counter }'
# nft 'add flowtable inet filter f1 { devices = { dummy1 } ; }'
Error: missing hook and priority in flowtable declaration
add flowtable inet filter f1 { devices = { dummy1 } ; }
^^^^^^^^^^^^^^^^^^^^^^^^
Fixes: 5ad475fce5a1 ("evaluate: bail out if new flowtable does not specify hook and priority") Reported-by: Martin Gignac <martin.gignac@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
tests: shell: restore pipapo and chain binding coverage in standalone 30s-stress
Do not disable pipapo and chain binding coverage for standalone runs by
default. Instead, turn them on by default and allow users to disable them
through:
tests: shell: skip if kernel does not support flowtable with no devices
Originally, flowtables required devices in place to work, this was later
relaxed to allow flowtable with no initial devices, see 05abe4456fa3
("netfilter: nf_tables: allow to register flowtable with no devices").
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Tue, 14 Nov 2023 16:08:31 +0000 (17:08 +0100)]
tools: check for consistency of .json-nft dumps in "check-tree.sh"
Add checks for the newly introduced .json-nft dump files.
Optimally, every test that has a .nft dump should also have a .json-nft
dump, and vice versa.
However, currently some JSON tests fail to validate, and are missing.
Only flag those missing files as warning, without failing the script.
The reason to warn about this, is that we really should fix those tests,
and having a annoying warning increases the pressure and makes it
discoverable.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Thomas Haller [Tue, 14 Nov 2023 16:08:30 +0000 (17:08 +0100)]
tools: check more strictly for bash shebang in "check-tree.sh"
There is no problem in principle to allow any executable/shebang. However,
it also not clear why we would want to use anything except bash. Unless
we have a good use case, check and reject anything else.
Also not that `./tests/shell/run-tests.sh -x` only works if the shebang
is either exactly "#!/bin/bash" or "#!/bin/bash -e". While it probably
could be made work with other shebangs, the simpler thing is to just use
bash consistently.
Just check that they are all bash scripts. If there ever is a use-case,
we can always adjust this check.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Thomas Haller [Tue, 14 Nov 2023 16:08:29 +0000 (17:08 +0100)]
tools: simplify error handling in "check-tree.sh" by adding msg_err()/msg_warn()
msg_err() also sets EXIT_CODE=, so we don't have to duplicate this.
Also add msg_warn() to print non-fatal warnings. Will be used in the
future. As "check-tree.sh" tests the consistency of the source tree, a
warning only makes sense to point something out that really should be
fixed, but is not yet.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Thomas Haller [Tue, 14 Nov 2023 16:08:28 +0000 (17:08 +0100)]
tests/shell: add JSON dump files
Generate and add ".json-nft" files. These files contain the output of
`nft -j list ruleset` after the test. Also, "test-wrapper.sh" will
compare the current ruleset against the ".json-nft" files and test them
with "nft -j --check -f $FILE`. These are useful extra tests, that we
almost get for free.
Note that for some JSON dumps, `nft -f --check` fails (or prints
something). For those tests no *.json-nft file is added. The bugs needs
to be fixed first.
internal:0:0-0: Error: Parsing command array at index 3 failed.
internal:0:0-0: Error: unqualified type integer specified in map definition. Try "typeof expression" instead of "type datatype".
<<<<
Tests like "tests/shell/testcases/nft-f/0012different_defines_0" and
"tests/shell/testcases/nft-f/0024priority_0" also don't get a .json-nft
dump yet, because their output is not stable. That needs fixing too.
Cc: Pablo Neira Ayuso <pablo@netfilter.org> Cc: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Thomas Haller [Tue, 14 Nov 2023 16:08:27 +0000 (17:08 +0100)]
tests/shell: check and generate JSON dump files
The rules after a successful test are good opportunity to test
`nft -j list ruleset` and `nft -j --check`. This quite possibly touches
code paths that are not hit by other tests yet.
The only downside is the increase of the test runtime (which seems
negligible, given the benefits of increasing test coverage).
Future commits will generate and commit those ".json-nft" dump files.
"DUMPGEN=y" will, like before, regenerate only the existing
"*.{nodump,nft,json-nft}" files (unless a test has none of the 3 files,
in which case they are all generated and the user is suggested to commit
the correct ones). Now also "DUMPGEN=all" is honored, that will generate
all 3 files, regardless of whether they already existed. That is useful
if you start out with a test that only has a .nft file, and then you
want to generate a .json-nft file too.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Thomas Haller [Tue, 14 Nov 2023 15:29:25 +0000 (16:29 +0100)]
json: fix use after free in table_flags_json()
Add `$NFT -j list ruleset` to the end of "tests/shell/testcases/transactions/table_onoff".
Then valgrind will find this issue:
$ make -j && ./tests/shell/run-tests.sh tests/shell/testcases/transactions/table_onoff -V
Gives:
==286== Invalid read of size 4
==286== at 0x49B0261: do_dump (dump.c:211)
==286== by 0x49B08B8: do_dump (dump.c:378)
==286== by 0x49B08B8: do_dump (dump.c:378)
==286== by 0x49B04F7: do_dump (dump.c:273)
==286== by 0x49B08B8: do_dump (dump.c:378)
==286== by 0x49B0E84: json_dump_callback (dump.c:465)
==286== by 0x48AF22A: do_command_list_json (json.c:2016)
==286== by 0x48732F1: do_command_list (rule.c:2335)
==286== by 0x48737F5: do_command (rule.c:2605)
==286== by 0x48A867D: nft_netlink (libnftables.c:42)
==286== by 0x48A92B1: nft_run_cmd_from_buffer (libnftables.c:597)
==286== by 0x402CBA: main (main.c:533)
Fixes: e70354f53e9f ("libnftables: Implement JSON output support") Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Split test in two, one for interval sets and another with concatenation
+ intervals, so at least intervals are tested in older kernels with no
pipapo support.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Wed, 8 Nov 2023 18:22:20 +0000 (19:22 +0100)]
netlink: fix buffer size for user data in netlink_delinearize_chain()
The correct define is NFTNL_UDATA_CHAIN_MAX and not NFTNL_UDATA_OBJ_MAX.
In current libnftnl, they both are defined as 1, so (with current libnftnl)
there is no difference.
Fixes: 702ac2b72c0e ("src: add comment support for chains") Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Tue, 24 Oct 2023 09:57:10 +0000 (11:57 +0200)]
src: remove xfree() and use plain free()
xmalloc() (and similar x-functions) are used for allocation. They wrap
malloc()/realloc() but will abort the program on ENOMEM.
The meaning of xmalloc() is that it wraps malloc() but aborts on
failure. I don't think x-functions should have the notion, that this
were potentially a different memory allocator that must be paired
with a particular xfree().
Even if the original intent was that the allocator is abstracted (and
possibly not backed by standard malloc()/free()), then that doesn't seem
a good idea. Nowadays libc allocators are pretty good, and we would need
a very special use cases to switch to something else. In other words,
it will never happen that xmalloc() is not backed by malloc().
Also there were a few places, where a xmalloc() was already "wrongly"
paired with free() (for example, iface_cache_release(), exit_cookie(),
nft_run_cmd_from_buffer()).
Or note how pid2name() returns an allocated string from fscanf(), which
needs to be freed with free() (and not xfree()). This requirement
bubbles up the callers portid2name() and name_by_portid(). This case was
actually handled correctly and the buffer was freed with free(). But it
shows that mixing different allocators is cumbersome to get right. Of
course, we don't actually have different allocators and whether to use
free() or xfree() makes no different. The point is that xfree() serves
no actual purpose except raising irrelevant questions about whether
x-functions are correctly paired with xfree().
Note that xfree() also used to accept const pointers. It is bad to
unconditionally for all deallocations. Instead prefer to use plain
free(). To free a const pointer use free_const() which obviously wraps
free, as indicated by the name.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Tue, 24 Oct 2023 09:57:09 +0000 (11:57 +0200)]
src: add free_const() and use it instead of xfree()
Almost everywhere xmalloc() and friends is used instead of malloc().
This is almost everywhere paired with xfree().
xfree() has two problems. First, it brings the wrong notion that
xmalloc() should be paired with xfree(), as if xmalloc() would not use
the plain malloc() allocator. In practices, xfree() just wraps free(),
and it wouldn't make sense any other way. xfree() should go away. This
will be addressed in the next commit.
The problem addressed by this commit is that xfree() accepts a const
pointer. Paired with the practice of almost always using xfree() instead
of free(), all our calls to xfree() cast away constness of the pointer,
regardless whether that is necessary. Declaring a pointer as const
should help us to catch wrong uses. If the xfree() function always casts
aways const, the compiler doesn't help.
There are many places that rightly cast away const during free. But not
all of them. Add a free_const() macro, which is like free(), but accepts
const pointers. We should always make an intentional choice whether to
use free() or free_const(). Having a free_const() macro makes this very
common choice clearer, instead of adding a (void*) cast at many places.
Note that we now pair xmalloc() allocations with a free() call (instead
of xfree(). That inconsistency will be resolved in the next commit.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Tue, 24 Oct 2023 09:57:08 +0000 (11:57 +0200)]
gmputil: add nft_gmp_free() to free strings from mpz_get_str()
mpz_get_str() (with NULL as first argument) will allocate a buffer using
the allocator functions (mp_set_memory_functions()). We should free
those buffers with the corresponding free function.
Add nft_gmp_free() for that and use it.
The name nft_gmp_free() is chosen because "mini-gmp.c" already has an
internal define called gmp_free(). There wouldn't be a direct conflict,
but using the same name is confusing. And maybe our own defines should
have a clear nft prefix.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
evaluate: place byteorder conversion before rshift in payload expressions
Use the key from the evaluation context to perform the byteorder
conversion in case that this expression is used for lookups and updates
on explicit sets.
Simple matches on values and implicit sets rely on the binary transfer
mechanism to propagate the shift to the constant, no explicit byteorder
is required in such case.
Fixes: 668c18f67203 ("evaluate: place byteorder conversion before rshift in payload statement") Reported-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
evaluate: reset statement length context only for set mappings
map expression (which is used a key to look up for the mapping) needs to
consider the statement length context, otherwise incorrect bytecode is
generated when {ct,meta} statement is generated.
# nft -f - <<EOF
add table ip6 t
add chain ip6 t c
add map ip6 t mapv6 { typeof ip6 dscp : meta mark; }
EOF
# nft -d netlink add rule ip6 t c meta mark set ip6 dscp map @mapv6
ip6 t c
[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
... missing byteorder conversion here before shift ...
[ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
[ lookup reg 1 set mapv6 dreg 1 ]
[ meta set mark with reg 1 ]
Reset statement length context only for the mapping side for the
elements in the set.
Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand") Reported-by: Brian Davidson <davidson.brian@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
meta: fix hour decoding when timezone offset is negative
Brian Davidson says:
meta hour rules don't display properly after being created when the
hour is on or after 00:00 UTC. The netlink debug looks correct for
seconds past midnight UTC, but displaying the rules looks like an
overflow or a byte order problem. I am in UTC-0400, so today, 20:00
and later exhibits the problem, while 19:00 and earlier hours are
fine.
meta.c only ever worked when the delta to UTC is positive.
We need to add in case the second counter turns negative after
offset adjustment.
Also add a test case for this.
Fixes: f8f32deda31d ("meta: Introduce new conditions 'time', 'day' and 'hour'") Reported-by: Brian Davidson <davidson.brian@gmail.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 2 Nov 2023 13:48:10 +0000 (14:48 +0100)]
tproxy: Drop artificial port printing restriction
It does not make much sense to omit printing the port expression if it's
not a value expression: On one hand, input allows for more advanced
uses. On the other, if it is in-kernel, best nft can do is to try and
print it no matter what. Just ignoring ruleset elements can't be
correct.
Fixes: 2be1d52644cf7 ("src: Add tproxy support") Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1721 Signed-off-by: Phil Sutter <phil@nwl.cc> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
This has several problems, which we an avoid with a single Makefile:
- recursive-make is harder to maintain and understand as a whole.
Recursive-make makes sense, when there are truly independent
sub-projects. Which is not the case here. The project needs to be
considered as a whole and not one directory at a time. When
we add unit tests (which we should), those would reside in separate
directories but have dependencies between directories. With a single
Makefile, we see all at once. The build setup has an inherent complexity,
and that complexity is not necessarily reduced by splitting it into more files.
On the contrary it helps to have it all in once place, provided that it's
sensibly structured, named and organized.
- typing `make` prints irrelevant "Entering directory" messages. So much
so, that at the end of the build, the terminal is filled with such
messages and we have to scroll to see what even happened.
- with recursive-make, during build we see:
make[3]: Entering directory '.../nftables/src'
CC meta.lo
meta.c:13:2: error: #warning hello test [-Werror=cpp]
13 | #warning hello test
| ^~~~~~~
With a single Makefile we get
CC src/meta.lo
src/meta.c:13:2: error: #warning hello test [-Werror=cpp]
13 | #warning hello test
| ^~~~~~~
This shows the full filename -- assuming that the developer works from
the top level directory. The full name is useful, for example to
copy+paste into the terminal.
- single Makefile is also faster:
$ make && perf stat -r 200 -B make -j
I measure 35msec vs. 80msec.
- recursive-make limits parallel make. You have to craft the SUBDIRS= in
the correct order. The dependencies between directories are limited,
as make only sees "LDADD = $(top_builddir)/src/libnftables.la" and
not the deeper dependencies for the library.
- I presume, some people like recursive-make because of `make -C $subdir`
to only rebuild one directory. Rebuilding the entire tree is already very
fast, so this feature seems not relevant. Also, as dependency handling
is limited, we might wrongly not rebuild a target. For example,
make check
touch src/meta.c
make -C examples check
does not rebuild "examples/nft-json-file".
What we now can do with single Makefile (and better than before), is
`make examples/nft-json-file`, which works as desired and rebuilds all
dependencies.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Not sure why this starts to fail. But arguably the command line
arguments were wrong. Fix it, we need a pseudo name for the device.
Fixes: df6f1a3e0803 ("tests/shell: bind mount private /var/run/netns in test container") Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Florian Westphal [Tue, 24 Oct 2023 10:37:47 +0000 (12:37 +0200)]
check-tree.sh: check and flag /bin/sh usage
Almost all shell tests use /bin/bash already.
In some cases we've had shell tests use /bin/sh, but still having
a bashism. This causes failures on systems where sh is dash or another,
strict bourne shell.
Thomas Haller [Mon, 23 Oct 2023 17:00:47 +0000 (19:00 +0200)]
parser_bison: fix length check for ifname in ifname_expr_alloc()
IFNAMSIZ is 16, and the allowed byte length of the name is one less than
that. Fix the length check and adjust a test for covering the longest
allowed interface name.
This is obviously a change in behavior, because previously interface
names with length 16 were accepted and were silently truncated along the
way. Now they are rejected as invalid.
Fixes: fa52bc225806 ("parser: reject zero-length interface names") Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Mon, 23 Oct 2023 17:00:46 +0000 (19:00 +0200)]
tests/shell: cover long interface name in "0042chain_variable_0" test
IFNAMSIZ is 16. Adjust "0042chain_variable_0" to use an interface name
with the maximum allowed bytes length.
Instead of adding an entirely different test, adjust an existing one to
use another interface name. The aspect for testing for a long interface
name is not special enough, to warrant a separate test. We can cover it
by extending an existing test.
Note that the length check in "parser_bison.y" is wrong. The test checks
still for the wrong behavior and that "d23456789012345x" is accepted.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Mon, 23 Oct 2023 16:13:16 +0000 (18:13 +0200)]
tools: reject unexpected files in "tests/shell/testcases/" with "check-tree.sh"
"check-tree.sh" does consistency checks on the source tree. Extend
the check to flag more unexpected files. We don't want to accidentally
have left over files.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Mon, 23 Oct 2023 16:13:15 +0000 (18:13 +0200)]
tests/shell: inline input data in "single_anon_set" test
The file "optimizations/dumps/single_anon_set.nft.input" was laying
around, and it was unclear how it was used.
Let's extend "check-patch.sh" to flag all unused files. But the script
cannot understand how "single_anon_set.nft.input" is used (aside allow
listing it).
Instead, inline the script to keep it inside the test (script).
We still write the data to a separate file and don't use `nft -f -`
(because reading stdin uses a different code path we want to cover).
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Mon, 23 Oct 2023 13:38:18 +0000 (15:38 +0200)]
tests/shell: test for maximum length of "comment" in "comments_objects_0"
The comment length is limited to NFTNL_UDATA_COMMENT_MAXLEN. Test for
that.
Adjust an existing test for that.
Also rename $EXPECTED to $RULESET. We don't compare the value of
$EXPECTED against the actually configured rules. It also wouldn't work,
because the input is not normalized and wouldn't match. It also isn't
necessary, because there is a .nft dump file.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This is an inconsistency. The test should have either a .nft or a
.nodump file. "./tools/check-tree.sh" enforces that and will in the
future run by `make check`.
Fixes: 22fab8681a50 ("parser_bison: Fix for broken compatibility with older dumps") Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Mon, 16 Oct 2023 19:43:08 +0000 (21:43 +0200)]
tests/shell: honor NFT_TEST_VERBOSE_TEST variable to debug tests via `bash -x`
It can be cumbersome to debug why a test fails. Our tests are just shell
scripts, which for the most part don't print much. That is good, but for
debugging, it can be useful to run the test via `bash -x`. Previously,
we would just patch the source file while debugging.
Add an option "-x" and NFT_TEST_VERBOSE_TEST=y environment variable. If set,
"test-wrapper.sh" will check whether the shebang is "#!/bin/bash" and add
"-x" to the command line.
While at it, let test-wrapper.sh also log a line like
Command: $CMD
With this, we see in the log the command that was run, and how
NFT_TEST_VERBOSE_TEST may have affected it. This is anyway useful,
because many tests don't print anything at all, and we end up with an
empty "testout.log". Empty files are cumbersome, e.g. I like to use
`grep -R ^` to show the content of all files, which does not show empty
files. Ensuring that something is always written is desirable.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Phil Sutter [Thu, 19 Oct 2023 16:40:04 +0000 (18:40 +0200)]
parser_bison: Fix for broken compatibility with older dumps
Commit e6d1d0d611958 ("src: add set element multi-statement
support") changed the order of expressions and other state attached to set
elements are expected in input. This broke parsing of ruleset dumps
created by nft commands prior to that commit.
Restore compatibility by also accepting the old ordering.
Fixes: e6d1d0d611958 ("src: add set element multi-statement support") Signed-off-by: Phil Sutter <phil@nwl.cc>
evaluate: validate maximum log statement prefix length
Otherwise too long string overruns the log prefix buffer.
Fixes: e76bb3794018 ("src: allow for variables in the log prefix string") Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1714 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thomas Haller [Mon, 16 Oct 2023 13:30:10 +0000 (15:30 +0200)]
tests/shell: use bash instead of /bin/sh for tests
All tests under "tests/shell" are shell scripts with shebang /bin/bash
or /bin/sh. This may seem expected, since these tests are under
"tests/shell" directory, but any executable file would work.
Anyway. The vast majority of the tests has "#!/bin/bash" as shebang.
A few tests had "#!/bin/sh" or "#!/bin/sh -e". Unify this and always use bash.
Since we anyway require bash, this is not a limitation.
Also, if we know that this is a bash script (by parsing the shebang), we
can let the test wrapper pass "-x" to the script. The next commit will
do that, and it is nicer if the shebangs are all uniform.
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
This is an inconsistency. The test should have either a .nft or a
.nodump file. "./tools/check-tree.sh" enforces that and will in the
future run by `make check`.
Fixes: 74cf3d16d8e9 ('tests: shell: add vlan match test case') Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
Florian Westphal [Fri, 13 Oct 2023 10:37:21 +0000 (12:37 +0200)]
evaluate: suggest != in negation error message
when I run sudo nft insert rule filter FORWARD iifname "ens2f1" ip saddr not @ip_macs counter drop comment \" BLOCK ALL NON REGISTERED IP/MACS \"
I get: Error: negation can only be used with singleton bitmask values
And even I did not spot the problem immediately.
I don't think "not" should have been added, its easily confused with
"not equal"/"neq"/!= and hides that this is allegedly a binop.
At least *mention* that the commandline is asking for a binary
operation here and suggest "!=".
Thomas Haller [Fri, 6 Oct 2023 09:42:20 +0000 (11:42 +0200)]
tests/shell: add "-S|--setup-host" option to set sysctl for rootless tests
Most tests can run just fine without root. A few of them will fail if
/proc/sys/net/core/{wmem_max,rmem_max} is too small (as it is by default
on the host).
The easy workaround is to bump those limits once. This has to be
repeated after each reboot.
Doing that manually (every time) is cumbersome. Add a "--setup-host"
option for that.
Usage:
$ sudo ./tests/shell/run-tests.sh -S
Setting up host for running as rootless (requires root).
echo 4096000 > /proc/sys/net/core/rmem_max (previous value 100000)
echo 4096000 > /proc/sys/net/core/wmem_max (previous value 100000)
Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
projects / thirdparty / nftables.git / log
