- dev - passt

[PATCH] vhost_user: Drop packet with unsupported iovec array
by Laurent Vivier 21 Jan '25

21 Jan '25

If the iovec array cannot be managed, drop it rather than passing the second entry to tap_add_packet(). Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- vu_common.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/vu_common.c b/vu_common.c index aa5ca7b0e750..f43d8ac45748 100644 --- a/vu_common.c +++ b/vu_common.c @@ -195,15 +195,17 @@ static void vu_handle_tx(struct vu_dev *vdev, int index, hdrlen); } else { /* vnet header can be in a separate iovec */ - if (elem[count].out_num != 2) + if (elem[count].out_num != 2) { debug("virtio-net transmit queue contains more than one buffer ([%d]: %u)", count, elem[count].out_num); - if (elem[count].out_sg[0].iov_len != (size_t)hdrlen) + } else if (elem[count].out_sg[0].iov_len != (size_t)hdrlen) { debug("virtio-net transmit queue entry not aligned on hdrlen ([%d]: %d != %zu)", - count, hdrlen, elem[count].out_sg[0].iov_len); - tap_add_packet(vdev->context, - elem[count].out_sg[1].iov_len, - (char *)elem[count].out_sg[1].iov_base); + count, hdrlen, elem[count].out_sg[0].iov_len); + } else { + tap_add_packet(vdev->context, + elem[count].out_sg[1].iov_len, + (char *)elem[count].out_sg[1].iov_base); + } } count++; -- 2.47.1

2 1

[PATCH v2] tcp: Disable Nagle's algorithm (set TCP_NODELAY) on all sockets
by Stefano Brivio 21 Jan '25

21 Jan '25

Following up on 725acd111ba3 ("tcp_splice: Set (again) TCP_NODELAY on both sides"), David argues that, in general, we don't know what kind of TCP traffic we're dealing with, on any side or path. TCP segments might have been delivered to our socket with a PSH flag, but we don't have a way to know about it. Similarly, the guest might send us segments with PSH or URG set, but we don't know if we should generally TCP_CORK sockets and uncork on those flags, because that would assume they're running a Linux kernel (and a particular version of it) matching the kernel that delivers outbound packets for us. Given that we can't make any assumption and everything might very well be interactive traffic, disable Nagle's algorithm on all non-spliced sockets as well. After all, John Nagle himself is nowadays recommending that delayed ACKs should never be enabled together with his algorithm, but we don't have a practical way to ensure that our environment is free from delayed ACKs (TCP_QUICKACK is not really usable for this purpose): https://news.ycombinator.com/item?id=34180239 Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Set TCP_NODELAY on inbound socket after accept4(), not on the listening sockets, and change failure message to debug() instead of trace() tcp.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tcp.c b/tcp.c index a012b81..4d6a6b3 100644 --- a/tcp.c +++ b/tcp.c @@ -756,6 +756,19 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s) 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 + */ +static void tcp_sock_set_nodelay(int s) +{ + if (s == -1) + return; + + if (setsockopt(s, SOL_TCP, TCP_NODELAY, &((int){ 1 }), sizeof(int))) + debug("TCP: failed to set TCP_NODELAY on socket %i", s); +} + /** * tcp_update_csum() - Calculate TCP checksum * @psum: Unfolded partial checksum of the IPv4 or IPv6 pseudo-header @@ -1285,6 +1298,7 @@ static int tcp_conn_new_sock(const struct ctx *c, sa_family_t af) return -errno; tcp_sock_set_bufsize(c, s); + tcp_sock_set_nodelay(s); return s; } @@ -2058,6 +2072,7 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref, goto cancel; tcp_sock_set_bufsize(c, s); + tcp_sock_set_nodelay(s); /* FIXME: When listening port has a specific bound address, record that * as our address -- 2.43.0

2 4

[PATCH] vhost_user: remove ASSERT() on iovec number
by Laurent Vivier 21 Jan '25

21 Jan '25

Replace ASSERT() on the number of iovec in the element and on the first entry length by a debug() message. Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- vu_common.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vu_common.c b/vu_common.c index 431fba6be0c0..d34ae6dc1df3 100644 --- a/vu_common.c +++ b/vu_common.c @@ -195,8 +195,12 @@ static void vu_handle_tx(struct vu_dev *vdev, int index, hdrlen); } else { /* vnet header can be in a separate iovec */ - ASSERT(elem[count].out_num == 2); - ASSERT(elem[count].out_sg[0].iov_len == (size_t)hdrlen); + if (elem[count].out_num != 2) + debug("virtio-net tranmit queue contains more than one buffer ([%d]: %u)", + count, elem[count].out_num); + if (elem[count].out_sg[0].iov_len != (size_t)hdrlen) + debug("virtio-net transmit queue entry not aligned on hdrlen ([%d]: %d != %zu)", + count, hdrlen, elem[count].out_sg[0].iov_len); tap_add_packet(vdev->context, elem[count].out_sg[1].iov_len, (char *)elem[count].out_sg[1].iov_base); -- 2.47.1

3 3

[PATCH] tcp: Buffer sizes are *not* inherited on accept()/accept4()
by Stefano Brivio 21 Jan '25

21 Jan '25

...so it's pointless to set SO_RCVBUF and SO_SNDBUF on listening sockets. Call tcp_sock_set_bufsize() after accept4(), for inbound sockets. As we didn't have large buffer sizes set for inbound sockets for a long time (they are set explicitly only if the maximum size is big enough, more than than the ~200 KiB default), I ran some more throughput tests for this one, and I see slightly better numbers (say, 17 gbps instead of 15 gbps guest to host without vhost-user). Fixes: 904b86ade7db ("tcp: Rework window handling, timers, add SO_RCVLOWAT and pools for sockets/pipes") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tcp.c b/tcp.c index 3b3193a..a012b81 100644 --- a/tcp.c +++ b/tcp.c @@ -2057,6 +2057,8 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref, if (s < 0) goto cancel; + tcp_sock_set_bufsize(c, s); + /* FIXME: When listening port has a specific bound address, record that * as our address */ @@ -2260,7 +2262,6 @@ static int tcp_sock_init_one(const struct ctx *c, const union inany_addr *addr, if (s < 0) return s; - tcp_sock_set_bufsize(c, s); return s; } @@ -2317,9 +2318,7 @@ static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port) s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback4, NULL, port, tref.u32); - if (s >= 0) - tcp_sock_set_bufsize(c, s); - else + if (s < 0) s = -1; if (c->tcp.fwd_out.mode == FWD_AUTO) @@ -2343,9 +2342,7 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port) s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback6, NULL, port, tref.u32); - if (s >= 0) - tcp_sock_set_bufsize(c, s); - else + if (s < 0) s = -1; if (c->tcp.fwd_out.mode == FWD_AUTO) -- 2.43.0

2 1

[PATCH 0/9] vhost-user: Migration support
by Laurent Vivier 20 Jan '25

20 Jan '25

This series allows a QEMU guest to be migrated while it is connected to Passt using vhost-user interface. There are two parts: - first part enables the migration of QEMU without transferring the internal state of Passt. All connections are lost. This is done by implementing VHOST_USER_SET_LOG_FD and VHOST_USER_SET_LOG_BASE commands (and enabling VHOST_USER_PROTOCOL_F_LOG_SHMFD feature) "vhost-user: add VHOST_USER_SET_LOG_FD command" "vhost-user: add VHOST_USER_SET_LOG_BASE command" "vhost-user: Report to front-end we support VHOST_USER_PROTOCOL_F_LOG_SHMFD" - second part allows source Passt instance to send its internal state using QEMU migration channel to destination Passt instance. This is done implementing VHOST_USER_SET_DEVICE_STATE_FD and VHOST_USER_CHECK_DEVICE_STATE (and enabling VHOST_USER_PROTOCOL_F_DEVICE_STATE feature). "vhost-user: add VHOST_USER_CHECK_DEVICE_STATE command" "vhost-user: add VHOST_USER_SET_DEVICE_STATE_FD command" "vhost-user: Report to front-end we support VHOST_USER_PROTOCOL_F_DEVICE_STATE" For now, it only implements the function needed to transfer the state but no state is transferred. VHOST_USER_PROTOCOL_F_DEVICE_STATE is implemented in QEMU only for vhost-user-fs, to be able to use it with virtio-net I have proposed a patch upstream: https://patchew.org/QEMU/20241218143453.1573185-1-lvivier@redhat.com/ Laurent Vivier (9): virtio: Use const pointer for vu_dev vhost-user: update protocol features and commands list vhost-user: add VHOST_USER_SET_LOG_FD command vhost-user: Pass vu_dev to more virtio functions vhost-user: add VHOST_USER_SET_LOG_BASE command vhost-user: Report to front-end we support VHOST_USER_PROTOCOL_F_LOG_SHMFD vhost-user: add VHOST_USER_CHECK_DEVICE_STATE command vhost-user: add VHOST_USER_SET_DEVICE_STATE_FD command vhost-user: Report to front-end we support VHOST_USER_PROTOCOL_F_DEVICE_STATE epoll_type.h | 2 + passt.c | 4 + util.h | 3 + vhost_user.c | 251 ++++++++++++++++++++++++++++++++++++++++++++++++++- vhost_user.h | 50 +++++++++- virtio.c | 116 +++++++++++++++++++++--- virtio.h | 32 +++++-- vu_common.c | 59 +++++++++++- vu_common.h | 3 +- 9 files changed, 484 insertions(+), 36 deletions(-) -- 2.47.1

3 30

[PATCH] tcp: Disable Nagle's algorithm (set TCP_NODELAY) on all sockets
by Stefano Brivio 20 Jan '25

20 Jan '25

Following up on 725acd111ba3 ("tcp_splice: Set (again) TCP_NODELAY on both sides"), David argues that, in general, we don't know what kind of TCP traffic we're dealing with, on any side or path. TCP segments might have been delivered to our socket with a PSH flag, but we don't have a way to know about it. Similarly, the guest might send us segments with PSH or URG set, but we don't know if we should generally TCP_CORK sockets and uncork on those flags, because that would assume they're running a Linux kernel (and a particular version of it) matching the kernel that delivers outbound packets for us. Given that we can't make any assumption and everything might very well be interactive traffic, disable Nagle's algorithm on all non-spliced sockets as well. After all, John Nagle himself is nowadays recommending that delayed ACKs should never be enabled together with his algorithm, but we don't have a practical way to ensure that our environment is free from delayed ACKs (TCP_QUICKACK is not really usable for this purpose): https://news.ycombinator.com/item?id=34180239 Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tcp.c b/tcp.c index 3b3193a..c570f42 100644 --- a/tcp.c +++ b/tcp.c @@ -756,6 +756,19 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s) 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 + */ +static void tcp_sock_set_nodelay(int s) +{ + if (s == -1) + return; + + if (setsockopt(s, SOL_TCP, TCP_NODELAY, &((int){ 1 }), sizeof(int))) + trace("TCP: failed to set TCP_NODELAY on socket %i", s); +} + /** * tcp_update_csum() - Calculate TCP checksum * @psum: Unfolded partial checksum of the IPv4 or IPv6 pseudo-header @@ -1285,6 +1298,7 @@ static int tcp_conn_new_sock(const struct ctx *c, sa_family_t af) return -errno; tcp_sock_set_bufsize(c, s); + tcp_sock_set_nodelay(s); return s; } @@ -2261,6 +2275,8 @@ static int tcp_sock_init_one(const struct ctx *c, const union inany_addr *addr, return s; tcp_sock_set_bufsize(c, s); + tcp_sock_set_nodelay(s); + return s; } @@ -2322,6 +2338,8 @@ static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port) else s = -1; + tcp_sock_set_nodelay(s); + if (c->tcp.fwd_out.mode == FWD_AUTO) tcp_sock_ns[port][V4] = s; } @@ -2348,6 +2366,8 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port) else s = -1; + tcp_sock_set_nodelay(s); + if (c->tcp.fwd_out.mode == FWD_AUTO) tcp_sock_ns[port][V6] = s; } -- 2.43.0

2 2

[net, v2.1] tcp: correct handling of extreme memory squeeze
by Jon Maloy 17 Jan '25

17 Jan '25

Testing with iperf3 using the "pasta" protocol splicer has revealed a bug in the way tcp handles window advertising in extreme memory squeeze situations. The problem occurs on the server side, and the socket in question is a completely regular socket with the default settings for the fedora40 kernel. We do not use SO_PEEK or SO_RCVBUF on this socket. A brief summary: Under memory pressure, a socket endpoint may temporarily advertise a zero-sized window, but this is not stored as part of the socket data. The reasoning behind this is that it is considered a temporary setting which shouldn't influence any further calculations. However, if we happen to stall at an unfortunate value of the current window size, the algorithm selecting a new value will consistently fail to advertise a non-zero window once we have freed up enough memory. This means that this side's notion of the current window size is different from the one last advertised to the peer, causing the latter to not send any data to resolve the sitution. The following excerpt of a logging session, with own comments added, shows more in detail what is happening: // tcp_v4_rcv(->) // tcp_rcv_established(->) [5201<->39222]: ==== Activating log @ net/ipv4/tcp_input.c/tcp_data_queue()/5257 ==== [5201<->39222]: tcp_data_queue(->) [5201<->39222]: DROPPING skb [265600160..265665640], reason: SKB_DROP_REASON_PROTO_MEM [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200, win_now 131184] [copied_seq 259909392->260034360 (124968), unread 5565800, qlen 85, ofoq 0] [5201<->39222]: tcp_data_queue(<-) OFO queue: gap: 65480, len: 0 [5201<->39222]: __tcp_transmit_skb(->) [5201<->39222]: tcp_select_window(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: (inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM) --> TRUE [5201<->39222]: tcp_select_window(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160, returning 0 [5201<->39222]: ADVERTISING WIN 0, ACK_SEQ: 265600160 [5201<->39222]: __tcp_transmit_skb(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_rcv_established(<-) [5201<->39222]: tcp_v4_rcv(<-) // Receive queue is at 85 buffers and we are out of memory. // We drop the incoming buffer, although it is in sequence, and decide // to send an advertisement with a window of zero. // We don't update tp->rcv_wnd and tp->rcv_wup accordingly, which means // we unconditionally shrink the window. [5201<->39222]: tcp_recvmsg_locked(->) [5201<->39222]: __tcp_cleanup_rbuf(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: [new_win = 0, win_now = 131184, 2 * win_now = 262368] [5201<->39222]: [new_win >= (2 * win_now) ? --> time_to_ack = 0] [5201<->39222]: NOT calling tcp_send_ack() [5201<->39222]: __tcp_cleanup_rbuf(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_recvmsg_locked(<-) returning 6104 bytes. [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200, win_now 131184] [copied_seq 260040464->260040464 (0), unread 5559696, qlen 85, ofoq 0] // After each read, the algorithm for calculating the new receive // window in __tcp_cleanup_rbuf() finds it is too small to advertise // or to update tp->rcv_wnd. // Meanwhile, the peer thinks the window is zero, and will not send // any more data to trigger an update from the interrupt mode side. [5201<->39222]: tcp_recvmsg_locked(->) [5201<->39222]: __tcp_cleanup_rbuf(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: [new_win = 262144, win_now = 131184, 2 * win_now = 262368] [5201<->39222]: [new_win >= (2 * win_now) ? --> time_to_ack = 0] [5201<->39222]: NOT calling tcp_send_ack() [5201<->39222]: __tcp_cleanup_rbuf(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_recvmsg_locked(<-) returning 131072 bytes. [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200, win_now 131184] [copied_seq 260099840->260171536 (71696), unread 5428624, qlen 83, ofoq 0] // The above pattern repeats again and again, since nothing changes // between the reads. [...] [5201<->39222]: tcp_recvmsg_locked(->) [5201<->39222]: __tcp_cleanup_rbuf(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: [new_win = 262144, win_now = 131184, 2 * win_now = 262368] [5201<->39222]: [new_win >= (2 * win_now) ? --> time_to_ack = 0] [5201<->39222]: NOT calling tcp_send_ack() [5201<->39222]: __tcp_cleanup_rbuf(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_recvmsg_locked(<-) returning 131072 bytes. [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200, win_now 131184] [copied_seq 265469200->265545488 (76288), unread 54672, qlen 1, ofoq 0] [5201<->39222]: tcp_recvmsg_locked(->) [5201<->39222]: __tcp_cleanup_rbuf(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: [new_win = 262144, win_now = 131184, 2 * win_now = 262368] [5201<->39222]: [new_win >= (2 * win_now) ? --> time_to_ack = 0] [5201<->39222]: NOT calling tcp_send_ack() [5201<->39222]: __tcp_cleanup_rbuf(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_recvmsg_locked(<-) returning 54672 bytes. [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200, win_now 131184] [copied_seq 265600160->265600160 (0), unread 0, qlen 0, ofoq 0] // The receive queue is empty, but no new advertisement is sent. // The peer still thinks the receive window is zero, and sends nothing. // We have ended up in a deadlock situation. Furthermore, we have observed that in these situations this side may send out an updated 'th->ack_seq´ which is not stored in tp->rcv_wup as it should be. Backing ack_seq seems to be harmless, but is of course still wrong from a protocol viewpoint. We fix this by setting tp->rcv_wnd and tp->rcv_wup even when a packet has been dropped because of memory exhaustion and we have to advertize a zero window. Further testing shows that the connection recovers neatly from the squeeze situation, and traffic can continue indefinitely. Fixes: e2142825c120 ("net: tcp: send zero-window ACK when no memory") Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v1: -Posted on Apr 6, 2024 v2: -Improved commit log to clarify how we end up in this situation. -After feedback from Eric Dumazet, removed references to use of SO_PEEK and SO_PEEK_OFF which may lead to a misunderstanding about how this situation occurs. Those flags are used at the peer side's incoming connection, and not on this one. --- net/ipv4/tcp_output.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 0e5b9a654254..ba295f798e5e 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -265,11 +265,13 @@ static u16 tcp_select_window(struct sock *sk) u32 cur_win, new_win; /* Make the window 0 if we failed to queue the data because we - * are out of memory. The window is temporary, so we don't store - * it on the socket. + * are out of memory. */ - if (unlikely(inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM)) + if (unlikely(inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM)) { + tp->rcv_wnd = 0; + tp->rcv_wup = tp->rcv_nxt; return 0; + } cur_win = tcp_receive_window(tp); new_win = __tcp_select_window(sk); -- 2.48.0

2 1

[net, v2] tcp: correct handling of extreme memory squeeze
by Jon Maloy 16 Jan '25

16 Jan '25

Testing with iperf3 using the "pasta" protocol splicer has revealed a bug in the way tcp handles window advertising in extreme memory squeeze situations. The problem occurs on the server side, and the socket in question is a completely regular socket with the default settings for the fedora40 kernel. We do not use SO_PEEK or SO_RCV_BUF on this socket. The following excerpt of a logging session, with own comments added, shows what is happening: // tcp_v4_rcv(<-) // tcp_rcv_established(<-) [5201<->39222]: ==== Activating log @ net/ipv4/tcp_input.c/tcp_data_queue()/5257 ==== [5201<->39222]: tcp_data_queue(->) [5201<->39222]: DROPPING skb [265600160..265665640], reason: SKB_DROP_REASON_PROTO_MEM [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200 (0), win_now 131184] [copied_seq 259909392->260034360 (124968), unread 5565800, qlen 85, ofoq 0] [5201<->39222]: tcp_data_queue(<-) OFO queue: gap: 65480, len: 0 [5201<->39222]: __tcp_transmit_skb(->) [5201<->39222]: tcp_select_window(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: (inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM) --> TRUE [5201<->39222]: tcp_select_window(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160, returning 0 [5201<->39222]: ADVERTISING WIN 0, ACK_SEQ: 265600160 [5201<->39222]: __tcp_transmit_skb(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_rcv_established(<-) [5201<->39222]: tcp_v4_rcv(<-) // Receive queue is at 85 buffers and we are out of memory. // We drop the incoming buffer, although it is in sequence, and decide // to send an advertisement with a window of zero. // We don't update tp->rcv_wnd and tp->rcv_wup accordingly, which means // we unconditionally shrink the window. [5201<->39222]: tcp_recvmsg_locked(->) [5201<->39222]: __tcp_cleanup_rbuf(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: [new_win = 0, win_now = 131184, 2 * win_now = 262368] [5201<->39222]: [new_win >= (2 * win_now) ? --> time_to_ack = 0] [5201<->39222]: NOT calling tcp_send_ack() [5201<->39222]: __tcp_cleanup_rbuf(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_recvmsg_locked(<-) returning 6104 bytes. [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200 (0), win_now 131184] [copied_seq 260040464->260040464 (0), unread 5559696, qlen 85, ofoq 0] // After each read, the algorithm for calculating the new receive // window in __tcp_cleanup_rbuf() finds it is too small to advertise // or to update tp->rcv_wnd. // Meanwhile, the peer thinks the window is zero, and will not send // any more data to trigger an update from the interrupt mode side. [5201<->39222]: tcp_recvmsg_locked(->) [5201<->39222]: __tcp_cleanup_rbuf(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: [new_win = 262144, win_now = 131184, 2 * win_now = 262368] [5201<->39222]: [new_win >= (2 * win_now) ? --> time_to_ack = 0] [5201<->39222]: NOT calling tcp_send_ack() [5201<->39222]: __tcp_cleanup_rbuf(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_recvmsg_locked(<-) returning 131072 bytes. [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200 (0), win_now 131184] [copied_seq 260099840->260171536 (71696), unread 5428624, qlen 83, ofoq 0] // The above pattern repeats again and again, since nothing changes // between the reads. [...] [5201<->39222]: tcp_recvmsg_locked(->) [5201<->39222]: __tcp_cleanup_rbuf(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: [new_win = 262144, win_now = 131184, 2 * win_now = 262368] [5201<->39222]: [new_win >= (2 * win_now) ? --> time_to_ack = 0] [5201<->39222]: NOT calling tcp_send_ack() [5201<->39222]: __tcp_cleanup_rbuf(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_recvmsg_locked(<-) returning 131072 bytes. [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200 (0), win_now 131184] [copied_seq 265469200->265545488 (76288), unread 54672, qlen 1, ofoq 0] [5201<->39222]: tcp_recvmsg_locked(->) [5201<->39222]: __tcp_cleanup_rbuf(->) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: [new_win = 262144, win_now = 131184, 2 * win_now = 262368] [5201<->39222]: [new_win >= (2 * win_now) ? --> time_to_ack = 0] [5201<->39222]: NOT calling tcp_send_ack() [5201<->39222]: __tcp_cleanup_rbuf(<-) tp->rcv_wup: 265469200, tp->rcv_wnd: 262144, tp->rcv_nxt 265600160 [5201<->39222]: tcp_recvmsg_locked(<-) returning 54672 bytes. [rcv_nxt 265600160, rcv_wnd 262144, snt_ack 265469200 (0), win_now 131184] [copied_seq 265600160->265600160 (0), unread 0, qlen 0, ofoq 0] // The receive queue is empty, but no new advertisement is sent. // The peer still thinks the receive window is zero, and sends nothing. // We have ended up in a deadlock situation. Furthermore, we have observed that in these situations this side may send out an updated 'th->ack_seq´ which is not stored in tp->rcv_wup as it should be. Backing ack_seq seems to be harmless, but is of course still wrong from a protocol viewpoint. We fix this by setting tp->rcv_wnd and tp->rcv_wup even when a packet has been dropped because of memory exhaustion and we have to advertize a zero window. Further testing shows that the connection recovers neatly from the squeeze situation, and traffic can continue indefinitely. Fixes: e2142825c120 ("net: tcp: send zero-window ACK when no memory") Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v1: -Posted on Apr 6, 2024 v2: -Improved commit log to clarify how we end up in this situation. -After feedback from Eric Dumazet, removed references to use of SO_PEEK and SO_PEEK_OFF which may lead to a misunderstanding about how this situation occurs. Those flags are used at the peer side's incoming connection, and not on this one. --- net/ipv4/tcp_output.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 0e5b9a654254..ba295f798e5e 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -265,11 +265,13 @@ static u16 tcp_select_window(struct sock *sk) u32 cur_win, new_win; /* Make the window 0 if we failed to queue the data because we - * are out of memory. The window is temporary, so we don't store - * it on the socket. + * are out of memory. */ - if (unlikely(inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM)) + if (unlikely(inet_csk(sk)->icsk_ack.pending & ICSK_ACK_NOMEM)) { + tp->rcv_wnd = 0; + tp->rcv_wup = tp->rcv_nxt; return 0; + } cur_win = tcp_receive_window(tp); new_win = __tcp_select_window(sk); -- 2.48.0

2 1

[PATCH] tcp: Set EPOLLET when when reading from a socket fails with EAGAIN
by Stefano Brivio 16 Jan '25

16 Jan '25

Before SO_PEEK_OFF support was introduced by commit e63d281871ef ("tcp: leverage support of SO_PEEK_OFF socket option when available"), we would peek data from sockets using a "discard" buffer as first iovec element, so that, unless we had no pending data at all, we would always get a positive return code from recvmsg() (except for closing connections or errors). If we couldn't send more data to the guest, in the window, we would set the STALLED flag (causing the epoll descriptor to switch to edge-triggered mode), and return early from tcp_data_from_sock(). With SO_PEEK_OFF, we don't have a discard buffer, and if there's data on the socket, but nothing beyond our current peeking offset, we'll get EAGAIN instead of our current "discard" length. In that case, we return even earlier, and we don't set EPOLLET on the socket as a result. As reported by Asahi Lina, this causes event loops where the kernel is signalling socket readiness, because there's data we didn't dequeue yet (waiting for the guest to acknowledge it), but we won't actually peek anything new, and return early without setting EPOLLET. This is the original report, mentioning the originally proposed fix: -- When there is unacknowledged data in the inbound socket buffer, passt leaves the socket in the epoll instance to accept new data from the server. Since there is already data in the socket buffer, an epoll without EPOLLET will repeatedly fire while no data is processed, busy-looping the CPU: epoll_pwait(3, [...], 8, 1000, NULL, 8) = 4 recvmsg(25, {msg_namelen=0}, MSG_PEEK) = -1 EAGAIN (Resource temporarily unavailable) recvmsg(169, {msg_namelen=0}, MSG_PEEK) = -1 EAGAIN (Resource temporarily unavailable) recvmsg(111, {msg_namelen=0}, MSG_PEEK) = -1 EAGAIN (Resource temporarily unavailable) recvmsg(180, {msg_namelen=0}, MSG_PEEK) = -1 EAGAIN (Resource temporarily unavailable) epoll_pwait(3, [...], 8, 1000, NULL, 8) = 4 recvmsg(25, {msg_namelen=0}, MSG_PEEK) = -1 EAGAIN (Resource temporarily unavailable) recvmsg(169, {msg_namelen=0}, MSG_PEEK) = -1 EAGAIN (Resource temporarily unavailable) recvmsg(111, {msg_namelen=0}, MSG_PEEK) = -1 EAGAIN (Resource temporarily unavailable) recvmsg(180, {msg_namelen=0}, MSG_PEEK) = -1 EAGAIN (Resource temporarily unavailable) Add in the missing EPOLLET flag for this case. This brings CPU usage down from around ~80% when downloading over TCP, to ~5% (use case: passt as network transport for muvm, downloading Steam games). -- we can't set EPOLLET unconditionally though, at least right now, because we don't monitor the guest tap for EPOLLOUT in case we fail to write on that side because we filled up that buffer (and not the window of a TCP connection). Instead, rely on the observation that we only get EAGAIN on recvmsg() if we are attempting to peek data from a socket with a non-zero peeking offset: we only peek when there's pending data on a socket, and in that case, if we peek without offset, we'll always see some data. And if we peek data with a non-zero offset and get EAGAIN, that means that we're either waiting for more data to arrive on the socket (which would cause further wake-ups, even with EPOLLET), or we're waiting for the guest to acknowledge some of it, which would anyway cause a wake-up. In that case, it's safe to set STALLED and, in turn, EPOLLET on the socket, which fixes the EPOLLIN event loop. Reported-by: Asahi Lina <lina(a)asahilina.net> Fixes: e63d281871ef ("tcp: leverage support of SO_PEEK_OFF socket option when available") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp_buf.c | 1 + tcp_vu.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tcp_buf.c b/tcp_buf.c index a975a55..577467b 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -359,6 +359,7 @@ int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) return -errno; } + conn_flag(c, conn, STALLED); return 0; } diff --git a/tcp_vu.c b/tcp_vu.c index 10e17d3..22ae71d 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -399,6 +399,8 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) tcp_rst(c, conn); return len; } + + conn_flag(c, conn, STALLED); return 0; } -- 2.43.0

1 1

[PATCH] vhost_user: fix multibuffer from linux
by Laurent Vivier 16 Jan '25

16 Jan '25

Under some conditions, linux can provide several buffers in the same element (multiple entries in the iovec array). I didn't identify what changed between the kernel guest that provides one buffer and the one that provides several (doesn't seem to be a kernel change or a configuration change). Fix the following assert: ASSERTION FAILED in virtqueue_map_desc (virtio.c:402): num_sg < max_num_sg What I can see is the buffer can be splitted in two iovecs: - vnet header - packet data This change manages this special case but the real fix will be to allow tap_add_packet() to manage iovec array. Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- vu_common.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/vu_common.c b/vu_common.c index 6d365bea5fe2..431fba6be0c0 100644 --- a/vu_common.c +++ b/vu_common.c @@ -18,6 +18,8 @@ #include "pcap.h" #include "vu_common.h" +#define VU_MAX_TX_BUFFER_NB 2 + /** * vu_packet_check_range() - Check if a given memory zone is contained in * a mapped guest memory region @@ -168,10 +170,15 @@ static void vu_handle_tx(struct vu_dev *vdev, int index, count = 0; out_sg_count = 0; - while (count < VIRTQUEUE_MAX_SIZE) { + while (count < VIRTQUEUE_MAX_SIZE && + out_sg_count + VU_MAX_TX_BUFFER_NB <= VIRTQUEUE_MAX_SIZE) { int ret; - vu_set_element(&elem[count], &out_sg[out_sg_count], NULL); + elem[count].out_num = VU_MAX_TX_BUFFER_NB; + elem[count].out_sg = &out_sg[out_sg_count]; + elem[count].in_num = 0; + elem[count].in_sg = NULL; + ret = vu_queue_pop(vdev, vq, &elem[count]); if (ret < 0) break; @@ -181,11 +188,20 @@ static void vu_handle_tx(struct vu_dev *vdev, int index, warn("virtio-net transmit queue contains no out buffers"); break; } - ASSERT(elem[count].out_num == 1); + if (elem[count].out_num == 1) { + tap_add_packet(vdev->context, + elem[count].out_sg[0].iov_len - hdrlen, + (char *)elem[count].out_sg[0].iov_base + + hdrlen); + } else { + /* vnet header can be in a separate iovec */ + ASSERT(elem[count].out_num == 2); + ASSERT(elem[count].out_sg[0].iov_len == (size_t)hdrlen); + tap_add_packet(vdev->context, + elem[count].out_sg[1].iov_len, + (char *)elem[count].out_sg[1].iov_base); + } - tap_add_packet(vdev->context, - elem[count].out_sg[0].iov_len - hdrlen, - (char *)elem[count].out_sg[0].iov_base + hdrlen); count++; } tap_handler(vdev->context, now); -- 2.47.1

3 2

2025

2024

2023

2022

2021

dev