Commit graph

1282 commits

Author SHA1 Message Date
David Gibson
41c02e10db tests: Use nmap-ncat instead of openbsd netcat for pasta tests
A number of the testcases use options specific the OpenBSD version of
netcat.  That's available in Debian, but not easily available in Fedora.
Switch the pasta tests to using the nmap version of netcat (a.k.a. ncat).
This is easily available in both Debian and Fedora, and appears to be a
bit more modern and maintained as well.

ncat generally requires explicit listen addresses (which is good for
clarity anywhere).  Its default options appear to remove the need for the
-N and -q options.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: changed one ncat listening address to IPv6 loopback]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-06-18 09:05:06 +02:00
David Gibson
e48373382f Use dhclient instead of udhcpc
For some reason, the passt/pasta tests and examples use dhclient for
DHCPv6, but in most cases use udhcpc for DHCPv4.  Change it to use dhclient
for both DHCPv4 and DHCPv6.  This means one less tool we need for testing,
plus dhclient is easily available on Fedora whereas udhcpc is not.

Note that the passt tests still rely on udhcpc indirectly because mbuto
wants to put it into the guest images it generates.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-06-15 09:38:10 +02:00
David Gibson
ca0c33ae5b Tweak dhclient arguments for readability
A number of tests and examples use dhclient in both IPv4 and IPv6 modes.
We use "dhclient -6" for IPv6, but usually just "dhclient" for IPv4.  Add
an explicit "-4" argument to make it more clear and explicit.

In addition, when dhclient is run from within pasta it usually won't be
"real" root, and so will not have access to write the default global pid
file.  This results in a mostly harmless but irritating error:
    Can't create /var/run/dhclient.pid: Permission denied
We can avoid that by using the --no-pid flag to dhclient.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-06-15 09:38:10 +02:00
David Gibson
2320ac3349 Don't abbreviate ip(8) arguments in examples and tests
ip(8)'s ability to take abbreviated arguments (e.g. "li sh" instead of
"link show") is very handy when using it interactively, but it doesn't make
for very readable scripts and examples when shown that way.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-06-15 09:38:10 +02:00
David Gibson
6703da44c1 tests: Use more explicit netcat options for distro/fedora tests
distro/fedora contains two versions of the basic tests, used for different
Fedora versions.  One uses explicit listening address for netcat in some
extra places, the other does not.  Apparently the older netcat versions
didn't require the explicit addresses.  Not supplying addresses doesn't
test anything useful though, just a detail in netcat's behaviour.  So,
it's cleaner to just always supply explicit addresses.

In addition, we're explicitly expecting the nmap version of ncat, also
known as "ncat".  So, it's more explicit what we're after if we invoke it
via that name rather than "nc", which will go via an /etc/alternatives
link.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: Fix port argument in distro_quick_pasta_test{,_fedora34} too]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-06-15 09:37:03 +02:00
Stefano Brivio
d7d467f60c README: Fix links to static builds
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-06-08 11:17:59 +02:00
Stefano Brivio
deda03bfc2 tcp: Silence warning from gcc 11.3 with -Ofast
If the first packet_get() call doesn't assign len, the second one
will also return NULL, but gcc doesn't see this.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-06-08 11:08:29 +02:00
Stefano Brivio
fb59cfc909 contrib/fedora: Use pre-processing macros in spec file
...they seem to be supported by COPR now and make things simpler.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-06-08 09:43:48 +02:00
Stefano Brivio
bb8a7b8a2a contrib/fedora: Drop dashes from version
COPR doesn't like them, and I'm trying to build packages there now.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-06-07 17:06:09 +02:00
Stefano Brivio
67103ea556 conf: Fix one Coverity CID 258163 warning, work around another one
In conf_runas(), Coverity reports that we might dereference uid and
gid despite possibly being NULL (CWE-476) because of the check after
the first sscanf(). They can't be NULL, but I actually wanted to
check that UID and GID are non-zero (the user could otherwise pass
--runas root:root and defy the whole mechanism).

Later on, we have the same type of warning for 'gr': it's compared
against NULL, so it might be NULL, which is actually the case: but
in that case, we don't dereference it, because we'll return -ENOENT
right away. Rewrite the clause to silence the warning.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-20 10:50:35 +02:00
Stefano Brivio
d27cc3e435 tcp: Work around gcc 12 bogus warning in tcp_rtt_dst_check()
gcc 12.1.x (e.g. current OpenSUSE Tumbleweed, x86_64 only,
gcc-12-1.4.x86_64) reports:

tcp.c: In function ‘tcp_send_flag’:
tcp.c:1014:9: warning: writing 16 bytes into a region of size 0 [-Wstringop-overflow=]
 1014 |         memcpy(low_rtt_dst + hole++, &conn->a.a6, sizeof(conn->a.a6));
      |         ^
tcp.c:559:24: note: at offset -16 into destination object ‘low_rtt_dst’ of size 128
  559 | static struct in6_addr low_rtt_dst[LOW_RTT_TABLE_SIZE];
      |

but 'hole' can't be -1, because the low_rtt_dst table is guaranteed
to have a hole: if we happened to write to the last entry, we'll go
back to index 0 and clear that one.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-20 10:36:11 +02:00
Stefano Brivio
a951e0b9ef conf: Add --runas option, changing to given UID and GID if started as root
On some systems, user and group "nobody" might not be available. The
new --runas option allows to override the default "nobody" choice if
started as root.

Now that we allow this, drop the initgroups() call that was used to
add any additional groups for the given user, as that might now
grant unnecessarily broad permissions. For instance, several
distributions have a "kvm" group to allow regular user access to
/dev/kvm, and we don't need that in passt or pasta.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 16:27:20 +02:00
Stefano Brivio
c318ffcb4c udp: Ignore bogus -Wstringop-overread for write() from gcc 12.1
With current OpenSUSE Tumbleweed on aarch64 (gcc-12-1.3.aarch64) and
on x86_64 (gcc-12-1.4.x86_64), but curiously not on armv7hl
(gcc-12-1.3.armv7hl), gcc warns about using the _pointer_ to the
802.3 header to write the whole frame to the tap descriptor:
  reading between 62 and 4294967357 bytes from a region of size 14

which is bogus:
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103483

Probably declaring udp_sock_fill_data_v{4,6}() as noinline would
"fix" this, but that's on the data path, so I'd rather not. Use
a gcc pragma instead.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 16:27:20 +02:00
Stefano Brivio
3d4c2a44a6 tests: Don't check exit code for every command in demo mode
Having all those 'echo $?' is rather distracting in demos.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Sefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:22 +02:00
Stefano Brivio
fe536d78ba tests: Don't count number of test units for demos
...there are no 'test' directives in demo, and this causes a
script failure.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:19 +02:00
Stefano Brivio
c447344158 demo/pasta: Fix bad sleep directive
'sleep' always needs an argument, this was meant to introduce
a 2 seconds delay.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:15 +02:00
Stefano Brivio
b4d41ac4b6 test/run: Return 0 from run(), exit value already reflects failures
There's no need to return non-zero if there have been failures in
run(), because the exit value is already determined from the number
of failures reported in the log file.

Return zero, so that this doesn't cause the script to fail, given we
now run it with -e.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:15 +02:00
Stefano Brivio
cfa8a78a39 test/perf/pasta_udp: Drop redundant assignment of ::1 to loopback interface
There are a few occurrences of this assignment, which are needed to
re-add ::1 as loopback address after the MTU has been increased
back from a value below 1280 bytes.

This one, however, is redundant, and causes an error in the
execution.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:15 +02:00
David Gibson
21e9cf7b95 tests: Simplify explicit checks for command success
A number of individual test cases use '*out' commands to check for success
of specific commands they've issued.  Now that the test harness is testing
for success of all issued commands as a matter of course, we no longer need
to do this.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-05-19 15:24:15 +02:00
David Gibson
4643b9b74c tests: Simplify *tools commands using pane_status
Now that we have pane_status to check the success of commands issued to
panes, we can more easily check for the success of the 'which' commands
used to check tool availability, rather than constructing, then parsing
special "skip" output.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-05-19 15:24:15 +02:00
David Gibson
13ad716f30 tests: Add pane_status command to check for success of issued commands
When we use pane_wait to wait for a command issued to a tmux pane to finish
we have no idea whether the command succeeded or not.  This means that the
test scripts can keep running long after the point something vital has
failed, making it difficult to work out what went wrong.

Add a new pane_status command that checks for success of the issued command
and use it in most places instead of pane_wait.  We still need explicit
pane_wait where we're gathering explicit output with pane_parse, because
the way we check the status with 'echo $?' means we lose track of that
output.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio:
 - instead of quitting the script, make a test fail if a command
    issued in a pane fails during a test, and loop until the status code is
    numeric in pane_status() as a hack to make it a bit more robust
 - retain usage of pane_wait() in iperf3 and teardown functions as we
   interrupt iperf3, passt, and pasta, so a non-zero exit code is expected
 - drop bogus ns_{1,2}_wait() calls in teardown_two_guests(), those
   functions were never implemented
 - use pane_status() for "guest" test directives too
]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:15 +02:00
David Gibson
3e0641f91f tests: Don't ignore errors during script
Most commands issued during the testing scripts aren't explicitly checked
for errors.  Therefore, if they fail, the shell will just keep on
executing.  This makes it difficult to figure out where things started
going wrong if things fall over.

Run the whole script with the set -e mode so that it will exit in the case
of any (unchecked) failing command.  To make this work we do need to add
explicit checks / fallbacks for some commands which we expect to fail.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: use sh -e instead of setting -e later, so that we don't miss
 anything before set -e is issued]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:15 +02:00
David Gibson
c638129a9e tests: Improve control character filtering in pane_parse
pane_parse() attempts to grab the output from the last command issued
into a tmux pane.  It strips out control characters using tr, which in
particular includes the final \r\n.  However, this won't fully strip
out terminal escape sequences.  In particular this breaks if the shell
in the pane is bash, with enable-bracketed-paste enabled in readline.
That issues terminal sequences to enable and disable bracketed paste
mode around every shell prompt.

We can work around this because these escapes are followed by a \r
(CR).  More generally, it seems reasonable to assume that any terminal
shenanigans followed by a CR, but not an LF is supposed to be hidden.
So, use sed to strip everything before the second last CR.  We still
need the tr to remove the final \r\n from the string (sed processes a
line at a time, and doesn't consider the CRLF part of the buffer it's
processing).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: modify regexp to keep foo\r\r\n unchanged, by matching on at
 least one CR and a non-CR afterwards: that's the usual output pattern
 for bash on Debian 8 and Debian 9]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:15 +02:00
David Gibson
ae83999b75 tests: Don't globally set tmux default-shell
run_term() uses tmux set-option -g to globally set the default shell.
Unfortunately this hits a chicken-and-egg problem that's common with many
of tmux's session options.  If there isn't already a tmux server running,
we can't connect to set the option.  If we attempt this after starting our
session (and therefore the server), then the session will already be
started with the previous default shell.

In any case it's not a good idea to set tmux global options, since that
might interfere with whatever else the user is doing in tmux.  So, instead
set the default-shell option locally to the session after starting it.  To
make sure we get the right shell for our initial script, explicitly invoke
/bin/sh to interpret it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-05-19 15:24:15 +02:00
David Gibson
14cbbd602c tests: Don't use tmux update-environment
The semantics of tmux's update-environment option are a bit confusing.
It says it means the given variables are copied into the session
environment from the source environment, but it's not entirely clear
what the "source" environment means.

From my experimentation it appeast to be the environment from which
the tmux *server* is launched, not the one issuing the 'new-session'
command.  That makes it pretty much useles, certainly in our case where
we have no way of knowing if the user has pre-existing tmux sessions.

Instead use the new-session -e option to explicitly pass in the variables
we want to propagate.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-05-19 15:24:15 +02:00
David Gibson
5f637a2060 tests: Add some debugging output for the test scripts themselves
The DEBUG option for tests/run enables debugging options to passt/pasta,
however that doesn't help with debugging the test scripts themselves, which
are fairly fragile.

Extend the DEBUG option so it also prints information on each command in
the test scripts to make it easier to work out where things are falling
over.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-05-19 15:24:15 +02:00
David Gibson
81c50acb21 tests: Remove unused XVFB variable
The XVFB variable is initialized at the beginning of test/run then never
used again.  I'm assuming it's a leftover from some ealier iteration.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-05-19 15:24:15 +02:00
Stefano Brivio
8e898ea71e tests: Update mbuto git URLs
The project is now at mbuto.sh, and git transport is enabled.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:24:15 +02:00
David Gibson
d3e842afc0 Add basic .gitignore files
Ignore various files generated during build or test.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-05-19 15:24:09 +02:00
Stefano Brivio
b164d257a1 qrap.1: Clarify it takes a qemu command, not a path
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-19 15:23:53 +02:00
Stefano Brivio
32210fb64f demo: podman: New port forwarding behaviour for pasta, minor fixes
Reflect the recent changes in the Podman adaptation (no port
forwarding by default).

It turns out that by running two iperf3 processes, sometimes
slirp4netns blocks the second connection until the first test is
done, thus doubling the throughput. Use a single process for
slirp4netns with slirp4netns port handling.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-02 23:14:37 +02:00
Stefano Brivio
bdcfe4749e contrib: podman: Add bound address configuration, update port specifications
Rebase the patch for Podman on top of current upstream, and:

- add support for configuration of specific addresses for forwarded
  ports

- by default, disable port forwarding, and reflect this in the man
  page changes

- adjust processing to a new, incompatible format for port storage,
  which I couldn't actually track down to a specific commit, but
  that resulted in https://github.com/containers/podman/issues/13643
  and commit eedaaf33cdbf ("fix slirp4netns port forwarding with
  ranges")

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-02 21:46:13 +02:00
Stefano Brivio
3f2e7098ac netlink: In nl_addr() and nl_route(), don't return before set request
Fixes: 22ed4467a4 ("treewide: Unchecked return value from library, CWE-252")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-01 07:19:05 +02:00
Stefano Brivio
3c6ae62510 conf, tcp, udp: Allow address specification for forwarded ports
This feature is available in slirp4netns but was missing in passt and
pasta.

Given that we don't do dynamic memory allocation, we need to bind
sockets while parsing port configuration. This means we need to
process all other options first, as they might affect addressing and
IP version support. It also implies a minor rework of how TCP and UDP
implementations bind sockets.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-01 07:19:05 +02:00
Stefano Brivio
df69be379e tcp_splice: Allow up to 8 MiB as pipe size
It actually improves throughput a bit, if allowed by user limits.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 14:28:27 +02:00
Stefano Brivio
6e9464ff0a test/lib: Add small delay before trying to parse output
Don't fetch the log too early, we might get output from previous
commands.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
a20d269630 test/distro: Set unprivileged_userns_clone on Debian Buster and earlier
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
081f7c8f4c test/lib: Consistent cols, rows, poster attributes for asciinema player
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
0bf6adc886 arch: Pointer to local outside scope, CWE-562
Reported by Coverity: if we fail to run the AVX2 version, once
execve() fails, we had already replaced argv[0] with the new
stack-allocated path string, and that's then passed back to
main(). Use a static variable instead.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
2b1fbf4631 udp: Out-of-bounds read, CWE-125 in udp_timer()
Not an actual issue due to how it's typically stored, but udp_act
can also be used for ports 65528-65535. Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
5ab2e12f98 tcp: False "Out-of-bounds read" positive, CWE-125
Reported by Coverity: it doesn't see that tcp{4,6}_l2_buf_used are
set to zero by tcp_l2_data_buf_flush(), repeat that explicitly here.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
2a3b8dad33 tcp, tcp_splice: False "Negative array index read" positives, CWE-129
A flag or event bit is always set by callers. Reported by Coverity.

Signed-by-off: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
264d68edcf tcp_splice: Logically dead code, CWE-561
Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
71a00f1449 tcp: Dereference null return value, CWE-476
Not an issue with a sane kernel behaviour. Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
ceddcac74a conf, tap: False "Buffer not null terminated" positives, CWE-170
Those strings are actually guaranteed to be NULL-terminated. Reported
by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
e46f67f152 conf: False "Assign instead of compare" positive, CWE-481
This really just needs to be an assignment before line_read() --
turn it into a for loop. Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
eb3d3f367e treewide: Argument cannot be negative, CWE-687
Actually harmless. Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
bb76470090 passt: Improper use of negative value (CWE-394)
Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
0786b2e60a conf, packet: Operands don't affect result, CWE-569
Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
48bc843d6e tap: Resource leak, CWE-404
Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00