- dev - passt

[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

[PATCH 0/3] Basic integration of ICMP with flow table
by David Gibson 13 Mar '24

13 Mar '24

I realized that even with the rudimentary form of the flow table we have now, it's possible to integrate ICMP (ping). This even simplifies a little bit of logic in the ICMP code. This is based on the address handling clean up series. David Gibson (3): icmp: Store ping socket information in flow table icmp: Flow based error reporting icmp: Use 'flowside' epoll references for ping sockets Makefile | 6 +- flow.c | 9 +++ flow.h | 4 ++ flow_table.h | 2 + icmp.c | 194 +++++++++++++++++++++++---------------------------- icmp.h | 15 +--- icmp_flow.h | 31 ++++++++ passt.c | 15 +--- passt.h | 7 +- util.c | 4 +- 10 files changed, 146 insertions(+), 141 deletions(-) create mode 100644 icmp_flow.h -- 2.44.0

2 11

[PATCH] conf: Don't warn if nameservers were found, but won't be advertised
by Stefano Brivio 08 Mar '24

08 Mar '24

Starting from commit 3a2afde87dd1 ("conf, udp: Drop mostly duplicated dns_send arrays, rename related fields"), we won't add to c->ip4.dns and c->ip6.dns nameservers that can't be used by the guest or container, and we won't advertise them. However, the fact that we don't advertise any nameserver doesn't mean that we didn't find any, and we should warn only if we couldn't find any. This is particularly relevant in case both --dns-forward and --no-map-gw are passed, and a single loopback address is listed in /etc/resolv.conf: we'll forward queries directed to the address specified by --dns-forward to the loopback address we found, we won't advertise that address, so we shouldn't warn: this is a perfectly legitimate usage. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/issues/19213 Fixes: 3a2afde87dd1 ("conf, udp: Drop mostly duplicated dns_send arrays, rename related fields") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/conf.c b/conf.c index 4a783b8..c50c039 100644 --- a/conf.c +++ b/conf.c @@ -399,6 +399,7 @@ static void get_dns(struct ctx *c) int dns4_set, dns6_set, dnss_set, dns_set, fd; struct fqdn *s = c->dns_search; struct lineread resolvconf; + unsigned int added = 0; char *line, *end; const char *p; int line_len; @@ -427,13 +428,17 @@ static void get_dns(struct ctx *c) if (!dns4_set && dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) - 1 - && inet_pton(AF_INET, p + 1, &dns4_tmp)) + && inet_pton(AF_INET, p + 1, &dns4_tmp)) { add_dns4(c, &dns4_tmp, &dns4); + added++; + } if (!dns6_set && dns6 - &c->ip6.dns[0] < ARRAY_SIZE(c->ip6.dns) - 1 - && inet_pton(AF_INET6, p + 1, &dns6_tmp)) + && inet_pton(AF_INET6, p + 1, &dns6_tmp)) { add_dns6(c, &dns6_tmp, &dns6); + added++; + } } else if (!dnss_set && strstr(line, "search ") == line && s == c->dns_search) { end = strpbrk(line, "\n"); @@ -459,7 +464,7 @@ static void get_dns(struct ctx *c) close(fd); out: - if (!dns_set && dns4 == c->ip4.dns && dns6 == c->ip6.dns) + if (!dns_set && !added) warn("Couldn't get any nameserver address"); } -- 2.39.2

3 6