September 2022 - dev

[PATCH v2 00/28] Fixes for static checkers
by David Gibson 30 Sep '22

30 Sep '22

The passt tests include two static checking tools: clang-tidy and cppcheck. However, newer versions of those tools have introduced extra checks, and may cause these tests to fail. This series fixes all the clang-tidy and cppcheck warnings, either by altering our code, or by suppressing them with relevant options to the checkers. With this series, the checks are now clean on both my Fedora 36 machine (clang-tools-extra-14.0.5-1.fc36.x86_64 and cppcheck-2.7.4-2.fc36.x86_64) and my Debian machine (bookworm with some pieces from sid: clang-tidy 1:14.0-55.1 and cppcheck 2.9-1). Changes since v1: * Fixed a whitespace error * Added extra background information and details to comments and commit messages when removing old suppressions * Improved conf_runas() rework to give better error messages David Gibson (28): Clean up parsing of port ranges clang-tidy: Suppress warning about unchecked error in logfn macro clang-tidy: Fix spurious null pointer warning in pasta_start_ns() clang-tidy: Remove duplicate #include from icmp.c Catch failures when installing signal handlers Pack DHCPv6 "on wire" structures Clean up parsing in conf_runas() cppcheck: Reduce scope of some variables Don't shadow 'i' in conf_ports() Don't shadow global function names Stricter checking for nsholder.c cppcheck: Work around false positive NULL pointer dereference error cppcheck: Use inline suppression for ffsl() cppcheck: Use inline suppressions for qrap.c cppcheck: Use inline suppression for strtok() in conf.c Avoid ugly 'end' members in netlink structures cppcheck: Broaden suppression for unused struct members cppcheck: Remove localtime suppression for pcap.c qrap: Handle case of PATH environment variable being unset cppcheck: Suppress same-value-in-ternary branches warning cppcheck: Suppress NULL pointer warning in tcp_sock_consume() Regenerate seccomp.h if seccomp.sh changes cppcheck: Avoid errors due to zeroes in bitwise ORs cppcheck: Remove unused knownConditionTrueFalse suppression cppcheck: Remove unused objectIndex suppressions cppcheck: Remove unused va_list_usedBeforeStarted suppression Mark unused functions for cppcheck cppcheck: Remove unused unmatchedSuppression suppressions Makefile | 26 +--- arch.c | 4 +- conf.c | 359 +++++++++++++++++++++++------------------------- dhcpv6.c | 26 ++-- icmp.c | 1 - igmp.c | 1 + netlink.c | 22 +-- passt.c | 7 +- pasta.c | 5 +- qrap.c | 18 ++- seccomp.sh | 2 + siphash.c | 1 + tap.c | 5 +- tcp.c | 5 + test/Makefile | 2 +- test/nsholder.c | 2 +- util.c | 2 +- util.h | 1 + 18 files changed, 236 insertions(+), 253 deletions(-) -- 2.37.3

2 29

[PATCH] Makefile: Hack for optimised-away store in ndp() before checksum calculation
by Stefano Brivio 29 Sep '22

29 Sep '22

With gcc 11 and 12, passing -flto, or -flto=auto, and -O2, intra-procedural optimisation gets rid of a fundamental bit in ndp(): the store of hop_limit in the IPv6 header, before the checksum is calculated, which on x86_64 looks like this: ip6hr->hop_limit = IPPROTO_ICMPV6; b8c0: c6 44 24 35 3a movb $0x3a,0x35(%rsp) Here, hop_limit is temporarily set to the protocol number, to conveniently get the IPv6 pseudo-header for ICMPv6 checksum calculation in memory. With LTO, the assignment just disappears from the binary. This is rather visible as NDP messages get a wrong checksum, namely the expected checksum plus 58, and they're ignored by the guest or in the namespace, meaning we can't get any IPv6 routes, as reported by Wenli Quan. The issue affects a significant number of distribution builds, including the ones for CentOS Stream 9, EPEL 9, Fedora >= 35, Mageia Cauldron, and openSUSE Tumbleweed. As a quick workaround, declare csum_unaligned() as "noipa" for gcc 11 and 12, with -flto and -O2. This disables inlining and cloning, which causes the assignment to be compiled again. Leave a TODO item: we should figure out if a gcc issue has already been reported, and report one otherwise. There's no apparent justification as to why the store could go away. Reported-by: Wenli Quan <wquan(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- Makefile | 7 +++++++ checksum.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 1d45f17..d4b623f 100644 --- a/Makefile +++ b/Makefile @@ -50,11 +50,18 @@ HEADERS = $(PASST_HEADERS) seccomp.h # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993 # from the pointer arithmetic used from the tcp_tap_handler() path to get the # remote connection address. +# +# TODO: With the same combination, in ndp(), gcc optimises away the store of +# hop_limit in the IPv6 header (temporarily set to the protocol number for +# convenience, to mimic the ICMPv6 checksum pseudo-header) before the call to +# csum_unaligned(). Mark csum_unaligned() as "noipa" as a quick work-around, +# while we figure out if a corresponding gcc issue has already been reported. ifeq (,$(filter-out 11 12, $(shell $(CC) -dumpversion))) ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS))) ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS))) FLAGS += -DTCP_HASH_NOINLINE FLAGS += -DSIPHASH_20B_NOINLINE + FLAGS += -DCSUM_UNALIGNED_NO_IPA endif endif endif diff --git a/checksum.c b/checksum.c index acb1e3e..56ad01e 100644 --- a/checksum.c +++ b/checksum.c @@ -97,6 +97,9 @@ uint16_t csum_fold(uint32_t sum) * * Return: 16-bit IPv4-style checksum */ +#if CSUM_UNALIGNED_NO_IPA +__attribute__((__noipa__)) /* See comment in Makefile */ +#endif uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init) { return (uint16_t)~csum_fold(sum_16b(buf, len) + init); -- 2.35.1

1 0

[PATCH 00/28] Fixes for static checkers
by David Gibson 29 Sep '22

29 Sep '22

The passt tests include two static checking tools: clang-tidy and cppcheck. However, newer versions of those tools have introduced extra checks, and may cause these tests to fail. This series fixes all the clang-tidy and cppcheck warnings, either by altering our code, or by suppressing them with relevant options to the checkers. With this series, the checks are now clean on both my Fedora 36 machine (clang-tools-extra-14.0.5-1.fc36.x86_64 and cppcheck-2.7.4-2.fc36.x86_64) and my Debian machine (bookworm with some pieces from sid: clang-tidy 1:14.0-55.1 and cppcheck 2.9-1). David Gibson (28): Clean up parsing of port ranges clang-tidy: Suppress warning about unchecked error in logfn macro clang-tidy: Fix spurious null pointer warning in pasta_start_ns() clang-tidy: Remove duplicate #include from icmp.c Catch failures when installing signal handlers Pack DHCPv6 "on wire" structures Clean up parsing in conf_runas() cppcheck: Reduce scope of some variables Don't shadow 'i' in conf_ports() Don't shadow global function names Stricter checking for nsholder.c cppcheck: Work around false positive NULL pointer dereference error cppcheck: Use inline suppression for ffsl() cppcheck: Use inline suppressions for qrap.c cppcheck: Use inline suppression for strtok() in conf.c Avoid ugly 'end' members in netlink structures cppcheck: Broaden suppression for unused struct members cppcheck: Remove localtime suppression for pcap.c qrap: Handle case of PATH environment variable being unset cppcheck: Suppress same-value-in-ternary branches warning cppcheck: Suppress NULL pointer warning in tcp_sock_consume() Regenerate seccomp.h if seccomp.sh changes cppcheck: Avoid errors due to zeroes in bitwise ORs cppcheck: Remove unused knownConditionTrueFalse suppression cppcheck: Remove unused objectIndex suppressions cppcheck: Remove unused va_list_usedBeforeStarted suppression Mark unused functions for cppcheck cppcheck: Remove unused unmatchedSuppression suppressions Makefile | 26 +--- arch.c | 4 +- conf.c | 344 ++++++++++++++++++++++-------------------------- dhcpv6.c | 26 ++-- icmp.c | 1 - igmp.c | 1 + netlink.c | 22 ++-- passt.c | 7 +- pasta.c | 5 +- qrap.c | 18 ++- seccomp.sh | 2 + siphash.c | 1 + tap.c | 5 +- tcp.c | 2 + test/Makefile | 2 +- test/nsholder.c | 2 +- util.c | 2 +- util.h | 1 + 18 files changed, 222 insertions(+), 249 deletions(-) -- 2.37.3

2 40

[PATCH 0/2] Address warnings from gcc 12.2 with -flto
by Stefano Brivio 28 Sep '22

28 Sep '22

The first one is a bogus report of an uninitialised field passed as input to hash functions for TCP connections, already seen with gcc 11. The second one is a bogus stringop-overread which already happened with gcc 12.1, but this time a "cute" pragma won't make it go away. Both harmless, but noisy. Based on pending series posted by David, if it matters. Stefano Brivio (2): Makefile: Extend noinline workarounds for LTO and -O2 to gcc 12 udp: Replace pragma to ignore bogus stringop-overread warning with workaround Makefile | 6 +++--- udp.c | 26 ++++++++++++++++++-------- util.h | 23 ----------------------- 3 files changed, 21 insertions(+), 34 deletions(-) -- 2.35.1

1 2

[PATCH v2 00/11] Assorted test fixes, batch 7
by David Gibson 26 Sep '22

26 Sep '22

Here's another bundle of test fixes, focused on making the tests faster and more reliable. Based on my earlier series cleaning up the port forwarding data structures. Changes since v1: * Rebased and fixed conflict with another Makefile change * Dropped patch removing sleeps from layout functions David Gibson (11): test: Add wait_for() shell helper test: Remove unnecessary sleeps from shutdown tests test: More robust wait for pasta/passt to be ready test: Use --config-net for namespace setup test: Simplify data handling for transfer tests test: Remove unneccessary pane naming from layout_two_guests clang-tidy: Disable 'readability-identifier-length' cppcheck: Avoid excessive scanning due to system headers cppcheck: Run quietly Makefile: Simplify getting target triple for compiler cppcheck: Add target specific headers Makefile | 33 +++---- test/.gitignore | 1 + test/Makefile | 15 ++- test/lib/context | 4 +- test/lib/layout | 4 - test/lib/setup | 32 +++---- test/lib/util | 8 ++ test/passt.mbuto | 6 +- test/passt/shutdown | 1 - test/passt/tcp | 53 +++++------ test/passt/udp | 29 +++--- test/passt_in_ns/shutdown | 1 - test/passt_in_ns/tcp | 187 ++++++++++++++++---------------------- test/passt_in_ns/udp | 93 ++++++++----------- test/pasta/tcp | 79 +++++++--------- test/pasta/udp | 43 ++++----- test/two_guests/basic | 2 +- 17 files changed, 251 insertions(+), 340 deletions(-) -- 2.37.3

1 11

[PATCH v2 0/8] Clean up and fix bugs in port forwarding data structures
by David Gibson 24 Sep '22

24 Sep '22

The information we need to keep track of which ports are forwarded where is currently split across several different values and data structures in several different places. Worse, a number of those structures are incorrectly sized due to an off by one error, which could lead to buffer overruns. Fix the sizing, and re-organize the data structures in a way that should make it less likely to repeat that mistake. While we're there, correct a similar off-by-one mis-sizing of a number of other arrays. Changes since v1: * Use a define for the array size, rather than a typedef to handle the bitmaps of ports David Gibson (8): Improve types and names for port forwarding configuration Consolidate port forwarding configuration into a common structure udp: Delay initialization of UDP reversed port mapping table Don't use indirect remap functions for conf_ports() Pass entire port forwarding configuration substructure to conf_ports() Treat port numbers as unsigned Fix widespread off-by-one error dealing with port numbers icmp: Correct off by one errors dealing with number of echo request ids Makefile | 2 +- conf.c | 136 +++++++++++++++++++++++---------------------------- icmp.c | 5 +- passt.h | 1 + port_fwd.h | 34 +++++++++++++ tcp.c | 68 ++++++++------------------ tcp.h | 13 ++--- tcp_splice.c | 4 +- udp.c | 59 ++++++++++------------ udp.h | 27 +++++----- 10 files changed, 168 insertions(+), 181 deletions(-) create mode 100644 port_fwd.h -- 2.37.3

2 10

[PATCH] Fix the name of the qemu-system-* executable
by Dario Faggioli 24 Sep '22

24 Sep '22

Hello everyone! So, while playing with out passt package for openSUSE, my colleague Vasiliy found out an issue in how passt assumes the qemu binary to be called. In fact, in openSUSE, while we have a package that allows people to launch qemu by invoking `qemu-kvm`, we don't install it by default, and we therefore only have the standard upstream names in the filesystem, which have the ARCH part in lowercase. This patch, which we're currently carrying as a downstream one in our package, should fix the problem. Thanks and Regards, Dario --- Vasiliy Ulyanov (1): Fix the name of the qemu-system-* executable Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ ------------------------------------------------------------------- <<This happens because _I_ choose it to happen!>> (Raistlin Majere)

2 3

[PATCH 0/8] Clean up and fix bugs in port forwarding data structures
by David Gibson 24 Sep '22

24 Sep '22

The information we need to keep track of which ports are forwarded where is currently split across several different values and data structures in several different places. Worse, a number of those structures are incorrectly sized due to an off by one error, which could lead to buffer overruns. Fix the sizing, and re-organize the data structures in a way that should make it less likely to repeat that mistake. While we're there, correct a similar off-by-one mis-sizing of a number of other arrays. David Gibson (8): Improve types and names for port forwarding configuration Consolidate port forwarding configuration into a common structure udp: Delay initialization of UDP reversed port mapping table Don't use indirect remap functions for conf_ports() Pass entire port forwarding configuration substructure to conf_ports() Treat port numbers as unsigned Fix widespread off-by-one error dealing with port numbers icmp: Correct off by one errors dealing with number of echo request ids Makefile | 2 +- conf.c | 136 +++++++++++++++++++++++---------------------------- icmp.c | 5 +- passt.h | 1 + port_fwd.h | 34 +++++++++++++ tcp.c | 68 ++++++++------------------ tcp.h | 13 ++--- tcp_splice.c | 4 +- udp.c | 59 ++++++++++------------ udp.h | 27 +++++----- 10 files changed, 168 insertions(+), 181 deletions(-) create mode 100644 port_fwd.h -- 2.37.3

2 12

[PATCH 00/12] Assorted test fixes, batch 7
by David Gibson 24 Sep '22

24 Sep '22

Here's another bundle of test fixes, focused on making the tests faster and more reliable. Based on my earlier series cleaning up the port forwarding data structures. David Gibson (12): test: Add wait_for() shell helper test: Remove unnecessary sleeps from layout functions test: Remove unnecessary sleeps from shutdown tests test: More robust wait for pasta/passt to be ready test: Use --config-net for namespace setup test: Simplify data handling for transfer tests test: Remove unneccessary pane naming from layout_two_guests clang-tidy: Disable 'readability-identifier-length' cppcheck: Avoid excessive scanning due to system headers cppcheck: Run quietly Makefile: Simplify getting target triple for compiler cppcheck: Add target specific headers Makefile | 32 +++---- test/.gitignore | 1 + test/Makefile | 15 ++- test/lib/context | 4 +- test/lib/layout | 24 ----- test/lib/setup | 32 +++---- test/lib/util | 8 ++ test/passt.mbuto | 6 +- test/passt/shutdown | 1 - test/passt/tcp | 53 +++++------ test/passt/udp | 29 +++--- test/passt_in_ns/shutdown | 1 - test/passt_in_ns/tcp | 187 ++++++++++++++++---------------------- test/passt_in_ns/udp | 93 ++++++++----------- test/pasta/tcp | 79 +++++++--------- test/pasta/udp | 43 ++++----- test/two_guests/basic | 2 +- 17 files changed, 250 insertions(+), 360 deletions(-) -- 2.37.3

2 21

[PATCH 00/11] Fixes for demos, pre-push hook, README
by Stefano Brivio 24 Sep '22

24 Sep '22

After the recent rework of command dispatch and handling of temporary files, a few issues prevent demos from completing, and CI files from being uploaded. Fix those. On top of that, rebase to Podman's latest upstream so that the Podman demo can run again, shorten paths used to change directories in demo scripts, and fix a number of inconsistencies in the README. Given that the current state of the website is inconsistent, I'd be inclined to apply these right away, while posting to ease later review anyway. Stefano Brivio (11): contrib/podman: Rebase to latest upstream hooks/pre_push: Fix upload of CI's logs and terminal capture file test/demo: Use relative paths to change directories when possible test/demo: Avoid using port 5201 on the host test/lib: Drop perf_report_append() from perf_report test/lib: Don't try to write to perf.js when running demos README: Fix misspellings of openSUSE README: Point openSUSE links to Dario's OBS repository README: Fix indentation in "Try It" section README: Fix paragraph in Try It section of passt README: Add legend for Features section README.md | 24 ++- ...001-libpod-Add-pasta-networking-mode.patch | 95 +++++----- hooks/pre-push | 11 +- test/demo/passt | 45 +++-- test/demo/pasta | 20 +- test/demo/podman | 176 +++++++++--------- test/lib/perf_report | 13 +- 7 files changed, 198 insertions(+), 186 deletions(-) -- 2.35.1

1 11