February 2024 - dev

tcp: leverage support of SO_PEEK_OFF socket option when available
by Jon Maloy 05 Feb '24

05 Feb '24

The kernel may support recvmsg(MSG_PEEK), starting reading data from a given offset set by the SO_PEEK_OFF socket option. This makes it possible to avoid repeated reading of already read initial bytes of a received message, hence saving read cycles when forwarding TCP messages in the host->name space direction. In this commit, we add functionality to leverage this feature when available, while we fall back to the previous behavior when not. Measurements with iperf3 shows that throughput increases with 15-20 percent in the host->namespace direction when this feature is used. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tcp.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/tcp.c b/tcp.c index 905d26f..58674eb 100644 --- a/tcp.c +++ b/tcp.c @@ -505,6 +505,7 @@ static struct tcp_buf_seq_update tcp6_l2_buf_seq_update[TCP_FRAMES_MEM]; static unsigned int tcp6_l2_buf_used; /* recvmsg()/sendmsg() data for tap */ +static bool peek_offset_cap = false; static char tcp_buf_discard [MAX_WINDOW]; static struct iovec iov_sock [TCP_FRAMES_MEM + 1]; @@ -582,6 +583,14 @@ static_assert(ARRAY_SIZE(tc_hash) >= FLOW_MAX, int init_sock_pool4 [TCP_SOCK_POOL_SIZE]; int init_sock_pool6 [TCP_SOCK_POOL_SIZE]; +static void set_peek_offset(int s, int offset) +{ + if (!peek_offset_cap) + return; + if (setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &offset, sizeof(offset))) + perror("Failed to set SO_PEEK_OFF\n"); +} + /** * tcp_conn_epoll_events() - epoll events mask for given connection state * @events: Current connection events @@ -1951,7 +1960,7 @@ static void tcp_conn_from_tap(struct ctx *c, if (bind(s, (struct sockaddr *)&addr6_ll, sizeof(addr6_ll))) goto cancel; } - + set_peek_offset(s, 0); conn = &flow->tcp; conn->f.type = FLOW_TCP; conn->sock = s; @@ -2174,6 +2183,12 @@ static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn) if (iov_rem) iov_sock[fill_bufs].iov_len = iov_rem; + /* Don't use discard buffer if SO_PEEK_OFF is supported. */ + if (peek_offset_cap) { + mh_sock.msg_iov = &iov_sock[1]; + mh_sock.msg_iovlen -= 1; + } + /* Receive into buffers, don't dequeue until acknowledged by guest. */ do len = recvmsg(s, &mh_sock, MSG_PEEK); @@ -2195,7 +2210,9 @@ static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn) return 0; } - sendlen = len - already_sent; + sendlen = len; + if (!peek_offset_cap) + sendlen -= already_sent; if (sendlen <= 0) { conn_flag(c, conn, STALLED); return 0; @@ -2367,6 +2384,7 @@ static int tcp_data_from_tap(struct ctx *c, struct tcp_tap_conn *conn, max_ack_seq, conn->seq_to_tap); conn->seq_ack_from_tap = max_ack_seq; conn->seq_to_tap = max_ack_seq; + set_peek_offset(conn->sock, 0); tcp_data_from_sock(c, conn); } @@ -2718,6 +2736,7 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref, tcp_splice_conn_from_sock(c, ref.tcp_listen, &flow->tcp_splice, s, (struct sockaddr *)&sa)) return; + set_peek_offset(s, 0); tcp_tap_conn_from_sock(c, ref.tcp_listen, &flow->tcp, s, (struct sockaddr *)&sa, now); @@ -3042,6 +3061,7 @@ static void tcp_sock_refill_init(const struct ctx *c) int tcp_init(struct ctx *c) { unsigned b; + int s; for (b = 0; b < TCP_HASH_TABLE_SIZE; b++) tc_hash[b] = FLOW_SIDX_NONE; @@ -3065,6 +3085,17 @@ int tcp_init(struct ctx *c) NS_CALL(tcp_ns_socks_init, c); } + /* Probe for SO_PEEK_OFF support */ + s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + if (s < 0) { + perror("Temporary tcp socket creation failed\n"); + } else { + if (!setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &(int){0}, sizeof(int))) { + peek_offset_cap = true; + } + close(s); + } + printf("SO_PEEK_OFF%ssupported\n", peek_offset_cap ? " " : " not "); return 0; } -- 2.42.0

2 1

[PATCH] netlink: Add support to fetch default gateway from multipath routes
by Stefano Brivio 05 Feb '24

05 Feb '24

If the default route for a given IP version is a multipath one, instead of refusing to start because there's no RTA_GATEWAY attribute in the set returned by the kernel, we can just pick one of the paths. To make this somewhat less arbitrary, pick the path with the highest weight, if weights differ. Reported-by: Ed Santiago <santiago(a)redhat.com> Link: https://github.com/containers/podman/issues/20927 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- netlink.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++--- passt.1 | 6 ++++-- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/netlink.c b/netlink.c index bf79dd4..f0b04cb 100644 --- a/netlink.c +++ b/netlink.c @@ -271,18 +271,60 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) for (rta = RTM_RTA(rtm), na = RTM_PAYLOAD(nh); RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) { - if (rta->rta_type != RTA_OIF) - continue; + if (rta->rta_type == RTA_OIF) { + ifi = *(unsigned int *)RTA_DATA(rta); + } else if (rta->rta_type == RTA_MULTIPATH) { + struct rtnexthop *rtnh; - ifi = *(unsigned int *)RTA_DATA(rta); + rtnh = (struct rtnexthop *)RTA_DATA(rta); + ifi = rtnh->rtnh_ifindex; + } } } + if (status < 0) warn("netlink: RTM_GETROUTE failed: %s", strerror(-status)); return ifi; } +/** + * nl_route_get_def_multipath() - Get lowest-weight route from nexthop list + * @rta: Routing netlink attribute with type RTA_MULTIPATH + * @gw: Default gateway to fill + * + * Return: true if a gateway was found, false otherwise + */ +bool nl_route_get_def_multipath(struct rtattr *rta, void *gw) +{ + struct rtnexthop *rtnh; + bool found = false; + int hops = -1; + + for (rtnh = (struct rtnexthop *)RTA_DATA(rta); + RTNH_OK(rtnh, RTA_PAYLOAD(rta)); rtnh = RTNH_NEXT(rtnh)) { + size_t len = rtnh->rtnh_len - sizeof(*rtnh); + struct rtattr *rta_inner; + + if (rtnh->rtnh_hops < hops) + continue; + + hops = rtnh->rtnh_hops; + + for (rta_inner = RTNH_DATA(rtnh); RTA_OK(rta_inner, len); + rta_inner = RTA_NEXT(rta_inner, len)) { + + if (rta_inner->rta_type != RTA_GATEWAY) + continue; + + memcpy(gw, RTA_DATA(rta_inner), RTA_PAYLOAD(rta_inner)); + found = true; + } + } + + return found; +} + /** * nl_route_get_def() - Get default route for given interface and address family * @s: Netlink socket @@ -326,6 +368,9 @@ int nl_route_get_def(int s, unsigned int ifi, sa_family_t af, void *gw) for (rta = RTM_RTA(rtm), na = RTM_PAYLOAD(nh); RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) { + if (rta->rta_type == RTA_MULTIPATH) + found = nl_route_get_def_multipath(rta, gw); + if (rta->rta_type != RTA_GATEWAY) continue; diff --git a/passt.1 b/passt.1 index efd6bb7..cc678ed 100644 --- a/passt.1 +++ b/passt.1 @@ -171,8 +171,10 @@ Assign IPv4 \fIaddr\fR as default gateway via DHCP (option 3), or IPv6 \fIaddr\fR as source for NDP Router Advertisement and DHCPv6 messages. This option can be specified zero (for defaults) to two times (once for IPv4, once for IPv6). -By default, IPv4 and IPv6 addresses are taken from the host interface with the -first default route for the corresponding IP version. +By default, IPv4 and IPv6 gateways are taken from the host interface with the +first default route for the corresponding IP version. If the default route is a +multipath one, the gateway is the first nexthop router returned by the kernel +which has the highest weight in the set of paths. Note: these addresses are also used as source address for packets directed to the guest or to the target namespace having a loopback or local source address, -- 2.39.2

2 1

[PATCH v2] log: setlogmask(0) can actually result in a system call, don't use it
by Stefano Brivio 05 Feb '24

05 Feb '24

Before commit 32d07f5e59f2 ("passt, pasta: Completely avoid dynamic memory allocation"), we didn't store the current log mask in a variable, and we fetched it using setlogmask(0) wherever needed. But after that commit, we can use our log_mask copy instead. And we should: with recent glibc versions, setlogmask(0) actually results in a system call, which causes a substantial overhead with high transfer rates: we use setlogmask(0) even to decide we don't want to print debug messages. Now that we rely on log_mask in early stages, before setlogmask() is called, we need to initialise that variable to the special LOG_EMERG mask value right away: define LOG_EARLY to make this clearer, and, while at it, group conditions in vlogmsg() into something more terse. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Initialise log_mask so that prints before the first __setlogmask() call still work. log.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/log.c b/log.c index 4a70e29..eafaca2 100644 --- a/log.c +++ b/log.c @@ -30,9 +30,12 @@ #include "util.h" #include "passt.h" +/* LOG_EARLY means we don't know yet: log everything. LOG_EMERG is unused */ +#define LOG_EARLY LOG_MASK(LOG_EMERG) + static int log_sock = -1; /* Optional socket to system logger */ static char log_ident[BUFSIZ]; /* Identifier string for openlog() */ -static int log_mask; /* Current log priority mask */ +static int log_mask = LOG_EARLY; /* Current log priority mask */ static int log_opt; /* Options for openlog() */ static int log_file = -1; /* Optional log file descriptor */ @@ -45,34 +48,33 @@ static time_t log_start; /* Start timestamp */ int log_trace; /* --trace mode enabled */ int log_to_stdout; /* Print to stdout instead of stderr */ -#define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) - void vlogmsg(int pri, const char *format, va_list ap) { + bool debug_print = (log_mask & LOG_MASK(LOG_DEBUG)) && log_file == -1; + bool early_print = LOG_PRI(log_mask) == LOG_EARLY; FILE *out = log_to_stdout ? stdout : stderr; struct timespec tp; - if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { + if (debug_print) { clock_gettime(CLOCK_REALTIME, &tp); fprintf(out, "%lli.%04lli: ", (long long int)tp.tv_sec - log_start, (long long int)tp.tv_nsec / (100L * 1000)); } - if ((LOG_MASK(LOG_PRI(pri)) & log_mask) || BEFORE_DAEMON) { + if ((log_mask & LOG_MASK(LOG_PRI(pri))) || early_print) { va_list ap2; va_copy(ap2, ap); /* Don't clobber ap, we need it again */ if (log_file != -1) logfile_write(pri, format, ap2); - else if (!(setlogmask(0) & LOG_MASK(LOG_DEBUG))) + else if (!(log_mask & LOG_MASK(LOG_DEBUG))) passt_vsyslog(pri, format, ap2); va_end(ap2); } - if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) || - (BEFORE_DAEMON && !(log_opt & LOG_PERROR))) { + if (debug_print || (early_print && !(log_opt & LOG_PERROR))) { (void)vfprintf(out, format, ap); if (format[strlen(format)] != '\n') fprintf(out, "\n"); -- 2.39.2

2 1

[PATCH] log: setlogmask(0) can actually result in a system call, don't use it
by Stefano Brivio 03 Feb '24

03 Feb '24

Before commit 32d07f5e59f2 ("passt, pasta: Completely avoid dynamic memory allocation"), we didn't store the current log mask in a variable, and we fetched it using setlogmask(0) wherever needed. But after that commit, we can just use our log_mask copy instead. And we should: with recent glibc versions, setlogmask(0) actually results in a system call, which causes a substantial overhead with high transfer rates: we use setlogmask(0) even to decide we don't want to print debug messages. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- log.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/log.c b/log.c index 4a70e29..a66ecb7 100644 --- a/log.c +++ b/log.c @@ -45,33 +45,33 @@ static time_t log_start; /* Start timestamp */ int log_trace; /* --trace mode enabled */ int log_to_stdout; /* Print to stdout instead of stderr */ -#define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) +#define BEFORE_DAEMON (log_mask == LOG_MASK(LOG_EMERG)) void vlogmsg(int pri, const char *format, va_list ap) { FILE *out = log_to_stdout ? stdout : stderr; struct timespec tp; - if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { + if (log_mask & LOG_MASK(LOG_DEBUG) && log_file == -1) { clock_gettime(CLOCK_REALTIME, &tp); fprintf(out, "%lli.%04lli: ", (long long int)tp.tv_sec - log_start, (long long int)tp.tv_nsec / (100L * 1000)); } - if ((LOG_MASK(LOG_PRI(pri)) & log_mask) || BEFORE_DAEMON) { + if ((log_mask & LOG_MASK(LOG_PRI(pri))) || BEFORE_DAEMON) { va_list ap2; va_copy(ap2, ap); /* Don't clobber ap, we need it again */ if (log_file != -1) logfile_write(pri, format, ap2); - else if (!(setlogmask(0) & LOG_MASK(LOG_DEBUG))) + else if (!(log_mask & LOG_MASK(LOG_DEBUG))) passt_vsyslog(pri, format, ap2); va_end(ap2); } - if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) || + if ((log_mask & LOG_MASK(LOG_DEBUG) && log_file == -1) || (BEFORE_DAEMON && !(log_opt & LOG_PERROR))) { (void)vfprintf(out, format, ap); if (format[strlen(format)] != '\n') -- 2.39.2

2 2