March 2024 - dev

[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

[PATCH v3 0/9] Add vhost-user support to passt (part 1)
by Laurent Vivier 08 Mar '24

08 Mar '24

v3: - add a patch that has been extracted from: "tcp: extract buffer management from tcp_send_flag()" -> "tcp: Introduce ipv4_fill_headers()/ipv6_fill_headers()" - see detailed v3 history log in each patch - I didn't address the alignment problem when we provide a pointer to a sub-structure in the internal buffer structure. (for the last patches of the series). v2 comparing to vhost-user full part: - part 1 includes only preliminary patches (checksum, iovec, cleanup) - see detailed v2 history log in each patch. Full series v1 available at: [PATCH 00/24] Add vhost-user support to passt. https://url.corp.redhat.com/passt-vhost-user-v1 Thanks, Laurent Laurent Vivier (9): iov: add some functions to manage iovec pcap: add pcap_iov() checksum: align buffers checksum: add csum_iov() util: move IP stuff from util.[ch] to ip.[ch] checksum: use csum_ip4_header() in udp.c and tcp.c checksum: introduce functions to compute the header part checksum for TCP/UDP tap: make tap_update_mac() generic tcp: Introduce ipv4_fill_headers()/ipv6_fill_headers() Makefile | 12 +-- checksum.c | 171 ++++++++++++++++++++++++++++++----------- checksum.h | 9 ++- conf.c | 1 + dhcp.c | 1 + flow.c | 1 + icmp.c | 1 + iov.c | 171 +++++++++++++++++++++++++++++++++++++++++ iov.h | 29 +++++++ ip.c | 72 +++++++++++++++++ ip.h | 86 +++++++++++++++++++++ ndp.c | 1 + pcap.c | 62 +++++++++++++-- pcap.h | 1 + port_fwd.c | 1 + qrap.c | 1 + tap.c | 14 ++-- tap.h | 2 +- tcp.c | 213 +++++++++++++++++++++++++++++---------------------- tcp_splice.c | 1 + udp.c | 35 +++------ util.c | 55 ------------- util.h | 76 ------------------ 23 files changed, 703 insertions(+), 313 deletions(-) create mode 100644 iov.c create mode 100644 iov.h create mode 100644 ip.c create mode 100644 ip.h -- 2.42.0

3 37

[PATCH] iov: Improve documentation of iov_skip_bytes()
by David Gibson 08 Mar '24

08 Mar '24

As pointed out in review, the documentation comments for iov_skip_bytes() are more confusing than they should be. Reword them, including updating parameter names, to make it clearer. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- iov.c | 28 ++++++++++++++-------------- iov.h | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/iov.c b/iov.c index e3312628..8e850180 100644 --- a/iov.c +++ b/iov.c @@ -26,31 +26,31 @@ #include "iov.h" -/* iov_skip_bytes() - Skip the first n bytes into an IO vector +/* iov_skip_bytes() - Skip leading bytes of an IO vector * @iov: IO vector * @n: Number of entries in @iov - * @vec_offset: Total byte offset into the IO vector - * @buf_offset: Offset into a single buffer of the IO vector + * @skip: Number of leading bytes of @iov to skip + * @offset: Offset of first unskipped byte in its @iov entry * - * Return: index I of individual struct iovec which contains the byte at - * @vec_offset bytes into the vector (as though all its buffers were - * contiguous). If @buf_offset is non-NULL, update it to the offset of - * that byte within @iov[I] (guaranteed to be less than @iov[I].iov_len) - * If the whole vector has <= @vec_offset bytes, return @n. + * Return: index I of individual struct iovec which contains the byte at @skip + * bytes into the vector (as though all its buffers were contiguous). + * If @offset is non-NULL, update it to the offset of that byte within + * @iov[I] (guaranteed to be less than @iov[I].iov_len) If the whole + * vector has <= @skip bytes, return @n. */ size_t iov_skip_bytes(const struct iovec *iov, size_t n, - size_t vec_offset, size_t *buf_offset) + size_t skip, size_t *offset) { - size_t offset = vec_offset, i; + size_t off = skip, i; for (i = 0; i < n; i++) { - if (offset < iov[i].iov_len) + if (off < iov[i].iov_len) break; - offset -= iov[i].iov_len; + off -= iov[i].iov_len; } - if (buf_offset) - *buf_offset = offset; + if (offset) + *offset = off; return i; } diff --git a/iov.h b/iov.h index e1becdea..6058af77 100644 --- a/iov.h +++ b/iov.h @@ -19,7 +19,7 @@ #include <string.h> size_t iov_skip_bytes(const struct iovec *iov, size_t n, - size_t vec_offset, size_t *buf_offset); + size_t skip, size_t *offset); size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt, size_t offset, const void *buf, size_t bytes); size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, -- 2.44.0

2 1

[PATCH v2 0/9] vhost-user part 1, v6
by David Gibson 06 Mar '24

06 Mar '24

When I rebased by own code, I hit the problems Stefano mentioned with static checker regressions in the vhost-user series. Laurent's most recent vhost-user part 1 series, but with fixes applied for the cppcheck and clang-tidy regressions introduced. Changes since 'v5 + static checker fixes' * Added my own alteration notes and S-o-b Changes since v5: * Fixed a number of static checker regressions Laurent Vivier (9): pcap: add pcap_iov() checksum: align buffers checksum: add csum_iov() util: move IP stuff from util.[ch] to ip.[ch] udp: little cleanup in udp_update_hdrX() to prepare future changes checksum: use csum_ip4_header() in udp.c and tcp.c checksum: introduce functions to compute the header part checksum for TCP/UDP tap: make tap_update_mac() generic tcp: Introduce tcp_fill_headers4()/tcp_fill_headers6() Makefile | 10 +-- checksum.c | 173 ++++++++++++++++++++++++++++++---------- checksum.h | 12 ++- conf.c | 1 + dhcp.c | 1 + flow.c | 1 + fwd.c | 1 + icmp.c | 1 + inany.c | 1 + iov.c | 1 - ip.c | 72 +++++++++++++++++ ip.h | 86 ++++++++++++++++++++ ndp.c | 1 + pcap.c | 27 ++++++- pcap.h | 1 + qrap.c | 1 + tap.c | 13 +-- tap.h | 2 +- tcp.c | 221 +++++++++++++++++++++++++++++---------------------- tcp_splice.c | 1 + udp.c | 82 ++++++++----------- util.c | 55 ------------- util.h | 76 ------------------ 23 files changed, 505 insertions(+), 335 deletions(-) create mode 100644 ip.c create mode 100644 ip.h -- 2.44.0

2 10