aboutgitcodebugslistschat
Commit message (Collapse)AuthorAgeFilesLines
...
* 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>
* test/lib: Restore IFS while executing directives in def blocksStefano Brivio2022-09-221-1/+1
| | | | | | | If we don't, guest command dispatch will fail altogether, given that we use cat(1) on the enter file, which contains spaces. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf, tcp, udp: Arrays for ports need 2^16 values, not 2^16-8Stefano Brivio2022-09-223-5/+5
| | | | | | | | | | | | Reported by David but also by Coverity (CWE-119): In conf_ports: Out-of-bounds access to a buffer ...not in practice, because the allocation size is rounded up anyway, but not nice either. Reported-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tap: Check return value of accept4() before calling getsockopt()Stefano Brivio2022-09-221-2/+4
| | | | | | | | | | | | | | | | Reported by Coverity (CWE-119): Negative value used as argument to a function expecting a positive value (for example, size of buffer or allocation) and harmless, because getsockopt() would return -EBADF if the socket is -1, so we wouldn't print anything. Check if accept4() returns a valid socket before calling getsockopt() on it. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* test/perf: Switch performance test duration to 10 seconds instead of 30Stefano Brivio2022-09-224-4/+4
| | | | | | | | | It looks like the workaround for the virtio_net TX hang issue is working less reliably with the new command dispatch mechanism, I'm not sure why. Switch to 10 seconds, at least for the moment. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* test/perf: Always use /sbin/sysctl in tcp testStefano Brivio2022-09-222-6/+6
| | | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* README: Update Availability and Try It sections with new packagesStefano Brivio2022-09-221-25/+32
| | | | | | | | | We now have official packages for Fedora, unofficial (Fedora Copr) for other common RPM-based distributions, and the existing packages with static builds for Debian, and for other RPM-based distributions. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* test/passt_in_ns: Consistent sleep commands before starting socat clientStefano Brivio2022-09-222-6/+41
| | | | | | | | | | | | | | | There are some 'sleep 1' commands between starting the socat server and its corresponding client to avoid races due to the server not being ready as we start sending data. However, those don't cover all the cases where we might need them, and in some cases the sleep command actually ended up being before the server even starts. This fixes occasional failures in TCP and UDP simple transfer tests, that became apparent with the new command dispatch mechanism. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* test/perf: Check for /sbin/sysctl with which(1), not simply sysctlStefano Brivio2022-09-223-4/+4
| | | | | | | | | | Otherwise, we're depending on having /sbin in $PATH. For some reason I didn't completely grasp, with the new command dispatch mechanism that's not the case anymore, even if I have /sbin in $PATH in the parent shell. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* doc/demo: Clone and use mbuto in init namespaceStefano Brivio2022-09-221-5/+17
| | | | | | | | ...and not in pasta's namespace: the fakeroot(1) version shipping with (at least) Fedora 36 assumes "nested" operation as it sees that the UID is 0, and claims it's not supported. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* doc/demo: Drop /sbin from dhclient command, pass script file explicitlyStefano Brivio2022-09-221-4/+4
| | | | | | | | | | | dhclient might be in /usr/sbin on recent versions of Fedora, and mbuto just adds it to the same location as it was on the host: just call dhclient instead of /sbin/dhclient. This also applies for dhclient-script: given that we create the file on boot, pass its explicit location with -sf. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Makefile: Include seccomp.h in HEADERS and require it for static checkersStefano Brivio2022-09-221-3/+3
| | | | | | | | Targets running static checkers (cppcheck and clang-tidy) need seccomp.h, but the latter is not included in HEADERS. Add it. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* Makefile: Allow define overrides by prepending, not appending, CFLAGSStefano Brivio2022-09-221-24/+25
| | | | | | | | | | | | | | | | | | | If we append CFLAGS to the ones passed via command line (if any), -D options we append will override -D options passed on command line (if any). For example, OpenSUSE build flags include -D_FORTIFY_SOURCE=3, and we want to have -D_FORTIFY_SOURCE=2, if and only if not overridden. The current behaviour implies we redefine _FORTIFY_SOURCE as 2, though. Instead of appending CFLAGS, prepend them by adding all the default build flags to another variable, a simply expanded one (defined with :=), named FLAGS, and pass that *before* CFLAGS in targets, so that defines from command line can override default flags. Reported-by: Dario Faggioli <dfaggioli@suse.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Tested-by: Dario Faggioli <dfaggioli@suse.com>
* test: term: When checking if status line is a number, hide errorsStefano Brivio2022-09-141-1/+1
| | | | | | | | | | We use the [ "$x" -eq "$x" ] syntax to check if $x is a number. The behaviour is clearly implied by POSIX, but some shells might actually report the (intended) error, and dash floods script.log with "Illegal number" error messages. Hide them. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* test: Simpler termination handling for UDP testsDavid Gibson2022-09-133-65/+62
| | | | | | | | | | | | | Because UDP is connectionless we don't have an in-built end-of-stream signal for our connectivity tests. We work around this by explicitly adding an end marker to our sample data and killing the listening end once it is seen. However, socat has some built-in options - null-eof and shut-null - which can be used to signal the end of stream with a zero-length UDP packet. Use these to simplify how the UDP tests are implemented. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* udp: Don't drop zero-length outbound UDP packetsDavid Gibson2022-09-131-7/+10
| | | | | | | | | | | | | udp_tap_handler() currently skips outbound packets if they have a payload length of zero. This is not correct, since in a datagram protocol zero length packets still have meaning. Adjust this to correctly forward the zero-length packets by using a msghdr with msg_iovlen == 0. Bugzilla: https://bugs.passt.top/show_bug.cgi?id=19 Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-14 06:40:13 +0000