- dev - passt

[PATCH 0/2] Fix bug #80 again
by Stefano Brivio 24 Jun '24

24 Jun '24

Oops. Stefano Brivio (2): Revert "udp: Make rport calculation more local" udp: Reduce scope of rport in udp_invert_portmap() udp.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) -- 2.43.0

2 5

passt: new version 2024_06_24.1ee2eca available
by Stefano Brivio 24 Jun '24

24 Jun '24

The new version with tag 2024_06_24.1ee2eca includes the following changes: 1ee2eca udp: Reduce scope of rport in udp_invert_portmap() 0546975 Revert "udp: Make rport calculation more local" c66f034 log: Don't report syslog failures to stderr after initialisation e7323e5 conf, passt: Don't call __openlog() if a log file is used dba7f0f treewide: Replace strerror() calls 92a22fe treewide: Replace perror() calls with calls to logging functions c1140df log: Add _perror() logging function variants afd9cdc log, passt: Always print to stderr before initialisation is complete 8c2f24a conf, log: Instead of abusing log levels, add log_conf_parsed flag bca0fef conf, passt: Make --stderr do nothing, and deprecate it b748016 conf, passt: Don't try to log to stderr after we close it 65923ba conf: Accept duplicate and conflicting options, the last one wins 62de614 netlink: Strip nexthop identifiers when duplicating routes 1544a43 passt.1, qrap.1: align license description with SPDX identifier f301bb1 netlink: Ignore EHOSTUNREACH failures when duplicating routes 450a613 netlink: With no default route, pick the first interface with a route 54a9d38 tcp: Don't rely on bind() to fail to decide that connection target is valid 020ff7a siphash: Remove stale prototypes 7e87bd9 udp: Move management of udp[46]_localname into udp_splice_send() ff57f8d udp: Rework how we divide queued datagrams between sending methods 63db7dc udp: Fold checking of splice flag into udp_mmh_splice_port() 523fbc5 util: Split construction of bind socket address from the rest of sock_l4() 4070bac tap: use in->buf_size rather than sizeof(pkt_buf) 7290335 iov: remove iov_copy() 0c335d7 vhost-user: compare mode MODE_PASTA and not MODE_PASST 377b666 udp: rename udp_sock_handler() to udp_buf_sock_handler() e7ac995 udp: refactor UDP header update functions 9ecf7fe tap: refactor packets handling functions fba2b54 tcp: move buffers management functions to their own file ec26fa0 tcp: extract buffer management from tcp_send_flag() d949667 cppcheck: Suppress constParameterCallback errors https://passt.top/passt/log/?qt=range&q=2024_06_07.8a83b53..2024_06_24.1ee2… Packages: - Alpine Linux: https://pkgs.alpinelinux.org/packages?name=passt - Arch Linux: https://www.archlinux.org/packages/extra/x86_64/passt/ https://archlinuxarm.org/packages/aarch64/passt https://archlinuxarm.org/packages/armv7h/passt - Chimera: https://pkgs.chimera-linux.org/packages?name=passt - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/7661289/ permanent mirror: https://passt.top/builds/copr/0^20240624.g1ee2eca/ - Fedora updates: https://bodhi.fedoraproject.org/updates/?packages=passt - Gentoo versions: https://packages.gentoo.org/packages/net-misc/passt - GNU Guix: https://packages.guix.gnu.org/packages/passt/ - openSUSE: https://software.opensuse.org/package/passt - PLD Linux: https://git.pld-linux.org/cgi-bin/gitweb.cgi?p=packages/passt.git - Solus: https://github.com/getsolus/packages/tree/main/packages/p/passt - Ubuntu tracker: https://launchpad.net/ubuntu/+source/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-g1ee2eca-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_1ee2eca-1_all.deb -- Stefano

1 0

[PATCH v3] conf: Accept duplicate and conflicting options, the last one wins
by Stefano Brivio 21 Jun '24

21 Jun '24

In multiple occasions, especially when passt(1) and pasta(1) are used in integrations such as the one with Podman, the ability to override earlier options on the command line with later one would have been convenient. Recently, to debug a number of issues happening with Podman, I would have liked to ask users to share a debug log by passing --debug as additional option, but pasta refuses --quiet (always passed by Podman) and --debug at the same time. On top of this, Podman lets users specify other pasta options in its containers.conf(5) file, as well as on the command line. The options from the configuration files are appended together with the ones from the command line, which makes it impossible for users to override options from the configuration file, if duplicated options are refused, unless Podman takes care of sorting them, which is clearly not sustainable. For --debug and --trace, somebody took care of this on Podman side at: https://github.com/containers/common/pull/2052 but this doesn't fix the issue with other options, and we'll have anyway older versions of Podman around, too. I think there's some value in telling users about duplicated or conflicting options, because that might reveal issues in integrations or accidental misconfigurations, but by now I'm fairly convinced that the downsides outweigh this. Drop checks about duplicate options and mutually exclusive ones. In some cases, we need to also undo a couple of initialisations caused by earlier options, but this looks like a simplification, overall. Notable exception: --stderr still conflicts with --log-file, because users might have the expectation that they don't actually conflict. But they do conflict in the existing implementation, so it's safer to make sure that the users notice that. Suggested-by: Paul Holzinger <pholzing(a)redhat.com> Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v3: - override --gateway and --dns-forward with the last given option for a given address family, too v2: - report --log-file and --stderr as conflicting, but accept multiple options if they don't conflict - for --log-file and --pcap, make it clear in the man page that the general principle still applies: only the last one (of each) takes effect - add some more context about Podman and how the list of pasta options is built there, in the commit message conf.c | 146 ++++++++++++++++---------------------------------------- passt.1 | 17 +++++++ 2 files changed, 59 insertions(+), 104 deletions(-) diff --git a/conf.c b/conf.c index 94b3ed6..4fff2b1 100644 --- a/conf.c +++ b/conf.c @@ -517,9 +517,6 @@ static void conf_netns_opt(char *netns, const char *arg) static void conf_pasta_ns(int *netns_only, char *userns, char *netns, int optind, int argc, char *argv[]) { - if (*netns_only && *userns) - die("Both --userns and --netns-only given"); - if (*netns && optind != argc) die("Both --netns and PID or command given"); @@ -1204,7 +1201,6 @@ void conf(struct ctx *c, int argc, char **argv) {"udp-ns", required_argument, NULL, 'U' }, {"userns", required_argument, NULL, 2 }, {"netns", required_argument, NULL, 3 }, - {"netns-only", no_argument, &netns_only, 1 }, {"ns-mac-addr", required_argument, NULL, 4 }, {"dhcp-dns", no_argument, NULL, 5 }, {"no-dhcp-dns", no_argument, NULL, 6 }, @@ -1221,6 +1217,7 @@ void conf(struct ctx *c, int argc, char **argv) {"config-net", no_argument, NULL, 17 }, {"no-copy-routes", no_argument, NULL, 18 }, {"no-copy-addrs", no_argument, NULL, 19 }, + {"netns-only", no_argument, NULL, 20 }, { 0 }, }; char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 }; @@ -1265,6 +1262,8 @@ void conf(struct ctx *c, int argc, char **argv) if (ret <= 0 || ret >= (int)sizeof(userns)) die("Invalid userns: %s", optarg); + netns_only = 0; + break; case 3: if (c->mode != MODE_PASTA) @@ -1303,14 +1302,12 @@ void conf(struct ctx *c, int argc, char **argv) c->no_dhcp_dns_search = 1; break; case 9: - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) && - inet_pton(AF_INET6, optarg, &c->ip6.dns_match) && + if (inet_pton(AF_INET6, optarg, &c->ip6.dns_match) && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) && !IN6_IS_ADDR_LOOPBACK(&c->ip6.dns_match)) break; - if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) && - inet_pton(AF_INET, optarg, &c->ip4.dns_match) && + if (inet_pton(AF_INET, optarg, &c->ip4.dns_match) && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) && !IN4_IS_ADDR_BROADCAST(&c->ip4.dns_match) && !IN4_IS_ADDR_LOOPBACK(&c->ip4.dns_match)) @@ -1325,24 +1322,13 @@ void conf(struct ctx *c, int argc, char **argv) c->no_netns_quit = 1; break; case 11: - if (c->trace) - die("Multiple --trace options given"); - - if (c->quiet) - die("Either --trace or --quiet"); - c->trace = c->debug = 1; + c->quiet = 0; break; case 12: - if (runas) - die("Multiple --runas options given"); - runas = optarg; break; case 13: - if (logsize) - die("Multiple --log-size options given"); - errno = 0; logsize = strtol(optarg, NULL, 0); @@ -1356,9 +1342,6 @@ void conf(struct ctx *c, int argc, char **argv) fprintf(stdout, VERSION_BLOB); exit(EXIT_SUCCESS); case 15: - if (*c->ip4.ifname_out) - die("Redundant outbound interface: %s", optarg); - ret = snprintf(c->ip4.ifname_out, sizeof(c->ip4.ifname_out), "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->ip4.ifname_out)) @@ -1366,9 +1349,6 @@ void conf(struct ctx *c, int argc, char **argv) break; case 16: - if (*c->ip6.ifname_out) - die("Redundant outbound interface: %s", optarg); - ret = snprintf(c->ip6.ifname_out, sizeof(c->ip6.ifname_out), "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->ip6.ifname_out)) @@ -1395,62 +1375,47 @@ void conf(struct ctx *c, int argc, char **argv) warn("--no-copy-addrs will be dropped soon"); c->no_copy_addrs = copy_addrs_opt = true; break; - case 'd': - if (c->debug) - die("Multiple --debug options given"); - - if (c->quiet) - die("Either --debug or --quiet"); + case 20: + if (c->mode != MODE_PASTA) + die("--netns-only is for pasta mode only"); + netns_only = 1; + *userns = 0; + break; + case 'd': c->debug = 1; + c->quiet = 0; break; case 'e': if (logfile) die("Can't log to both file and stderr"); - if (c->force_stderr) - die("Multiple --stderr options given"); - c->force_stderr = 1; + logfile = NULL; break; case 'l': if (c->force_stderr) die("Can't log to both stderr and file"); - if (logfile) - die("Multiple --log-file options given"); - logfile = optarg; + c->force_stderr = 0; break; case 'q': - if (c->quiet) - die("Multiple --quiet options given"); - - if (c->debug) - die("Either --debug or --quiet"); - c->quiet = 1; + c->debug = c->trace = 0; break; case 'f': - if (c->foreground) - die("Multiple --foreground options given"); - c->foreground = 1; break; case 's': - if (*c->sock_path) - die("Multiple --socket options given"); - ret = snprintf(c->sock_path, UNIX_SOCK_MAX - 1, "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->sock_path)) die("Invalid socket path: %s", optarg); + c->fd_tap = -1; break; case 'F': - if (c->fd_tap >= 0) - die("Multiple --fd options given"); - errno = 0; c->fd_tap = strtol(optarg, NULL, 0); @@ -1458,12 +1423,9 @@ void conf(struct ctx *c, int argc, char **argv) die("Invalid --fd: %s", optarg); c->one_off = true; - + *c->sock_path = 0; break; case 'I': - if (*c->pasta_ifn) - die("Multiple --ns-ifname options given"); - ret = snprintf(c->pasta_ifn, IFNAMSIZ, "%s", optarg); if (ret <= 0 || ret >= IFNAMSIZ) @@ -1471,18 +1433,12 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'p': - if (*c->pcap) - die("Multiple --pcap options given"); - ret = snprintf(c->pcap, sizeof(c->pcap), "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->pcap)) die("Invalid pcap path: %s", optarg); break; case 'P': - if (*c->pidfile) - die("Multiple --pid options given"); - ret = snprintf(c->pidfile, sizeof(c->pidfile), "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->pidfile)) @@ -1490,9 +1446,6 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'm': - if (c->mtu) - die("Multiple --mtu options given"); - errno = 0; c->mtu = strtol(optarg, NULL, 0); @@ -1510,8 +1463,7 @@ void conf(struct ctx *c, int argc, char **argv) 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) && + if (inet_pton(AF_INET6, optarg, &c->ip6.addr) && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr) && !IN6_IS_ADDR_LOOPBACK(&c->ip6.addr) && !IN6_IS_ADDR_V4MAPPED(&c->ip6.addr) && @@ -1519,8 +1471,7 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_MULTICAST(&c->ip6.addr)) break; - if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr) && - inet_pton(AF_INET, optarg, &c->ip4.addr) && + if (inet_pton(AF_INET, optarg, &c->ip4.addr) && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr) && !IN4_IS_ADDR_BROADCAST(&c->ip4.addr) && !IN4_IS_ADDR_LOOPBACK(&c->ip4.addr) && @@ -1542,14 +1493,12 @@ void conf(struct ctx *c, int argc, char **argv) if (c->mode == MODE_PASTA) c->no_copy_routes = 1; - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw) && - inet_pton(AF_INET6, optarg, &c->ip6.gw) && + if (inet_pton(AF_INET6, optarg, &c->ip6.gw) && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw) && !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw)) break; - if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw) && - inet_pton(AF_INET, optarg, &c->ip4.gw) && + if (inet_pton(AF_INET, optarg, &c->ip4.gw) && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw) && !IN4_IS_ADDR_BROADCAST(&c->ip4.gw) && !IN4_IS_ADDR_LOOPBACK(&c->ip4.gw)) @@ -1558,16 +1507,12 @@ void conf(struct ctx *c, int argc, char **argv) die("Invalid gateway address: %s", optarg); break; case 'i': - if (ifi4 || ifi6) - die("Redundant interface: %s", optarg); - if (!(ifi4 = ifi6 = if_nametoindex(optarg))) die("Invalid interface name %s: %s", optarg, strerror(errno)); break; case 'o': - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out) && - inet_pton(AF_INET6, optarg, &c->ip6.addr_out) && + if (inet_pton(AF_INET6, optarg, &c->ip6.addr_out) && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out) && !IN6_IS_ADDR_LOOPBACK(&c->ip6.addr_out) && !IN6_IS_ADDR_V4MAPPED(&c->ip6.addr_out) && @@ -1575,8 +1520,7 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_MULTICAST(&c->ip6.addr_out)) break; - if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) && - inet_pton(AF_INET, optarg, &c->ip4.addr_out) && + if (inet_pton(AF_INET, optarg, &c->ip4.addr_out) && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) && !IN4_IS_ADDR_BROADCAST(&c->ip4.addr_out) && !IN4_IS_ADDR_MULTICAST(&c->ip4.addr_out)) @@ -1587,18 +1531,23 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'D': if (!strcmp(optarg, "none")) { - if (c->no_dns) - die("Redundant DNS options"); + c->no_dns = 1; - if (dns4 - c->ip4.dns || dns6 - c->ip6.dns) - die("Conflicting DNS options"); + dns4 = &c->ip4.dns[0]; + memset(c->ip4.dns, 0, sizeof(c->ip4.dns)); + c->ip4.dns[0] = (struct in_addr){ 0 }; + c->ip4.dns_match = (struct in_addr){ 0 }; + c->ip4.dns_host = (struct in_addr){ 0 }; + + dns6 = &c->ip6.dns[0]; + memset(c->ip6.dns, 0, sizeof(c->ip6.dns)); + c->ip6.dns_match = (struct in6_addr){ 0 }; + c->ip6.dns_host = (struct in6_addr){ 0 }; - c->no_dns = 1; break; } - if (c->no_dns) - die("Conflicting DNS options"); + c->no_dns = 0; if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) && inet_pton(AF_INET, optarg, &dns4_tmp)) { @@ -1616,18 +1565,14 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'S': if (!strcmp(optarg, "none")) { - if (c->no_dns_search) - die("Redundant DNS search options"); + c->no_dns_search = 1; - if (dnss != c->dns_search) - die("Conflicting DNS search options"); + memset(c->dns_search, 0, sizeof(c->dns_search)); - c->no_dns_search = 1; break; } - if (c->no_dns_search) - die("Conflicting DNS search options"); + c->no_dns_search = 0; if (dnss - c->dns_search < ARRAY_SIZE(c->dns_search)) { ret = snprintf(dnss->n, sizeof(*c->dns_search), @@ -1643,17 +1588,16 @@ void conf(struct ctx *c, int argc, char **argv) break; case '4': v4_only = true; + v6_only = false; break; case '6': v6_only = true; + v4_only = false; break; case '1': if (c->mode == MODE_PASTA) die("--one-off is for passt mode only"); - if (c->one_off) - die("Redundant --one-off option"); - c->one_off = true; break; case 't': @@ -1672,12 +1616,6 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); - if (v4_only && v6_only) - die("Options ipv4-only and ipv6-only are mutually exclusive"); - - if (*c->sock_path && c->fd_tap >= 0) - die("Options --socket and --fd are mutually exclusive"); - if (c->mode == MODE_PASTA && !c->pasta_conf_ns) { if (copy_routes_opt) die("--no-copy-routes needs --config-net"); diff --git a/passt.1 b/passt.1 index 7676fe3..7b9915b 100644 --- a/passt.1 +++ b/passt.1 @@ -73,6 +73,9 @@ for performance reasons. .SH OPTIONS +Unless otherwise noted below, \fBif conflicting or multiple options are given, +the last one takes effect.\fR + .TP .BR \-d ", " \-\-debug Be verbose, don't log to the system logger. @@ -97,10 +100,21 @@ Log to standard error too. Default is to log to the system logger only, if started from an interactive terminal, and to both system logger and standard error otherwise. +This option cannot be specified together with \fB--log-file\fR, because there +might be a reasonable expectation that messages are logged to standard error as +well as to a file, which is however not supported. + .TP .BR \-l ", " \-\-log-file " " \fIPATH\fR Log to file \fIPATH\fR, not to standard error, and not to the system logger. +This option cannot be specified together with \fB--stderr\fR, because there +might be a reasonable expectation that messages are logged to a file as well as +to standard error, which is however not supported. + +Specifying this option multiple times does \fInot\fR lead to multiple log files: +the last given option takes effect. + .TP .BR \-\-log-size " " \fISIZE\fR Limit log file size to \fISIZE\fR bytes. When the log file is full, make room @@ -128,6 +142,9 @@ Show version and exit. Capture tap-facing (that is, guest-side or namespace-side) network packets to \fIfile\fR in \fBpcap\fR format. +Specifying this option multiple times does \fInot\fR lead to multiple capture +files: the last given option takes effect. + .TP .BR \-P ", " \-\-pid " " \fIfile Write own PID to \fIfile\fR once initialisation is done, before forking to -- 2.43.0

3 3

[PATCH 0/8] Open socket and PID files as root, before switching
by Stefano Brivio 21 Jun '24

21 Jun '24

If libguestfs tools run as root, with the 'direct' backend (without libvirt), we'll start as root as well. As guest images might be owned by root, there are valid reasons to use libguestfs tools as root, so be nice to them: open socket and PID files *before* switching to nobody, so that we can still access their paths. Stefano Brivio (8): conf: Don't lecture user about starting us as root tap: Move all-ones initialisation of mac_guest to tap_sock_init() passt, tap: Don't use -1 as uninitialised value for fd_tap_listen tap: Split tap_sock_unix_init() into opening and listening parts util: Rename write_pidfile() to pidfile_write() passt, util: Move opening of PID file to its own function conf, passt, tap: Open socket and PID files before switching UID/GID conf, passt.h: Rename pid_file in struct ctx to pidfile conf.c | 23 +++++++++++++++++++---- passt.c | 17 ++++------------- passt.h | 8 ++++++-- tap.c | 57 +++++++++++++++++++++++++++++++++++---------------------- tap.h | 1 + util.c | 28 +++++++++++++++++++++++++--- util.h | 3 ++- 7 files changed, 92 insertions(+), 45 deletions(-) -- 2.43.0

3 29

[PATCH v2] conf: Accept duplicate and conflicting options, the last one wins
by Stefano Brivio 20 Jun '24

20 Jun '24

In multiple occasions, especially when passt(1) and pasta(1) are used in integrations such as the one with Podman, the ability to override earlier options on the command line with later one would have been convenient. Recently, to debug a number of issues happening with Podman, I would have liked to ask users to share a debug log by passing --debug as additional option, but pasta refuses --quiet (always passed by Podman) and --debug at the same time. On top of this, Podman lets users specify other pasta options in its containers.conf(5) file, as well as on the command line. The options from the configuration files are appended together with the ones from the command line, which makes it impossible for users to override options from the configuration file, if duplicated options are refused, unless Podman takes care of sorting them, which is clearly not sustainable. For --debug and --trace, somebody took care of this on Podman side at: https://github.com/containers/common/pull/2052 but this doesn't fix the issue with other options, and we'll have anyway older versions of Podman around, too. I think there's some value in telling users about duplicated or conflicting options, because that might reveal issues in integrations or accidental misconfigurations, but by now I'm fairly convinced that the downsides outweigh this. Drop checks about duplicate options and mutually exclusive ones. In some cases, we need to also undo a couple of initialisations caused by earlier options, but this looks like a simplification, overall. Notable exception: --stderr still conflicts with --log-file, because users might have the expectation that they don't actually conflict. But they do conflict in the existing implementation, so it's safer to make sure that the users notice that. Suggested-by: Paul Holzinger <pholzing(a)redhat.com> Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: - report --log-file and --stderr as conflicting, but accept multiple options if they don't conflict - for --log-file and --pcap, make it clear in the man page that the general principle still applies: only the last one (of each) takes effect - add some more context about Podman and how the list of pasta options is built there, in the commit message conf.c | 134 ++++++++++++++++---------------------------------------- passt.1 | 17 +++++++ 2 files changed, 55 insertions(+), 96 deletions(-) diff --git a/conf.c b/conf.c index 94b3ed6..c5f13ac 100644 --- a/conf.c +++ b/conf.c @@ -517,9 +517,6 @@ static void conf_netns_opt(char *netns, const char *arg) static void conf_pasta_ns(int *netns_only, char *userns, char *netns, int optind, int argc, char *argv[]) { - if (*netns_only && *userns) - die("Both --userns and --netns-only given"); - if (*netns && optind != argc) die("Both --netns and PID or command given"); @@ -1204,7 +1201,6 @@ void conf(struct ctx *c, int argc, char **argv) {"udp-ns", required_argument, NULL, 'U' }, {"userns", required_argument, NULL, 2 }, {"netns", required_argument, NULL, 3 }, - {"netns-only", no_argument, &netns_only, 1 }, {"ns-mac-addr", required_argument, NULL, 4 }, {"dhcp-dns", no_argument, NULL, 5 }, {"no-dhcp-dns", no_argument, NULL, 6 }, @@ -1221,6 +1217,7 @@ void conf(struct ctx *c, int argc, char **argv) {"config-net", no_argument, NULL, 17 }, {"no-copy-routes", no_argument, NULL, 18 }, {"no-copy-addrs", no_argument, NULL, 19 }, + {"netns-only", no_argument, NULL, 20 }, { 0 }, }; char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 }; @@ -1265,6 +1262,8 @@ void conf(struct ctx *c, int argc, char **argv) if (ret <= 0 || ret >= (int)sizeof(userns)) die("Invalid userns: %s", optarg); + netns_only = 0; + break; case 3: if (c->mode != MODE_PASTA) @@ -1325,24 +1324,13 @@ void conf(struct ctx *c, int argc, char **argv) c->no_netns_quit = 1; break; case 11: - if (c->trace) - die("Multiple --trace options given"); - - if (c->quiet) - die("Either --trace or --quiet"); - c->trace = c->debug = 1; + c->quiet = 0; break; case 12: - if (runas) - die("Multiple --runas options given"); - runas = optarg; break; case 13: - if (logsize) - die("Multiple --log-size options given"); - errno = 0; logsize = strtol(optarg, NULL, 0); @@ -1356,9 +1344,6 @@ void conf(struct ctx *c, int argc, char **argv) fprintf(stdout, VERSION_BLOB); exit(EXIT_SUCCESS); case 15: - if (*c->ip4.ifname_out) - die("Redundant outbound interface: %s", optarg); - ret = snprintf(c->ip4.ifname_out, sizeof(c->ip4.ifname_out), "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->ip4.ifname_out)) @@ -1366,9 +1351,6 @@ void conf(struct ctx *c, int argc, char **argv) break; case 16: - if (*c->ip6.ifname_out) - die("Redundant outbound interface: %s", optarg); - ret = snprintf(c->ip6.ifname_out, sizeof(c->ip6.ifname_out), "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->ip6.ifname_out)) @@ -1395,62 +1377,47 @@ void conf(struct ctx *c, int argc, char **argv) warn("--no-copy-addrs will be dropped soon"); c->no_copy_addrs = copy_addrs_opt = true; break; - case 'd': - if (c->debug) - die("Multiple --debug options given"); - - if (c->quiet) - die("Either --debug or --quiet"); + case 20: + if (c->mode != MODE_PASTA) + die("--netns-only is for pasta mode only"); + netns_only = 1; + *userns = 0; + break; + case 'd': c->debug = 1; + c->quiet = 0; break; case 'e': if (logfile) die("Can't log to both file and stderr"); - if (c->force_stderr) - die("Multiple --stderr options given"); - c->force_stderr = 1; + logfile = NULL; break; case 'l': if (c->force_stderr) die("Can't log to both stderr and file"); - if (logfile) - die("Multiple --log-file options given"); - logfile = optarg; + c->force_stderr = 0; break; case 'q': - if (c->quiet) - die("Multiple --quiet options given"); - - if (c->debug) - die("Either --debug or --quiet"); - c->quiet = 1; + c->debug = c->trace = 0; break; case 'f': - if (c->foreground) - die("Multiple --foreground options given"); - c->foreground = 1; break; case 's': - if (*c->sock_path) - die("Multiple --socket options given"); - ret = snprintf(c->sock_path, UNIX_SOCK_MAX - 1, "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->sock_path)) die("Invalid socket path: %s", optarg); + c->fd_tap = -1; break; case 'F': - if (c->fd_tap >= 0) - die("Multiple --fd options given"); - errno = 0; c->fd_tap = strtol(optarg, NULL, 0); @@ -1458,12 +1425,9 @@ void conf(struct ctx *c, int argc, char **argv) die("Invalid --fd: %s", optarg); c->one_off = true; - + *c->sock_path = 0; break; case 'I': - if (*c->pasta_ifn) - die("Multiple --ns-ifname options given"); - ret = snprintf(c->pasta_ifn, IFNAMSIZ, "%s", optarg); if (ret <= 0 || ret >= IFNAMSIZ) @@ -1471,18 +1435,12 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'p': - if (*c->pcap) - die("Multiple --pcap options given"); - ret = snprintf(c->pcap, sizeof(c->pcap), "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->pcap)) die("Invalid pcap path: %s", optarg); break; case 'P': - if (*c->pidfile) - die("Multiple --pid options given"); - ret = snprintf(c->pidfile, sizeof(c->pidfile), "%s", optarg); if (ret <= 0 || ret >= (int)sizeof(c->pidfile)) @@ -1490,9 +1448,6 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'm': - if (c->mtu) - die("Multiple --mtu options given"); - errno = 0; c->mtu = strtol(optarg, NULL, 0); @@ -1542,14 +1497,12 @@ void conf(struct ctx *c, int argc, char **argv) if (c->mode == MODE_PASTA) c->no_copy_routes = 1; - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw) && - inet_pton(AF_INET6, optarg, &c->ip6.gw) && + if (inet_pton(AF_INET6, optarg, &c->ip6.gw) && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.gw) && !IN6_IS_ADDR_LOOPBACK(&c->ip6.gw)) break; - if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw) && - inet_pton(AF_INET, optarg, &c->ip4.gw) && + if (inet_pton(AF_INET, optarg, &c->ip4.gw) && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.gw) && !IN4_IS_ADDR_BROADCAST(&c->ip4.gw) && !IN4_IS_ADDR_LOOPBACK(&c->ip4.gw)) @@ -1558,16 +1511,12 @@ void conf(struct ctx *c, int argc, char **argv) die("Invalid gateway address: %s", optarg); break; case 'i': - if (ifi4 || ifi6) - die("Redundant interface: %s", optarg); - if (!(ifi4 = ifi6 = if_nametoindex(optarg))) die("Invalid interface name %s: %s", optarg, strerror(errno)); break; case 'o': - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out) && - inet_pton(AF_INET6, optarg, &c->ip6.addr_out) && + if (inet_pton(AF_INET6, optarg, &c->ip6.addr_out) && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out) && !IN6_IS_ADDR_LOOPBACK(&c->ip6.addr_out) && !IN6_IS_ADDR_V4MAPPED(&c->ip6.addr_out) && @@ -1575,8 +1524,7 @@ void conf(struct ctx *c, int argc, char **argv) !IN6_IS_ADDR_MULTICAST(&c->ip6.addr_out)) break; - if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) && - inet_pton(AF_INET, optarg, &c->ip4.addr_out) && + if (inet_pton(AF_INET, optarg, &c->ip4.addr_out) && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) && !IN4_IS_ADDR_BROADCAST(&c->ip4.addr_out) && !IN4_IS_ADDR_MULTICAST(&c->ip4.addr_out)) @@ -1587,18 +1535,23 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'D': if (!strcmp(optarg, "none")) { - if (c->no_dns) - die("Redundant DNS options"); + c->no_dns = 1; - if (dns4 - c->ip4.dns || dns6 - c->ip6.dns) - die("Conflicting DNS options"); + dns4 = &c->ip4.dns[0]; + memset(c->ip4.dns, 0, sizeof(c->ip4.dns)); + c->ip4.dns[0] = (struct in_addr){ 0 }; + c->ip4.dns_match = (struct in_addr){ 0 }; + c->ip4.dns_host = (struct in_addr){ 0 }; + + dns6 = &c->ip6.dns[0]; + memset(c->ip6.dns, 0, sizeof(c->ip6.dns)); + c->ip6.dns_match = (struct in6_addr){ 0 }; + c->ip6.dns_host = (struct in6_addr){ 0 }; - c->no_dns = 1; break; } - if (c->no_dns) - die("Conflicting DNS options"); + c->no_dns = 0; if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) && inet_pton(AF_INET, optarg, &dns4_tmp)) { @@ -1616,18 +1569,14 @@ void conf(struct ctx *c, int argc, char **argv) break; case 'S': if (!strcmp(optarg, "none")) { - if (c->no_dns_search) - die("Redundant DNS search options"); + c->no_dns_search = 1; - if (dnss != c->dns_search) - die("Conflicting DNS search options"); + memset(c->dns_search, 0, sizeof(c->dns_search)); - c->no_dns_search = 1; break; } - if (c->no_dns_search) - die("Conflicting DNS search options"); + c->no_dns_search = 0; if (dnss - c->dns_search < ARRAY_SIZE(c->dns_search)) { ret = snprintf(dnss->n, sizeof(*c->dns_search), @@ -1643,17 +1592,16 @@ void conf(struct ctx *c, int argc, char **argv) break; case '4': v4_only = true; + v6_only = false; break; case '6': v6_only = true; + v4_only = false; break; case '1': if (c->mode == MODE_PASTA) die("--one-off is for passt mode only"); - if (c->one_off) - die("Redundant --one-off option"); - c->one_off = true; break; case 't': @@ -1672,12 +1620,6 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); - if (v4_only && v6_only) - die("Options ipv4-only and ipv6-only are mutually exclusive"); - - if (*c->sock_path && c->fd_tap >= 0) - die("Options --socket and --fd are mutually exclusive"); - if (c->mode == MODE_PASTA && !c->pasta_conf_ns) { if (copy_routes_opt) die("--no-copy-routes needs --config-net"); diff --git a/passt.1 b/passt.1 index 7676fe3..7b9915b 100644 --- a/passt.1 +++ b/passt.1 @@ -73,6 +73,9 @@ for performance reasons. .SH OPTIONS +Unless otherwise noted below, \fBif conflicting or multiple options are given, +the last one takes effect.\fR + .TP .BR \-d ", " \-\-debug Be verbose, don't log to the system logger. @@ -97,10 +100,21 @@ Log to standard error too. Default is to log to the system logger only, if started from an interactive terminal, and to both system logger and standard error otherwise. +This option cannot be specified together with \fB--log-file\fR, because there +might be a reasonable expectation that messages are logged to standard error as +well as to a file, which is however not supported. + .TP .BR \-l ", " \-\-log-file " " \fIPATH\fR Log to file \fIPATH\fR, not to standard error, and not to the system logger. +This option cannot be specified together with \fB--stderr\fR, because there +might be a reasonable expectation that messages are logged to a file as well as +to standard error, which is however not supported. + +Specifying this option multiple times does \fInot\fR lead to multiple log files: +the last given option takes effect. + .TP .BR \-\-log-size " " \fISIZE\fR Limit log file size to \fISIZE\fR bytes. When the log file is full, make room @@ -128,6 +142,9 @@ Show version and exit. Capture tap-facing (that is, guest-side or namespace-side) network packets to \fIfile\fR in \fBpcap\fR format. +Specifying this option multiple times does \fInot\fR lead to multiple capture +files: the last given option takes effect. + .TP .BR \-P ", " \-\-pid " " \fIfile Write own PID to \fIfile\fR once initialisation is done, before forking to -- 2.43.0

3 3

[PATCH v4 0/8] Fixes for early logging/prints and related cleanups
by Stefano Brivio 20 Jun '24

20 Jun '24

The most apparent issue fixed by this series is the one from 4/6: with a log file configured, we wouldn't print to standard error anymore, during initialisation, which means that users such as libvirt lost the ability to report meaningful error messages that occurred during initialisation, in that case. v4: - in 4/8, name the new flag 'log_daemonised' instead of 'log_daemon_ready' v3: - add 2/8: we don't really need --stderr anymore - in 5/8, save errno at the beginning of the _perror() helper - in 7/8, avoid assigning errno to whatever return code we have just for the sake of using the new helpers: strerror() is actually less convoluted than that - add 8/8: there's no need to call __openlog() with a log file v2: - turn flag bitmap into simple, separate boolean flags - move errno description after message in _perror() functions - make some of the old perror() messages more descriptive Stefano Brivio (8): conf, passt: Don't try to log to stderr after we close it conf, passt: Make --stderr do nothing, and deprecate it conf, log: Instead of abusing log levels, add log_conf_parsed flag log, passt: Always print to stderr before initialisation is complete log: Add _perror() logging function variants treewide: Replace perror() calls with calls to logging functions treewide: Replace strerror() calls conf, passt: Don't call __openlog() if a log file is used arch.c | 10 ++++---- conf.c | 41 +++++++++++--------------------- fwd.c | 2 +- isolation.c | 46 +++++++++++++++--------------------- log.c | 63 ++++++++++++++++++++++++++++++------------------- log.h | 25 ++++++++++++++++---- netlink.c | 4 ++-- passt.1 | 9 +++---- passt.c | 68 ++++++++++++++++++++--------------------------------- passt.h | 2 -- pasta.c | 27 +++++++++------------ pcap.c | 8 +++---- tap.c | 14 +++++------ tcp.c | 24 +++++++------------ util.c | 12 ++++------ 15 files changed, 165 insertions(+), 190 deletions(-) -- 2.43.0

2 9

[PATCH v3 0/8] Fixes for early logging/prints and related cleanups
by Stefano Brivio 20 Jun '24

20 Jun '24

The most apparent issue fixed by this series is the one from 4/6: with a log file configured, we wouldn't print to standard error anymore, during initialisation, which means that users such as libvirt lost the ability to report meaningful error messages that occurred during initialisation, in that case. v3: - add 2/8: we don't really need --stderr anymore - in 5/8, save errno at the beginning of the _perror() helper - in 7/8, avoid assigning errno to whatever return code we have just for the sake of using the new helpers: strerror() is actually less convoluted than that - add 8/8: there's no need to call __openlog() with a log file v2: - turn flag bitmap into simple, separate boolean flags - move errno description after message in _perror() functions - make some of the old perror() messages more descriptive *** BLURB HERE *** Stefano Brivio (8): conf, passt: Don't try to log to stderr after we close it conf, passt: Make --stderr do nothing, and deprecate it conf, log: Instead of abusing log levels, add log_conf_parsed flag log, passt: Always print to stderr before initialisation is complete log: Add _perror() logging function variants treewide: Replace perror() calls with calls to logging functions treewide: Replace strerror() calls conf, passt: Don't call __openlog() if a log file is used arch.c | 10 ++++---- conf.c | 41 +++++++++++--------------------- fwd.c | 2 +- isolation.c | 46 +++++++++++++++--------------------- log.c | 63 ++++++++++++++++++++++++++++++------------------- log.h | 25 ++++++++++++++++---- netlink.c | 4 ++-- passt.1 | 9 +++---- passt.c | 68 ++++++++++++++++++++--------------------------------- passt.h | 2 -- pasta.c | 27 +++++++++------------ pcap.c | 8 +++---- tap.c | 14 +++++------ tcp.c | 24 +++++++------------ util.c | 12 ++++------ 15 files changed, 165 insertions(+), 190 deletions(-) -- 2.43.0

2 16

[PATCH v2] netlink: Strip nexthop identifiers when duplicating routes
by Stefano Brivio 20 Jun '24

20 Jun '24

If routing daemons set up host routes, for example FRR via OSPF as in the reported issue, they might add nexthop identifiers (not objects) that are generally not valid in the target namespace. Strip them off as well, otherwise we'll get EINVAL from the kernel. Link: https://github.com/containers/podman/issues/22960 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: oops, it looks like I didn't run this through clang-tidy :( and it reported a bugprone-branch-clone if I have two branches both doing the same thing (rta->rta_type = RTA_UNSPEC). I condensed comments under the same branch, probably more elegant than carrying around yet another suppression. netlink.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/netlink.c b/netlink.c index 2c9e71f..c082991 100644 --- a/netlink.c +++ b/netlink.c @@ -600,13 +600,22 @@ int nl_route_dup(int s_src, unsigned int ifi_src, if (discard) break; - } else if (rta->rta_type == RTA_PREFSRC) { - /* Host routes might include a preferred source - * address, which must be one of the host's - * addresses. However, with -a pasta will use a - * different namespace address, making such a - * route invalid in the namespace. Strip off - * RTA_PREFSRC attributes to avoid that. */ + } else if (rta->rta_type == RTA_PREFSRC || + rta->rta_type == RTA_NH_ID) { + /* Strip RTA_PREFSRC attributes: host routes + * might include a preferred source address, + * which must be one of the host's addresses. + * However, with -a, pasta will use a different + * namespace address, making such a route + * invalid in the namespace. + * + * Strip RTA_NH_ID attributes: host routes set + * up via routing protocols (e.g. OSPF) might + * contain a nexthop ID (and not nexthop + * objects, which are taken care of in the + * RTA_MULTIPATH case above) that's not valid + * in the target namespace. + */ rta->rta_type = RTA_UNSPEC; } } -- 2.43.0

2 1

[PATCH v2 0/6] Fixes for early logging/prints and related cleanups
by Stefano Brivio 20 Jun '24

20 Jun '24

The most apparent issue fixed by this series is the one from 3/6: with a log file configured, we wouldn't print to standard error anymore, during initialisation, which means that users such as libvirt lost the ability to report meaningful error messages that occurred during initialisation, in that case. v2: - turn flag bitmap into simple, separate boolean flags - move errno description after message in _perror() functions - make some of the old perror() messages more descriptive Stefano Brivio (6): conf, passt: Don't try to log to stderr after we close it conf, log: Instead of abusing log levels, add log_conf_parsed flag log, passt: Always print to stderr before initialisation is complete log: Add _perror() logging function variants treewide: Replace perror() calls with calls to logging functions treewide: Replace strerror() calls arch.c | 10 ++++---- conf.c | 45 ++++++++++++++++++---------------- fwd.c | 2 +- isolation.c | 46 +++++++++++++++-------------------- log.c | 49 +++++++++++++++++++++++++------------ log.h | 25 ++++++++++++++++--- netlink.c | 4 +-- passt.1 | 3 ++- passt.c | 70 +++++++++++++++++++++++------------------------------ pasta.c | 41 +++++++++++++++---------------- pcap.c | 8 +++--- tap.c | 14 +++++------ tcp.c | 24 ++++++------------ util.c | 12 ++++----- 14 files changed, 179 insertions(+), 174 deletions(-) -- 2.43.0

2 14

[PATCH 0/6] Fixes for early logging/prints and related cleanups
by Stefano Brivio 20 Jun '24

20 Jun '24

The most apparent issue fixed by this series is the one from 3/6: with a log file configured, we wouldn't print to standard error anymore, during initialisation, which means that users such as libvirt lost the ability to report meaningful error messages that occurred during initialisation, in that case. Stefano Brivio (6): conf, passt: Don't try to log to stderr after we close it conf, log: Introduce internal log flags, instead of abusing log levels log, passt: Always print to stderr before initialisation is complete log: Add _perror() logging function variants treewide: Replace perror() calls with calls to logging functions treewide: Replace strerror() calls arch.c | 10 ++++---- conf.c | 45 ++++++++++++++++++---------------- fwd.c | 2 +- isolation.c | 46 +++++++++++++++-------------------- log.c | 57 +++++++++++++++++++++++++++++++------------ log.h | 26 +++++++++++++++++--- netlink.c | 4 +-- passt.1 | 3 ++- passt.c | 70 +++++++++++++++++++++++------------------------------ pasta.c | 41 +++++++++++++++---------------- pcap.c | 8 +++--- tap.c | 14 +++++------ tcp.c | 24 ++++++------------ util.c | 12 ++++----- 14 files changed, 189 insertions(+), 173 deletions(-) -- 2.43.0

2 26