aboutgitcodebugslistschat
Commit message (Collapse)AuthorAgeFilesLines
...
* Don't shadow 'i' in conf_ports()David Gibson2022-09-291-2/+5
| | | | | | | | | | | | | | 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>
* cppcheck: Reduce scope of some variablesDavid Gibson2022-09-293-5/+7
| | | | | | | Minor style improvement suggested by cppcheck. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Clean up parsing in conf_runas()David Gibson2022-09-292-47/+51
| | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Pack DHCPv6 "on wire" structuresDavid Gibson2022-09-291-13/+13
| | | | | | | | | | | | | | | | | | | 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>
* Catch failures when installing signal handlersDavid Gibson2022-09-291-2/+4
| | | | | | | | | 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>
* clang-tidy: Remove duplicate #include from icmp.cDavid Gibson2022-09-291-1/+0
| | | | | Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* clang-tidy: Fix spurious null pointer warning in pasta_start_ns()David Gibson2022-09-291-1/+4
| | | | | | | | | | | | 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>
* clang-tidy: Suppress warning about unchecked error in logfn macroDavid Gibson2022-09-291-1/+1
| | | | | | | | | 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>
* Clean up parsing of port rangesDavid Gibson2022-09-291-140/+102
| | | | | | | | | | | | | | | | | | | | 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>
* cppcheck: Add target specific headersDavid Gibson2022-09-291-1/+1
| | | | | | | | | 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>
* Makefile: Simplify getting target triple for compilerDavid Gibson2022-09-291-2/+2
| | | | | | | | | | 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>
* cppcheck: Run quietlyDavid Gibson2022-09-291-1/+1
| | | | | | | | 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>
* cppcheck: Avoid excessive scanning due to system headersDavid Gibson2022-09-291-14/+6
| | | | | | | | | | | | | | | | | 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>
* clang-tidy: Disable 'readability-identifier-length'David Gibson2022-09-291-1/+6
| | | | | | | | 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>
* test: Remove unneccessary pane naming from layout_two_guestsDavid Gibson2022-09-291-4/+0
| | | | | | | | | 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>
* test: Simplify data handling for transfer testsDavid Gibson2022-09-2910-294/+214
| | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* test: Use --config-net for namespace setupDavid Gibson2022-09-291-14/+3
| | | | | | | | | | | | | | 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>
* test: More robust wait for pasta/passt to be readyDavid Gibson2022-09-291-5/+10
| | | | | | | | | | | | | 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>
* test: Remove unnecessary sleeps from shutdown testsDavid Gibson2022-09-292-2/+0
| | | | | | | | 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>
* test: Add wait_for() shell helperDavid Gibson2022-09-292-3/+9
| | | | | | | | | | 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>
* icmp: Correct off by one errors dealing with number of echo request ids2022_09_24.8978f65David Gibson2022-09-241-2/+3
| | | | | | | | | | 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>
* Fix widespread off-by-one error dealing with port numbersDavid Gibson2022-09-245-16/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Treat port numbers as unsignedDavid Gibson2022-09-243-8/+9
| | | | | | | | | | | 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>
* Pass entire port forwarding configuration substructure to conf_ports()David Gibson2022-09-241-40/+22
| | | | | | | | | 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>
* Don't use indirect remap functions for conf_ports()David Gibson2022-09-245-55/+7
| | | | | | | | | | 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>
* udp: Delay initialization of UDP reversed port mapping tableDavid Gibson2022-09-242-4/+23
| | | | | | | | | | | | | | | 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>
* Consolidate port forwarding configuration into a common structureDavid Gibson2022-09-247-98/+106
| | | | | | | | | | | | | | | 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>
* Improve types and names for port forwarding configurationDavid Gibson2022-09-247-55/+76
| | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Fix the name of the qemu-system-* executableVasiliy Ulyanov2022-09-241-4/+4
| | | | | | | | | | | | | | | 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>
* README: Add missing parenthesis in Try It sectionStefano Brivio2022-09-241-1/+1
| | | | Signed-off-by: Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* README: Drop excess whitespace in Try It sectionStefano Brivio2022-09-241-2/+2
| | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* README: Add legend for Features sectionStefano Brivio2022-09-241-0/+3
| | | | | | | 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>
* README: Fix paragraph in Try It section of passtStefano Brivio2022-09-241-3/+4
| | | | | | | The qemu patch isn't mentioned there anymore: replace reference with a link. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* README: Fix indentation in "Try It" sectionStefano Brivio2022-09-241-3/+3
| | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* README: Point openSUSE links to Dario's OBS repositoryStefano Brivio2022-09-241-4/+4
| | | | | | | ...instead of my Copr. It's also not official yet, but surely more appropriate now. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* README: Fix misspellings of openSUSEStefano Brivio2022-09-241-4/+4
| | | | | | For some reason, I used a capital O everywhere. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* test/lib: Don't try to write to perf.js when running demosStefano Brivio2022-09-241-0/+6
| | | | | | | ...it doesn't actually exist, and this error now causes the demo to stop. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* test/lib: Drop perf_report_append() from perf_reportStefano Brivio2022-09-241-6/+1
| | | | | | | 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>
* test/demo: Avoid using port 5201 on the hostStefano Brivio2022-09-243-115/+115
| | | | | | | | | 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>
* test/demo: Use relative paths to change directories when possibleStefano Brivio2022-09-242-6/+5
| | | | | | | | 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>
* hooks/pre_push: Fix upload of CI's logs and terminal capture fileStefano Brivio2022-09-241-4/+7
| | | | | | | | | | 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>
* contrib/podman: Rebase to latest upstreamStefano Brivio2022-09-241-45/+50
| | | | | | One check moved from networking_linux.go to networking_common.go. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* test/passt.mbuto: Don't fail on missing guest public keyStefano Brivio2022-09-231-1/+1
| | | | | | | | | | | 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>
* test/distro: Update workarounds for Ubuntu 22.04 on s390x2022_09_23.d6f865aStefano Brivio2022-09-231-1/+3
| | | | | | | | | | | | | | 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>
* test/lib: Wait for DHCPv4 before starting DHCPv6 client in two_guests testStefano Brivio2022-09-231-0/+1
| | | | | | | | | | 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>
* test/perf: Wait for neper servers in guest to be ready before starting clientStefano Brivio2022-09-232-0/+6
| | | | | | | | 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>
* test/lib: Wait for kernel to free up ports used by iperf3 before reusing themStefano Brivio2022-09-221-0/+2
| | | | | | | | 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>
* test/lib: Run also iperf3 clients in background, revert to time-based waitStefano Brivio2022-09-221-3/+6
| | | | | | | | | | | | | | | 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>
* test/perf: Disable periodic throughput reports to avoid vhost hangStefano Brivio2022-09-225-5/+5
| | | | | | | | | | | | | | | | | | 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>
* test/lib: Wait on iperf3 clients to be done, then send SIGINT to serversStefano Brivio2022-09-221-8/+8
| | | | | | | | | | | | | | | | An iperf3 client might fail to send the control message indicating the end of the test, if the kernel buffer doesn't accept it, and exit without having sent it, as the control socket is non-blocking. Should this happen, the server will just wait forever for this message, instead of terminating. Restore some of the behaviour that went away with the "test: Rewrite test_iperf3" patch: instead of waiting on servers to terminate, wait on the clients. When they are done, wait 2 seconds, and then send SIGINT to the servers, which make them still write out the JSON report before terminating. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-09 19:56:09 +0000