- dev - passt

[PATCH 0/4] Some improvements to the tap send path
by David Gibson 14 Mar '24

14 Mar '24

This series has a handful of small improvements to the tap send path. See individual commit messages for the details. I expect this will conflict with Laurent's upcoming work. I hope the conflicts won't be too bad, and indeed will set us up for less duplication there in the end. This is based on Laurent's patch fixing pcap_multiple() not to capture frames we failed to send. David Gibson (4): tap: Extend tap_send_frames() to allow multi-buffer frames tap: Simplify some casts in the tap "slow path" functions tap: Implement tap_send() "slow path" in terms of fast path tap: Rename tap_iov_{base,len} arp.c | 4 +- tap.c | 158 +++++++++++++++++++++++++++++++--------------------------- tap.h | 19 +++---- tcp.c | 20 ++++---- udp.c | 10 ++-- 5 files changed, 111 insertions(+), 100 deletions(-) -- 2.44.0

3 17

[PATCH] conf: Handle addresses passed via --dns just like the ones from resolv.conf
by Stefano Brivio 14 Mar '24

14 Mar '24

...that is, call add_dns4() and add_dns6() instead of simply adding those to the list of servers we advertise. Most importantly, this will set the 'dns_host' field for the matching IP version, so that, as mentioned in the man page, servers passed via --dns are used for DNS mapping as well, if used in combination with --dns-forward. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://bugs.passt.top/show_bug.cgi?id=82 Fixes: 89678c515755 ("conf, udp: Introduce basic DNS forwarding") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/conf.c b/conf.c index c50c039..17c667a 100644 --- a/conf.c +++ b/conf.c @@ -1164,11 +1164,11 @@ void conf(struct ctx *c, int argc, char **argv) }; char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 }; bool copy_addrs_opt = false, copy_routes_opt = false; + struct in6_addr *dns6 = c->ip6.dns, dns6_tmp; + struct in_addr *dns4 = c->ip4.dns, dns4_tmp; enum fwd_ports_mode fwd_default = FWD_NONE; bool v4_only = false, v6_only = false; - struct in6_addr *dns6 = c->ip6.dns; struct fqdn *dnss = c->dns_search; - struct in_addr *dns4 = c->ip4.dns; unsigned int ifi4 = 0, ifi6 = 0; const char *logfile = NULL; const char *optstring; @@ -1554,14 +1554,14 @@ void conf(struct ctx *c, int argc, char **argv) die("Conflicting DNS options"); if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) && - inet_pton(AF_INET, optarg, dns4)) { - dns4++; + inet_pton(AF_INET, optarg, &dns4_tmp)) { + add_dns4(c, &dns4_tmp, &dns4); break; } if (dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) && - inet_pton(AF_INET6, optarg, dns6)) { - dns6++; + inet_pton(AF_INET6, optarg, &dns6_tmp)) { + add_dns6(c, &dns6_tmp, &dns6); break; } -- 2.39.2

3 4

[PATCH] ip: Use regular htons() for non-constant protocol number in L2_BUF_IP4_PSUM
by Stefano Brivio 14 Mar '24

14 Mar '24

instead of htons_constant(), which is for... constants. Fixes: 5bf200ae8a1a ("tcp, udp: Don't include destination address in partially precomputed csums") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- ip.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ip.h b/ip.h index 9be4778..b9aedf6 100644 --- a/ip.h +++ b/ip.h @@ -38,7 +38,7 @@ .daddr = 0, \ } #define L2_BUF_IP4_PSUM(proto) ((uint32_t)htons_constant(0x4500) + \ - (uint32_t)htons_constant(0xff00 | (proto))) + (uint32_t)htons(0xff00 | (proto))) #define L2_BUF_IP6_INIT(proto) \ { \ -- 2.39.2

2 3

[PATCH v2] passt, log: Call __openlog() earlier, log to stderr until we detach
by Stefano Brivio 14 Mar '24

14 Mar '24

Paul reports that, with commit 15001b39ef1d ("conf: set the log level much earlier"), early messages aren't reported to standard error anymore. The reason is that, once the log mask is changed from LOG_EARLY, we don't force logging to stderr, and this mechanism was abused to have early errors on stderr. Now that we drop LOG_EARLY earlier on, this doesn't work anymore. Call __openlog() as soon as we know the mode we're running as, using LOG_PERROR. Then, once we detach, if we're not running from an interactive terminal and logging to standard error is not forced, drop LOG_PERROR from the options. While at it, check if the standard error descriptor refers to a terminal, instead of checking standard output: if the user redirects standard output to /dev/null, they might still want to see messages from standard error. Further, make sure we don't print messages to standard error reporting that we couldn't log to the system logger, if we didn't open a connection yet. That's expected. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Fixes: 15001b39ef1d ("conf: set the log level much earlier") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Change the isatty() test to use stderr instead of stdout log.c | 2 +- passt.c | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/log.c b/log.c index eafaca2..bdd31b4 100644 --- a/log.c +++ b/log.c @@ -174,7 +174,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap) if (log_opt & LOG_PERROR) fprintf(stderr, "%s", buf + prefix_len); - if (send(log_sock, buf, n, 0) != n) + if (log_sock >= 0 && send(log_sock, buf, n, 0) != n) fprintf(stderr, "Failed to send %i bytes to syslog\n", n); } diff --git a/passt.c b/passt.c index f430648..59ab501 100644 --- a/passt.c +++ b/passt.c @@ -225,6 +225,8 @@ int main(int argc, char **argv) strncpy(argv0, argv[0], PATH_MAX - 1); name = basename(argv0); if (strstr(name, "pasta")) { + __openlog(log_name = "pasta", LOG_PERROR, LOG_DAEMON); + sa.sa_handler = pasta_child_handler; if (sigaction(SIGCHLD, &sa, NULL)) { die("Couldn't install signal handlers: %s", @@ -237,18 +239,16 @@ int main(int argc, char **argv) } c.mode = MODE_PASTA; - log_name = "pasta"; } else if (strstr(name, "passt")) { + __openlog(log_name = "passt", LOG_PERROR, LOG_DAEMON); + c.mode = MODE_PASST; - log_name = "passt"; } else { exit(EXIT_FAILURE); } madvise(pkt_buf, TAP_BUF_BYTES, MADV_HUGEPAGE); - __openlog(log_name, 0, LOG_DAEMON); - c.epollfd = epoll_create1(EPOLL_CLOEXEC); if (c.epollfd == -1) { perror("epoll_create1"); @@ -269,9 +269,6 @@ int main(int argc, char **argv) conf(&c, argc, argv); trace_init(c.trace); - if (c.force_stderr || isatty(fileno(stdout))) - __openlog(log_name, LOG_PERROR, LOG_DAEMON); - pasta_netns_quit_init(&c); tap_sock_init(&c); @@ -314,6 +311,9 @@ int main(int argc, char **argv) if (isolate_prefork(&c)) die("Failed to sandbox process, exiting"); + if (!c.force_stderr && !isatty(fileno(stderr))) + __openlog(log_name, 0, LOG_DAEMON); + if (!c.foreground) __daemon(pidfile_fd, devnull_fd); else -- 2.39.2

2 1

[PATCH] conf: Warn if we can't advertise any nameserver via DHCP, NDP, or DHCPv6
by Stefano Brivio 14 Mar '24

14 Mar '24

We might have read from resolv.conf, or from the command line, a resolver that's reachable via loopback address, but that doesn't mean we can offer that via DHCP, NDP or DHCPv6: warn if there are no resolvers we can offer for a given IP version. Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index 17c667a..ac9fb34 100644 --- a/conf.c +++ b/conf.c @@ -464,8 +464,21 @@ static void get_dns(struct ctx *c) close(fd); out: - if (!dns_set && !added) - warn("Couldn't get any nameserver address"); + if (!dns_set) { + if (!added) + warn("Couldn't get any nameserver address"); + + if (c->no_dhcp_dns) + return; + + if (c->ifi4 && !c->no_dhcp && + IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[0])) + warn("No IPv4 nameserver available for DHCP"); + + if (c->ifi6 && ((!c->no_ndp && !c->no_ra) || !c->no_dhcpv6) && + IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[0])) + warn("No IPv6 nameserver available for NDP/DHCPv6"); + } } /** -- 2.39.2

2 1

[PATCH] passt.1: --{no-,}dhcp-dns and --{no-,}dhcp-search don't take addresses
by Stefano Brivio 14 Mar '24

14 Mar '24

...they are simple enable/disable options. Fixes: 89678c515755 ("conf, udp: Introduce basic DNS forwarding") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt.1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/passt.1 b/passt.1 index e97ef73..9c492f5 100644 --- a/passt.1 +++ b/passt.1 @@ -237,28 +237,28 @@ list altogether (if you need to search a domain called "none" you can use \fB--search none.\fR). .TP -.BR \-\-no-dhcp-dns " " \fIaddr +.BR \-\-no-dhcp-dns In \fIpasst\fR mode, do not assign IPv4 addresses via DHCP (option 23) or IPv6 addresses via NDP Router Advertisement (option type 25) and DHCPv6 (option 23) as DNS resolvers. By default, all the configured addresses are passed. .TP -.BR \-\-dhcp-dns " " \fIaddr +.BR \-\-dhcp-dns In \fIpasta\fR mode, assign IPv4 addresses via DHCP (option 23) or IPv6 addresses via NDP Router Advertisement (option type 25) and DHCPv6 (option 23) as DNS resolvers. By default, configured addresses, if any, are not passed. .TP -.BR \-\-no-dhcp-search " " \fIaddr +.BR \-\-no-dhcp-search In \fIpasst\fR mode, do not send the DNS domain search list addresses via DHCP (option 119), via NDP Router Advertisement (option type 31) and DHCPv6 (option 24). By default, the DNS domain search list resulting from configuration is passed. .TP -.BR \-\-dhcp-search " " \fIaddr +.BR \-\-dhcp-search In \fIpasta\fR mode, send the DNS domain search list addresses via DHCP (option 119), via NDP Router Advertisement (option type 31) and DHCPv6 (option 24). By default, the DNS domain search list resulting from configuration is not -- 2.39.2

2 1

[PATCH] pcap: Use clock_gettime() instead of gettimeofday()
by Stefano Brivio 14 Mar '24

14 Mar '24

POSIX.1-2008 declared gettimeofday() as obsolete, but I'm a dinosaur. Usually, C libraries translate that to the clock_gettime() system call anyway, but this doesn't happen in Jon's environment, and, there, seccomp happily kills pasta(1) when started with --pcap, because we didn't add gettimeofday() to our seccomp profiles. Use clock_gettime() instead. Reported-by: Jon Maloy <jmaloy(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- pcap.c | 26 +++++++++++++------------- util.h | 1 + 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/pcap.c b/pcap.c index a0f01ad..45bbfcd 100644 --- a/pcap.c +++ b/pcap.c @@ -72,17 +72,17 @@ struct pcap_pkthdr { * @iov: IO vector containing frame (with L2 headers and tap headers) * @iovcnt: Number of buffers (@iov entries) in frame * @offset: Byte offset of the L2 headers within @iov - * @tv: Timestamp + * @now: Timestamp * * Returns: 0 on success, -errno on error writing to the file */ static void pcap_frame(const struct iovec *iov, size_t iovcnt, - size_t offset, const struct timeval *tv) + size_t offset, const struct timespec *now) { size_t len = iov_size(iov, iovcnt) - offset; struct pcap_pkthdr h = { - .tv_sec = tv->tv_sec, - .tv_usec = tv->tv_usec, + .tv_sec = now->tv_sec, + .tv_usec = DIV_ROUND_CLOSEST(now->tv_nsec, 1000), .caplen = len, .len = len }; @@ -103,13 +103,13 @@ static void pcap_frame(const struct iovec *iov, size_t iovcnt, void pcap(const char *pkt, size_t len) { struct iovec iov = { (char *)pkt, len }; - struct timeval tv; + struct timespec now; if (pcap_fd == -1) return; - gettimeofday(&tv, NULL); - pcap_frame(&iov, 1, 0, &tv); + clock_gettime(CLOCK_REALTIME, &now); + pcap_frame(&iov, 1, 0, &now); } /** @@ -122,16 +122,16 @@ void pcap(const char *pkt, size_t len) void pcap_multiple(const struct iovec *iov, size_t frame_parts, unsigned int n, size_t offset) { - struct timeval tv; + struct timespec now; unsigned int i; if (pcap_fd == -1) return; - gettimeofday(&tv, NULL); + clock_gettime(CLOCK_REALTIME, &now); for (i = 0; i < n; i++) - pcap_frame(iov + i * frame_parts, frame_parts, offset, &tv); + pcap_frame(iov + i * frame_parts, frame_parts, offset, &now); } /* @@ -145,13 +145,13 @@ void pcap_multiple(const struct iovec *iov, size_t frame_parts, unsigned int n, /* cppcheck-suppress unusedFunction */ void pcap_iov(const struct iovec *iov, size_t iovcnt) { - struct timeval tv; + struct timespec now; if (pcap_fd == -1) return; - gettimeofday(&tv, NULL); - pcap_frame(iov, iovcnt, 0, &tv); + clock_gettime(CLOCK_REALTIME, &now); + pcap_frame(iov, iovcnt, 0, &now); } /** diff --git a/util.h b/util.h index 25e54a7..48f3560 100644 --- a/util.h +++ b/util.h @@ -40,6 +40,7 @@ #endif #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) +#define DIV_ROUND_CLOSEST(n, d) (((n) + (d) / 2) / (d)) #define ROUND_DOWN(x, y) ((x) & ~((y) - 1)) #define ROUND_UP(x, y) (((x) + (y) - 1) & ~((y) - 1)) -- 2.39.2

2 1

[PATCH] tap: Capture only packets that are actually sent
by Laurent Vivier 13 Mar '24

13 Mar '24

In tap_send_frames(), if we failed to send all the frames, we must only log the frames that have been sent, not all the frames we wanted to send. Fixes: dda7945ca9c9 ("pcap: Handle short writes in pcap_frame()") Cc: david(a)gibson.dropbear.id.au Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- Notes: David, I don't understand why you changed the 'm' by an 'n' in your commit. tap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tap.c b/tap.c index c7b9372668ec..f4051cecab84 100644 --- a/tap.c +++ b/tap.c @@ -413,7 +413,7 @@ size_t tap_send_frames(const struct ctx *c, const struct iovec *iov, size_t n) if (m < n) debug("tap: failed to send %zu frames of %zu", n - m, n); - pcap_multiple(iov, 1, n, c->mode == MODE_PASST ? sizeof(uint32_t) : 0); + pcap_multiple(iov, 1, m, c->mode == MODE_PASST ? sizeof(uint32_t) : 0); return m; } -- 2.42.0

3 2

[PATCH v2 0/6] udp: Small cleanups
by David Gibson 13 Mar '24

13 Mar '24

Following on from the recent set of small fixes for UDP, here are a number of small cleanups to the UDP code, which will simplify later more complex fixes and improvements. Laurent, I expect this will have some conflicts with part 2 of your vhost-user work, though I hope they won't be too bad. This is now based on part 1 of Laurent's vhost-user series (specifically the version I posted with the static checker regressions fixed). Changes since v1: * Rebased on Laurent's vhost-user part 1 patches * Some improved commit messages David Gibson (6): udp: Refactor udp_sock[46]_iov_init() udp: Consistent port variable names in udp_update_hdr[46] udp: Pass data length explicitly to to udp_update_hdr[46] udp: Re-order udp_update_hdr[46] for clarity and brevity udp: Avoid unnecessary pointer in udp_update_hdr4() udp: Use existing helper for UDP checksum on inbound IPv6 packets udp.c | 211 +++++++++++++++++++++++++++------------------------------- 1 file changed, 99 insertions(+), 112 deletions(-) -- 2.44.0

2 7

[PATCH] passt, log: Call __openlog() earlier, log to stderr until we detach
by Stefano Brivio 13 Mar '24

13 Mar '24

Paul reports that, with commit 15001b39ef1d ("conf: set the log level much earlier"), early messages aren't reported to standard error anymore. The reason is that, once the log mask is changed from LOG_EARLY, we don't force logging to stderr, and this mechanism was abused to have early errors on stderr. Now that we drop LOG_EARLY earlier on, this doesn't work anymore. Call __openlog() as soon as we know the mode we're running as, using LOG_PERROR. Then, once we detach, if we're not running from an interactive terminal and logging to standard error is not forced, drop LOG_PERROR from the options. While at it, make sure we don't print messages to standard error reporting that we couldn't log to the system logger, if we didn't open a connection yet. That's expected. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Fixes: 15001b39ef1d ("conf: set the log level much earlier") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- log.c | 2 +- passt.c | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/log.c b/log.c index eafaca2..bdd31b4 100644 --- a/log.c +++ b/log.c @@ -174,7 +174,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap) if (log_opt & LOG_PERROR) fprintf(stderr, "%s", buf + prefix_len); - if (send(log_sock, buf, n, 0) != n) + if (log_sock >= 0 && send(log_sock, buf, n, 0) != n) fprintf(stderr, "Failed to send %i bytes to syslog\n", n); } diff --git a/passt.c b/passt.c index f430648..e4cb11e 100644 --- a/passt.c +++ b/passt.c @@ -225,6 +225,8 @@ int main(int argc, char **argv) strncpy(argv0, argv[0], PATH_MAX - 1); name = basename(argv0); if (strstr(name, "pasta")) { + __openlog(log_name = "pasta", LOG_PERROR, LOG_DAEMON); + sa.sa_handler = pasta_child_handler; if (sigaction(SIGCHLD, &sa, NULL)) { die("Couldn't install signal handlers: %s", @@ -237,18 +239,16 @@ int main(int argc, char **argv) } c.mode = MODE_PASTA; - log_name = "pasta"; } else if (strstr(name, "passt")) { + __openlog(log_name = "passt", LOG_PERROR, LOG_DAEMON); + c.mode = MODE_PASST; - log_name = "passt"; } else { exit(EXIT_FAILURE); } madvise(pkt_buf, TAP_BUF_BYTES, MADV_HUGEPAGE); - __openlog(log_name, 0, LOG_DAEMON); - c.epollfd = epoll_create1(EPOLL_CLOEXEC); if (c.epollfd == -1) { perror("epoll_create1"); @@ -269,9 +269,6 @@ int main(int argc, char **argv) conf(&c, argc, argv); trace_init(c.trace); - if (c.force_stderr || isatty(fileno(stdout))) - __openlog(log_name, LOG_PERROR, LOG_DAEMON); - pasta_netns_quit_init(&c); tap_sock_init(&c); @@ -314,6 +311,9 @@ int main(int argc, char **argv) if (isolate_prefork(&c)) die("Failed to sandbox process, exiting"); + if (!c.force_stderr && !isatty(fileno(stdout))) + __openlog(log_name, 0, LOG_DAEMON); + if (!c.foreground) __daemon(pidfile_fd, devnull_fd); else -- 2.39.2

2 2