- dev - passt

[PATCH v2 00/13] Clean up to tap errors and epoll dispatch
by David Gibson 11 Aug '23

11 Aug '23

Getting from an epoll event to the relevant handler function is currently several levels of functions and tests. This series simplifies this to be pretty close to a single switch on a value in the epoll ref dispatching directly to the appropriate handler. Doing this requires some preliminary cleaning up of the handling of errors or disconnects on the tap device. Changes since v1: * Give listening TCP sockets their own reference type * Fold "tap reset" series into this one Changes since v2 of tap reset series: * More thorough cleanup of handling error events on the listening Unix socket. Changes since v1 of the tap reset series: * Two extra patches that further clean up the reset path David Gibson (13): tap: Clean up tap reset path tap: Clean up behaviour for errors on listening Unix socket tap: Fold reset handling into tap_handler_pasta() tap: Fold reset handling into tap_handler_passt() epoll: Generalize epoll_ref to cover things other than sockets epoll: Always use epoll_ref for the epoll data variable epoll: Fold sock_handler into general switch on epoll event fd epoll: Split handling of ICMP and ICMPv6 sockets epoll: Tiny cleanup to udp_sock_handler() epoll: Split handling of TCP timerfds into its own handler function epoll: Split handling of listening TCP sockets into their own handler epoll: Split listening Unix domain socket into its own type epoll: Use different epoll types for passt and pasta tap fds icmp.c | 118 +++++++++++++++++++++++++-------------------- icmp.h | 9 ++-- passt.c | 90 ++++++++++++++++++++-------------- passt.h | 56 +++++++++++++++++----- pasta.c | 8 +++- tap.c | 133 ++++++++++++++++++++++++++------------------------- tap.h | 7 ++- tcp.c | 84 ++++++++++++++------------------ tcp.h | 28 +++++++---- tcp_conn.h | 4 +- tcp_splice.c | 8 ++-- tcp_splice.h | 2 +- udp.c | 16 +++---- util.c | 27 ++++++++--- 14 files changed, 334 insertions(+), 256 deletions(-) -- 2.41.0

2 17

[PATCH v2 0/4] Clean up reset of tap connections
by David Gibson 07 Aug '23

07 Aug '23

I ran into some mildly confusing stuff in the tap reset path while working on the epoll rework. Here are some fixes for it. Changes since v1: * Two extra patches that further clean up the reset path David Gibson (4): tap: Clean up tap reset path tap: More sensible behaviour for error on listening qemu socket tap: Fold reset handling into tap_handler_pasta() tap: Fold reset handling into tap_handler_passt() tap.c | 94 ++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 51 insertions(+), 43 deletions(-) -- 2.41.0

1 4

[PATCH 0/3] Better report errors failing to open namespace tap device
by David Gibson 04 Aug '23

04 Aug '23

In https://github.com/containers/podman/issues/19428, pasta is failing to open the namespace tap device. Paul Holzinger correctly noted that pasta isn't very helpful in this case, with no information beyond "it failed". He suggested a patch for that, however it wasn't quite sufficient: errno may not be propagated back from the ephemeral thread which enters the namespace, and even if it does the errno alone won't tell us which of the possible failure points actually failed. 2/3 here is a more robust change to address the problem. The other patches are minor cleanups I noticed along the way. Link: https://bugs.passt.top/show_bug.cgi?id=69 David Gibson (3): util: Make ns_enter() a void function and report setns() errors tap: More detailed error reporting in tap_ns_tun() tap: Remove unnecessary global tun_ns_fd conf.c | 3 ++- tap.c | 33 ++++++++++++++++++--------------- udp.c | 6 ++---- util.c | 8 +++----- util.h | 2 +- 5 files changed, 26 insertions(+), 26 deletions(-) -- 2.41.0

2 5

[PATCH 0/2] Clean up reset of tap connections
by David Gibson 04 Aug '23

04 Aug '23

I ran into some mildly confusing stuff in the tap reset path while working on the epoll rework. Here's a couple of fixes for it. David Gibson (2): tap: Clean up tap reset path tap: More sensible behaviour for error on listening qemu socket tap.c | 57 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 25 deletions(-) -- 2.41.0

1 2

[PATCH v2 00/17] netlink fixes and cleanups
by David Gibson 04 Aug '23

04 Aug '23

We've had several bugs in the past that were quite tricky to debug, but would have been much easier if we'd known that a netlink operation had failed. So, it would be desirable to actually detect and report failures of netlink operations. While working on that, I discovered that there are a number of other issues ranging from very small to medium sized with the way we use netlink. This series addresses many of them. Link: https://bugs.passt.top/show_bug.cgi?id=60 Link: https://bugs.passt.top/show_bug.cgi?id=67 This series was tested as based on the pending patches adding C11 support, though I believe it trivially rebases onto current main. Changes since v1: * Assorted minor fixes based on Stefano's review * Rebased on C11 branch David Gibson (17): netlink: Split up functionality of nl_link() netlink: Split nl_addr() into separate operation functions netlink: Split nl_route() into separate operation functions netlink: Use struct in_addr for IPv4 addresses, not bare uint32_t netlink: Explicitly pass netlink sockets to operations netlink: Make nl_*_dup() use a separate datagram for each request netlink: Start sequence number from 1 instead of 0 netlink: Treat send() or recv() errors as fatal netlink: Fill in netlink header fields from nl_req() netlink: Add nl_do() helper for simple operations with error checking netlink: Clearer reasoning about the netlink response buffer size netlink: Split nl_req() to allow processing multiple response datagrams netlink: Add nl_foreach_oftype to filter response message types netlink: Propagate errors for "set" operations netlink: Always process all responses to a netlink request netlink: Propagate errors for "dump" operations netlink: Propagate errors for "dup" operations conf.c | 66 ++++- netlink.c | 843 ++++++++++++++++++++++++++++++++++-------------------- netlink.h | 27 +- pasta.c | 75 +++-- 4 files changed, 658 insertions(+), 353 deletions(-) -- 2.41.0

2 18

[PATCH 0/3] RFC: Allow C11 extensions in the passt/pasta code
by David Gibson 04 Aug '23

04 Aug '23

As discussed on our recent calls, the C11 standard introduces anonymous structure and union members and static assertions, amongst other things. Both of these could be useful in a few places in passt/pasta to make the code more readable and safer. However, at the moment, the compiler flags we use only allow C99 code. This series allows C11 code, and makes some fairly obvious cleanups by using it. It would be nice to get an opinion on this reasonably quickly, because I have other patches in the works that will look different depending on whether or not they can use C11 features. David Gibson (3): Allow C11 code, not just C99 code Use C11 anonymous members to make poll refs less verbose to use Use static assertion to verify that union epoll_ref is the right size Makefile | 4 ++-- icmp.c | 22 +++++++++++----------- icmp.h | 2 +- passt.c | 8 ++++---- passt.h | 8 ++++++-- tcp.c | 46 +++++++++++++++++++++++----------------------- tcp.h | 2 +- tcp_splice.c | 11 +++++------ udp.c | 50 +++++++++++++++++++++++--------------------------- udp.h | 2 +- util.c | 4 ++-- 11 files changed, 79 insertions(+), 80 deletions(-) -- 2.41.0

2 7

[PATCH 00/17] netlink fixes and cleanups
by David Gibson 03 Aug '23

03 Aug '23

We've had several bugs in the past that were quite tricky to debug, but would have been much easier if we'd known that a netlink operation had failed. So, it would be desirable to actually detect and report failures of netlink operations. While working on that, I discovered that there are a number of other issues ranging from very small to medium sized with the way we use netlink. This series addresses many of them. Link: https://bugs.passt.top/show_bug.cgi?id=60 Link: https://bugs.passt.top/show_bug.cgi?id=67 David Gibson (17): netlink: Split up functionality if nl_link() netlink: Split nl_addr() into separate operation functions netlink: Split nl_route() into separate operation functions netlink: Use struct in_addr for IPv4 addresses, not bare uint32_t netlink: Explicitly pass netlink sockets to operations netlink: Make nl_*_dup() use a separate datagram for each request netlink: Start sequence number from 1 instead of 0 netlink: Treat send() or recv() errors as fatal netlink: Fill in netlink header fields from nl_req() netlink: Add nl_do() helper for simple operations with error checking netlink: Clearer reasoning about the netlink response buffer size netlink: Split nl_req() to allow processing multiple response datagrams netlink: Add nl_foreach_oftype to filter response message types netlink: Propagate errors for "set" operations netlink: Always process all responses to a netlink request netlink: Propagate errors for "dump" operations netlink: Propagate errors for "dup" operations conf.c | 66 ++++- netlink.c | 844 ++++++++++++++++++++++++++++++++++-------------------- netlink.h | 27 +- pasta.c | 75 +++-- 4 files changed, 659 insertions(+), 353 deletions(-) -- 2.41.0

2 34

[PATCH] tap: include errno in error when tap_ns_tun() fails
by Paul Holzinger 03 Aug '23

03 Aug '23

It is important to know why a syscall failed so pasta should include the errno in the error message. This is still not perfect as we do not know which of functions (open, ioctl, if_nametoindex) failed but it should at least include more important context. This change was inspiered by a podman issue[1]. [1] https://github.com/containers/podman/issues/19428 Signed-off-by: Paul Holzinger <pholzing(a)redhat.com> --- tap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tap.c b/tap.c index a6a73d3..c212616 100644 --- a/tap.c +++ b/tap.c @@ -1205,7 +1205,8 @@ static void tap_sock_tun_init(struct ctx *c) NS_CALL(tap_ns_tun, c); if (tun_ns_fd == -1) - die("Failed to open tun socket in namespace"); + die("Failed to open tun socket in namespace: %s", + strerror(errno)); pasta_ns_conf(c); -- 2.41.0

2 3

[PATCH 0/8] RFC: Generalize flow tracking, part 1
by David Gibson 28 Jul '23

28 Jul '23

This is a first draft of the first part of implementing a more general flow table (connection tracking) as described at: https://pad.passt.top/p/NewForwardingModel This is by no means complete. So far it doesn't really do anything new, it just reorganizes the TCP connection table to be closer to the more general flow table. Still it's ready for preliminary review. David Gibson (8): tap: Don't clobber source address in tap6_handler() tap: Pass source address to protocol handler functions tcp: More precise terms for addresses and ports tcp, udp: Don't include destination address in partially precomputed csums tcp, udp: Don't pre-fill IPv4 destination address in headers tcp: Track guest-side correspondent address tcp, flow: Introduce struct demiflow tcp, flow: Perform TCP hash calculations based on demiflow structure flow.h | 66 +++++++++++++++ icmp.c | 12 ++- icmp.h | 3 +- passt.c | 10 +-- passt.h | 4 +- pasta.c | 2 +- siphash.c | 1 + tap.c | 29 ++++--- tcp.c | 227 ++++++++++++++++++++------------------------------- tcp.h | 5 +- tcp_conn.h | 9 +- tcp_splice.c | 2 + udp.c | 37 +++------ udp.h | 5 +- util.h | 4 +- 15 files changed, 209 insertions(+), 207 deletions(-) create mode 100644 flow.h -- 2.41.0

1 8

ip_nonlocal_bind causes havoc with local connection detection
by Valtteri Vuorikoski 20 Jul '23

20 Jul '23

If net.ipv4.ip_nonlocal_bind is enabled, the following code in tcp_conn_from_tap gets very confused: if (!bind(s, sa, sl)) { tcp_rst(c, conn); /* Nobody is listening then */ return; } if (errno != EADDRNOTAVAIL && errno != EACCES) conn_flag(c, conn, LOCAL); This is especially visible if net.ipv4.ip_unprivileged_port_start is set to a value lower than the default. For example, if net.ipv4.ip_unprivileged_port_start=443 and net.ipv4.ip_nonlocal_bind=1, the bind()==0 branch will be hit for all outgoing connections going to port 443 because bind() succeeds even when "sa" contains the remote address, and pretty much nothing will work. It might the best to skip the check and marking connections as LOCAL if net.ipv4.ip_nonlocal_bind is enabled? If that doesn't seem reasonable, then maybe show a warning at start and/or just document that the ip_nonlocal_bind setting shouldn't be used with passt? -Valtteri

2 2

2024

2023

2022

2021

dev