- dev - passt

[PATCH] tcp: Clamp MSS value when queueing data to tap, also for pasta
by Stefano Brivio 08 Mar '23

08 Mar '23

Tom reports that a pattern of repated ~1 MiB chunks downloads over NNTP over TLS, on Podman 4.4 using pasta as network back-end, results in pasta taking one full CPU thread after a while, and the download never succeeds. On that setup, we end up re-sending the same frame over and over, with a consistent 65 534 bytes size, and never get an acknowledgement from the tap-side client. This only happens for the default MTU value (65 520 bytes) or for values that are slightly smaller than that (down to 64 499 bytes). We hit this condition because the MSS value we use in tcp_data_from_sock(), only in pasta mode, is simply clamped to USHRT_MAX, and not to the actual size of the buffers we pre-cooked for sending, which is a bit less than that. It looks like we got away with it until commit 0fb7b2b9080a ("tap: Use different io vector bases depending on tap type") fixed the setting of iov_len. Luckily, since it's pasta, we're queueing up to two frames at a time, so the worst that can happen is a badly segmented TCP stream: we always have some space at the tail of the buffer. Clamp the MSS value to the appropriate maximum given by struct tcp{4,6}_buf_data_t, no matter if we're running in pasta or passt mode. While at it, fix the comments to those structs to reflect the current struct size. This is not really relevant for any further calculation or consideration, but it's convenient to know while debugging this kind of issues. Thanks to Tom for reporting the issue in a very detailed way and for providing a test setup. Reported-by: Tom Mombourquette <tom(a)devnode.com> Link: https://github.com/containers/podman/issues/17703 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/tcp.c b/tcp.c index a29e387..0214087 100644 --- a/tcp.c +++ b/tcp.c @@ -461,7 +461,7 @@ static struct tcp4_l2_buf_t { struct iphdr iph; /* 44 28 */ struct tcphdr th; /* 64 48 */ uint8_t data[MSS4]; /* 84 68 */ - /* 65541 65525 */ + /* 65536 65532 */ #ifdef __AVX2__ } __attribute__ ((packed, aligned(32))) #else @@ -489,7 +489,7 @@ struct tcp6_l2_buf_t { struct ipv6hdr ip6h; /* 32 20 */ struct tcphdr th; /* 72 60 */ uint8_t data[MSS6]; /* 92 80 */ - /* 65639 65627 */ + /* 65536 65532 */ #ifdef __AVX2__ } __attribute__ ((packed, aligned(32))) #else @@ -1917,15 +1917,13 @@ int tcp_conn_new_sock(const struct ctx *c, sa_family_t af) /** * tcp_conn_tap_mss() - Get MSS value advertised by tap/guest - * @c: Execution context * @conn: Connection pointer * @opts: Pointer to start of TCP options * @optlen: Bytes in options: caller MUST ensure available length * * Return: clamped MSS value */ -static uint16_t tcp_conn_tap_mss(const struct ctx *c, - const struct tcp_tap_conn *conn, +static uint16_t tcp_conn_tap_mss(const struct tcp_tap_conn *conn, const char *opts, size_t optlen) { unsigned int mss; @@ -1936,13 +1934,10 @@ static uint16_t tcp_conn_tap_mss(const struct ctx *c, else mss = ret; - /* Don't upset qemu */ - if (c->mode == MODE_PASST) { - if (CONN_V4(conn)) - mss = MIN(MSS4, mss); - else - mss = MIN(MSS6, mss); - } + if (CONN_V4(conn)) + mss = MIN(MSS4, mss); + else + mss = MIN(MSS6, mss); return MIN(mss, USHRT_MAX); } @@ -2066,7 +2061,7 @@ static void tcp_conn_from_tap(struct ctx *c, int af, const void *addr, conn->wnd_to_tap = WINDOW_DEFAULT; - mss = tcp_conn_tap_mss(c, conn, opts, optlen); + mss = tcp_conn_tap_mss(conn, opts, optlen); if (setsockopt(s, SOL_TCP, TCP_MAXSEG, &mss, sizeof(mss))) trace("TCP: failed to set TCP_MAXSEG on socket %i", s); MSS_SET(conn, mss); @@ -2533,7 +2528,7 @@ static void tcp_conn_from_sock_finish(struct ctx *c, struct tcp_tap_conn *conn, if (!(conn->wnd_from_tap >>= conn->ws_from_tap)) conn->wnd_from_tap = 1; - MSS_SET(conn, tcp_conn_tap_mss(c, conn, opts, optlen)); + MSS_SET(conn, tcp_conn_tap_mss(conn, opts, optlen)); conn->seq_init_from_tap = ntohl(th->seq) + 1; conn->seq_from_tap = conn->seq_init_from_tap; -- 2.39.2

2 1

[PATCH v3 0/2] Implement explicit outbound address and interface selection
by Stefano Brivio 08 Mar '23

08 Mar '23

v3: In 2/2, *really* pass the correct bind_addr reference to sock_l4() in udp_tap_handler() -- I just sent v2 from the wrong directory :/ Stefano Brivio (2): conf, passt.h: Rename "outbound" interface to "template" interface conf, icmp, tcp, udp: Add options to bind to outbound address and interface conf.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++------ icmp.c | 24 +++++++++++++--- passt.1 | 19 ++++++++++++ passt.h | 14 +++++++-- tcp.c | 60 ++++++++++++++++++++++++++++++++++++++ udp.c | 54 +++++++++++++++++++++++----------- 6 files changed, 228 insertions(+), 32 deletions(-) -- 2.39.2

2 3

[PATCH 0/3] Fail gracefully on too many open files
by Stefano Brivio 08 Mar '23

08 Mar '23

If we can't bind a few ports out of a port specifier, fine, we don't want to bother the user with that. But running out of files on '-t all' or similar is something different: patch 3/3 fixes this, 1/3 and 2/3 are preparation steps. Stefano Brivio (3): tcp, udp, util: Pass socket creation errors all the way up tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init() conf: Terminate on EMFILE or ENFILE on sockets for port mapping conf.c | 36 +++++++++++++++++++++++++++++------- tcp.c | 29 ++++++++++++++--------------- udp.c | 44 +++++++++++++++++++++----------------------- util.c | 31 ++++++++++++++++++------------- 4 files changed, 82 insertions(+), 58 deletions(-) -- 2.39.2

2 6

[PATCH v2 0/2] Implement explicit outbound address and interface selection
by Stefano Brivio 08 Mar '23

08 Mar '23

v2: In 2/2, pass the correct bind_addr reference to sock_l4() in udp_tap_handler() Stefano Brivio (2): conf, passt.h: Rename "outbound" interface to "template" interface conf, icmp, tcp, udp: Add options to bind to outbound address and interface conf.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++------ icmp.c | 24 +++++++++++++--- passt.1 | 19 ++++++++++++ passt.h | 14 +++++++-- tcp.c | 60 ++++++++++++++++++++++++++++++++++++++ udp.c | 54 +++++++++++++++++++++++----------- 6 files changed, 228 insertions(+), 32 deletions(-) -- 2.39.2

1 2

[PATCH 0/4] Fix build against musl C library
by Stefano Brivio 08 Mar '23

08 Mar '23

This series, partially coming from Chris Kuhn, enables passt to be built against musl. Tested on Alpine (x86_64) 3.17.2, musl 1.2.3-r4. See also: https://bugs.passt.top/show_bug.cgi?id=4 https://github.com/void-linux/void-packages/pull/42517 Chris Kuhn (2): conf, passt: Rename stderr to force_stderr treewide: Fix header includes to build with musl Stefano Brivio (2): netlink: Use 8 KiB * netlink message header size as response buffer util: Carry own definition of __bswap_constant{16,32} conf.c | 8 +++++--- isolation.c | 1 + netlink.c | 16 ++++++++++------ passt.c | 4 +++- passt.h | 4 ++-- tap.c | 1 + tcp.c | 1 + tcp_splice.c | 1 + udp.c | 1 + util.c | 1 + util.h | 11 +++++++++++ 11 files changed, 37 insertions(+), 12 deletions(-) -- 2.39.2

2 8

[PATCH 0/4] Further adjustments for SELinux policy files
by Stefano Brivio 08 Mar '23

08 Mar '23

This series addresses a number of issues or inconveniences shown by further testing against libvirt, essentially a 9.1.0 version patched up to fix the current breakage by starting passt in the "passt_t" domain. Stefano Brivio (4): contrib/selinux: Drop duplicate init_daemon_domain() rule contrib/selinux: Let passt write to stdout and stderr when it starts contrib/selinux: Allow binding and connecting to all UDP and TCP ports contrib/selinux: Let interface users set paths for log, PID, socket files contrib/selinux/passt.if | 26 +++++++++++++++++++++++++- contrib/selinux/passt.te | 29 ++++++++++++++++------------- 2 files changed, 41 insertions(+), 14 deletions(-) -- 2.39.2

2 5

[PATCH 1/3] musl adaptations
by KuhnChris 04 Mar '23

04 Mar '23

--- conf.c | 6 +++--- passt.c | 2 +- passt.h | 4 ++-- util.h | 14 ++++++++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/conf.c b/conf.c index 0e512f4..c1dd9ba 100644 --- a/conf.c +++ b/conf.c @@ -1305,13 +1305,13 @@ void conf(struct ctx *c, int argc, char **argv) if (logfile) die("Can't log to both file and stderr"); - if (c->stderr) + if (c->force_stderr) die("Multiple --stderr options given"); - c->stderr = 1; + c->force_stderr = 1; break; case 'l': - if (c->stderr) + if (c->force_stderr) die("Can't log to both stderr and file"); if (logfile) diff --git a/passt.c b/passt.c index 5b8146e..f67213a 100644 --- a/passt.c +++ b/passt.c @@ -241,7 +241,7 @@ int main(int argc, char **argv) conf(&c, argc, argv); trace_init(c.trace); - if (c.stderr || isatty(fileno(stdout))) + if (c.force_stderr || isatty(fileno(stdout))) __openlog(log_name, LOG_PERROR, LOG_DAEMON); quit_fd = pasta_netns_quit_init(&c); diff --git a/passt.h b/passt.h index e0383eb..71d3602 100644 --- a/passt.h +++ b/passt.h @@ -32,7 +32,7 @@ struct tap_l4_msg { union epoll_ref; #include <stdbool.h> - +#include <bits/limits.h> #include "packet.h" #include "icmp.h" #include "port_fwd.h" @@ -197,7 +197,7 @@ struct ctx { int trace; int quiet; int foreground; - int stderr; + int force_stderr; int nofile; char sock_path[UNIX_PATH_MAX]; char pcap[PATH_MAX]; diff --git a/util.h b/util.h index 570094c..7315ce2 100644 --- a/util.h +++ b/util.h @@ -7,7 +7,10 @@ #define UTIL_H #include <stdlib.h> +#include <stdio.h> #include <stdarg.h> +#include <signal.h> +#include <byteswap.h> #include "log.h" @@ -88,6 +91,8 @@ #define MAC_ZERO ((uint8_t [ETH_ALEN]){ 0 }) #define MAC_IS_ZERO(addr) (!memcmp((addr), MAC_ZERO, ETH_ALEN)) +#if defined(__GLIBC__) || defined(__UCLIBC__) + #if __BYTE_ORDER == __BIG_ENDIAN #define htons_constant(x) (x) #define htonl_constant(x) (x) @@ -96,6 +101,15 @@ #define htonl_constant(x) (__bswap_constant_32(x)) #endif +#else + +/* mainly musl fallback */ + +#define htons_constant(x) (x) +#define htonl_constant(x) (x) + +#endif + #define IN4_IS_ADDR_UNSPECIFIED(a) \ ((a)->s_addr == htonl(INADDR_ANY)) #define IN4_IS_ADDR_BROADCAST(a) \ -- 2.38.4

2 5

[PATCH] udp: Fix signedness warning on 32-bits architectures
by Stefano Brivio 01 Mar '23

01 Mar '23

When a ssize_t is an int: udp.c: In function ‘udp_sock_handler’: udp.c:774:23: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘ssize_t’ {aka ‘int’} [-Wsign-compare] 774 | for (i = 0; i < n; i += m) { | ^ udp.c:781:43: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘ssize_t’ {aka ‘int’} [-Wsign-compare] 781 | for (m = 1; i + m < n; m++) { | Change 'i' and 'm' counters in udp_sock_handler() to signed versions, to match ssize_t n. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- udp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/udp.c b/udp.c index 20a9ea0..b7cbfdc 100644 --- a/udp.c +++ b/udp.c @@ -754,7 +754,7 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, in_port_t dstport = ref.r.p.udp.udp.port; bool v6 = ref.r.p.udp.udp.v6; struct mmsghdr *mmh_recv; - unsigned int i, m; + int i, m; if (!(events & EPOLLIN)) return; -- 2.39.1

2 1

[PATCH] Makefile, seccomp.sh: Fix cross-builds, adjust syscalls list to compiler
by Stefano Brivio 01 Mar '23

01 Mar '23

Debian cross-building automatic checks: http://crossqa.debian.net/src/passt currently fail because we don't use the right target architecture and compiler while building the system call lists and resolving their numbers in seccomp.sh. Pass ARCH and CC to seccomp.sh and use them. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- Makefile | 2 +- seccomp.sh | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 667ddfb..e5723a7 100644 --- a/Makefile +++ b/Makefile @@ -105,7 +105,7 @@ static: FLAGS += -static -DGLIBC_NO_STATIC_NSS static: clean all seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS) - @ EXTRA_SYSCALLS="$(EXTRA_SYSCALLS)" ./seccomp.sh $(PASST_SRCS) $(PASST_HEADERS) + @ EXTRA_SYSCALLS="$(EXTRA_SYSCALLS)" ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh $(PASST_SRCS) $(PASST_HEADERS) passt: $(PASST_SRCS) $(HEADERS) $(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) diff --git a/seccomp.sh b/seccomp.sh index 31ea8da..b075efd 100755 --- a/seccomp.sh +++ b/seccomp.sh @@ -17,6 +17,9 @@ TMP="$(mktemp)" IN="$@" OUT="seccomp.h" +[ -z "${ARCH}" ] && ARCH="$(uname -m)" +[ -z "${CC}" ] && CC="cc" + HEADER="/* This file was automatically generated by $(basename ${0}) */ #ifndef AUDIT_ARCH_PPC64LE @@ -110,7 +113,7 @@ log2() { # $1: Name of syscall syscall_nr() { __in="$(printf "#include <asm-generic/unistd.h>\n#include <sys/syscall.h>\n__NR_%s" ${1})" - __out="$(echo "${__in}" | cc -E -xc - -o - | tail -1)" + __out="$(echo "${__in}" | ${CC} -E -xc - -o - | tail -1)" [ "${__out}" = "__NR_$1" ] && return 1 # Output might be in the form "(x + y)" (seen on armv6l, armv7l) @@ -125,7 +128,7 @@ filter() { case ${__c} in *:*) case ${__c} in - $(uname -m):*) + ${ARCH}:*) __arch_match=1 __c=${__c##*:} ;; -- 2.39.1

2 1

[PATCH] Makefile: Fix SuperH 4 builds: it's AUDIT_ARCH_SH, not AUDIT_ARCH_SH4
by Stefano Brivio 28 Feb '23

28 Feb '23

sh4 builds currently fail because of this: https://buildd.debian.org/status/fetch.php?pkg=passt&arch=sh4&ver=0.0%7Egit… Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index e5723a7..6f8bd81 100644 --- a/Makefile +++ b/Makefile @@ -31,6 +31,7 @@ AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/PPC64/PPC/') AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/PPCLE/PPC64LE/') AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/MIPS64EL/MIPSEL64/') AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/HPPA/PARISC/') +AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/SH4/SH/') FLAGS := -Wall -Wextra -pedantic -std=c99 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE FLAGS += -D_FORTIFY_SOURCE=2 -O2 -pie -fPIE -- 2.39.1

1 0