On 17/10/2024 02:10, Stefano Brivio wrote:
On Wed, 16 Oct 2024 11:41:34 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:
On 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().
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 socket