June 2023 - dev

[PATCH] conf: Accept -a and -g without --config-net in pasta mode
by Stefano Brivio 23 Jun '23

23 Jun '23

While --no-copy-addrs and --no-copy-routes only make sense with --config-net, and they are implied on -g and -a, respectively, that doesn't mean we should refuse -a or -g without --config-net: they are still relevant for a number of things (including DHCP/DHCPv6/NDP configuration). Reported-by: Gianluca Stivan <me(a)yawnt.com> Fixes: cc9d16758be6 ("conf, pasta: With --config-net, copy all addresses by default") Fixes: da54641f140e ("conf, pasta: With --config-net, copy all routes by default") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- This is based on my previous patch: "conf: Make -a/--address really imply --no-copy-addrs" conf.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/conf.c b/conf.c index 16a3e64..820d597 100644 --- a/conf.c +++ b/conf.c @@ -1197,6 +1197,7 @@ void conf(struct ctx *c, int argc, char **argv) }; struct get_bound_ports_ns_arg ns_ports_arg = { .c = c }; char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 }; + bool copy_addrs_opt = false, copy_routes_opt = false; bool v4_only = false, v6_only = false; char *runas = NULL, *logfile = NULL; struct in6_addr *dns6 = c->ip6.dns; @@ -1362,14 +1363,14 @@ void conf(struct ctx *c, int argc, char **argv) die("--no-copy-routes is for pasta mode only"); warn("--no-copy-routes will be dropped soon"); - c->no_copy_routes = 1; + c->no_copy_routes = copy_routes_opt = true; break; case 19: if (c->mode != MODE_PASTA) die("--no-copy-addrs is for pasta mode only"); warn("--no-copy-addrs will be dropped soon"); - c->no_copy_addrs = 1; + c->no_copy_addrs = copy_addrs_opt = true; break; case 20: if (logfile) @@ -1682,10 +1683,10 @@ void conf(struct ctx *c, int argc, char **argv) die("Options --socket and --fd are mutually exclusive"); if (c->mode == MODE_PASTA && !c->pasta_conf_ns) { - if (c->no_copy_routes) - die("Option --no-copy-routes needs --config-net"); - if (c->no_copy_addrs) - die("Option --no-copy-addrs needs --config-net"); + if (copy_routes_opt) + die("--no-copy-routes needs --config-net"); + if (copy_addrs_opt) + die("--no-copy-addrs needs --config-net"); } if (!ifi4 && *c->ip4.ifname_out) -- 2.39.2

1 0

[RFC v2] tcp: add support for MSG_PEEK with offset
by Jon Maloy 23 Jun '23

23 Jun '23

When reading received messages with MSG_PEEK, we would like to eliminate the need to read the leading bytes over and over again only to reach the part of the message we really want. We now introduce support for this feature, when supported by the kernel. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tcp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tcp.c b/tcp.c index 0ed9bfa..ebaf520 100644 --- a/tcp.c +++ b/tcp.c @@ -500,7 +500,6 @@ tcp6_l2_buf[TCP_FRAMES_MEM]; static unsigned int tcp6_l2_buf_used; /* recvmsg()/sendmsg() data for tap */ -static char tcp_buf_discard [MAX_WINDOW]; static struct iovec iov_sock [TCP_FRAMES_MEM + 1]; static struct iovec tcp4_l2_iov [TCP_FRAMES_MEM]; @@ -2239,7 +2238,7 @@ static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn) mh_sock.msg_iov = iov_sock; mh_sock.msg_iovlen = fill_bufs + 1; - iov_sock[0].iov_base = tcp_buf_discard; + iov_sock[0].iov_base = NULL; iov_sock[0].iov_len = already_sent; if (( v4 && tcp4_l2_buf_used + fill_bufs > ARRAY_SIZE(tcp4_l2_buf)) || -- 2.39.0

1 0

[PATCH] conf: Make -a/--address really imply --no-copy-addrs
by Stefano Brivio 23 Jun '23

23 Jun '23

I wrote it in commit message and man page, but not in conf()... Note that -g/--gateway correctly implies --no-copy-routes already. This fixes Podman's tests: podman networking with pasta(1) - IPv4 address assignment podman networking with pasta(1) - IPv4 default route assignment where we pass -a and -g to assign an address and a default gateway that's compatible with it, but -a doesn't disable the copy of addresses, so we ignore -a, and the default gateway is incompatible with the addresses we copy -- hence no routes in the container. Link: https://github.com/containers/podman/pull/18612 Fixes: cc9d16758be6 ("conf, pasta: With --config-net, copy all addresses by default") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/conf.c b/conf.c index db9d095..16a3e64 100644 --- a/conf.c +++ b/conf.c @@ -1502,6 +1502,9 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'a': + if (c->mode == MODE_PASTA) + c->no_copy_addrs = 1; + if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr) && inet_pton(AF_INET6, optarg, &c->ip6.addr) && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr) && -- 2.39.2

1 0

Option parsing: Allow the same option multiple times
by Paul Holzinger 20 Jun '23

20 Jun '23

Hi all, following up on a quick discussion with Stefano on IRC. passt/pasta currently rejects most (not all) options when specified multiple times, i.e. pasta -I eth0 -I eth1 ... fails. I think it makes more sense to just use the last one instead. My use case: In podman I added a new containers.conf option[1] which allows users to set default pasta cli options. However users can also add options on the podman cli with podman run --net=pasta:... For me it would make the most sense to just append those to the config options and then let pasta deal with it. This allows some form of overwrite mechanism, i.e. by default I may have "-I" , "eth0" in containers.conf but for one specific container I want to use a different interface name and set --net=pasta:-I,eth1 on the cli. Then podman should just hand "-I eth0 -I eth1" to pasta and then pasta picks the last one. If we keep the current behavior it means I am forced to parse the options in podman and dedup them which is hard to maintain as podman would need to keep up with pasta upstream. I am willing to send a patch to change this so please let me know if anyone would object to that. [1] https://github.com/containers/common/commit/da3ac5d6d025f2933e655a0541eddbc… Thanks, Paul

3 5

[PATCH 0/2] Introduce --log-fd option
by Michal Privoznik 13 Jun '23

13 Jun '23

The idea behind this is so that libvirt can create the log file, set all DAC/SELinux labels and just pass pre-opened FD to passt. You can view my WIP patches for libvirt here: https://gitlab.com/MichalPrivoznik/libvirt/-/commit/5bbe40087d6888a7c4031a2… Michal Privoznik (2): util: Introduce set_cloexec() conf, log: Introduce --log-fd option conf.c | 29 +++++++++++++++++++++++++---- log.c | 17 ++++++++++++----- log.h | 2 +- passt.1 | 5 +++++ util.c | 17 +++++++++++++++++ util.h | 1 + 6 files changed, 61 insertions(+), 10 deletions(-) -- 2.39.3

3 9

[PATCH v2] conf, log: On -h / --help, print usage to stdout, not stderr
by Stefano Brivio 05 Jun '23

05 Jun '23

Erik suggests that this makes it easier to grep for options, and with --help we're anyway printing usage information as expected, not as part of an error report. While at it: on -h, we should exit with 0. Reported-by: Erik Sjölund <erik.sjolund(a)gmail.com> Link: https://bugs.passt.top/show_bug.cgi?id=52 Link: https://bugs.passt.top/show_bug.cgi?id=53 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Exit with 0, non with 1 as also reported by Erik conf.c | 23 ++++++++++++++++++----- log.c | 8 +++++--- log.h | 1 + 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/conf.c b/conf.c index ffff235..87dec50 100644 --- a/conf.c +++ b/conf.c @@ -717,10 +717,11 @@ static unsigned int conf_ip6(unsigned int ifi, } /** - * usage() - Print usage and exit + * print_usage() - Print usage, exit with given status code * @name: Executable name + * @status: Status code for exit() */ -static void usage(const char *name) +static void print_usage(const char *name, int status) { if (strstr(name, "pasta")) { info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name); @@ -856,7 +857,7 @@ static void usage(const char *name) info( " SPEC is as described for TCP above"); info( " default: none"); - exit(EXIT_FAILURE); + exit(status); pasta_opts: @@ -906,7 +907,16 @@ pasta_opts: info( " Don't copy all addresses to namespace"); info( " --ns-mac-addr ADDR Set MAC address on tap interface"); - exit(EXIT_FAILURE); + exit(status); +} + +/** + * usage() - Print usage and exit with failure + * @name: Executable name + */ +static void usage(const char *name) +{ + print_usage(name, EXIT_FAILURE); } /** @@ -1630,8 +1640,11 @@ void conf(struct ctx *c, int argc, char **argv) case 'U': /* Handle these later, once addresses are configured */ break; - case '?': case 'h': + log_to_stdout = 1; + print_usage(argv[0], EXIT_SUCCESS); + break; + case '?': default: usage(argv[0]); break; diff --git a/log.c b/log.c index 3a3d101..63d7801 100644 --- a/log.c +++ b/log.c @@ -43,17 +43,19 @@ static char log_header[BUFSIZ]; /* File header, written back on cuts */ static time_t log_start; /* Start timestamp */ int log_trace; /* --trace mode enabled */ +int log_to_stdout; /* Print to stdout instead of stderr */ #define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) #define logfn(name, level) \ void name(const char *format, ...) { \ + FILE *out = log_to_stdout ? stdout : stderr; \ struct timespec tp; \ va_list args; \ \ if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { \ clock_gettime(CLOCK_REALTIME, &tp); \ - fprintf(stderr, "%lli.%04lli: ", \ + fprintf(out, "%lli.%04lli: ", \ (long long int)tp.tv_sec - log_start, \ (long long int)tp.tv_nsec / (100L * 1000)); \ } \ @@ -70,10 +72,10 @@ void name(const char *format, ...) { \ if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) || \ (BEFORE_DAEMON && !(log_opt & LOG_PERROR))) { \ va_start(args, format); \ - (void)vfprintf(stderr, format, args); \ + (void)vfprintf(out, format, args); \ va_end(args); \ if (format[strlen(format)] != '\n') \ - fprintf(stderr, "\n"); \ + fprintf(out, "\n"); \ } \ } diff --git a/log.h b/log.h index 3aab29d..a17171a 100644 --- a/log.h +++ b/log.h @@ -22,6 +22,7 @@ void debug(const char *format, ...); } while (0) extern int log_trace; +extern int log_to_stdout; void trace_init(int enable); #define trace(...) \ do { \ -- 2.39.2

2 1

[PATCH] conf, log: On -h / --help, print usage to stdout, not stderr
by Stefano Brivio 05 Jun '23

05 Jun '23

Erik suggests that this makes it easier to grep for options, and with --help we're anyway printing usage information as expected, not as part of an error report. Reported-by: Erik Sjölund <erik.sjolund(a)gmail.com> Link: https://bugs.passt.top/show_bug.cgi?id=52 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 4 +++- log.c | 8 +++++--- log.h | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/conf.c b/conf.c index ffff235..9eb6992 100644 --- a/conf.c +++ b/conf.c @@ -1630,8 +1630,10 @@ void conf(struct ctx *c, int argc, char **argv) case 'U': /* Handle these later, once addresses are configured */ break; - case '?': case 'h': + log_to_stdout = 1; + /* Falls through */ + case '?': default: usage(argv[0]); break; diff --git a/log.c b/log.c index 3a3d101..63d7801 100644 --- a/log.c +++ b/log.c @@ -43,17 +43,19 @@ static char log_header[BUFSIZ]; /* File header, written back on cuts */ static time_t log_start; /* Start timestamp */ int log_trace; /* --trace mode enabled */ +int log_to_stdout; /* Print to stdout instead of stderr */ #define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) #define logfn(name, level) \ void name(const char *format, ...) { \ + FILE *out = log_to_stdout ? stdout : stderr; \ struct timespec tp; \ va_list args; \ \ if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { \ clock_gettime(CLOCK_REALTIME, &tp); \ - fprintf(stderr, "%lli.%04lli: ", \ + fprintf(out, "%lli.%04lli: ", \ (long long int)tp.tv_sec - log_start, \ (long long int)tp.tv_nsec / (100L * 1000)); \ } \ @@ -70,10 +72,10 @@ void name(const char *format, ...) { \ if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) || \ (BEFORE_DAEMON && !(log_opt & LOG_PERROR))) { \ va_start(args, format); \ - (void)vfprintf(stderr, format, args); \ + (void)vfprintf(out, format, args); \ va_end(args); \ if (format[strlen(format)] != '\n') \ - fprintf(stderr, "\n"); \ + fprintf(out, "\n"); \ } \ } diff --git a/log.h b/log.h index 3aab29d..a17171a 100644 --- a/log.h +++ b/log.h @@ -22,6 +22,7 @@ void debug(const char *format, ...); } while (0) extern int log_trace; +extern int log_to_stdout; void trace_init(int enable); #define trace(...) \ do { \ -- 2.39.2

2 2

[PATCH] tap: With pasta, don't reset on tap errors, handle write failures
by Stefano Brivio 05 Jun '23

05 Jun '23

Since commit 0515adceaa8f ("passt, pasta: Namespace-based sandboxing, defer seccomp policy application"), it makes no sense to close and reopen the tap device on error: we don't have access to /dev/net/tun after the initial setup phase. If we hit ENOBUFS while writing (as reported: in one case because the kernel actually ran out of memory, with another case under investigation), or ENOSPC, we're supposed to drop whatever data we were trying to send: there's no room for it. Handle EINTR just like we handled EAGAIN/EWOULDBLOCK: there's no particular reason why sending the same data should fail again. Anything else I can think of would be an unrecoverable error: exit with failure then. While at it, drop a useless cast on the write() call: it takes a const void * anyway. Reported-by: Gianluca Stivan <me(a)yawnt.com> Reported-by: Chris Kuhn <kuhnchris(a)kuhnchris.eu> Fixes: 0515adceaa8f ("passt, pasta: Namespace-based sandboxing, defer seccomp policy application") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tap.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tap.c b/tap.c index c0b7f33..e323529 100644 --- a/tap.c +++ b/tap.c @@ -320,12 +320,23 @@ static size_t tap_send_frames_pasta(struct ctx *c, size_t i; for (i = 0; i < n; i++) { - if (write(c->fd_tap, (char *)iov[i].iov_base, - iov[i].iov_len) < 0) { + if (write(c->fd_tap, iov[i].iov_base, iov[i].iov_len) < 0) { debug("tap write: %s", strerror(errno)); - if (errno != EAGAIN && errno != EWOULDBLOCK) - tap_handler(c, c->fd_tap, EPOLLERR, NULL); - i--; + + switch (errno) { + case EAGAIN: +#if EAGAIN != EWOULDBLOCK + case EWOULDBLOCK: +#endif + case EINTR: + i--; + break; + case ENOBUFS: + case ENOSPC: + break; + default: + die("Write error on tap device, exiting"); + } } } @@ -1237,6 +1248,9 @@ void tap_handler(struct ctx *c, int fd, uint32_t events, exit(EXIT_SUCCESS); } + if (c->mode == MODE_PASTA) + die("Error on tap device, exiting"); + tap_sock_init(c); } } -- 2.39.2

2 1

passt: new version 2023_06_03.429e1a7 available
by Stefano Brivio 03 Jun '23

03 Jun '23

The new version with tag 2023_06_03.429e1a7 includes the following changes: 429e1a7 conf: Fix erroneous check of ip6->gw e3b1953 test/nstool: Fix fd leak in accept() loop 527c822 test/nstool: Provide useful error if given a path that's too long 9f61c5b passt.h: Fix description of pasta_ifi in struct ctx cc9d167 conf, pasta: With --config-net, copy all addresses by default e89da3c netlink: Add functionality to copy addresses from outer namespace a7359f0 conf: Don't exit if sourced default route has no gateway e8fef75 Revert "conf: Adjust netmask on mismatch between IPv4 address/netmask and gateway" da54641 conf, pasta: With --config-net, copy all routes by default 468f19a conf: --config-net option is for pasta mode only 2fe0461 netlink: Add functionality to copy routes from outer namespace f099afb pasta: Improve error handling on failure to join network namespace 1c3c689 netlink: Fix comment about response buffer size for nl_req() 770d1a4 isolation: Initially Keep CAP_SETFCAP if running as UID 0 in non-init b0e450a pasta: Detach mount namespace, (re)mount procfs before spawning command b0881aa util, conf: Add and use ns_is_init() helper 25f1d1a tap: Don't update ip6.addr_seen to :: https://passt.top/passt/log/?qt=range&q=2023_05_09.96f8d55..2023_06_03.429e… Packages: - Arch Linux: https://www.archlinux.org/packages/extra/x86_64/passt/ https://archlinuxarm.org/packages/aarch64/passt https://archlinuxarm.org/packages/armv7h/passt - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/6000164/ permanent mirror: https://passt.top/builds/copr/0^20230603.g429e1a7/ - Fedora updates: https://bodhi.fedoraproject.org/updates/?packages=passt - Ubuntu tracker: https://packages.ubuntu.com/lunar/passt - Void Linux: https://voidlinux.org/packages/?q=passt - Static builds: - Package for other RPM-based distributions, x86_64 only: https://passt.top/builds/latest/x86_64/passt-g429e1a7-1.x86_64.rpm - x86_64 static binaries: https://passt.top/builds/latest/x86_64/ - Debian package, from x86_64 static build: https://passt.top/builds/latest/x86_64/passt_429e1a7-1_all.deb -- Stefano

1 0

[PATCH] conf: Fix erroneous check of ip6->gw
by David Gibson 03 Jun '23

03 Jun '23

a7359f094898 ("conf: Don't exit if sourced default route has no gateway") was supposed to allow passt/pasta to run even if given a template interface which has no default gateway. However a mistake in the patch means it still requires the gateway, but doesn't require a global address for the guest which we really do need. This is one part (but not the only part) of the problem seen in https://bugs.passt.top/show_bug.cgi?id=50. Fixes: a7359f094898 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf.c b/conf.c index d8414fe7..ffff235b 100644 --- a/conf.c +++ b/conf.c @@ -708,7 +708,7 @@ static unsigned int conf_ip6(unsigned int ifi, if (MAC_IS_ZERO(mac)) nl_link(0, ifi, mac, 0, 0); - if (IN6_IS_ADDR_UNSPECIFIED(&ip6->gw) || + if (IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) || IN6_IS_ADDR_UNSPECIFIED(&ip6->addr_ll) || MAC_IS_ZERO(mac)) return 0; -- 2.40.1

2 1