Re: [PATCH v8 7/8] vhost-user: add vhost-user

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 22 Oct 2024 14:59:19 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 17/10/2024 02:10, Stefano Brivio wrote:
> > On Wed, 16 Oct 2024 11:41:34 +1100
> > David Gibson <david@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?

I guess the connection from the first test is not closed properly, so
it's still in TIME_WAIT or similar. Given the topic, I can imagine that
something goes wrong as we check:

        if (ack && conn->events & TAP_FIN_SENT &&
            conn->seq_ack_from_tap == conn->seq_to_tap)
                conn_event(c, conn, TAP_FIN_ACKED);

in tcp_data_from_tap(). Or something around FIN segments anyway.

You can give socat the 'reuseaddr' option, say:

  socat -u TCP4-LISTEN:10001,reuseaddr OPEN:test_big.bin,create,trunc

to see if that's actually the case.

> The patch is:
> 
> [...]

I can try that in a bit.

From --debug (or --trace) output the issue might be obvious, by the
way (you can enable that in the test suite with DEBUG=1). I guess
you'll see FIN from one side and not from the other one.

-- 
Stefano