- dev - passt

[PATCH] util: Don't stop on unrelated values when looking for --fd in close_open_files()
by Stefano Brivio 20 Aug '24

20 Aug '24

Seen with krun: we get a file descriptor via --fd, but we close it and happily use the same number for TCP files. The issue is that if we also get other options before --fd, with arguments, getopt_long() stops parsing them because it sees them as non-option values. Use the - modifier at the beginning of optstring, instead of +, which means we'll continue parsing after finding unrelated option values, but getopt_long() won't reorder them anyway: they'll be passed with option value '1', which we can ignore. By the way, we also need to add : after F in the optstring, so that we're able to parse the option when given as short name as well. Link: https://github.com/slp/krun/issues/17#issuecomment-2294943828 Fixes: baccfb95ce0e ("conf: Stop parsing options at first non-option argument") Fixes: 09603cab28f9 ("passt, util: Close any open file that the parent might have leaked") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util.c b/util.c index 0b41404..3fce3c2 100644 --- a/util.c +++ b/util.c @@ -710,7 +710,7 @@ void close_open_files(int argc, char **argv) int name, rc; do { - name = getopt_long(argc, argv, "+:F", optfd, NULL); + name = getopt_long(argc, argv, "-:F:", optfd, NULL); if (name == 'F') { errno = 0; -- 2.43.0

1 0

[PATCH] test: Update list of dependencies in README.md
by Stefano Brivio 20 Aug '24

20 Aug '24

Mostly packages we now need to run Podman-based tests. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- test/README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/README.md b/test/README.md index 0936b04..91ca603 100644 --- a/test/README.md +++ b/test/README.md @@ -28,10 +28,11 @@ on a system, i.e. common utilities such as a shell are not included here. Example for Debian, and possibly most Debian-based distributions: - build-essential git jq strace iperf3 qemu-system-x86 tmux sipcalc bats bc - catatonit clang-tidy cppcheck go isc-dhcp-common psmisc linux-cpupower socat - netcat-openbsd fakeroot lz4 lm-sensors qemu-system-arm qemu-system-ppc - qemu-system-misc qemu-system-x86 valgrind + bats bc build-essential catatonit clang-tidy conmon cppcheck crun fakeroot + git go iperf3 isc-dhcp-common jq libgpgme-dev libseccomp-dev linux-cpupower + lm-sensors lz4 netavark netcat-openbsd psmisc qemu-efi-aarch64 + qemu-system-arm qemu-system-misc qemu-system-ppc qemu-system-x86 + qemu-system-x86 sipcalc socat strace tmux uidmap valgrind NOTE: the tests need a qemu version >= 7.2, or one that contains commit 13c6be96618c ("net: stream: add unix socket"): this change introduces support -- 2.43.0

2 1

[PATCH] tcp, udp: Allow timerfd_gettime64() and recvmmsg_time64() on arm (armhf)
by Stefano Brivio 20 Aug '24

20 Aug '24

These system calls are needed after the conversion of time_t to 64-bit types on 32-bit architectures. Tested by running some transfer tests with passt and pasta on Debian Bookworm (glibc 2.36) and Trixie (glibc 2.39), running on armv6l. Suggested-by: Faidon Liambotis <paravoid(a)debian.org> Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1078981 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 2 +- udp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tcp.c b/tcp.c index c542a78..501c56c 100644 --- a/tcp.c +++ b/tcp.c @@ -2143,7 +2143,7 @@ cancel: * @c: Execution context * @ref: epoll reference of timer (not connection) * - * #syscalls timerfd_gettime i686:timerfd_gettime64 + * #syscalls timerfd_gettime arm:timerfd_gettime64 i686:timerfd_gettime64 */ void tcp_timer_handler(struct ctx *c, union epoll_ref ref) { diff --git a/udp.c b/udp.c index 851881e..f8fd597 100644 --- a/udp.c +++ b/udp.c @@ -460,7 +460,7 @@ static bool udp_sock_recverr(int s) * @events: epoll events bitmap * @mmh mmsghdr array to receive into * - * #syscalls recvmmsg i686:recvmmsg_time64 + * #syscalls recvmmsg arm:recvmmsg_time64 i686:recvmmsg_time64 */ static int udp_sock_recv(const struct ctx *c, int s, uint32_t events, struct mmsghdr *mmh) -- 2.43.0

2 1

[PATCH 0/2] Build fixes for musl, glibc < 2.34
by Stefano Brivio 20 Aug '24

20 Aug '24

Stefano Brivio (2): udp_flow: Add missing unistd.h include for close() util: Provide own version of close_range(), and no-op fallback udp_flow.c | 1 + util.h | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) -- 2.43.0

2 4

[PATCH] treewide: Allow additional system calls for i386/i686
by Stefano Brivio 20 Aug '24

20 Aug '24

I haven't tested i386 for a long time (after playing with some openSUSE i586 image a couple of years ago). It turns out that a number of system calls we actually need were denied by the seccomp filter, and not even basic functionality works. Add some system calls that glibc started using with the 64-bit time ("t64") transition, see also: https://wiki.debian.org/ReleaseGoals/64bit-time that is: clock_gettime64, timerfd_gettime64, fcntl64, and recvmmsg_time64. Add further system calls that are needed regardless of time_t width, that is, mmap2 (valgrind profile only), _llseek and sigreturn (common outside x86_64), and socketcall (same as s390x). I validated this against an almost full run of the test suite, with just a few selected tests skipped. Fixes needed to run most tests on i386/i686, and other assorted fixes for tests, are included in upcoming patches. Reported-by: Uroš Knupleš <uros(a)knuples.net> Analysed-by: Faidon Liambotis <paravoid(a)debian.org> Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1078981 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- Makefile | 2 +- log.c | 2 +- passt.c | 6 +++--- pasta.c | 2 +- tcp.c | 2 +- tcp_splice.c | 2 +- udp.c | 2 +- udp_flow.c | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index b6329e3..01fada4 100644 --- a/Makefile +++ b/Makefile @@ -129,7 +129,7 @@ qrap: $(QRAP_SRCS) passt.h valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime mmap \ - munmap open unlink gettimeofday futex + mmap2 munmap open unlink gettimeofday futex valgrind: FLAGS += -g -DVALGRIND valgrind: all diff --git a/log.c b/log.c index e7202d0..433b552 100644 --- a/log.c +++ b/log.c @@ -98,7 +98,7 @@ const char *logfile_prefix[] = { * @fd: Log file descriptor * @now: Current timestamp * - * #syscalls lseek ppc64le:_llseek ppc64:_llseek arm:_llseek + * #syscalls lseek ppc64le:_llseek ppc64:_llseek arm:_llseek i686:_llseek */ static void logfile_rotate_fallocate(int fd, const struct timespec *now) { diff --git a/passt.c b/passt.c index 4b3c306..f9670b3 100644 --- a/passt.c +++ b/passt.c @@ -191,11 +191,11 @@ void exit_handler(int signal) * Return: non-zero on failure * * #syscalls read write writev - * #syscalls socket bind connect getsockopt setsockopt s390x:socketcall close - * #syscalls recvfrom sendto shutdown + * #syscalls socket getsockopt setsockopt s390x:socketcall i686:socketcall close + * #syscalls bind connect recvfrom sendto shutdown * #syscalls arm:recv ppc64le:recv arm:send ppc64le:send * #syscalls accept4|accept listen epoll_ctl epoll_wait|epoll_pwait epoll_pwait - * #syscalls clock_gettime arm:clock_gettime64 + * #syscalls clock_gettime arm:clock_gettime64 i686:clock_gettime64 */ int main(int argc, char **argv) { diff --git a/pasta.c b/pasta.c index 1142f03..49b56db 100644 --- a/pasta.c +++ b/pasta.c @@ -13,7 +13,7 @@ * * #syscalls:pasta clone waitid exit exit_group rt_sigprocmask * #syscalls:pasta rt_sigreturn|sigreturn - * #syscalls:pasta arm:sigreturn ppc64:sigreturn s390x:sigreturn + * #syscalls:pasta arm:sigreturn ppc64:sigreturn s390x:sigreturn i686:sigreturn */ #include <sched.h> diff --git a/tcp.c b/tcp.c index c0820ce..c542a78 100644 --- a/tcp.c +++ b/tcp.c @@ -2143,7 +2143,7 @@ cancel: * @c: Execution context * @ref: epoll reference of timer (not connection) * - * #syscalls timerfd_gettime + * #syscalls timerfd_gettime i686:timerfd_gettime64 */ void tcp_timer_handler(struct ctx *c, union epoll_ref ref) { diff --git a/tcp_splice.c b/tcp_splice.c index 483e45d..9f5cc27 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -28,7 +28,7 @@ * - FIN_SENT_0: FIN (write shutdown) sent to accepted socket * - FIN_SENT_1: FIN (write shutdown) sent to target socket * - * #syscalls:pasta pipe2|pipe fcntl arm:fcntl64 ppc64:fcntl64 + * #syscalls:pasta pipe2|pipe fcntl arm:fcntl64 ppc64:fcntl64 i686:fcntl64 */ #include <sched.h> diff --git a/udp.c b/udp.c index 7731257..851881e 100644 --- a/udp.c +++ b/udp.c @@ -460,7 +460,7 @@ static bool udp_sock_recverr(int s) * @events: epoll events bitmap * @mmh mmsghdr array to receive into * - * #syscalls recvmmsg + * #syscalls recvmmsg i686:recvmmsg_time64 */ static int udp_sock_recv(const struct ctx *c, int s, uint32_t events, struct mmsghdr *mmh) diff --git a/udp_flow.c b/udp_flow.c index 8b25ad1..b1133c0 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -174,7 +174,7 @@ cancel: * @s_in: Source socket address, filled in by recvmmsg() * @now: Timestamp * - * #syscalls fcntl + * #syscalls fcntl arm:fcntl64 ppc64:fcntl64 i686:fcntl64 * * Return: sidx for the destination side of the flow for this packet, or * FLOW_SIDX_NONE if we couldn't find or create a flow. -- 2.43.0

2 1

[PATCH v4 0/7] Prevent DAD for link-local addresses in containers
by Stefano Brivio 17 Aug '24

17 Aug '24

There's no point in letting a container perform duplicate address detection as we'll silently discard neighbour solicitations with unspecified source addresses anyway, without relaying them to anybody. And we realised that it's not harmless, see the whole discussion around https://github.com/containers/podman/pull/23561#discussion_r1711639663: we can't communicate with the container right away because of that, which is surely annoying for tests, but it could also be an issue for use cases with very short-lived containers or namespaces. Disabling DAD via procfs configuration would be simpler than all this, but we don't own the namespace (unless we spawn a shell), so we shouldn't mess up with procfs entries, assuming it's even possible. Set the nodad attribute, and prevent DAD from being triggered before on link up, before we can set that attribute. v4: - fix botched rebase, changes meant for 4/7 in v3 ended up in 6/7 v3: - in 4/7, actually handle all the netlink responses for the case where we change multiple addresses v2: - in 4/7, instead of doing the whole nl_routes_dup()-vendored dance to keep addresses in a single buffer, send NLM_F_REPLACE requests right away, but use nlmsg_send() instead of nl_do(), and check for answers to our further requests later. Use warn() instead of die() if we can't set nodad attributes - in 5/7, make nl_addr_get_ll() get a pointer to struct in6_addr instead of a generic void pointer, and warn(), don't die(), if it fails Stefano Brivio (7): netlink: Fix typo in function comment for nl_addr_get() netlink, pasta: Split MTU setting functionality out of nl_link_up() netlink, pasta: Turn nl_link_up() into a generic function to set link flags netlink, pasta: Disable DAD for link-local addresses on namespace interface netlink, pasta: Fetch link-local address from namespace interface once it's up pasta: Disable neighbour solicitations on device up to prevent DAD netlink: Fix typo in function comment for nl_addr_set() netlink.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++----- netlink.h | 6 ++- pasta.c | 29 ++++++++++- 3 files changed, 166 insertions(+), 15 deletions(-) -- 2.43.0

2 8

[PATCH v3 0/7] Prevent DAD for link-local addresses in containers
by Stefano Brivio 17 Aug '24

17 Aug '24

There's no point in letting a container perform duplicate address detection as we'll silently discard neighbour solicitations with unspecified source addresses anyway, without relaying them to anybody. And we realised that it's not harmless, see the whole discussion around https://github.com/containers/podman/pull/23561#discussion_r1711639663: we can't communicate with the container right away because of that, which is surely annoying for tests, but it could also be an issue for use cases with very short-lived containers or namespaces. Disabling DAD via procfs configuration would be simpler than all this, but we don't own the namespace (unless we spawn a shell), so we shouldn't mess up with procfs entries, assuming it's even possible. Set the nodad attribute, and prevent DAD from being triggered before on link up, before we can set that attribute. v3: - in 4/7, actually handle all the netlink responses for the case where we change multiple addresses v2: - in 4/7, instead of doing the whole nl_routes_dup()-vendored dance to keep addresses in a single buffer, send NLM_F_REPLACE requests right away, but use nlmsg_send() instead of nl_do(), and check for answers to our further requests later. Use warn() instead of die() if we can't set nodad attributes - in 5/7, make nl_addr_get_ll() get a pointer to struct in6_addr instead of a generic void pointer, and warn(), don't die(), if it fails Stefano Brivio (7): netlink: Fix typo in function comment for nl_addr_get() netlink, pasta: Split MTU setting functionality out of nl_link_up() netlink, pasta: Turn nl_link_up() into a generic function to set link flags netlink, pasta: Disable DAD for link-local addresses on namespace interface netlink, pasta: Fetch link-local address from namespace interface once it's up pasta: Disable neighbour solicitations on device up to prevent DAD netlink: Fix typo in function comment for nl_addr_set() netlink.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++----- netlink.h | 6 ++- pasta.c | 29 ++++++++++- 3 files changed, 166 insertions(+), 15 deletions(-) -- 2.43.0

2 10

[PATCH v2 0/7] Prevent DAD for link-local addresses in containers
by Stefano Brivio 16 Aug '24

16 Aug '24

There's no point in letting a container perform duplicate address detection as we'll silently discard neighbour solicitations with unspecified source addresses anyway, without relaying them to anybody. And we realised that it's not harmless, see the whole discussion around https://github.com/containers/podman/pull/23561#discussion_r1711639663: we can't communicate with the container right away because of that, which is surely annoying for tests, but it could also be an issue for use cases with very short-lived containers or namespaces. Disabling DAD via procfs configuration would be simpler than all this, but we don't own the namespace (unless we spawn a shell), so we shouldn't mess up with procfs entries, assuming it's even possible. Set the nodad attribute, and prevent DAD from being triggered before on link up, before we can set that attribute. v2: - in 4/7, instead of doing the whole nl_routes_dup()-vendored dance to keep addresses in a single buffer, send NLM_F_REPLACE requests right away, but use nlmsg_send() instead of nl_do(), and check for answers to our further requests later. Use warn() instead of die() if we can't set nodad attributes - in 5/7, make nl_addr_get_ll() get a pointer to struct in6_addr instead of a generic void pointer, and warn(), don't die(), if it fails Stefano Brivio (7): netlink: Fix typo in function comment for nl_addr_get() netlink, pasta: Split MTU setting functionality out of nl_link_up() netlink, pasta: Turn nl_link_up() into a generic function to set link flags netlink, pasta: Disable DAD for link-local addresses on namespace interface netlink, pasta: Fetch link-local address from namespace interface once it's up pasta: Disable neighbour solicitations on device up to prevent DAD netlink: Fix typo in function comment for nl_addr_set() netlink.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++++----- netlink.h | 6 ++- pasta.c | 29 ++++++++++- 3 files changed, 164 insertions(+), 15 deletions(-) -- 2.43.0

2 12

[PATCH 0/7] Prevent DAD for link-local addresses in containers
by Stefano Brivio 15 Aug '24

15 Aug '24

There's no point in letting a container perform duplicate address detection as we'll silently discard neighbour solicitations with unspecified source addresses anyway, without relaying them to anybody. And we realised that it's not harmless, see the whole discussion around https://github.com/containers/podman/pull/23561#discussion_r1711639663: we can't communicate with the container right away because of that, which is surely annoying for tests, but it could also be an issue for use cases with very short-lived containers or namespaces. Disabling DAD via procfs configuration would be simpler than all this, but we don't own the namespace (unless we spawn a shell), so we shouldn't mess up with procfs entries, assuming it's even possible. Set the nodad attribute, and prevent DAD from being triggered before on link up, before we can set that attribute. Stefano Brivio (7): netlink: Fix typo in function comment for nl_addr_get() netlink, pasta: Split MTU setting functionality out of nl_link_up() netlink, pasta: Turn nl_link_up() into a generic function to set link flags netlink, pasta: Disable DAD for link-local addresses on namespace interface netlink, pasta: Fetch link-local address from namespace interface once it's up pasta: Disable neighbour solicitations on device up to prevent DAD netlink: Fix typo in function comment for nl_addr_set() netlink.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++++++---- netlink.h | 6 +- pasta.c | 29 ++++++++- 3 files changed, 206 insertions(+), 15 deletions(-) -- 2.43.0

2 16

[PATCH 0/3] Assorted fixes related to addressing
by David Gibson 15 Aug '24

15 Aug '24

Here are fixes for a number of bugs I encountered while working toward configurable host mapping. There are more coming, but I'm still working on getting them ready. David Gibson (3): Correct inaccurate comments on ip[46]_ctx::addr conf: Delay handling -D option until after addresses are configured fwd: Rework inconsistencies in translation of inbound flows conf.c | 84 +++++++++++++++++++++++++++++------------------------- fwd.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-------- passt.h | 4 +-- 3 files changed, 124 insertions(+), 52 deletions(-) -- 2.46.0

2 5