July 2024 - dev

[PATCH 00/11] Minor assorted fixes, mostly logging and tests
by Stefano Brivio 26 Jul '24

26 Jul '24

Here are a bunch of mostly unrelated assorted fixes for small issues I found in the past few weeks but didn't really find time to report. Stefano Brivio (11): tap: Don't quit if pasta gets EIO on writev() to tap, interface might be down tcp: Change SO_PEEK_OFF support message to debug() log: Drop newlines in the middle of the perror()-like messages log: Fix sub-second part in relative log time calculation log: Initialise timestamp for relative log time also if we use a log file test: Fix memory/passt tests, --netns-only is not a valid option for passt test: Update names of symbols and slabinfo entries test: iperf3 3.16 introduces multiple threads, drop our own implementation of that tap: Exit if we fail to bind a UNIX domain socket with explicit path tap: Discard guest data on length descriptor mismatch conf: Accept addresses enclosed by square brackets in port forwarding specifiers conf.c | 24 ++++++++---- flow.c | 2 +- log.c | 76 +++++++++++++++++++++----------------- log.h | 19 +++++----- passt.c | 2 + tap.c | 18 ++++++--- tcp.c | 2 +- test/lib/perf_report | 2 +- test/lib/test | 31 +++++----------- test/memory/passt | 35 ++++++++---------- test/passt.mem.mbuto | 9 +---- test/perf/passt_tcp | 39 ++++++++++---------- test/perf/passt_udp | 55 ++++++++++++++-------------- test/perf/pasta_tcp | 58 ++++++++++++++--------------- test/perf/pasta_udp | 87 ++++++++++++++++++++++---------------------- 15 files changed, 229 insertions(+), 230 deletions(-) -- 2.43.0

2 32

[PATCH] test: Fix spurious test failure with systemd-resolved
by David Gibson 25 Jul '24

25 Jul '24

systemd-resolved has the rather strange behaviour of listening on the non-standard loopback address 127.0.0.53. Various changes we've made in passt mean that we now usually work fine on a host using systemd-resolved. However our tests still fail in this case. We have a special case for when the guest's resolv.conf needs to differ from the host's because the resolver is on a host loopback address. However, we only consider the case where the host resolver is on 127.0.0.1, not other loopback addresses. Correct this with a different test condition. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- test/passt/dhcp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/passt/dhcp b/test/passt/dhcp index 3ec2faf2..e05a4bb1 100644 --- a/test/passt/dhcp +++ b/test/passt/dhcp @@ -38,7 +38,7 @@ check [ __MTU__ = 65520 ] test DHCP: DNS gout DNS sed -n 's/^nameserver $[0-9]*\.$$.*$/\1\2/p' /etc/resolv.conf | tr '\n' ',' | sed 's/,$//;s/$/\n/' hout HOST_DNS sed -n 's/^nameserver $[0-9]*\.$$.*$/\1\2/p' /etc/resolv.conf | head -n3 | tr '\n' ',' | sed 's/,$//;s/$/\n/' -check [ "__DNS__" = "__HOST_DNS__" ] || [ "__DNS__" = "__HOST_GW__" -a "__HOST_DNS__" = "127.0.0.1" ] +check [ "__DNS__" = "__HOST_DNS__" ] || ( [ "__DNS__" = "__HOST_GW__" ] && expr "__HOST_DNS__" : "127[.]" ) # FQDNs should be terminated by dots, but the guest DHCP client might omit them: # strip them first -- 2.45.2

2 1

[PATCH v2 0/2] Broaden DNS forwarding
by David Gibson 25 Jul '24

25 Jul '24

When looking through outstanding bugs in various trackers, I noticed this one: https://github.com/containers/podman/issues/23239 Now that the flow table is merged, this is very easy to fix, so, here's a fix. While we're at it, handle encrypted DNS on port 853 as well, which Red Hat certainly seems to be interested in for one. Changes since v1: * Fix some stylistic errors in 1/2 * Update man page to reflect new behaviour in 2/2 David Gibson (2): fwd: Refactor tests in fwd_nat_from_tap() for clarity fwd: Broaden what we consider for DNS specific forwarding rules fwd.c | 39 ++++++++++++++++++++++++++------------- passt.1 | 10 +++++----- 2 files changed, 31 insertions(+), 18 deletions(-) -- 2.45.2

3 7

[PATCH 0/2] Small fixes for SO_PEEK_OFF
by David Gibson 24 Jul '24

24 Jul '24

We discovered some problems caused by the recent merge of SO_PEEK_OFF support. 1/2 is a fix from Jon, modified by Stefano. 2/2 is a fix for a different issue. David Gibson (1): tcp: Correctly update SO_PEEK_OFF when tcp_send_frames() drops frames Jon Maloy (1): tcp: probe for SO_PEEK_OFF both in tcpv4 and tcp6 tcp.c | 37 +++++++++++++++++++++++++------------ tcp_buf.c | 23 ++++++++++++++--------- tcp_buf.h | 2 +- 3 files changed, 40 insertions(+), 22 deletions(-) -- 2.45.2

2 3

[PATCH 0/2] Broaden DNS forwarding
by David Gibson 24 Jul '24

24 Jul '24

When looking through outstanding bugs in various trackers, I noticed this one: https://github.com/containers/podman/issues/23239 Now that the flow table is merged, this is very easy to fix, so, here's a fix. While we're at it, handle encrypted DNS on port 853 as well, which Red Hat certainly seems to be interested in for one. David Gibson (2): fwd: Refactor tests in fwd_nat_from_tap() for clarity fwd: Broaden what we consider for DNS specific forwarding rules fwd.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) -- 2.45.2

2 5

[PATCH v2] tcp: probe for SO_PEEK_OFF both in tcpv4 and tcp6
by Stefano Brivio 24 Jul '24

24 Jul '24

From: Jon Maloy <jmaloy(a)redhat.com> Based on an original patch by Jon Maloy: -- The recently added socket option SO_PEEK_OFF is not supported for TCP/IPv6 sockets. Until we get that support into the kernel we need to test for support in both protocols to set the global 'peek_offset_cap´ to true. -- Compared to the original patch: - only check for SO_PEEK_OFF support for enabled IP versions - use sa_family_t instead of int to pass the address family around Fixes: e63d281871ef ("tcp: leverage support of SO_PEEK_OFF socket option when available") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/tcp.c b/tcp.c index 0c66ac8..c031f13 100644 --- a/tcp.c +++ b/tcp.c @@ -2470,6 +2470,29 @@ static void tcp_sock_refill_init(const struct ctx *c) } } +/** + * tcp_probe_peek_offset_cap() - Check if SO_PEEK_OFF is supported by kernel + * @af: Address family, IPv4 or IPv6 + * + * Return: true if supported, false otherwise + */ +bool tcp_probe_peek_offset_cap(sa_family_t af) +{ + bool ret = false; + int s, optv = 0; + + s = socket(af, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP); + if (s < 0) { + warn_perror("Temporary TCP socket creation failed"); + } else { + if (!setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &optv, sizeof(int))) + ret = true; + close(s); + } + + return ret; +} + /** * tcp_init() - Get initial sequence, hash secret, initialise per-socket data * @c: Execution context @@ -2478,9 +2501,6 @@ static void tcp_sock_refill_init(const struct ctx *c) */ int tcp_init(struct ctx *c) { - unsigned int optv = 0; - int s; - ASSERT(!c->no_tcp); if (c->ifi4) @@ -2502,15 +2522,8 @@ int tcp_init(struct ctx *c) NS_CALL(tcp_ns_socks_init, c); } - /* Probe for SO_PEEK_OFF support */ - s = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP); - if (s < 0) { - warn_perror("Temporary TCP socket creation failed"); - } else { - if (!setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &optv, sizeof(int))) - peek_offset_cap = true; - close(s); - } + peek_offset_cap = (!c->ifi4 || tcp_probe_peek_offset_cap(AF_INET)) && + (!c->ifi6 || tcp_probe_peek_offset_cap(AF_INET6)); info("SO_PEEK_OFF%ssupported", peek_offset_cap ? " " : " not "); return 0; -- 2.43.0

2 5

[PATCH] tcp: probe for SO_PEEK_OFF both in tcpv4 and tcp6
by Jon Maloy 22 Jul '24

22 Jul '24

The recently added socket option SO_PEEK_OFF is not supported for TCP/IPv6 sockets. Until we get that support into the kernel we need to test for support in both protocols to set the global 'peek_offset_cap´ to true. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tcp.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/tcp.c b/tcp.c index c5431f1..32026ca 100644 --- a/tcp.c +++ b/tcp.c @@ -2717,6 +2717,28 @@ static void tcp_sock_refill_init(const struct ctx *c) } } +/** + * tcp_probe_peek_offset_cap() - Check if SO_PEEK_OFF is supported by kernel + * @af: Address family, IPv4 or IPv6 + * + * Return: true if supported, false otherwise + */ +bool tcp_probe_peek_offset_cap(int af) +{ + bool ret = false; + int s, optv = 0; + + s = socket(af, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP); + if (s < 0) { + warn_perror("Temporary TCP socket creation failed"); + } else { + if (!setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &optv, sizeof(int))) + ret = true; + close(s); + } + return ret; +} + /** * tcp_init() - Get initial sequence, hash secret, initialise per-socket data * @c: Execution context @@ -2725,8 +2747,7 @@ static void tcp_sock_refill_init(const struct ctx *c) */ int tcp_init(struct ctx *c) { - unsigned int b, optv = 0; - int s; + unsigned int b; ASSERT(!c->no_tcp); @@ -2752,15 +2773,8 @@ int tcp_init(struct ctx *c) NS_CALL(tcp_ns_socks_init, c); } - /* Probe for SO_PEEK_OFF support */ - s = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP); - if (s < 0) { - warn_perror("Temporary TCP socket creation failed"); - } else { - if (!setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &optv, sizeof(int))) - peek_offset_cap = true; - close(s); - } + peek_offset_cap = tcp_probe_peek_offset_cap(AF_INET) && + tcp_probe_peek_offset_cap(AF_INET6); info("SO_PEEK_OFF%ssupported", peek_offset_cap ? " " : " not "); return 0; -- 2.45.2

3 4

[PATCH v8 00/27] Unified flow table
by David Gibson 20 Jul '24

20 Jul '24

This is the seventh draft of an implementation of more general "connection" tracking, as described at: https://pad.passt.top/p/NewForwardingModel This series changes the TCP connection table and hash table into a more general flow table that can track other protocols as well. Each flow uniformly keeps track of all the relevant addresses and ports, which will allow for more robust control of NAT and port forwarding. ICMP and UDP are converted to use the new flow table. This is based on the recent series of UDP flow table preliminaries. Caveats: * We roughly double the size of a connection/flow entry * We don't yet record the local address of flows initiated from a socket, even in cases where it's bound to a specific address. Changes since v7: * Rebase * Fix unintended regression in forwarding logic (we weren't applying map_gw logic to DNS packets, if they didn't hit explicit DNS forwarding rules). * Remove return value from pif_sockaddr(), in turned out not to be very useful. * More robust discarding of datagrams received between bind() and connect() on UDP reply sockets. * Avoid the name 'fside' for variables which was confusing in some contexts * Assorted minor changes based on feedback. Changes since v6: * Complete redesign of the UDP flow handling * Rebased (handling the change to bind() probing for local addresses was surprisingly fiddly) * Replace sockaddr_from_inany() with pif_sockaddr() which can correctly handle scope_id for different interfaces, and returns whether the address is non-trivial for convenience * Preserve specific loopback addresses in forwarding logic Changes since v5: * flowside_from_af() is now static * Small fixes to state verification * Pass protocol specific types into deferred/timer callbacks * No longer require complete forwarding address info for the hash table (we won't have it for UDP) * Fix bugs with logging of flow addresses * Make sure to initialise sin_zero field sockaddr_from_inany * Added patch better typing parameters to flow type specific callbacks * Terminology change "forwarded side" to "target side" * Assorted wording and style tweaks based on Stefano's review * Fold introduction of struct flowside and populating the initiating side together * Manage outbound addresses via the flow table as well * Support for UDP * Correct type of 'b' in flowside_lookup() (was a signed int) Changes since v4: * flowside_from_af() no longer fills in unspecified addresses when passed NULL * Split and rename flow hash lookup function * Clarified flow state transitions, and enforced where practical * Made side 0 always the initiating side of a flow, rather than letting the protocol specific code decide * Separated pifs from flowside addresses to allow better structure packing Changes since v3: * Complex rebase on top of the many things that have happened upstream since v2. * Assorted other changes. * Replace TAPFSIDE() and SOCKFSIDE() macros with local variables. Changes since v2: * Cosmetic fixes based on review * Extra doc comments for enum flow_type * Rename flowside to flowaddrs which turns out to make more sense in light of future changes * Fix bug where the socket flowaddrs for tap initiated connections wasn't initialised to match the socket address we were using in the case of map-gw NAT * New flowaddrs_from_sock() helper used in most cases which is cleaner and should avoid bugs like the above * Using newer centralised workarounds for clang-tidy issue 58992 * Remove duplicate definition of FLOW_MAX as maximum flow type and maximum number of tracked flows * Rebased on newer versions of preliminary work (ICMP, flow based dispatch and allocation, bind/address cleanups) * Unified hash table as well as base flow table * Integrated ICMP Changes since v1: * Terminology changes - "Endpoint" address/port instead of "correspondent" address/port - "flowside" instead of "demiflow" * Actually move the connection table to a new flow table structure in new files * Significant rearrangement of earlier patchs on top of that new table, to reduce churn David Gibson (27): flow: Common address information for initiating side flow: Common address information for target side tcp, flow: Remove redundant information, repack connection structures tcp: Obtain guest address from flowside tcp: Manage outbound address via flow table tcp: Simplify endpoint validation using flowside information tcp_splice: Eliminate SPLICE_V6 flag tcp, flow: Replace TCP specific hash function with general flow hash flow, tcp: Generalise TCP hash table to general flow hash table tcp: Re-use flow hash for initial sequence number generation icmp: Remove redundant id field from flow table entry icmp: Obtain destination addresses from the flowsides icmp: Look up ping flows using flow hash icmp: Eliminate icmp_id_map flow: Helper to create sockets based on flowside icmp: Manage outbound socket address via flow table flow, tcp: Flow based NAT and port forwarding for TCP flow, icmp: Use general flow forwarding rules for ICMP fwd: Update flow forwarding logic for UDP udp: Create flows for datagrams from originating sockets udp: Handle "spliced" datagrams with per-flow sockets udp: Remove obsolete splice tracking udp: Find or create flows for datagrams from tap interface udp: Direct datagrams from host to guest via flow table udp: Remove obsolete socket tracking udp: Remove rdelta port forwarding maps udp: Rename UDP listening sockets Makefile | 4 +- conf.c | 14 +- epoll_type.h | 6 +- flow.c | 483 +++++++++++++++++++++- flow.h | 47 +++ flow_table.h | 45 +- fwd.c | 187 ++++++++- fwd.h | 9 + icmp.c | 105 ++--- icmp_flow.h | 2 - inany.h | 2 - passt.c | 10 +- passt.h | 5 +- pif.c | 40 ++ pif.h | 17 + tap.c | 11 - tap.h | 1 - tcp.c | 522 ++++++----------------- tcp_buf.c | 6 +- tcp_conn.h | 47 +-- tcp_internal.h | 10 +- tcp_splice.c | 98 +---- tcp_splice.h | 5 +- udp.c | 1079 ++++++++++++++++++++---------------------------- udp.h | 35 +- udp_flow.h | 27 ++ util.c | 9 +- util.h | 3 + 28 files changed, 1563 insertions(+), 1266 deletions(-) create mode 100644 udp_flow.h -- 2.45.2

2 29

[PATCH 0/4] RFC: Proof-of-concept based exeter+Avocado tests
by David Gibson 19 Jul '24

19 Jul '24

Here's a rough proof of concept showing how we could run tests for passt with Avocado and the exeter library I recently created. It includes Cleber's patch adding some basic Avocado tests and builds on that. The current draft is pretty janky: * The build rules to download and install the necessary pieces are messy * We create the Avocado job files from the exeter sources in the Makefile. Ideally Avocado would eventually be extended to handle this itself * We have more ugly make rules to mangle the right venv path into the exeter based python script * The names that Avocado sees for each test are terrible (nondescriptive and duplicated). This is the point I made in an earlier reply to Cleber's patch - we need some way of setting the test name from the job files. * There's some hacks to make sure things are executed from the right working directory But, it's a starting point. Stefano, I'm hoping most of the jank in the setup can be cleaned up. The bit I really want your opinion on is how the test scripts themslves look: test/build/static_checkers.exeter (shell) and test/build/build.exeter.in (Python). Cleber, I hope this concrete example makes it a bit clearer how I'm thinking about running tests from Avocado, without making the test themselves inherently tied to Avocado. Cleber Rosa (1): test: run static checkers with Avocado and JSON definitions David Gibson (3): test: Extend make targets to run Avocado tests test: Exeter based static tests test: Add exeter+Avocado based build tests test/.gitignore | 2 + test/Makefile | 33 +++++++++++- test/avocado/static_checkers.json | 16 ++++++ test/build/.gitignore | 2 + test/build/build.exeter.in | 89 +++++++++++++++++++++++++++++++ test/build/static_checkers.exeter | 17 ++++++ test/run_avocado | 49 +++++++++++++++++ 7 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 test/avocado/static_checkers.json create mode 100644 test/build/.gitignore create mode 100644 test/build/build.exeter.in create mode 100755 test/build/static_checkers.exeter create mode 100755 test/run_avocado -- 2.45.2

1 4

[PATCH 0/6] Of course there are more flow table preliminaries
by David Gibson 17 Jul '24

17 Jul '24

This series has some further preliminaries for the flow table. Specifically this is focused on some clean ups to handling of indicies for flows and sides. We also add some additional test programs for platform requirements. This is based on the series adding UDP error handling. David Gibson (6): flow, icmp, tcp: Clean up helpers for getting flow from index flow, tcp_splice: Prefer 'sidei' for variables referring to side index flow: Introduce flow_foreach_sidei() macro tcp_splice: Use parameterised macros for per-side event/flag bits doc: Test behaviour of closing duplicate UDP sockets doc: Extend zero-recv test with methods using msghdr doc/platform-requirements/.gitignore | 1 + doc/platform-requirements/Makefile | 4 +- doc/platform-requirements/recv-zero.c | 60 +++++++-- doc/platform-requirements/udp-close-dup.c | 105 +++++++++++++++ flow.h | 16 +-- flow_table.h | 48 +++++-- icmp.c | 22 +++- tcp.c | 28 +++- tcp_conn.h | 15 +-- tcp_splice.c | 152 ++++++++++++---------- 10 files changed, 332 insertions(+), 119 deletions(-) create mode 100644 doc/platform-requirements/udp-close-dup.c -- 2.45.2

2 13