On Wed, 23 Oct 2024 17:27:49 +0200 Laurent Vivier <lvivier(a)redhat.com> wrote:On 22/10/2024 14:59, Laurent Vivier wrote:This is expected, I think (and unfortunately not documented).On 17/10/2024 02:10, Stefano Brivio wrote:For the moment, I can see a behavior change of recvmsg() with the new kernel. without peek_offset_cap, if no new data is available, it returns "already_sent", so it enters in (found with tcp_vu.c but code samples from tcp_buf.c): ==> recvmsg() returns already_sent, so len > 0 ... sendlen -= already_sent; ==> here sendlen becomes 0 if (sendlen <= 0) { conn_flag(c, conn, STALLED); return 0; } With peek_offset, it returns -1, so it enters in:On Wed, 16 Oct 2024 11:41:34 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:I have a fix for that but there is an error I don't understand: when I run twice the test, the second time I have: guest: # socat -u TCP4-LISTEN:10001 OPEN:test_big.bin,create,trunc # socat -u TCP4-LISTEN:10001 OPEN:test_big.bin,create,trunc 2024/10/22 08:51:58 socat[1485] E bind(5, {AF=2 0.0.0.0:10001}, 16): Address already in use host: $ socat -u OPEN:test/big.bin TCP4:127.0.0.1:10001 If I wait a little it can work again several times and fails again. Any idea? The patch is: diff --git a/tcp_vu.c b/tcp_vu.c index 78884c673215..83e40fb07a03 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -379,6 +379,10 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) conn->seq_ack_from_tap, conn->seq_to_tap); conn->seq_to_tap = conn->seq_ack_from_tap; already_sent = 0; + if (tcp_set_peek_offset(conn->sock, 0)) { + tcp_rst(c, conn); + return -1; + } } if (!wnd_scaled || already_sent >= wnd_scaled) { @@ -389,14 +393,13 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) /* Set up buffer descriptors we'll fill completely and partially. */ - fillsize = wnd_scaled; + fillsize = wnd_scaled - already_sent; if (peek_offset_cap) already_sent = 0; iov_vu[0].iov_base = tcp_buf_discard; iov_vu[0].iov_len = already_sent; - fillsize -= already_sent; /* collect the buffers from vhost-user and fill them with the * data from the socketOn Tue, Oct 15, 2024 at 09:54:38PM +0200, Stefano Brivio wrote: > [Still partial review] [snip] >> + if (peek_offset_cap) >> + already_sent = 0; >> + >> + iov_vu[0].iov_base = tcp_buf_discard; >> + iov_vu[0].iov_len = already_sent; > > I think I had a similar comment to a previous revision. Now, I haven't > tested this (yet) on a kernel with support for SO_PEEK_OFF on TCP, but > I think this should eventually follow the same logic as the (updated) > tcp_buf_data_from_sock(): we should use tcp_buf_discard only if > (!peek_offset_cap). > > It's fine to always initialise VIRTQUEUE_MAX_SIZE iov_vu items, > starting from 1, for simplicity. But I'm not sure if it's safe to pass a > zero iov_len if (peek_offset_cap). > I'll test that (unless you already did) -- if it works, we can fix this > up later as well. I believe I tested it at some point, and I think we're already using it somewhere.I tested it again just to be sure on a recent net.git kernel: sometimes the first test in passt_vu_in_ns/tcp, "TCP/IPv4: host to guest: big transfer" hangs on my setup, sometimes it's the "TCP/IPv4: ns to guest (using loopback address): big transfer" test instead. I can reproduce at least one of the two issues consistently (tests stopped 5 times out of 5). The socat client completes the transfer, the server is still waiting for something. I haven't taken captures yet or tried to re-send from the client. It all works (consistently) with an older kernel without support for SO_PEEK_OFF on TCP, but also on this kernel if I force peek_offset_cap to false in tcp_init().if (len < 0) goto err; ... err: if (errno != EAGAIN && errno != EWOULDBLOCK) {But errno here should be EAGAIN, so yes, it looks buggy to me in the sense that:ret = -errno; tcp_rst(c, conn); }we return 0 here without setting the STALLED flag. While it should be fixed, that flag is some kind of optimisation, so this doesn't really explain the issue that I mentioned in 20241022201914.072f7c7d@elisabeth: https://archives.passt.top/passt-dev/20241022201914.072f7c7d@elisabeth/ As a quick fix, you should probably do this in tcp_vu_data_from_sock(): if (peek_offset_cap) /* add this condition */ len -= already_sent; if (len <= 0 || (peek_offset_cap && len == -1 && errno == EAGAIN)) /* change this condition */ ... ...or you mean that due to this behaviour you don't call vu_queue_rewind() and that causes troubles? -- Stefano