December 2024 - dev

[PATCH] treewide: Dodge dynamic memory allocation in strerror() from glibc > 2.40
by Stefano Brivio 11 Dec '24

11 Dec '24

With glibc commit 25a5eb4010df ("string: strerror, strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() now needs, at least on x86, the getrandom() and brk() system calls, in order to fill in the locale-translated error message. But getrandom() and brk() are not allowed by our seccomp profiles. This became visible on Fedora Rawhide with the "podman login and logout" Podman tests, defined at test/e2e/login_logout_test.go in the Podman source tree, where pasta would terminate upon printing error descriptions (at least the ones related to the SO_ERROR queue for spliced connections). Avoid dynamic memory allocation by calling strerrordesc_np() instead, which is a GNU function returning a static, untranslated version of the error description. If it's not available, keep calling strerror(), which at that point should be simple enough as to be usable (at least, that's currently the case for musl). Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/issues/24804 Analysed-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 10 +++++----- icmp.c | 4 ++-- log.c | 2 +- netlink.c | 2 +- pasta.c | 22 +++++++++++----------- tcp.c | 22 +++++++++++----------- tcp_splice.c | 16 ++++++++-------- udp.c | 5 +++-- udp_flow.c | 8 ++++---- util.c | 6 +++--- util.h | 32 ++++++++++++++++++++++++++++++++ 11 files changed, 81 insertions(+), 48 deletions(-) diff --git a/conf.c b/conf.c index eaa7d99..6d03111 100644 --- a/conf.c +++ b/conf.c @@ -365,7 +365,7 @@ mode_conflict: die("Port forwarding mode '%s' conflicts with previous mode", optarg); bind_fail: die("Failed to bind port %u (%s) for option '-%c %s', exiting", - i, strerror(-ret), optname, optarg); + i, __strerror(-ret), optname, optarg); bind_all_fail: die("Failed to bind any port for '-%c %s', exiting", optname, optarg); } @@ -655,7 +655,7 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) &ip4->guest_gw); if (rc < 0) { debug("Couldn't discover IPv4 gateway address: %s", - strerror(-rc)); + __strerror(-rc)); return 0; } } @@ -665,7 +665,7 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) &ip4->addr, &ip4->prefix_len, NULL); if (rc < 0) { debug("Couldn't discover IPv4 address: %s", - strerror(-rc)); + __strerror(-rc)); return 0; } } @@ -729,7 +729,7 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) rc = nl_route_get_def(nl_sock, ifi, AF_INET6, &ip6->guest_gw); if (rc < 0) { debug("Couldn't discover IPv6 gateway address: %s", - strerror(-rc)); + __strerror(-rc)); return 0; } } @@ -738,7 +738,7 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL, &prefix_len, &ip6->our_tap_ll); if (rc < 0) { - debug("Couldn't discover IPv6 address: %s", strerror(-rc)); + debug("Couldn't discover IPv6 address: %s", __strerror(-rc)); return 0; } diff --git a/icmp.c b/icmp.c index f514dbc..bb0e30b 100644 --- a/icmp.c +++ b/icmp.c @@ -85,7 +85,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref) n = recvfrom(ref.fd, buf, sizeof(buf), 0, &sr.sa, &sl); if (n < 0) { - flow_err(pingf, "recvfrom() error: %s", strerror(errno)); + flow_err(pingf, "recvfrom() error: %s", __strerror(errno)); return; } @@ -301,7 +301,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0); if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) { flow_dbg(pingf, "failed to relay request to socket: %s", - strerror(errno)); + __strerror(errno)); } else { flow_dbg(pingf, "echo request to socket, ID: %"PRIu16", seq: %"PRIu16, diff --git a/log.c b/log.c index 239c8ce..c0eae4f 100644 --- a/log.c +++ b/log.c @@ -322,7 +322,7 @@ void logmsg_perror(int pri, const char *format, ...) vlogmsg(false, false, pri, format, ap); va_end(ap); - logmsg(true, true, pri, ": %s", strerror(errno_copy)); + logmsg(true, true, pri, ": %s", __strerror(errno_copy)); } /** diff --git a/netlink.c b/netlink.c index 4aba2a3..983f45b 100644 --- a/netlink.c +++ b/netlink.c @@ -320,7 +320,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) } if (status < 0) - warn("netlink: RTM_GETROUTE failed: %s", strerror(-status)); + warn("netlink: RTM_GETROUTE failed: %s", __strerror(-status)); if (defifi) { if (ndef > 1) { diff --git a/pasta.c b/pasta.c index 96dacc3..d3fb36c 100644 --- a/pasta.c +++ b/pasta.c @@ -296,7 +296,7 @@ void pasta_ns_conf(struct ctx *c) rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP); if (rc < 0) die("Couldn't bring up loopback interface in namespace: %s", - strerror(-rc)); + __strerror(-rc)); /* Get or set MAC in target namespace */ if (MAC_IS_ZERO(c->guest_mac)) @@ -305,7 +305,7 @@ void pasta_ns_conf(struct ctx *c) rc = nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->guest_mac); if (rc < 0) die("Couldn't set MAC address in namespace: %s", - strerror(-rc)); + __strerror(-rc)); if (c->pasta_conf_ns) { unsigned int flags = IFF_UP; @@ -332,7 +332,7 @@ void pasta_ns_conf(struct ctx *c) if (rc < 0) { die("Couldn't set IPv4 address(es) in namespace: %s", - strerror(-rc)); + __strerror(-rc)); } if (c->ip4.no_copy_routes) { @@ -346,7 +346,7 @@ void pasta_ns_conf(struct ctx *c) if (rc < 0) { die("Couldn't set IPv4 route(s) in guest: %s", - strerror(-rc)); + __strerror(-rc)); } } @@ -355,13 +355,13 @@ void pasta_ns_conf(struct ctx *c) &c->ip6.addr_ll_seen); if (rc < 0) { warn("Can't get LL address from namespace: %s", - strerror(-rc)); + __strerror(-rc)); } rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); if (rc < 0) { warn("Can't set nodad for LL in namespace: %s", - strerror(-rc)); + __strerror(-rc)); } /* We dodged DAD: re-enable neighbour solicitations */ @@ -382,7 +382,7 @@ void pasta_ns_conf(struct ctx *c) if (rc < 0) { die("Couldn't set IPv6 address(es) in namespace: %s", - strerror(-rc)); + __strerror(-rc)); } if (c->ip6.no_copy_routes) { @@ -397,7 +397,7 @@ void pasta_ns_conf(struct ctx *c) if (rc < 0) { die("Couldn't set IPv6 route(s) in guest: %s", - strerror(-rc)); + __strerror(-rc)); } } } @@ -446,18 +446,18 @@ void pasta_netns_quit_init(const struct ctx *c) return; if ((dir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY)) < 0) - die("netns dir open: %s, exiting", strerror(errno)); + die("netns dir open: %s, exiting", __strerror(errno)); if (fstatfs(dir_fd, &s) || s.f_type == DEVPTS_SUPER_MAGIC || s.f_type == PROC_SUPER_MAGIC || s.f_type == SYSFS_MAGIC) try_inotify = false; if (try_inotify && (fd = inotify_init1(flags)) < 0) - warn("inotify_init1(): %s, use a timer", strerror(errno)); + warn("inotify_init1(): %s, use a timer", __strerror(errno)); if (fd >= 0 && inotify_add_watch(fd, c->netns_dir, IN_DELETE) < 0) { warn("inotify_add_watch(): %s, use a timer", - strerror(errno)); + __strerror(errno)); close(fd); fd = -1; } diff --git a/tcp.c b/tcp.c index 1872ccb..640b25a 100644 --- a/tcp.c +++ b/tcp.c @@ -516,7 +516,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) fd = timerfd_create(CLOCK_MONOTONIC, 0); if (fd == -1 || fd > FD_REF_MAX) { flow_dbg(conn, "failed to get timer: %s", - strerror(errno)); + __strerror(errno)); if (fd > -1) close(fd); conn->timer = -1; @@ -526,7 +526,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, conn->timer, &ev)) { flow_dbg(conn, "failed to add timer: %s", - strerror(errno)); + __strerror(errno)); close(conn->timer); conn->timer = -1; return; @@ -551,7 +551,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) (unsigned long long)it.it_value.tv_nsec / 1000 / 1000); if (timerfd_settime(conn->timer, 0, &it, NULL)) - flow_err(conn, "failed to set timer: %s", strerror(errno)); + flow_err(conn, "failed to set timer: %s", __strerror(errno)); } /** @@ -1307,7 +1307,7 @@ int tcp_conn_sock(const struct ctx *c, sa_family_t af) return s; err("TCP: Unable to open socket for new connection: %s", - strerror(-s)); + __strerror(-s)); return -1; } @@ -1360,7 +1360,7 @@ static void tcp_bind_outbound(const struct ctx *c, flow_dbg(conn, "Can't bind TCP outbound socket to %s:%hu: %s", inany_ntop(&tgt->oaddr, sstr, sizeof(sstr)), - tgt->oport, strerror(errno)); + tgt->oport, __strerror(errno)); } } @@ -1371,7 +1371,7 @@ static void tcp_bind_outbound(const struct ctx *c, strlen(c->ip4.ifname_out))) { flow_dbg(conn, "Can't bind IPv4 TCP socket to" " interface %s: %s", c->ip4.ifname_out, - strerror(errno)); + __strerror(errno)); } } } else if (bind_sa.sa_family == AF_INET6) { @@ -1381,7 +1381,7 @@ static void tcp_bind_outbound(const struct ctx *c, strlen(c->ip6.ifname_out))) { flow_dbg(conn, "Can't bind IPv6 TCP socket to" " interface %s: %s", c->ip6.ifname_out, - strerror(errno)); + __strerror(errno)); } } } @@ -2113,7 +2113,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) * and we just set the timer to a new point in the future: discard it. */ if (timerfd_gettime(conn->timer, &check_armed)) - flow_err(conn, "failed to read timer: %s", strerror(errno)); + flow_err(conn, "failed to read timer: %s", __strerror(errno)); if (check_armed.it_value.tv_sec || check_armed.it_value.tv_nsec) return; @@ -2154,7 +2154,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) */ if (timerfd_settime(conn->timer, 0, &new, &old)) flow_err(conn, "failed to set timer: %s", - strerror(errno)); + __strerror(errno)); if (old.it_value.tv_sec == ACT_TIMEOUT) { flow_dbg(conn, "activity timeout"); @@ -2422,13 +2422,13 @@ static void tcp_sock_refill_init(const struct ctx *c) int rc = tcp_sock_refill_pool(c, init_sock_pool4, AF_INET); if (rc < 0) warn("TCP: Error refilling IPv4 host socket pool: %s", - strerror(-rc)); + __strerror(-rc)); } if (c->ifi6) { int rc = tcp_sock_refill_pool(c, init_sock_pool6, AF_INET6); if (rc < 0) warn("TCP: Error refilling IPv6 host socket pool: %s", - strerror(-rc)); + __strerror(-rc)); } } diff --git a/tcp_splice.c b/tcp_splice.c index 93f8bce..d35bfd5 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -160,7 +160,7 @@ static int tcp_splice_epoll_ctl(const struct ctx *c, if (epoll_ctl(c->epollfd, m, conn->s[0], &ev[0]) || epoll_ctl(c->epollfd, m, conn->s[1], &ev[1])) { int ret = -errno; - flow_err(conn, "ERROR on epoll_ctl(): %s", strerror(errno)); + flow_err(conn, "ERROR on epoll_ctl(): %s", __strerror(errno)); return ret; } @@ -314,7 +314,7 @@ static int tcp_splice_connect_finish(const struct ctx *c, if (conn->pipe[sidei][0] < 0) { if (pipe2(conn->pipe[sidei], O_NONBLOCK | O_CLOEXEC)) { flow_err(conn, "cannot create %d->%d pipe: %s", - sidei, !sidei, strerror(errno)); + sidei, !sidei, __strerror(errno)); conn_flag(c, conn, CLOSING); return -EIO; } @@ -370,7 +370,7 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn) if (connect(conn->s[1], &sa.sa, sl)) { if (errno != EINPROGRESS) { flow_trace(conn, "Couldn't connect socket for splice: %s", - strerror(errno)); + __strerror(errno)); return -errno; } @@ -469,10 +469,10 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref, rc = getsockopt(ref.fd, SOL_SOCKET, SO_ERROR, &err, &sl); if (rc) flow_err(conn, "Error retrieving SO_ERROR: %s", - strerror(errno)); + __strerror(errno)); else flow_trace(conn, "Error event on socket: %s", - strerror(err)); + __strerror(err)); goto close; } @@ -551,7 +551,7 @@ eintr: &lowat, sizeof(lowat))) { flow_trace(conn, "Setting SO_RCVLOWAT %i: %s", - lowat, strerror(errno)); + lowat, __strerror(errno)); } else { conn_flag(c, conn, lowat_set_flag); conn_flag(c, conn, lowat_act_flag); @@ -696,13 +696,13 @@ static int tcp_sock_refill_ns(void *arg) int rc = tcp_sock_refill_pool(c, ns_sock_pool4, AF_INET); if (rc < 0) warn("TCP: Error refilling IPv4 ns socket pool: %s", - strerror(-rc)); + __strerror(-rc)); } if (c->ifi6) { int rc = tcp_sock_refill_pool(c, ns_sock_pool6, AF_INET6); if (rc < 0) warn("TCP: Error refilling IPv6 ns socket pool: %s", - strerror(-rc)); + __strerror(-rc)); } return 0; diff --git a/udp.c b/udp.c index c89f031..0154a1a 100644 --- a/udp.c +++ b/udp.c @@ -453,7 +453,7 @@ static int udp_sock_recverr(int s) /* TODO: When possible propagate and otherwise handle errors */ debug("%s error on UDP socket %i: %s", - str_ee_origin(ee), s, strerror(ee->ee_errno)); + str_ee_origin(ee), s, __strerror(ee->ee_errno)); return 1; } @@ -492,7 +492,8 @@ int udp_sock_errs(const struct ctx *c, int s, uint32_t events) } if (err) { - debug("Unqueued error on UDP socket %i: %s", s, strerror(err)); + debug("Unqueued error on UDP socket %i: %s", s, + __strerror(err)); n_err++; } diff --git a/udp_flow.c b/udp_flow.c index c8fdb5f..627cc15 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -95,7 +95,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, if (uflow->s[INISIDE] < 0) { flow_err(uflow, "Couldn't duplicate listening socket: %s", - strerror(errno)); + __strerror(errno)); goto cancel; } } @@ -115,14 +115,14 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, if (uflow->s[TGTSIDE] < 0) { flow_dbg(uflow, "Couldn't open socket for spliced flow: %s", - strerror(errno)); + __strerror(errno)); goto cancel; } if (flowside_connect(c, uflow->s[TGTSIDE], tgtpif, tgt) < 0) { flow_dbg(uflow, "Couldn't connect flow socket: %s", - strerror(errno)); + __strerror(errno)); goto cancel; } @@ -144,7 +144,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, } else if (errno != EAGAIN) { flow_err(uflow, "Unexpected error discarding datagrams: %s", - strerror(errno)); + __strerror(errno)); } } diff --git a/util.c b/util.c index 55cae3f..386bf76 100644 --- a/util.c +++ b/util.c @@ -90,7 +90,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, ret = -errno; if (fd < 0) { - warn("L4 socket: %s", strerror(-ret)); + warn("L4 socket: %s", __strerror(-ret)); return ret; } @@ -162,7 +162,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, if (type == EPOLL_TYPE_TCP_LISTEN && listen(fd, 128) < 0) { ret = -errno; - warn("TCP socket listen: %s", strerror(-ret)); + warn("TCP socket listen: %s", __strerror(-ret)); close(fd); return ret; } @@ -171,7 +171,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, ev.data.u64 = ref.u64; if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) { ret = -errno; - warn("L4 epoll_ctl: %s", strerror(-ret)); + warn("L4 epoll_ctl: %s", __strerror(-ret)); return ret; } diff --git a/util.h b/util.h index 41bbd60..b766dc1 100644 --- a/util.h +++ b/util.h @@ -274,6 +274,38 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m) void raw_random(void *buf, size_t buflen); +/* + * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, + * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs + * getrandom(2) and brk(2) as it allocates memory for the locale-translated + * error description, but our seccomp profiles forbid both. + * + * Use the __strerror() wrapper instead, calling into strerrordesc_np() to get + * a static untranslated string. It's a GNU implementation, but also defined by + * bionic. + * + * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries + * not defining strerrordesc_np() are expected to provide strerror() + * implementations that are simple enough for us to call. + */ +__attribute__ ((weak)) const char *strerrordesc_np(int errnum); + +/** + * __strerror() - strerror() wrapper calling strerrordesc_np() if available + * @errnum: Error code + * + * Return: error description string + */ +static inline const char *__strerror(int errnum) +{ + if (strerrordesc_np) + return strerrordesc_np(errnum); + + return strerror(errnum); +} + +#define strerror(x) @ "Don't call strerror() directly, use __strerror() instead" + /* * Workarounds for https://github.com/llvm/llvm-project/issues/58992 * -- 2.43.0

3 5

[PATCH v2] treewide: Dodge dynamic memory allocation in strerror() from glibc > 2.40
by Stefano Brivio 11 Dec '24

11 Dec '24

With glibc commit 25a5eb4010df ("string: strerror, strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() now needs, at least on x86, the getrandom() and brk() system calls, in order to fill in the locale-translated error message. But getrandom() and brk() are not allowed by our seccomp profiles. This became visible on Fedora Rawhide with the "podman login and logout" Podman tests, defined at test/e2e/login_logout_test.go in the Podman source tree, where pasta would terminate upon printing error descriptions (at least the ones related to the SO_ERROR queue for spliced connections). Avoid dynamic memory allocation by calling strerrordesc_np() instead, which is a GNU function returning a static, untranslated version of the error description. If it's not available, keep calling strerror(), which at that point should be simple enough as to be usable (at least, that's currently the case for musl). Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/issues/24804 Analysed-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Name the wrapper strerror_() instead of __strerror() conf.c | 10 +++++----- icmp.c | 4 ++-- log.c | 2 +- netlink.c | 2 +- pasta.c | 22 +++++++++++----------- tcp.c | 22 +++++++++++----------- tcp_splice.c | 16 ++++++++-------- udp.c | 4 ++-- udp_flow.c | 8 ++++---- util.c | 6 +++--- util.h | 32 ++++++++++++++++++++++++++++++++ 11 files changed, 80 insertions(+), 48 deletions(-) diff --git a/conf.c b/conf.c index eaa7d99..43daeca 100644 --- a/conf.c +++ b/conf.c @@ -365,7 +365,7 @@ mode_conflict: die("Port forwarding mode '%s' conflicts with previous mode", optarg); bind_fail: die("Failed to bind port %u (%s) for option '-%c %s', exiting", - i, strerror(-ret), optname, optarg); + i, strerror_(-ret), optname, optarg); bind_all_fail: die("Failed to bind any port for '-%c %s', exiting", optname, optarg); } @@ -655,7 +655,7 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) &ip4->guest_gw); if (rc < 0) { debug("Couldn't discover IPv4 gateway address: %s", - strerror(-rc)); + strerror_(-rc)); return 0; } } @@ -665,7 +665,7 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) &ip4->addr, &ip4->prefix_len, NULL); if (rc < 0) { debug("Couldn't discover IPv4 address: %s", - strerror(-rc)); + strerror_(-rc)); return 0; } } @@ -729,7 +729,7 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) rc = nl_route_get_def(nl_sock, ifi, AF_INET6, &ip6->guest_gw); if (rc < 0) { debug("Couldn't discover IPv6 gateway address: %s", - strerror(-rc)); + strerror_(-rc)); return 0; } } @@ -738,7 +738,7 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL, &prefix_len, &ip6->our_tap_ll); if (rc < 0) { - debug("Couldn't discover IPv6 address: %s", strerror(-rc)); + debug("Couldn't discover IPv6 address: %s", strerror_(-rc)); return 0; } diff --git a/icmp.c b/icmp.c index f514dbc..143e93b 100644 --- a/icmp.c +++ b/icmp.c @@ -85,7 +85,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref) n = recvfrom(ref.fd, buf, sizeof(buf), 0, &sr.sa, &sl); if (n < 0) { - flow_err(pingf, "recvfrom() error: %s", strerror(errno)); + flow_err(pingf, "recvfrom() error: %s", strerror_(errno)); return; } @@ -301,7 +301,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0); if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) { flow_dbg(pingf, "failed to relay request to socket: %s", - strerror(errno)); + strerror_(errno)); } else { flow_dbg(pingf, "echo request to socket, ID: %"PRIu16", seq: %"PRIu16, diff --git a/log.c b/log.c index 239c8ce..95e4576 100644 --- a/log.c +++ b/log.c @@ -322,7 +322,7 @@ void logmsg_perror(int pri, const char *format, ...) vlogmsg(false, false, pri, format, ap); va_end(ap); - logmsg(true, true, pri, ": %s", strerror(errno_copy)); + logmsg(true, true, pri, ": %s", strerror_(errno_copy)); } /** diff --git a/netlink.c b/netlink.c index 4aba2a3..0407692 100644 --- a/netlink.c +++ b/netlink.c @@ -320,7 +320,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) } if (status < 0) - warn("netlink: RTM_GETROUTE failed: %s", strerror(-status)); + warn("netlink: RTM_GETROUTE failed: %s", strerror_(-status)); if (defifi) { if (ndef > 1) { diff --git a/pasta.c b/pasta.c index 96dacc3..ff41c95 100644 --- a/pasta.c +++ b/pasta.c @@ -296,7 +296,7 @@ void pasta_ns_conf(struct ctx *c) rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP); if (rc < 0) die("Couldn't bring up loopback interface in namespace: %s", - strerror(-rc)); + strerror_(-rc)); /* Get or set MAC in target namespace */ if (MAC_IS_ZERO(c->guest_mac)) @@ -305,7 +305,7 @@ void pasta_ns_conf(struct ctx *c) rc = nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->guest_mac); if (rc < 0) die("Couldn't set MAC address in namespace: %s", - strerror(-rc)); + strerror_(-rc)); if (c->pasta_conf_ns) { unsigned int flags = IFF_UP; @@ -332,7 +332,7 @@ void pasta_ns_conf(struct ctx *c) if (rc < 0) { die("Couldn't set IPv4 address(es) in namespace: %s", - strerror(-rc)); + strerror_(-rc)); } if (c->ip4.no_copy_routes) { @@ -346,7 +346,7 @@ void pasta_ns_conf(struct ctx *c) if (rc < 0) { die("Couldn't set IPv4 route(s) in guest: %s", - strerror(-rc)); + strerror_(-rc)); } } @@ -355,13 +355,13 @@ void pasta_ns_conf(struct ctx *c) &c->ip6.addr_ll_seen); if (rc < 0) { warn("Can't get LL address from namespace: %s", - strerror(-rc)); + strerror_(-rc)); } rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); if (rc < 0) { warn("Can't set nodad for LL in namespace: %s", - strerror(-rc)); + strerror_(-rc)); } /* We dodged DAD: re-enable neighbour solicitations */ @@ -382,7 +382,7 @@ void pasta_ns_conf(struct ctx *c) if (rc < 0) { die("Couldn't set IPv6 address(es) in namespace: %s", - strerror(-rc)); + strerror_(-rc)); } if (c->ip6.no_copy_routes) { @@ -397,7 +397,7 @@ void pasta_ns_conf(struct ctx *c) if (rc < 0) { die("Couldn't set IPv6 route(s) in guest: %s", - strerror(-rc)); + strerror_(-rc)); } } } @@ -446,18 +446,18 @@ void pasta_netns_quit_init(const struct ctx *c) return; if ((dir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY)) < 0) - die("netns dir open: %s, exiting", strerror(errno)); + die("netns dir open: %s, exiting", strerror_(errno)); if (fstatfs(dir_fd, &s) || s.f_type == DEVPTS_SUPER_MAGIC || s.f_type == PROC_SUPER_MAGIC || s.f_type == SYSFS_MAGIC) try_inotify = false; if (try_inotify && (fd = inotify_init1(flags)) < 0) - warn("inotify_init1(): %s, use a timer", strerror(errno)); + warn("inotify_init1(): %s, use a timer", strerror_(errno)); if (fd >= 0 && inotify_add_watch(fd, c->netns_dir, IN_DELETE) < 0) { warn("inotify_add_watch(): %s, use a timer", - strerror(errno)); + strerror_(errno)); close(fd); fd = -1; } diff --git a/tcp.c b/tcp.c index 1872ccb..ec433f7 100644 --- a/tcp.c +++ b/tcp.c @@ -516,7 +516,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) fd = timerfd_create(CLOCK_MONOTONIC, 0); if (fd == -1 || fd > FD_REF_MAX) { flow_dbg(conn, "failed to get timer: %s", - strerror(errno)); + strerror_(errno)); if (fd > -1) close(fd); conn->timer = -1; @@ -526,7 +526,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, conn->timer, &ev)) { flow_dbg(conn, "failed to add timer: %s", - strerror(errno)); + strerror_(errno)); close(conn->timer); conn->timer = -1; return; @@ -551,7 +551,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) (unsigned long long)it.it_value.tv_nsec / 1000 / 1000); if (timerfd_settime(conn->timer, 0, &it, NULL)) - flow_err(conn, "failed to set timer: %s", strerror(errno)); + flow_err(conn, "failed to set timer: %s", strerror_(errno)); } /** @@ -1307,7 +1307,7 @@ int tcp_conn_sock(const struct ctx *c, sa_family_t af) return s; err("TCP: Unable to open socket for new connection: %s", - strerror(-s)); + strerror_(-s)); return -1; } @@ -1360,7 +1360,7 @@ static void tcp_bind_outbound(const struct ctx *c, flow_dbg(conn, "Can't bind TCP outbound socket to %s:%hu: %s", inany_ntop(&tgt->oaddr, sstr, sizeof(sstr)), - tgt->oport, strerror(errno)); + tgt->oport, strerror_(errno)); } } @@ -1371,7 +1371,7 @@ static void tcp_bind_outbound(const struct ctx *c, strlen(c->ip4.ifname_out))) { flow_dbg(conn, "Can't bind IPv4 TCP socket to" " interface %s: %s", c->ip4.ifname_out, - strerror(errno)); + strerror_(errno)); } } } else if (bind_sa.sa_family == AF_INET6) { @@ -1381,7 +1381,7 @@ static void tcp_bind_outbound(const struct ctx *c, strlen(c->ip6.ifname_out))) { flow_dbg(conn, "Can't bind IPv6 TCP socket to" " interface %s: %s", c->ip6.ifname_out, - strerror(errno)); + strerror_(errno)); } } } @@ -2113,7 +2113,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) * and we just set the timer to a new point in the future: discard it. */ if (timerfd_gettime(conn->timer, &check_armed)) - flow_err(conn, "failed to read timer: %s", strerror(errno)); + flow_err(conn, "failed to read timer: %s", strerror_(errno)); if (check_armed.it_value.tv_sec || check_armed.it_value.tv_nsec) return; @@ -2154,7 +2154,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) */ if (timerfd_settime(conn->timer, 0, &new, &old)) flow_err(conn, "failed to set timer: %s", - strerror(errno)); + strerror_(errno)); if (old.it_value.tv_sec == ACT_TIMEOUT) { flow_dbg(conn, "activity timeout"); @@ -2422,13 +2422,13 @@ static void tcp_sock_refill_init(const struct ctx *c) int rc = tcp_sock_refill_pool(c, init_sock_pool4, AF_INET); if (rc < 0) warn("TCP: Error refilling IPv4 host socket pool: %s", - strerror(-rc)); + strerror_(-rc)); } if (c->ifi6) { int rc = tcp_sock_refill_pool(c, init_sock_pool6, AF_INET6); if (rc < 0) warn("TCP: Error refilling IPv6 host socket pool: %s", - strerror(-rc)); + strerror_(-rc)); } } diff --git a/tcp_splice.c b/tcp_splice.c index 93f8bce..3a0f868 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -160,7 +160,7 @@ static int tcp_splice_epoll_ctl(const struct ctx *c, if (epoll_ctl(c->epollfd, m, conn->s[0], &ev[0]) || epoll_ctl(c->epollfd, m, conn->s[1], &ev[1])) { int ret = -errno; - flow_err(conn, "ERROR on epoll_ctl(): %s", strerror(errno)); + flow_err(conn, "ERROR on epoll_ctl(): %s", strerror_(errno)); return ret; } @@ -314,7 +314,7 @@ static int tcp_splice_connect_finish(const struct ctx *c, if (conn->pipe[sidei][0] < 0) { if (pipe2(conn->pipe[sidei], O_NONBLOCK | O_CLOEXEC)) { flow_err(conn, "cannot create %d->%d pipe: %s", - sidei, !sidei, strerror(errno)); + sidei, !sidei, strerror_(errno)); conn_flag(c, conn, CLOSING); return -EIO; } @@ -370,7 +370,7 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn) if (connect(conn->s[1], &sa.sa, sl)) { if (errno != EINPROGRESS) { flow_trace(conn, "Couldn't connect socket for splice: %s", - strerror(errno)); + strerror_(errno)); return -errno; } @@ -469,10 +469,10 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref, rc = getsockopt(ref.fd, SOL_SOCKET, SO_ERROR, &err, &sl); if (rc) flow_err(conn, "Error retrieving SO_ERROR: %s", - strerror(errno)); + strerror_(errno)); else flow_trace(conn, "Error event on socket: %s", - strerror(err)); + strerror_(err)); goto close; } @@ -551,7 +551,7 @@ eintr: &lowat, sizeof(lowat))) { flow_trace(conn, "Setting SO_RCVLOWAT %i: %s", - lowat, strerror(errno)); + lowat, strerror_(errno)); } else { conn_flag(c, conn, lowat_set_flag); conn_flag(c, conn, lowat_act_flag); @@ -696,13 +696,13 @@ static int tcp_sock_refill_ns(void *arg) int rc = tcp_sock_refill_pool(c, ns_sock_pool4, AF_INET); if (rc < 0) warn("TCP: Error refilling IPv4 ns socket pool: %s", - strerror(-rc)); + strerror_(-rc)); } if (c->ifi6) { int rc = tcp_sock_refill_pool(c, ns_sock_pool6, AF_INET6); if (rc < 0) warn("TCP: Error refilling IPv6 ns socket pool: %s", - strerror(-rc)); + strerror_(-rc)); } return 0; diff --git a/udp.c b/udp.c index c89f031..923cc38 100644 --- a/udp.c +++ b/udp.c @@ -453,7 +453,7 @@ static int udp_sock_recverr(int s) /* TODO: When possible propagate and otherwise handle errors */ debug("%s error on UDP socket %i: %s", - str_ee_origin(ee), s, strerror(ee->ee_errno)); + str_ee_origin(ee), s, strerror_(ee->ee_errno)); return 1; } @@ -492,7 +492,7 @@ int udp_sock_errs(const struct ctx *c, int s, uint32_t events) } if (err) { - debug("Unqueued error on UDP socket %i: %s", s, strerror(err)); + debug("Unqueued error on UDP socket %i: %s", s, strerror_(err)); n_err++; } diff --git a/udp_flow.c b/udp_flow.c index c8fdb5f..343caae 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -95,7 +95,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, if (uflow->s[INISIDE] < 0) { flow_err(uflow, "Couldn't duplicate listening socket: %s", - strerror(errno)); + strerror_(errno)); goto cancel; } } @@ -115,14 +115,14 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, if (uflow->s[TGTSIDE] < 0) { flow_dbg(uflow, "Couldn't open socket for spliced flow: %s", - strerror(errno)); + strerror_(errno)); goto cancel; } if (flowside_connect(c, uflow->s[TGTSIDE], tgtpif, tgt) < 0) { flow_dbg(uflow, "Couldn't connect flow socket: %s", - strerror(errno)); + strerror_(errno)); goto cancel; } @@ -144,7 +144,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, } else if (errno != EAGAIN) { flow_err(uflow, "Unexpected error discarding datagrams: %s", - strerror(errno)); + strerror_(errno)); } } diff --git a/util.c b/util.c index 55cae3f..11973c4 100644 --- a/util.c +++ b/util.c @@ -90,7 +90,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, ret = -errno; if (fd < 0) { - warn("L4 socket: %s", strerror(-ret)); + warn("L4 socket: %s", strerror_(-ret)); return ret; } @@ -162,7 +162,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, if (type == EPOLL_TYPE_TCP_LISTEN && listen(fd, 128) < 0) { ret = -errno; - warn("TCP socket listen: %s", strerror(-ret)); + warn("TCP socket listen: %s", strerror_(-ret)); close(fd); return ret; } @@ -171,7 +171,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, ev.data.u64 = ref.u64; if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) { ret = -errno; - warn("L4 epoll_ctl: %s", strerror(-ret)); + warn("L4 epoll_ctl: %s", strerror_(-ret)); return ret; } diff --git a/util.h b/util.h index 41bbd60..3fa1d12 100644 --- a/util.h +++ b/util.h @@ -274,6 +274,38 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m) void raw_random(void *buf, size_t buflen); +/* + * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, + * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs + * getrandom(2) and brk(2) as it allocates memory for the locale-translated + * error description, but our seccomp profiles forbid both. + * + * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get + * a static untranslated string. It's a GNU implementation, but also defined by + * bionic. + * + * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries + * not defining strerrordesc_np() are expected to provide strerror() + * implementations that are simple enough for us to call. + */ +__attribute__ ((weak)) const char *strerrordesc_np(int errnum); + +/** + * strerror_() - strerror() wrapper calling strerrordesc_np() if available + * @errnum: Error code + * + * Return: error description string + */ +static inline const char *strerror_(int errnum) +{ + if (strerrordesc_np) + return strerrordesc_np(errnum); + + return strerror(errnum); +} + +#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead" + /* * Workarounds for https://github.com/llvm/llvm-project/issues/58992 * -- 2.43.0

3 2

[PATCH v5] pasta: make it possible to disable socket splicing
by Jon Maloy 11 Dec '24

11 Dec '24

During testing it is sometimes useful to force traffic which would normally be forwared by socket splicing through the tap interface. In this commit, we add a command switch enabling such funtionality for inbound local traffic. For outbound local traffic this is much trickier, if even possible, so leave that for a later commit. Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: Some minor changes based on feedback from PASST team v3: More changes based on feedback from D. Gibson and S. Brivio -Moved new option to pasta-only section -Added description to man-page v4: -Changed test on (mode == PASST) to (mode != PASTA) as suggested by Stefano Brivio. v5: -Updated text in man pages as suggested by Stefano Brivio. --- conf.c | 7 ++++++- fwd.c | 2 +- passt.1 | 5 +++++ passt.h | 2 ++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index eaa7d99..97d8beb 100644 --- a/conf.c +++ b/conf.c @@ -977,7 +977,8 @@ pasta_opts: " Don't copy all routes to namespace\n" " --no-copy-addrs DEPRECATED:\n" " Don't copy all addresses to namespace\n" - " --ns-mac-addr ADDR Set MAC address on tap interface\n"); + " --ns-mac-addr ADDR Set MAC address on tap interface\n" + " --no-splice Disable inbound socket splicing\n"); exit(status); } @@ -1319,6 +1320,7 @@ void conf(struct ctx *c, int argc, char **argv) {"no-dhcpv6", no_argument, &c->no_dhcpv6, 1 }, {"no-ndp", no_argument, &c->no_ndp, 1 }, {"no-ra", no_argument, &c->no_ra, 1 }, + {"no-splice", no_argument, &c->no_splice, 1 }, {"freebind", no_argument, &c->freebind, 1 }, {"no-map-gw", no_argument, &no_map_gw, 1 }, {"ipv4-only", no_argument, NULL, '4' }, @@ -1756,6 +1758,9 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); + if (c->mode != MODE_PASTA) + c->no_splice = 1; + if (c->mode == MODE_PASTA && !c->pasta_conf_ns) { if (copy_routes_opt) die("--no-copy-routes needs --config-net"); diff --git a/fwd.c b/fwd.c index 0b7f8b1..2829cd2 100644 --- a/fwd.c +++ b/fwd.c @@ -443,7 +443,7 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, else if (proto == IPPROTO_UDP) tgt->eport += c->udp.fwd_in.delta[tgt->eport]; - if (c->mode == MODE_PASTA && inany_is_loopback(&ini->eaddr) && + if (!c->no_splice && inany_is_loopback(&ini->eaddr) && (proto == IPPROTO_TCP || proto == IPPROTO_UDP)) { /* spliceable */ diff --git a/passt.1 b/passt.1 index b2896a2..d9cd33e 100644 --- a/passt.1 +++ b/passt.1 @@ -695,6 +695,11 @@ Configure MAC address \fIaddr\fR on the tap interface in the namespace. Default is to let the tap driver build a pseudorandom hardware address. +.TP +.BR \-\-no-splice +Disable the bypass path for inbound, local traffic. See the section \fBHandling +of local traffic in pasta\fR in the \fBNOTES\fR for more details. + .SH EXAMPLES .SS \fBpasta diff --git a/passt.h b/passt.h index c038630..0dd4efa 100644 --- a/passt.h +++ b/passt.h @@ -229,6 +229,7 @@ struct ip6_ctx { * @no_dhcpv6: Disable DHCPv6 server * @no_ndp: Disable NDP handler altogether * @no_ra: Disable router advertisements + * @no_splice: Disable socket splicing for inbound traffic * @host_lo_to_ns_lo: Map host loopback addresses to ns loopback addresses * @freebind: Allow binding of non-local addresses for forwarding * @low_wmem: Low probed net.core.wmem_max @@ -291,6 +292,7 @@ struct ctx { int no_dhcpv6; int no_ndp; int no_ra; + int no_splice; int host_lo_to_ns_lo; int freebind; -- 2.47.1

3 2

[PATCH 1/2] tap: Use a common function to start a new connection
by Laurent Vivier 10 Dec '24

10 Dec '24

Merge code from tap_backend_init(), tap_sock_tun_init() and tap_listen_handler() to set epoll_ref entry and to add it to epollfd. No functionality change Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- tap.c | 66 +++++++++++++++++++++++++++-------------------------------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/tap.c b/tap.c index c4180643bd0a..b2d30456e8dc 100644 --- a/tap.c +++ b/tap.c @@ -1255,6 +1255,33 @@ static void tap_sock_unix_init(const struct ctx *c) epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap_listen, &ev); } +/** + * tap_start_connection() - start a new connection + * @c: Execution context + */ +static void tap_start_connection(const struct ctx *c) +{ + struct epoll_event ev = { 0 }; + union epoll_ref ref = { 0 }; + + ref.fd = c->fd_tap; + switch (c->mode) { + case MODE_PASST: + ref.type = EPOLL_TYPE_TAP_PASST; + break; + case MODE_PASTA: + ref.type = EPOLL_TYPE_TAP_PASTA; + break; + case MODE_VU: + ref.type = EPOLL_TYPE_VHOST_CMD; + break; + } + + ev.events = EPOLLIN | EPOLLRDHUP; + ev.data.u64 = ref.u64; + epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); +} + /** * tap_listen_handler() - Handle new connection on listening socket * @c: Execution context @@ -1262,8 +1289,6 @@ static void tap_sock_unix_init(const struct ctx *c) */ void tap_listen_handler(struct ctx *c, uint32_t events) { - struct epoll_event ev = { 0 }; - union epoll_ref ref = { 0 }; int v = INT_MAX / 2; struct ucred ucred; socklen_t len; @@ -1302,14 +1327,7 @@ void tap_listen_handler(struct ctx *c, uint32_t events) setsockopt(c->fd_tap, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v))) trace("tap: failed to set SO_SNDBUF to %i", v); - ref.fd = c->fd_tap; - if (c->mode == MODE_VU) - ref.type = EPOLL_TYPE_VHOST_CMD; - else - ref.type = EPOLL_TYPE_TAP_PASST; - ev.events = EPOLLIN | EPOLLRDHUP; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); + tap_start_connection(c); } /** @@ -1353,19 +1371,13 @@ static int tap_ns_tun(void *arg) */ static void tap_sock_tun_init(struct ctx *c) { - union epoll_ref ref = { .type = EPOLL_TYPE_TAP_PASTA }; - struct epoll_event ev = { 0 }; - NS_CALL(tap_ns_tun, c); if (c->fd_tap == -1) die("Failed to set up tap device in namespace"); pasta_ns_conf(c); - ref.fd = c->fd_tap; - ev.events = EPOLLIN | EPOLLRDHUP; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); + tap_start_connection(c); } /** @@ -1399,26 +1411,8 @@ void tap_backend_init(struct ctx *c) tap_sock_update_pool(pkt_buf, sizeof(pkt_buf)); if (c->fd_tap != -1) { /* Passed as --fd */ - struct epoll_event ev = { 0 }; - union epoll_ref ref; - ASSERT(c->one_off); - ref.fd = c->fd_tap; - switch (c->mode) { - case MODE_PASST: - ref.type = EPOLL_TYPE_TAP_PASST; - break; - case MODE_PASTA: - ref.type = EPOLL_TYPE_TAP_PASTA; - break; - case MODE_VU: - ref.type = EPOLL_TYPE_VHOST_CMD; - break; - } - - ev.events = EPOLLIN | EPOLLRDHUP; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); + tap_start_connection(c); return; } -- 2.47.1

3 6

[PATCH v4] pasta: make it possible to disable socket splicing
by Jon Maloy 09 Dec '24

09 Dec '24

During testing it is sometimes useful to force traffic which would normally be forwared by socket splicing through the tap interface. In this commit, we add a command switch enabling such funtionality for inbound local traffic. For outbound local traffic this is much trickier, if even possible, so leave that for a later commit. Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: Some minor changes based on feedback from PASST team v3: More changes based on feedback from D. Gibson and S. Brivio -Moved new option to pasta-only section -Added description to man-page v4: -Changed test on (mode == PASST) to (mode != PASTA) as suggested by Stefano Brivio. --- conf.c | 7 ++++++- fwd.c | 2 +- passt.1 | 4 ++++ passt.h | 2 ++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index eaa7d99..97d8beb 100644 --- a/conf.c +++ b/conf.c @@ -977,7 +977,8 @@ pasta_opts: " Don't copy all routes to namespace\n" " --no-copy-addrs DEPRECATED:\n" " Don't copy all addresses to namespace\n" - " --ns-mac-addr ADDR Set MAC address on tap interface\n"); + " --ns-mac-addr ADDR Set MAC address on tap interface\n" + " --no-splice Disable inbound socket splicing\n"); exit(status); } @@ -1319,6 +1320,7 @@ void conf(struct ctx *c, int argc, char **argv) {"no-dhcpv6", no_argument, &c->no_dhcpv6, 1 }, {"no-ndp", no_argument, &c->no_ndp, 1 }, {"no-ra", no_argument, &c->no_ra, 1 }, + {"no-splice", no_argument, &c->no_splice, 1 }, {"freebind", no_argument, &c->freebind, 1 }, {"no-map-gw", no_argument, &no_map_gw, 1 }, {"ipv4-only", no_argument, NULL, '4' }, @@ -1756,6 +1758,9 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); + if (c->mode != MODE_PASTA) + c->no_splice = 1; + if (c->mode == MODE_PASTA && !c->pasta_conf_ns) { if (copy_routes_opt) die("--no-copy-routes needs --config-net"); diff --git a/fwd.c b/fwd.c index 0b7f8b1..2829cd2 100644 --- a/fwd.c +++ b/fwd.c @@ -443,7 +443,7 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, else if (proto == IPPROTO_UDP) tgt->eport += c->udp.fwd_in.delta[tgt->eport]; - if (c->mode == MODE_PASTA && inany_is_loopback(&ini->eaddr) && + if (!c->no_splice && inany_is_loopback(&ini->eaddr) && (proto == IPPROTO_TCP || proto == IPPROTO_UDP)) { /* spliceable */ diff --git a/passt.1 b/passt.1 index b2896a2..c8a5783 100644 --- a/passt.1 +++ b/passt.1 @@ -695,6 +695,10 @@ Configure MAC address \fIaddr\fR on the tap interface in the namespace. Default is to let the tap driver build a pseudorandom hardware address. +.TP +.BR \-\-no-splice +Disable socket splicing for host to NS traffic. + .SH EXAMPLES .SS \fBpasta diff --git a/passt.h b/passt.h index c038630..0dd4efa 100644 --- a/passt.h +++ b/passt.h @@ -229,6 +229,7 @@ struct ip6_ctx { * @no_dhcpv6: Disable DHCPv6 server * @no_ndp: Disable NDP handler altogether * @no_ra: Disable router advertisements + * @no_splice: Disable socket splicing for inbound traffic * @host_lo_to_ns_lo: Map host loopback addresses to ns loopback addresses * @freebind: Allow binding of non-local addresses for forwarding * @low_wmem: Low probed net.core.wmem_max @@ -291,6 +292,7 @@ struct ctx { int no_dhcpv6; int no_ndp; int no_ra; + int no_splice; int host_lo_to_ns_lo; int freebind; -- 2.45.2

3 2

[PATCH v3] pasta: make it possible to disable socket splicing
by Jon Maloy 05 Dec '24

05 Dec '24

During testing it is sometimes useful to force traffic which would normally be forwared by socket splicing through the tap interface. In this commit, we add a command switch enabling such funtionality for inbound local traffic. For outbound local traffic this is much trickier, if even possible, so leave that for a later commit. Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: Some minor changes based on feedback from PASST team v3: More changes based on feedback from D. Gibson and S. Brivio -Moved new option to pasta-only section -Added description to man-page --- conf.c | 7 ++++++- fwd.c | 2 +- passt.1 | 4 ++++ passt.h | 2 ++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index eaa7d99..53f6770 100644 --- a/conf.c +++ b/conf.c @@ -977,7 +977,8 @@ pasta_opts: " Don't copy all routes to namespace\n" " --no-copy-addrs DEPRECATED:\n" " Don't copy all addresses to namespace\n" - " --ns-mac-addr ADDR Set MAC address on tap interface\n"); + " --ns-mac-addr ADDR Set MAC address on tap interface\n" + " --no-splice Disable inbound socket splicing\n"); exit(status); } @@ -1319,6 +1320,7 @@ void conf(struct ctx *c, int argc, char **argv) {"no-dhcpv6", no_argument, &c->no_dhcpv6, 1 }, {"no-ndp", no_argument, &c->no_ndp, 1 }, {"no-ra", no_argument, &c->no_ra, 1 }, + {"no-splice", no_argument, &c->no_splice, 1 }, {"freebind", no_argument, &c->freebind, 1 }, {"no-map-gw", no_argument, &no_map_gw, 1 }, {"ipv4-only", no_argument, NULL, '4' }, @@ -1756,6 +1758,9 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); + if (c->mode == MODE_PASST) + c->no_splice = 1; + if (c->mode == MODE_PASTA && !c->pasta_conf_ns) { if (copy_routes_opt) die("--no-copy-routes needs --config-net"); diff --git a/fwd.c b/fwd.c index 0b7f8b1..2829cd2 100644 --- a/fwd.c +++ b/fwd.c @@ -443,7 +443,7 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, else if (proto == IPPROTO_UDP) tgt->eport += c->udp.fwd_in.delta[tgt->eport]; - if (c->mode == MODE_PASTA && inany_is_loopback(&ini->eaddr) && + if (!c->no_splice && inany_is_loopback(&ini->eaddr) && (proto == IPPROTO_TCP || proto == IPPROTO_UDP)) { /* spliceable */ diff --git a/passt.1 b/passt.1 index b2896a2..c8a5783 100644 --- a/passt.1 +++ b/passt.1 @@ -695,6 +695,10 @@ Configure MAC address \fIaddr\fR on the tap interface in the namespace. Default is to let the tap driver build a pseudorandom hardware address. +.TP +.BR \-\-no-splice +Disable socket splicing for host to NS traffic. + .SH EXAMPLES .SS \fBpasta diff --git a/passt.h b/passt.h index c038630..0dd4efa 100644 --- a/passt.h +++ b/passt.h @@ -229,6 +229,7 @@ struct ip6_ctx { * @no_dhcpv6: Disable DHCPv6 server * @no_ndp: Disable NDP handler altogether * @no_ra: Disable router advertisements + * @no_splice: Disable socket splicing for inbound traffic * @host_lo_to_ns_lo: Map host loopback addresses to ns loopback addresses * @freebind: Allow binding of non-local addresses for forwarding * @low_wmem: Low probed net.core.wmem_max @@ -291,6 +292,7 @@ struct ctx { int no_dhcpv6; int no_ndp; int no_ra; + int no_splice; int host_lo_to_ns_lo; int freebind; -- 2.45.2

3 4

[PATCH v2] udp_vu: update segment size
by Laurent Vivier 05 Dec '24

05 Dec '24

In udp_vu_sock_recv(), collect a segment with a size defined to IP_MAX_MTU + ETH_HLEN + sizeof(struct virtio_net_hdr_mrg_rxbuf) The original version double counted the IP header: IP_MAX_MTU includes the IP header, and so did hdrlen. Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- udp_vu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/udp_vu.c b/udp_vu.c index 9c697f3be247..41235101b69e 100644 --- a/udp_vu.c +++ b/udp_vu.c @@ -104,7 +104,8 @@ static int udp_vu_sock_recv(const struct ctx *c, int s, uint32_t events, vu_init_elem(elem, iov_vu, VIRTQUEUE_MAX_SIZE); iov_cnt = vu_collect(vdev, vq, elem, VIRTQUEUE_MAX_SIZE, - IP_MAX_MTU - sizeof(struct udphdr) + hdrlen, + IP_MAX_MTU + ETH_HLEN + + sizeof(struct virtio_net_hdr_mrg_rxbuf), NULL); if (iov_cnt == 0) return 0; -- 2.47.1

2 1

[PATCH 0/2] Fix handling of ICMP flows with 0 id
by David Gibson 05 Dec '24

05 Dec '24

Well, that's the impetus, but it's done as a more general changing of where we sanity check flow endpoints. Link: https://bugs.passt.top/show_bug.cgi?id=105 David Gibson (2): udp: Improve detail of UDP endpoint sanity checking flow: Remove over-zealous sanity checks in flow_sidx_hash() flow.c | 7 +------ udp_flow.c | 32 ++++++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 14 deletions(-) -- 2.47.0

2 3

[PATCH] udp_vu: update segment size
by Laurent Vivier 05 Dec '24

05 Dec '24

In udp_vu_sock_recv(), collect a segment with a size defined to IP_MAX_MTU + ETH_HLEN + sizeof(struct virtio_net_hdr_mrg_rxbuf) Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- udp_vu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/udp_vu.c b/udp_vu.c index 9c697f3be247..41235101b69e 100644 --- a/udp_vu.c +++ b/udp_vu.c @@ -104,7 +104,8 @@ static int udp_vu_sock_recv(const struct ctx *c, int s, uint32_t events, vu_init_elem(elem, iov_vu, VIRTQUEUE_MAX_SIZE); iov_cnt = vu_collect(vdev, vq, elem, VIRTQUEUE_MAX_SIZE, - IP_MAX_MTU - sizeof(struct udphdr) + hdrlen, + IP_MAX_MTU + ETH_HLEN + + sizeof(struct virtio_net_hdr_mrg_rxbuf), NULL); if (iov_cnt == 0) return 0; -- 2.47.0

3 2

[PATCH v14 0/9] Add vhost-user support to passt. (part 3)
by Laurent Vivier 03 Dec '24

03 Dec '24

This series of patches adds vhost-user support to passt and then allows passt to connect to QEMU network backend using virtqueue rather than a socket. With QEMU, rather than using to connect: -netdev stream,id=s,server=off,addr.type=unix,addr.path=/tmp/passt_1.socket we will use: -chardev socket,id=chr0,path=/tmp/passt_1.socket -netdev vhost-user,id=netdev0,chardev=chr0 -device virtio-net,netdev=netdev0 -object memory-backend-memfd,id=memfd0,share=on,size=$RAMSIZE -numa node,memdev=memfd0 The memory backend is needed to share data between passt and QEMU. Performance comparison between "-netdev stream" and "-netdev vhost-user": $ iperf3 -c localhost -p 10001 -t 60 -6 -u -b 50G socket: [ 5] 0.00-60.05 sec 95.6 GBytes 13.7 Gbits/sec 0.017 ms 6998988/10132413 (69%) receiver vhost-user: [ 5] 0.00-60.04 sec 237 GBytes 33.9 Gbits/sec 0.006 ms 53673/7813770 (0.69%) receiver $ iperf3 -c localhost -p 10001 -t 60 -4 -u -b 50G socket: [ 5] 0.00-60.05 sec 98.9 GBytes 14.1 Gbits/sec 0.018 ms 6260735/9501832 (66%) receiver vhost-user: [ 5] 0.00-60.05 sec 235 GBytes 33.7 Gbits/sec 0.008 ms 37581/7752699 (0.48%) receiver $ iperf3 -c localhost -p 10001 -t 60 -6 socket: [ 5] 0.00-60.00 sec 17.3 GBytes 2.48 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 17.3 GBytes 2.48 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 191 GBytes 27.4 Gbits/sec 0 sender [ 5] 0.00-60.05 sec 191 GBytes 27.3 Gbits/sec receiver $ iperf3 -c localhost -p 10001 -t 60 -4 socket: [ 5] 0.00-60.00 sec 15.6 GBytes 2.24 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 15.6 GBytes 2.24 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 189 GBytes 27.1 Gbits/sec 0 sender [ 5] 0.00-60.04 sec 189 GBytes 27.0 Gbits/sec receiver v14: - merge "tcp_vu: Share more header construction between IPv4 and IPv6 paths" into "vhost-user: add vhost-user" to simplify rework - address comments from David - in tcp_vu.c, use an array to point to the iovecs that contain headers v13: - fix TCP big file transfer test with SO_PEEK_OFF v12: - rebase - address comments from Stefano v11: - rebase - address comments from David on v10 v10: - rebase v9 on top of my changes - remove last 4 patches from v9 as 'tcp: Pass TCP header and payload separately to tcp_fill_headers[46]()' introduces a regression in "make check" for me. - addressed comments from David - I tried to cleanup iov management, but I was not able to remove the update of the first iov to point to the header or to the data - upd_vu_hdrlen()/tcp_vu_hdrlen() includes now the size of the virtio-net header - I didn't address comments from Stefano. - I didn't fix the bug with seek_offset_cap v9: [David Gibson] - Rebased on current main - Conflicts with v4/v6 buffer merge addressed - Conflicts with TCP options construction rework addressed - Added several cleanup patches on top - Added a number of IOV and buffer cleanups on top - The aim is that these should allow more sharing of logic between the vhost-user and non-vhost-user pathes, although they've only minimally accomplished that so far. v8: - remove iov_size() from vu_collect_one_frame() - move vu_packet_check_range() to vu_common.c - fix UDP when dlen is 0. v7: - rebase - use vu_collect_one_frame() to do vu_collect() (collect multiple frame) - add vhost-user tests from Stefano v6: - rebase - extract 3 patches from "vhost-user: add vhost-user": passt: rename tap_sock_init() to tap_backend_init() tcp: Export headers functions udp: Prepare udp.c to be shared with vhost-user - introduce new functions vu_collect_one_frame(), vu_collect(), vu_set_vnethdr(), vu_flush(), vu_send_single() to be called from tcp_vu.c, udp_vu.c and ICMP/DHCP where vhost-user code was duplicated. v5: - rebase on top of 2024_09_06.6b38f07 - rework udp_vu.c as ref.udp.v6 has been removed and we need to know if we receive IPv4 or IPv6 frame when we prepare the guest buffers for recvmsg() - remove vnet->hdrlen as the size is always the same with virtio-net v1 - address comments from David and Stefano v4: - rebase on top of 2024_08_21.1d6142f (rebasing on top of 620e19a1b48a ("udp: Merge udp[46]_mh_recv arrays") introduces a regression in the measure of the latency with UDP because I think I don't replace correctly ref.udp.v6 that is removed by this commit) - Addressed most of the comments from David and Stefano (I didn't want to postpone this version to next week, so I'll address the remaining comments in the next version). v3: - rebase on top of flow table - update tcp_vu.c to look like udp_vu.c (recv()/prepare()/send_frame()) - address comments from Stefano and David on version 2 v2: - remove PATCH 4 - rewrite PATCH 2 and 3 to follow passt coding style - move some code from PATCH 3 to PATCH 4 (previously PATCH 5) - partially addressed David's comment on PATCH 5 David Gibson (1): tcp: Move tcp_l2_buf_fill_headers() to tcp_buf.c Laurent Vivier (7): packet: replace struct desc by struct iovec vhost-user: introduce virtio API vhost-user: introduce vhost-user API udp: Prepare udp.c to be shared with vhost-user tcp: Export headers functions passt: rename tap_sock_init() to tap_backend_init() vhost-user: add vhost-user Stefano Brivio (1): test: Add tests for passt in vhost-user mode Makefile | 9 +- conf.c | 19 +- epoll_type.h | 4 + iov.c | 1 - isolation.c | 17 +- packet.c | 91 ++-- packet.h | 22 +- passt.1 | 10 +- passt.c | 11 +- passt.h | 7 + pcap.c | 1 - tap.c | 129 ++++-- tap.h | 7 +- tcp.c | 76 +--- tcp_buf.c | 39 +- tcp_internal.h | 19 +- tcp_vu.c | 493 +++++++++++++++++++++ tcp_vu.h | 12 + test/lib/perf_report | 15 + test/lib/setup | 77 +++- test/lib/setup_ugly | 2 +- test/passt_vu | 1 + test/passt_vu_in_ns | 1 + test/perf/passt_vu_tcp | 211 +++++++++ test/perf/passt_vu_udp | 159 +++++++ test/run | 25 ++ test/two_guests_vu | 1 + udp.c | 85 ++-- udp_internal.h | 34 ++ udp_vu.c | 343 ++++++++++++++ udp_vu.h | 13 + util.h | 9 + vhost_user.c | 981 +++++++++++++++++++++++++++++++++++++++++ vhost_user.h | 206 +++++++++ virtio.c | 645 +++++++++++++++++++++++++++ virtio.h | 184 ++++++++ vu_common.c | 282 ++++++++++++ vu_common.h | 60 +++ 38 files changed, 4099 insertions(+), 202 deletions(-) create mode 100644 tcp_vu.c create mode 100644 tcp_vu.h create mode 120000 test/passt_vu create mode 120000 test/passt_vu_in_ns create mode 100644 test/perf/passt_vu_tcp create mode 100644 test/perf/passt_vu_udp create mode 120000 test/two_guests_vu create mode 100644 udp_internal.h create mode 100644 udp_vu.c create mode 100644 udp_vu.h create mode 100644 vhost_user.c create mode 100644 vhost_user.h create mode 100644 virtio.c create mode 100644 virtio.h create mode 100644 vu_common.c create mode 100644 vu_common.h -- 2.47.0

3 26