1
0
mirror of https://passt.top/passt synced 2024-11-09 14:19:56 +00:00
Commit Graph

1465 Commits

Author SHA1 Message Date
David Gibson
a668d87e7e Don't shadow global function names
cppcheck points out that qrap's main shadows the global err() function with
a local.  Rename it to rc to avoid the clash.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:22:07 +02:00
David Gibson
ab96da98cd Don't shadow 'i' in conf_ports()
The counter 'i' is used in a number of places in conf_ports(), but in one
of those we unnecessarily shadow it in an inner scope.  We could re-use the
same 'i' every time, but each use is logically separate, so instead remove
the outer declaration and declare it locally in each of the clauses where
we need it.

While we're there change it from a signed to unsigned int, since it's used
to iterate over port numbers which are generally treated as unsigned.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:22:05 +02:00
David Gibson
eb5e123038 cppcheck: Reduce scope of some variables
Minor style improvement suggested by cppcheck.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:22:01 +02:00
David Gibson
68ef4931cb Clean up parsing in conf_runas()
conf_runas() handles several of the different possible cases for the
--runas argument in a slightly odd order.  Although it can parse both
numeric UIDs/GIDs and user/group names, it can't parse a numeric UID
combined with a group name or vice versa.  That's not obviously useful, but
it's slightly surprising gap to have.

Rework the parsing to be more systematic: first split the option into
user and (optional) group parts, then separately parse each part as either
numeric or a name.  As a bonus this removes some clang-tidy warnings.

While we're there also add cppcheck suppressions for getpwnam() and
getgrnam().  It complains about those because they're not reentrant.
passt is single threaded though, and is always likely to be during
this initialization code, even if we multithread later.

There were some existing suppressions for these in the cppcheck
invocation but they're no longer up to date.  Replace them with inline
suppressions which, being next to the code, are more likely to stay
correct.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:58 +02:00
David Gibson
7d4e50827c Pack DHCPv6 "on wire" structures
dhcpv6.c contains a number of structures which represent actual DHCPv6
packets as they appear on the wire, which will break if the structures
don't have exactly the in-memory layout we expect.

Therefore, we should mark these structures as ((packed)).  The contents of
them means this is unlikely to change the layout in practice - and since
it was working, presumably didn't on any arch we were testing on.  However
it's not impossible for the compiler on some arch to insert unexpected
padding in one of these structures, so we should be explicit.

clang-tidy warned about this since we were using memcmp() to compare some
of these structures, which it thought might not have a unique
representation.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:55 +02:00
David Gibson
8534be076c Catch failures when installing signal handlers
Stop ignoring the return codes from sigaction() and signal().  Unlikely to
happen in practice, but if it ever did it could lead to really hard to
debug problems.  So, take clang-tidy's advice and check for errors here.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:53 +02:00
David Gibson
8a19f36864 clang-tidy: Remove duplicate #include from icmp.c
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:50 +02:00
David Gibson
5823dc5c68 clang-tidy: Fix spurious null pointer warning in pasta_start_ns()
clang-tidy isn't quite clever enough to figure out that getenv("SHELL")
will return the same thing both times here, which makes it conclude that
shell could be NULL, causing problems later.

It's a bit ugly that we call getenv() twice in any case, so rework this in
a way that clang-tidy can figure out shell won't be NULL.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:48 +02:00
David Gibson
798b7ff1c0 clang-tidy: Suppress warning about unchecked error in logfn macro
clang-tidy complains that we're not checking the result of vfprintf in
logfn().  There's not really anything we can do if this fails here, so just
suppress the error with a cast to void.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:45 +02:00
David Gibson
84fec4e998 Clean up parsing of port ranges
conf_ports() parses ranges of ports for the -t, -u, -T and -U options.
The code is quite difficult to the follow, to the point that clang-tidy
and cppcheck disagree on whether one of the pointers can be NULL at some
points.

Rework the code with the use of two new helper functions:
  * parse_port_range() operates a bit like strtoul(), but can parse a whole
    port range specification (e.g. '80' or '1000-1015')
  * next_chunk() does the necessary wrapping around strchr() to advance to
    just after the next given delimiter, while cleanly handling if there
    are no more delimiters

The new version is easier to follow, and also removes some cppcheck
warnings.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:41 +02:00
David Gibson
a1a058533c cppcheck: Add target specific headers
Debian and similar distros put target specific header files in
/usr/include/<arch-vendor-os>, rather than directly in /usr/include.  Add
this directory to the includes for cppcheck so it can find them.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:39 +02:00
David Gibson
6d171cb357 Makefile: Simplify getting target triple for compiler
We do some manipulation of the output of cc -v to get the target triple
for the platform, to locate headers for cppcheck.  However, we can get
this more easily with cc -dumpmachine - and in fact we do so elsewhere in
the Makefile.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:37 +02:00
David Gibson
2e7f9da6d1 cppcheck: Run quietly
Adding the --quiet option to cppcheck makes the actual errors and warnings
easier to find.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:34 +02:00
David Gibson
d0629ff2b2 cppcheck: Avoid excessive scanning due to system headers
make cppcheck takes a long time, because it checks a large number of
different configurations.  It's assembling this very large set of
configurations not because of conditionals in the passt code itself,
but from those in the system headers.  By adding --config-exclude
directives to stop considering those configs, make cppcheck becomes
around 60x faster on my system.

Similarly, any problems that are found in the system headers are not our
problem, and so we can uniformly suppress them, rather than having specific
suppressions for particular problems in particular files (which might not
be correct for all different distro / version combinations either).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:30 +02:00
David Gibson
1fcce70caa clang-tidy: Disable 'readability-identifier-length'
This check complains about any identifier of less than 3 characters.  For
locals and parameters this is often pointlessly verbose.  Disable it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:27 +02:00
David Gibson
dc3f200a0b test: Remove unneccessary pane naming from layout_two_guests
This loop goes through and gives a numeric label to each pane, even though
we name the panes properly shortly thereafter.  Looks like a leftover from
some earlier version.  Remove it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:23 +02:00
David Gibson
feb8946ff5 test: Simplify data handling for transfer tests
Many of our tests are based around performing transfers of sample data
across passt/pasta created links.  The data flow here can be a bit
hard to follow since, e.g. we create a file transfer it to the guest,
then transfer it back to the host across several different tests.
This also means that the test cases aren't independent of each other.

Because we don't have the original file available at both ends in some
cases, we compare them by generating md5sums at each end and comparing
them, which is a bit complicated.

Make a number of changes to simplify this:
  1. Pre-generate the sample data files as a test asset, rather than
     building them on the fly during the tests proper
  2. Include the sample data files in the mbuto guest image
  3. Because we have good copies of the original data available in all
     contexts, we can now simply use 'cmp' to check if the transfer
     has worked, avoiding md5sum complications.
  4. Similarly we can always use the original copy of the sample data
     on the send side of each transfer, meaning that the tests become
     more independent of each other.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:19 +02:00
David Gibson
0a15b467d4 test: Use --config-net for namespace setup
The setup functions for passt_in_ns and two_guests perform some fairly slow
dhclient calls to configure the network in the namespace before starting
the guest.  This isn't really part of the tests, just necessary for the
operations later.

We can simplify and speed this up a bit by using pasta's '--config-net'
option to configure the networking for us.  As a bonus this means we have
at least a minimal test of the --config-net option itself.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:13 +02:00
David Gibson
5b899dce7a test: More robust wait for pasta/passt to be ready
When we start passt or pasta, it may take a short time to be ready to
handle packets, especially if running under valgrind.  We have a
number of semi-arbitrary fixed sleeps to account for this.

We can do this more robustly by exploiting the fact that pasta/passt
doesn't write its pidfile until it's ready to go, so if we wait for
the pidfile to be created, we can proceed with confidence.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:07 +02:00
David Gibson
33983de46b test: Remove unnecessary sleeps from shutdown tests
These are hangovers from older ways of shutting down the pasta/passt
processes and no longer serve any purpose.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:21:01 +02:00
David Gibson
05a2c7ae3c test: Add wait_for() shell helper
Add a shell helper function to wait for some command to succeed - typically
a test for something to be done by a background process.  Use it in the
context code which waits for the guest to respond to ssh-over-vsock
connections.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:20:44 +02:00
David Gibson
8978f6552b icmp: Correct off by one errors dealing with number of echo request ids
ICMP echo request and reply packets include a 16-bit 'id' value.  We have
some arrays indexed by this id value.  Unfortunately we size those arrays
with USHRT_MAX (65535) when they need to be sized by the total number of
id values (65536).  This could lead to buffer overruns.  Resize the arrays
correctly, using a new define for the purpose.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
d5b80ccc72 Fix widespread off-by-one error dealing with port numbers
Port numbers (for both TCP and UDP) are 16-bit, and so fit exactly into a
'short'.  USHRT_MAX is therefore the maximum port number and this is widely
used in the code.  Unfortunately, a lot of those places don't actually
want the maximum port number (USHRT_MAX == 65535), they want the total
number of ports (65536).  This leads to a number of potentially nasty
consequences:

 * We have buffer overruns on the port_fwd::delta array if we try to use
   port 65535
 * We have similar potential overruns for the tcp_sock_* arrays
 * Interestingly udp_act had the correct size, but we can calculate it in
   a more direct manner
 * We have a logical overrun of the ports bitmap as well, although it will
   just use an unused bit in the last byte so isnt harmful
 * Many loops don't consider port 65535 (which does mitigate some but not
   all of the buffer overruns above)
 * In udp_invert_portmap() we incorrectly compute the reverse port
   translation for return packets

Correct all these by using a new NUM_PORTS defined explicitly for this
purpose.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
3ede07aac9 Treat port numbers as unsigned
Port numbers are unsigned values, but we're storing them in (signed) int
variables in some places.  This isn't actually harmful, because int is
large enough to hold the entire range of ports.  However in places we don't
want to use an in_port_t (usually to avoid overflow on the last iteration
of a loop) it makes more conceptual sense to use an unsigned int. This will
also avoid some problems with later cleanups.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
0d1886dca0 Pass entire port forwarding configuration substructure to conf_ports()
conf_ports() switches on the optname argument to select the target array
for several updates.  Now that all these maps are in a common structure, we
can simplify by just passing in a pointer to the whole struct port_fwd to
update.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
f5a31ee94c Don't use indirect remap functions for conf_ports()
Now that we've delayed initialization of the UDP specific "reverse" map
until udp_init(), the only difference between the various 'remap' functions
used in conf_ports() is which array they target.  So, simplify by open
coding the logic into conf_ports() with a pointer to the correct mapping
array.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
1467a35b5a udp: Delay initialization of UDP reversed port mapping table
Because it's connectionless, when mapping UDP ports we need, in addition
to the table of deltas for destination ports needed by TCP, we need an
inverted table to translate the source ports on return packets.

Currently we fill out the inverted table at the same time we construct the
main table in udp_remap_to_tap() and udp_remap_to_init().  However, we
don't use either table until after we've initialized UDP, so we can delay
the construction of the reverse table to udp_init().  This makes the
configuration more symmetric between TCP and UDP which will enable further
cleanups.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
163dc5f188 Consolidate port forwarding configuration into a common structure
The configuration for how to forward ports in and out of the guest/ns is
divided between several different variables.  For each connect direction
and protocol we have a mode in the udp/tcp context structure, a bitmap
of which ports to forward also in the context structure and an array of
deltas to apply if the outward facing and inward facing port numbers are
different.  This last is a separate global variable, rather than being in
the context structure, for no particular reason.  UDP also requires an
additional array which has the reverse mapping used for return packets.

Consolidate these into a re-used substructure in the context structure.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
1128fa03fe Improve types and names for port forwarding configuration
enum conf_port_type is local to conf.c and is used to track the port
forwarding mode during configuration.  We don't keep it around in the
context structure, however the 'init_detect_ports' and 'ns_detect_ports'
fields in the context are based solely on this.  Rather than changing
encoding, just include the forwarding mode into the context structure.
Move the type definition to a new port_fwd.h, which is kind of trivial at
the moment but will have more stuff later.

While we're there, "conf_port_type" doesn't really convey that this enum is
describing how port forwarding is configured.  Rename it to port_fwd_mode.
The variables (now fields) of this type also have mildly confusing names
since it's not immediately obvious whether 'ns' and 'init' refer to the
source or destination of the packets.  Use "in" (host to guest / init to
ns) and "out" (guest to host / ns to init) instead.

This has the added bonus that we no longer have locals 'udp_init' and
'tcp_init' which shadow global functions.

In addition, add a typedef 'port_fwd_map' for a bitmap of each port number,
which is used in several places.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
Vasiliy Ulyanov
11e285df8f Fix the name of the qemu-system-* executable
Define the target machine architecture in lowercase.

The name of the executable qemu-system-* is defined from the build flags
and should be in lowercase:

    ( "qemu-system-" ARCH ),

I.e. qemu-system-x86_64 instead of qemu-system-X86_64. Otherwise, the
exec call will fail.

Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
2022-09-24 09:12:35 +02:00
Stefano Brivio
8338135777 README: Add missing parenthesis in Try It section
Signed-off-by: Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 09:00:22 +02:00
Stefano Brivio
9232065641 README: Drop excess whitespace in Try It section
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 08:59:16 +02:00
Stefano Brivio
16ad76d680 README: Add legend for Features section
As suggested by David: those emojis might not be entirely obvious.

Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:33:15 +02:00
Stefano Brivio
715677b699 README: Fix paragraph in Try It section of passt
The qemu patch isn't mentioned there anymore: replace reference with
a link.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:28:52 +02:00
Stefano Brivio
229b16cba3 README: Fix indentation in "Try It" section
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:23:38 +02:00
Stefano Brivio
be41639c20 README: Point openSUSE links to Dario's OBS repository
...instead of my Copr. It's also not official yet, but surely more
appropriate now.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:18:40 +02:00
Stefano Brivio
8b3443c561 README: Fix misspellings of openSUSE
For some reason, I used a capital O everywhere.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:14:47 +02:00
Stefano Brivio
e0f415c025 test/lib: Don't try to write to perf.js when running demos
...it doesn't actually exist, and this error now causes the demo to
stop.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:11:10 +02:00
Stefano Brivio
2e93cb6ed8 test/lib: Drop perf_report_append() from perf_report
It's not used anymore. While at it, fix the function name in the
comment to perf_report_append_js().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:09:23 +02:00
Stefano Brivio
b3549093f7 test/demo: Avoid using port 5201 on the host
That's the default port for iperf3, which also means that it's quite
likely in use on my test machine. Use different port numbers: recycle
the scheme we use in tests for passt and pasta's demo, use 5221-5224
(a bit shorter) for the slirp4netns container in Podman's demo.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:07:18 +02:00
Stefano Brivio
6d08bfc3e0 test/demo: Use relative paths to change directories when possible
A cd to __STATEDIR__ results in a rather long command, that's not
very readable. Jump between directories using .. and relative paths,
once we're there.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:05:20 +02:00
Stefano Brivio
57fa9dd4c9 hooks/pre_push: Fix upload of CI's logs and terminal capture file
The test_logs directory contains a directory: fix the wildcard so
that scp doesn't fail.

Terminal capture files are now deleted every time we re-run the
demo script: upload CI's .cast file before it's gone.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:01:39 +02:00
Stefano Brivio
bd3e6f373f contrib/podman: Rebase to latest upstream
One check moved from networking_linux.go to networking_common.go.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-24 00:00:32 +02:00
Stefano Brivio
85de88ff31 test/passt.mbuto: Don't fail on missing guest public key
We won't necessarily run mbuto as part of regular tests: it can also
be used for demos or out-of-tree tests.

To keep the profile simple, leave the whole sshd setup there, which
is otherwise harmless, but don't fail if guest-key.pub is missing in
the current directory.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-23 17:55:10 +02:00
Stefano Brivio
d6f865a40a test/distro: Update workarounds for Ubuntu 22.04 on s390x
If we use dhclient without creating a complete network configuration,
systemd-resolved will stop working after a while, and this sometimes
happens while we're still installing packages.

Disable it, together with systemd-networkd, while taking care of
removing the dhclient hook that prevents overriding /etc/resolv.conf.

While at it, it looks like removing snapd and needrestart actually
takes more time than keeping them: drop that line.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-23 02:46:24 +02:00
Stefano Brivio
cff565a1f6 test/lib: Wait for DHCPv4 before starting DHCPv6 client in two_guests test
I'm not sure why, but dhclient hangs otherwise. This reflects what we
do in the passt_in_ns setup steps.

Eventually, this whole block could go away if we let pasta configure
this network namespace with --config-net.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-23 02:46:24 +02:00
Stefano Brivio
1134ce88fe test/perf: Wait for neper servers in guest to be ready before starting client
Starting tcp_rr, tcp_crr, udp_rr servers in the guest takes a bit
longer than starting the corresponding clients on the host, and we
end up starting clients before servers unless we add a delay there.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-23 02:46:24 +02:00
Stefano Brivio
d1dbc4f992 test/lib: Wait for kernel to free up ports used by iperf3 before reusing them
If we start another server on the same port right away, we might fail
to bind the port. A small delay appears to be needed -- I'm not
entirely sure why at this point.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-22 16:54:09 +02:00
Stefano Brivio
856b04490a test/lib: Run also iperf3 clients in background, revert to time-based wait
Unfortunately, this partially counters recent efforts by David to
speed up these tests, but it looks like iperf3 clients don't reliably
terminate, in some rare cases I couldn't isolate yet.

For the moment being, reintroduce the time-based wait approach, now
using the configurable test duration, and terminate the servers at
the end of it, in case they're stuck. There's no point in keeping
the 'sleep 2' later, so drop that, and while at it, make sure that
the stuck servers have time to flush the JSON output before we use
it.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-22 16:54:09 +02:00
Stefano Brivio
df29ebfe02 test/perf: Disable periodic throughput reports to avoid vhost hang
It appears that if we run throughput tests with one-second periodic
reports, the sending side of the vhost channel used for SSH-based
command dispatch occasionally stops working altogether. I haven't
investigated this further, all I see is that output is truncated
at some point, and doesn't resume.

If we use gzip compression (ssh -C) this happens less frequently,
but it still happens, seemingly indicating the issue is probably
related to vhost itself.

Disable periodic reports in iperf3 clients. The -i options were
actually redundant, so remove them from both test files as well as
from test_iperf3().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-22 16:54:09 +02:00