February 2025 - dev

[PATCH] tap: always set the no_frag flag in IPv4 headers.
by Jon Maloy 16 Feb '25

16 Feb '25

When studying the Linux source code and Wireshark dumps it seems like the no_frag flag in the IPv4 header is always set. Following discussions in the Internet on this subject indicates that modern routers never fragment packets, and that it isn't even supported in many cases. Adding to this that incoming messages forwarded on the tap interface never even pass through a router it seems safe to always set this flag. This makes the IPv4 headers of forwarded messages identical to those sent by the external sockets, something we must consider desirable. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tap.c b/tap.c index d0673e5..44b0fc0 100644 --- a/tap.c +++ b/tap.c @@ -153,7 +153,7 @@ static void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src, ip4h->tos = 0; ip4h->tot_len = htons(l3len); ip4h->id = 0; - ip4h->frag_off = 0; + ip4h->frag_off = htons(IP_DF); ip4h->ttl = 255; ip4h->protocol = proto; ip4h->saddr = src.s_addr; -- 2.48.1

2 4

[PATCH v2] repair, passt-repair: Build and warning fixes for musl
by Stefano Brivio 15 Feb '25

15 Feb '25

Checked against musl 1.2.5. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Now 100% more tested: don't try to use the whole buffer as msg_controllen. passt-repair.c | 4 +++- repair.c | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/passt-repair.c b/passt-repair.c index 1174ae3..e0c366e 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -63,6 +63,7 @@ int main(int argc, char **argv) struct cmsghdr *cmsg; struct msghdr msg; struct iovec iov; + size_t cmsg_len; int op; prctl(PR_SET_DUMPABLE, 0); @@ -138,8 +139,9 @@ loop: } } if (!n) { + cmsg_len = cmsg->cmsg_len; /* socklen_t is 'unsigned' on musl */ fprintf(stderr, "Invalid ancillary data length %zu from peer\n", - cmsg->cmsg_len); + cmsg_len); _exit(1); } diff --git a/repair.c b/repair.c index d288617..1d26fe6 100644 --- a/repair.c +++ b/repair.c @@ -13,6 +13,7 @@ */ #include <errno.h> +#include <sys/socket.h> #include <sys/uio.h> #include "util.h" @@ -155,8 +156,12 @@ int repair_flush(struct ctx *c) if (!repair_nfds) return 0; - msg = (struct msghdr){ NULL, 0, &iov, 1, - buf, CMSG_SPACE(sizeof(int) * repair_nfds), 0 }; + msg = (struct msghdr){ .msg_name = NULL, .msg_namelen = 0, + .msg_iov = &iov, .msg_iovlen = 1, + .msg_control = buf, + .msg_controllen = CMSG_SPACE(sizeof(int) * + repair_nfds), + .msg_flags = 0 }; cmsg = CMSG_FIRSTHDR(&msg); cmsg->cmsg_level = SOL_SOCKET; -- 2.43.0

1 0

[PATCH v25 0/2] State migration, somewhat polished
by Stefano Brivio 15 Feb '25

15 Feb '25

Notable changes compared to v24: - dropped thaw/freeze mechanism, as duplicate keep-alives gave me issues with migration of half-closed connections and in general they proved to be too rigid to quickly fix things as I needed to do. Kept all the fixes, though - fixed error handling on target, e.g. clearing flags, calling flow_cancel() as needed, etc. - added tests for half-closed sockets, fixed headers and descriptions - fix build with musl (1.25), and warnings on i686, armv6l, cppcheck and clang-tidy reports - drop "queue plugging" mechanism: it turns out that the kernel actually generates ACKs from the padding we send to our own receive queue, even in repair mode (!) All tests are happy. This looks quite solid and mergeable to me as it is. Stefano Brivio (2): migrate: Migrate TCP flows test: Add migration tests contrib/selinux/passt.te | 4 +- flow.c | 243 ++++++++++ flow.h | 8 + migrate.c | 10 + passt.c | 6 +- repair.c | 1 - tcp.c | 919 ++++++++++++++++++++++++++++++++++++ tcp_conn.h | 103 ++++ test/lib/layout | 55 ++- test/lib/setup | 138 +++++- test/lib/test | 48 ++ test/migrate/basic | 59 +++ test/migrate/bidirectional | 64 +++ test/migrate/iperf3_bidir6 | 58 +++ test/migrate/iperf3_in4 | 50 ++ test/migrate/iperf3_in6 | 58 +++ test/migrate/iperf3_out4 | 47 ++ test/migrate/iperf3_out6 | 58 +++ test/migrate/rampstream_in | 12 +- test/migrate/rampstream_out | 8 +- test/run | 34 +- 21 files changed, 1964 insertions(+), 19 deletions(-) create mode 100644 test/migrate/basic create mode 100644 test/migrate/bidirectional create mode 100644 test/migrate/iperf3_bidir6 create mode 100644 test/migrate/iperf3_in4 create mode 100644 test/migrate/iperf3_in6 create mode 100644 test/migrate/iperf3_out4 create mode 100644 test/migrate/iperf3_out6 -- 2.43.0

1 2

[PATCH] repair, passt-repair: Build and warning fixes for musl
by Stefano Brivio 15 Feb '25

15 Feb '25

Checked against musl 1.2.5. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt-repair.c | 4 +++- repair.c | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/passt-repair.c b/passt-repair.c index 1174ae3..e0c366e 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -63,6 +63,7 @@ int main(int argc, char **argv) struct cmsghdr *cmsg; struct msghdr msg; struct iovec iov; + size_t cmsg_len; int op; prctl(PR_SET_DUMPABLE, 0); @@ -138,8 +139,9 @@ loop: } } if (!n) { + cmsg_len = cmsg->cmsg_len; /* socklen_t is 'unsigned' on musl */ fprintf(stderr, "Invalid ancillary data length %zu from peer\n", - cmsg->cmsg_len); + cmsg_len); _exit(1); } diff --git a/repair.c b/repair.c index d288617..64f18b7 100644 --- a/repair.c +++ b/repair.c @@ -13,6 +13,7 @@ */ #include <errno.h> +#include <sys/socket.h> #include <sys/uio.h> #include "util.h" @@ -155,8 +156,11 @@ int repair_flush(struct ctx *c) if (!repair_nfds) return 0; - msg = (struct msghdr){ NULL, 0, &iov, 1, - buf, CMSG_SPACE(sizeof(int) * repair_nfds), 0 }; + msg = (struct msghdr){ .msg_name = NULL, .msg_namelen = 0, + .msg_iov = &iov, .msg_iovlen = 1, + .msg_control = buf, + .msg_controllen = sizeof(buf), + .msg_flags = 0 }; cmsg = CMSG_FIRSTHDR(&msg); cmsg->cmsg_level = SOL_SOCKET; -- 2.43.0

1 0

[PATCH v24 0/5] State migration
by David Gibson 15 Feb '25

15 Feb '25

Rebased / integrated Stefano's patch for allowing us to carry on by sacrificing some flows in more cases. David Gibson (3): vhost_user: Clear ring address on GET_VRING_BASE flow: Flow table traversing macros flow, migrate: Flow migration skeleton Stefano Brivio (2): migrate: Migrate TCP flows test: Add migration tests contrib/selinux/passt.te | 4 +- flow.c | 335 ++++++++++++- flow.h | 13 + flow_table.h | 54 ++ migrate.c | 61 ++- migrate.h | 2 + passt.c | 6 +- repair.c | 1 - tcp.c | 950 ++++++++++++++++++++++++++++++++++++ tcp_conn.h | 99 ++++ test/lib/layout | 55 ++- test/lib/setup | 138 +++++- test/lib/test | 48 ++ test/migrate/basic | 59 +++ test/migrate/bidirectional | 64 +++ test/migrate/iperf3_bidir6 | 58 +++ test/migrate/iperf3_in4 | 50 ++ test/migrate/iperf3_in6 | 58 +++ test/migrate/iperf3_out4 | 50 ++ test/migrate/iperf3_out6 | 58 +++ test/migrate/rampstream_in | 10 +- test/migrate/rampstream_out | 6 +- test/run | 34 +- vhost_user.c | 1 + 24 files changed, 2176 insertions(+), 38 deletions(-) create mode 100644 test/migrate/basic create mode 100644 test/migrate/bidirectional create mode 100644 test/migrate/iperf3_bidir6 create mode 100644 test/migrate/iperf3_in4 create mode 100644 test/migrate/iperf3_in6 create mode 100644 test/migrate/iperf3_out4 create mode 100644 test/migrate/iperf3_out6 -- 2.48.1

2 6

[PATCH v23 0/4] State migration
by David Gibson 14 Feb '25

14 Feb '25

Brain dead today, didn't make much progress on bugs. In particular migrate/iperf3_bidir6 still fails for me with an EAGAIN trying to restore the sndq (the sent but not acked part, in repair mode). I did, however, do a substantial restructure of how we handle flow migration, which should make it easier to tweak our "pre" and "post" steps for each flow. I *think* disabling/re-enabling nonblocking mode on our sockets in the new freeze/thaw handlers will fix the EAGAIN problem, but I ran out of time before trying it. David Gibson (2): flow: Flow table traversing macros flow, migrate: Flow migration skeleton Stefano Brivio (2): migrate: Migrate TCP flows test: Add migration tests contrib/selinux/passt.te | 4 +- flow.c | 309 +++++++++++- flow.h | 13 + flow_table.h | 52 ++ migrate.c | 47 +- migrate.h | 2 + passt.c | 6 +- repair.c | 1 - tcp.c | 918 ++++++++++++++++++++++++++++++++++++ tcp_conn.h | 99 ++++ test/lib/layout | 55 ++- test/lib/setup | 138 +++++- test/lib/test | 48 ++ test/migrate/basic | 59 +++ test/migrate/bidirectional | 64 +++ test/migrate/iperf3_bidir6 | 58 +++ test/migrate/iperf3_in4 | 50 ++ test/migrate/iperf3_in6 | 58 +++ test/migrate/iperf3_out4 | 50 ++ test/migrate/iperf3_out6 | 58 +++ test/migrate/rampstream_in | 10 +- test/migrate/rampstream_out | 6 +- test/run | 34 +- 23 files changed, 2108 insertions(+), 31 deletions(-) create mode 100644 test/migrate/basic create mode 100644 test/migrate/bidirectional create mode 100644 test/migrate/iperf3_bidir6 create mode 100644 test/migrate/iperf3_in4 create mode 100644 test/migrate/iperf3_in6 create mode 100644 test/migrate/iperf3_out4 create mode 100644 test/migrate/iperf3_out6 -- 2.48.1

1 4

[PATCH] tcp, tcp_splice: Don't set SO_SNDBUF and SO_RCVBUF to maximum values
by Stefano Brivio 14 Feb '25

14 Feb '25

I added this a long long time ago because it dramatically improved throughput back then: with rmem_max and wmem_max >= 4 MiB, we would force send and receive buffer sizes for TCP sockets to the maximum allowed value. This effectively disables TCP auto-tuning, which would otherwise allow us to exceed those limits, as crazy as it might sound. But in any case, it made sense. Now that we have zero (internal) copies on every path, plus vhost-user support, it turns out that these settings are entirely obsolete. I get substantially the same throughput in every test we perform, even with very short durations (one second). The settings are not just useless: they actually cause us quite some trouble on guest state migration, because they lead to huge queues that need to be moved as well. Drop those settings. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 41 +++++++++-------------------------------- tcp_conn.h | 4 ++-- tcp_splice.c | 6 +++--- 3 files changed, 14 insertions(+), 37 deletions(-) diff --git a/tcp.c b/tcp.c index 16d01f6..b978b30 100644 --- a/tcp.c +++ b/tcp.c @@ -738,24 +738,6 @@ static void tcp_get_sndbuf(struct tcp_tap_conn *conn) SNDBUF_SET(conn, MIN(INT_MAX, v)); } -/** - * tcp_sock_set_bufsize() - Set SO_RCVBUF and SO_SNDBUF to maximum values - * @s: Socket, can be -1 to avoid check in the caller - */ -static void tcp_sock_set_bufsize(const struct ctx *c, int s) -{ - int v = INT_MAX / 2; /* Kernel clamps and rounds, no need to check */ - - if (s == -1) - return; - - if (!c->low_rmem && setsockopt(s, SOL_SOCKET, SO_RCVBUF, &v, sizeof(v))) - trace("TCP: failed to set SO_RCVBUF to %i", v); - - if (!c->low_wmem && setsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v))) - trace("TCP: failed to set SO_SNDBUF to %i", v); -} - /** * tcp_sock_set_nodelay() - Set TCP_NODELAY option (disable Nagle's algorithm) * @s: Socket, can be -1 to avoid check in the caller @@ -1278,12 +1260,11 @@ int tcp_conn_pool_sock(int pool[]) /** * tcp_conn_new_sock() - Open and prepare new socket for connection - * @c: Execution context * @af: Address family * * Return: socket number on success, negative code if socket creation failed */ -static int tcp_conn_new_sock(const struct ctx *c, sa_family_t af) +static int tcp_conn_new_sock(sa_family_t af) { int s; @@ -1297,7 +1278,6 @@ static int tcp_conn_new_sock(const struct ctx *c, sa_family_t af) if (s < 0) return -errno; - tcp_sock_set_bufsize(c, s); tcp_sock_set_nodelay(s); return s; @@ -1305,12 +1285,11 @@ static int tcp_conn_new_sock(const struct ctx *c, sa_family_t af) /** * tcp_conn_sock() - Obtain a connectable socket in the host/init namespace - * @c: Execution context * @af: Address family (AF_INET or AF_INET6) * * Return: Socket fd on success, -errno on failure */ -int tcp_conn_sock(const struct ctx *c, sa_family_t af) +int tcp_conn_sock(sa_family_t af) { int *pool = af == AF_INET6 ? init_sock_pool6 : init_sock_pool4; int s; @@ -1321,7 +1300,7 @@ int tcp_conn_sock(const struct ctx *c, sa_family_t af) /* If the pool is empty we just open a new one without refilling the * pool to keep latency down. */ - if ((s = tcp_conn_new_sock(c, af)) >= 0) + if ((s = tcp_conn_new_sock(af)) >= 0) return s; err("TCP: Unable to open socket for new connection: %s", @@ -1462,7 +1441,7 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af, goto cancel; } - if ((s = tcp_conn_sock(c, af)) < 0) + if ((s = tcp_conn_sock(af)) < 0) goto cancel; pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, tgt->eport); @@ -1483,7 +1462,7 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af, } else { /* Not a local, bound destination, inconclusive test */ close(s); - if ((s = tcp_conn_sock(c, af)) < 0) + if ((s = tcp_conn_sock(af)) < 0) goto cancel; } @@ -2090,7 +2069,6 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref, if (s < 0) goto cancel; - tcp_sock_set_bufsize(c, s); tcp_sock_set_nodelay(s); /* FIXME: If useful: when the listening port has a specific bound @@ -2434,13 +2412,12 @@ static int tcp_ns_socks_init(void *arg) /** * tcp_sock_refill_pool() - Refill one pool of pre-opened sockets - * @c: Execution context * @pool: Pool of sockets to refill * @af: Address family to use * * Return: 0 on success, negative error code if there was at least one error */ -int tcp_sock_refill_pool(const struct ctx *c, int pool[], sa_family_t af) +int tcp_sock_refill_pool(int pool[], sa_family_t af) { int i; @@ -2450,7 +2427,7 @@ int tcp_sock_refill_pool(const struct ctx *c, int pool[], sa_family_t af) if (pool[i] >= 0) continue; - if ((fd = tcp_conn_new_sock(c, af)) < 0) + if ((fd = tcp_conn_new_sock(af)) < 0) return fd; pool[i] = fd; @@ -2466,13 +2443,13 @@ int tcp_sock_refill_pool(const struct ctx *c, int pool[], sa_family_t af) static void tcp_sock_refill_init(const struct ctx *c) { if (c->ifi4) { - int rc = tcp_sock_refill_pool(c, init_sock_pool4, AF_INET); + int rc = tcp_sock_refill_pool(init_sock_pool4, AF_INET); if (rc < 0) warn("TCP: Error refilling IPv4 host socket pool: %s", strerror_(-rc)); } if (c->ifi6) { - int rc = tcp_sock_refill_pool(c, init_sock_pool6, AF_INET6); + int rc = tcp_sock_refill_pool(init_sock_pool6, AF_INET6); if (rc < 0) warn("TCP: Error refilling IPv6 host socket pool: %s", strerror_(-rc)); diff --git a/tcp_conn.h b/tcp_conn.h index d342680..8c20805 100644 --- a/tcp_conn.h +++ b/tcp_conn.h @@ -143,8 +143,8 @@ bool tcp_flow_defer(const struct tcp_tap_conn *conn); bool tcp_splice_flow_defer(struct tcp_splice_conn *conn); void tcp_splice_timer(const struct ctx *c, struct tcp_splice_conn *conn); int tcp_conn_pool_sock(int pool[]); -int tcp_conn_sock(const struct ctx *c, sa_family_t af); -int tcp_sock_refill_pool(const struct ctx *c, int pool[], sa_family_t af); +int tcp_conn_sock(sa_family_t af); +int tcp_sock_refill_pool(int pool[], sa_family_t af); void tcp_splice_refill(const struct ctx *c); #endif /* TCP_CONN_H */ diff --git a/tcp_splice.c b/tcp_splice.c index f048a82..f1a9223 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -351,7 +351,7 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn) int one = 1; if (tgtpif == PIF_HOST) - conn->s[1] = tcp_conn_sock(c, af); + conn->s[1] = tcp_conn_sock(af); else if (tgtpif == PIF_SPLICE) conn->s[1] = tcp_conn_sock_ns(c, af); else @@ -703,13 +703,13 @@ static int tcp_sock_refill_ns(void *arg) ns_enter(c); if (c->ifi4) { - int rc = tcp_sock_refill_pool(c, ns_sock_pool4, AF_INET); + int rc = tcp_sock_refill_pool(ns_sock_pool4, AF_INET); if (rc < 0) warn("TCP: Error refilling IPv4 ns socket pool: %s", strerror_(-rc)); } if (c->ifi6) { - int rc = tcp_sock_refill_pool(c, ns_sock_pool6, AF_INET6); + int rc = tcp_sock_refill_pool(ns_sock_pool6, AF_INET6); if (rc < 0) warn("TCP: Error refilling IPv6 ns socket pool: %s", strerror_(-rc)); -- 2.43.0

2 2

[PATCH v22 0/2] State migration
by Stefano Brivio 14 Feb '25

14 Feb '25

Not many changes compared to v21: - Drop Bash-only == from tests, restore functionality with dash - Rebase: this is now based on top of "tcp: Keep updating window and checking for socket data after FIN from guest" and "tcp, tcp_splice: Don't set SO_SNDBUF and SO_RCVBUF to maximum values" Still missing: - tests with half-closed socket - complete error handling with (maybe?) tcpi_state == 0 as indication that we failed to dump a given flow and we just want to restore the next one Stefano Brivio (2): migrate: Migrate TCP flows test: Add migration tests contrib/selinux/passt.te | 4 +- flow.c | 260 ++++++++++ flow.h | 8 + migrate.c | 19 + passt.c | 6 +- repair.c | 1 - tcp.c | 918 ++++++++++++++++++++++++++++++++++++ tcp_conn.h | 103 ++++ test/lib/layout | 55 ++- test/lib/setup | 138 +++++- test/lib/test | 48 ++ test/migrate/basic | 59 +++ test/migrate/bidirectional | 64 +++ test/migrate/iperf3_bidir6 | 58 +++ test/migrate/iperf3_in4 | 50 ++ test/migrate/iperf3_in6 | 58 +++ test/migrate/iperf3_out4 | 50 ++ test/migrate/iperf3_out6 | 58 +++ test/migrate/rampstream_in | 10 +- test/migrate/rampstream_out | 6 +- test/run | 34 +- 21 files changed, 1990 insertions(+), 17 deletions(-) create mode 100644 test/migrate/basic create mode 100644 test/migrate/bidirectional create mode 100644 test/migrate/iperf3_bidir6 create mode 100644 test/migrate/iperf3_in4 create mode 100644 test/migrate/iperf3_in6 create mode 100644 test/migrate/iperf3_out4 create mode 100644 test/migrate/iperf3_out6 -- 2.43.0

1 2

[PATCH] tcp: Keep updating window and checking for socket data after FIN from guest
by Stefano Brivio 14 Feb '25

14 Feb '25

Once we get a FIN segment from the container/guest, we enter something resembling CLOSE_WAIT (from the perspective of the peer), but that doesn't mean that we should stop processing window updates from the guest and checking for socket data if the guest acknowledges something. If we don't do that, we can very easily run into a situation where we send a burst of data to the tap, get a zero window update, along with a FIN segment, because the flow is meant to be unidirectional, and now the connection will be stuck forever, because we'll ignore updates. Reproducer, server: $ pasta --config-net -t 9999 -- sh -c 'echo DONE | socat TCP-LISTEN:9997,shut-down STDIO' and client: $ ./test/rampstream send 50000 | socat -u STDIN TCP:$LOCAL_ADDR:9997 2025/02/13 09:14:45 socat[2997126] E write(5, 0x55f5dbf47000, 8192): Broken pipe while at it, update the message string for the third passive close state (which we see in this case): it's CLOSE_WAIT, not LAST_ACK. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tcp.c b/tcp.c index a1d6c53..16d01f6 100644 --- a/tcp.c +++ b/tcp.c @@ -338,7 +338,7 @@ static const char *tcp_state_str[] __attribute((__unused__)) = { "SYN_RCVD", /* approximately maps to TAP_SYN_ACK_SENT */ /* Passive close: */ - "CLOSE_WAIT", "CLOSE_WAIT", "LAST_ACK", "LAST_ACK", "LAST_ACK", + "CLOSE_WAIT", "CLOSE_WAIT", "CLOSE_WAIT", "LAST_ACK", "LAST_ACK", /* Active close (+5): */ "CLOSING", "FIN_WAIT_1", "FIN_WAIT_1", "FIN_WAIT_2", "TIME_WAIT", }; @@ -1968,6 +1968,8 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, /* Established connections not accepting data from tap */ if (conn->events & TAP_FIN_RCVD) { tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq)); + tcp_tap_window_update(conn, ntohs(th->window)); + tcp_data_from_sock(c, conn); if (conn->events & SOCK_FIN_RCVD && conn->seq_ack_from_tap == conn->seq_to_tap) -- 2.43.0

2 1

[PATCH] udp: create and send ICMPv4 to local peer when applicable
by Jon Maloy 13 Feb '25

13 Feb '25

When a local peer sends a UDP message to a non-existing port on an existing remote host, that host will return an ICMP message containing the error code ICMP_PORT_UNREACH, plus the header and the first eight bytes of the original message. If the sender socket has been connected, it uses this message to issue a "Connection Refused" event to the user. Until now, we have only read such events from the externally facing socket, but we don't forward them back to the local sender because we cannot read the ICMP message directly to user space. Because of this, the local peer will hang and wait for a response that never arrives. We now fix this for IPv4 by recreating and forwarding a correct ICMP message back to the internal sender. We synthesize the message based on the information in the extended error structure, plus the returned part of the original message body. Note that for the sake of completeness, we even produce ICMP messages for other error codes. We have noticed that at least ICMP_PROT_UNREACH is propagated as an error event back to the user. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tap.c | 4 +-- tap.h | 2 ++ udp.c | 71 ++++++++++++++++++++++++++++++++++++++++++-------- udp_internal.h | 2 +- udp_vu.c | 4 +-- 5 files changed, 67 insertions(+), 16 deletions(-) diff --git a/tap.c b/tap.c index cd32a90..f83aac5 100644 --- a/tap.c +++ b/tap.c @@ -142,8 +142,8 @@ static void *tap_push_l2h(const struct ctx *c, void *buf, uint16_t proto) * * Return: pointer at which to write the packet's payload */ -static void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src, - struct in_addr dst, size_t l4len, uint8_t proto) +void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src, + struct in_addr dst, size_t l4len, uint8_t proto) { uint16_t l3len = l4len + sizeof(*ip4h); diff --git a/tap.h b/tap.h index dfbd8b9..3ba00c1 100644 --- a/tap.h +++ b/tap.h @@ -44,6 +44,8 @@ static inline void tap_hdr_update(struct tap_hdr *thdr, size_t l2len) thdr->vnet_len = htonl(l2len); } +void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src, + struct in_addr dst, size_t l4len, uint8_t proto); void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport, struct in_addr dst, in_port_t dport, const void *in, size_t dlen); diff --git a/udp.c b/udp.c index 923cc38..4b800b6 100644 --- a/udp.c +++ b/udp.c @@ -87,6 +87,7 @@ #include <netinet/in.h> #include <netinet/ip.h> #include <netinet/udp.h> +#include <netinet/ip_icmp.h> #include <stdint.h> #include <stddef.h> #include <string.h> @@ -402,25 +403,70 @@ static void udp_tap_prepare(const struct mmsghdr *mmh, (*tap_iov)[UDP_IOV_PAYLOAD].iov_len = l4len; } +/** + * udp_send_conn_fail_icmp4() - Construct and send ICMP to local peer + * @c: Execution context + * @ee: Extended error descriptor + * @ref: epoll reference + * @iov: First bytes (max 8) of original UDP message body + */ +static void udp_send_conn_fail_icmp4(const struct ctx *c, + const struct sock_extended_err *ee, + union epoll_ref ref, + struct iovec *iov) +{ + flow_sidx_t tosidx = flow_sidx_opposite(ref.flowside); + const struct flowside *toside = flowside_at_sidx(tosidx); + struct in_addr oaddr = toside->oaddr.v4mapped.a4; + struct in_addr eaddr = toside->eaddr.v4mapped.a4; + struct iov_tail data = IOV_TAIL(iov, 1, 0); + struct { + struct icmphdr icmp4h; + struct iphdr ip4h; + struct udphdr uh; + char data[8]; + } msg; + size_t udplen = sizeof(msg.uh) + iov->iov_len; + size_t msglen = sizeof(msg.icmp4h) + sizeof(msg.ip4h) + udplen; + + memset(&msg, 0, sizeof(msg)); + msg.icmp4h.type = ee->ee_type; + msg.icmp4h.code = ee->ee_code; + tap_push_ip4h(&msg.ip4h, eaddr, oaddr, udplen, IPPROTO_UDP); + msg.uh.source = htons(toside->eport); + msg.uh.dest = htons(toside->oport); + msg.uh.len = htons(udplen); + csum_udp4(&msg.uh, oaddr, eaddr, &data); + memcpy(&msg.data, iov->iov_base, iov->iov_len); + tap_icmp4_send(c, oaddr, eaddr, &msg, msglen); +} + /** * udp_sock_recverr() - Receive and clear an error from a socket - * @s: Socket to receive from + * @c: Execution context + * @ref: epoll reference * * Return: 1 if error received and processed, 0 if no more errors in queue, < 0 * if there was an error reading the queue * * #syscalls recvmsg */ -static int udp_sock_recverr(int s) +static int udp_sock_recverr(const struct ctx *c, union epoll_ref ref) { const struct sock_extended_err *ee; const struct cmsghdr *hdr; char buf[CMSG_SPACE(sizeof(*ee))]; + char udp_data[8]; + int s = ref.fd; + struct iovec iov = { + .iov_base = udp_data, + .iov_len = sizeof(udp_data) + }; struct msghdr mh = { .msg_name = NULL, .msg_namelen = 0, - .msg_iov = NULL, - .msg_iovlen = 0, + .msg_iov = &iov, + .msg_iovlen = 1, .msg_control = buf, .msg_controllen = sizeof(buf), }; @@ -450,8 +496,10 @@ static int udp_sock_recverr(int s) } ee = (const struct sock_extended_err *)CMSG_DATA(hdr); - - /* TODO: When possible propagate and otherwise handle errors */ + if (ee->ee_type == ICMP_DEST_UNREACH) { + iov.iov_len = rc; + udp_send_conn_fail_icmp4(c, ee, ref, &iov); + } debug("%s error on UDP socket %i: %s", str_ee_origin(ee), s, strerror_(ee->ee_errno)); @@ -461,15 +509,16 @@ static int udp_sock_recverr(int s) /** * udp_sock_errs() - Process errors on a socket * @c: Execution context - * @s: Socket to receive from + * @ref: epoll reference * @events: epoll events bitmap * * Return: Number of errors handled, or < 0 if we have an unrecoverable error */ -int udp_sock_errs(const struct ctx *c, int s, uint32_t events) +int udp_sock_errs(const struct ctx *c, union epoll_ref ref, uint32_t events) { unsigned n_err = 0; socklen_t errlen; + int s = ref.fd; int rc, err; ASSERT(!c->no_udp); @@ -478,7 +527,7 @@ int udp_sock_errs(const struct ctx *c, int s, uint32_t events) return 0; /* Nothing to do */ /* Empty the error queue */ - while ((rc = udp_sock_recverr(s)) > 0) + while ((rc = udp_sock_recverr(c, ref)) > 0) n_err += rc; if (rc < 0) @@ -558,7 +607,7 @@ static void udp_buf_listen_sock_handler(const struct ctx *c, const socklen_t sasize = sizeof(udp_meta[0].s_in); int n, i; - if (udp_sock_errs(c, ref.fd, events) < 0) { + if (udp_sock_errs(c, ref, events) < 0) { err("UDP: Unrecoverable error on listening socket:" " (%s port %hu)", pif_name(ref.udp.pif), ref.udp.port); /* FIXME: what now? close/re-open socket? */ @@ -661,7 +710,7 @@ static void udp_buf_reply_sock_handler(const struct ctx *c, union epoll_ref ref, from_s = uflow->s[ref.flowside.sidei]; - if (udp_sock_errs(c, from_s, events) < 0) { + if (udp_sock_errs(c, ref, events) < 0) { flow_err(uflow, "Unrecoverable error on reply socket"); flow_err_details(uflow); udp_flow_close(c, uflow); diff --git a/udp_internal.h b/udp_internal.h index cc80e30..3b081f5 100644 --- a/udp_internal.h +++ b/udp_internal.h @@ -30,5 +30,5 @@ size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp, size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp, const struct flowside *toside, size_t dlen, bool no_udp_csum); -int udp_sock_errs(const struct ctx *c, int s, uint32_t events); +int udp_sock_errs(const struct ctx *c, union epoll_ref ref, uint32_t events); #endif /* UDP_INTERNAL_H */ diff --git a/udp_vu.c b/udp_vu.c index 4123510..c26a223 100644 --- a/udp_vu.c +++ b/udp_vu.c @@ -227,7 +227,7 @@ void udp_vu_listen_sock_handler(const struct ctx *c, union epoll_ref ref, struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE]; int i; - if (udp_sock_errs(c, ref.fd, events) < 0) { + if (udp_sock_errs(c, ref, events) < 0) { err("UDP: Unrecoverable error on listening socket:" " (%s port %hu)", pif_name(ref.udp.pif), ref.udp.port); return; @@ -302,7 +302,7 @@ void udp_vu_reply_sock_handler(const struct ctx *c, union epoll_ref ref, ASSERT(!c->no_udp); - if (udp_sock_errs(c, from_s, events) < 0) { + if (udp_sock_errs(c, ref, events) < 0) { flow_err(uflow, "Unrecoverable error on reply socket"); flow_err_details(uflow); udp_flow_close(c, uflow); -- 2.48.0

3 7