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

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 17 Oct 2024 19:33:38 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:

> On Thu, 17 Oct 2024 19:18:57 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
> > On 17/10/2024 13:25, Stefano Brivio wrote:  
> > > On Thu, 17 Oct 2024 02:10:31 +0200
> > > Stefano Brivio <sbrivio@redhat.com> 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.    
> > > 
> > > ...Laurent, let me know if I should dig into this any further.
> > > 
> > > For reference, the kernel commit introducing SO_PEEK_OFF support for TCP
> > > on IPv6 is be9a4fb831b8 ("tcp: add SO_PEEK_OFF socket option tor
> > > TCPv6"). Without that commit, passt won't set peek_offset_cap.
> > > 
> > > It was added in 6.11-rc5, so it's part of kernel-6.11.3-200.fc40 (latest
> > > stable kernel) for Fedora 40. passt will print "SO_PEEK_OFF supported"
> > > if you run it with -d -f.
> > >     
> > 
> > I have kernel 6.11.3-200.fc40.x86_64 but the message is "SO_PEEK_OFF not supported".
> > 
> > Any idea?  
> 
> Grr, sorry, I used 'git describe' wrong. That commit will be in 6.12
> (not released yet), it's not in 6.11.
> 
> For testing, you can force peek_offset_cap = true in tcp.c, as long as
> you don't use IPv6 (you can pass "-4" to passt just to be sure) it's
> fine.

Or... I didn't know about this until now:

  https://fedoraproject.org/wiki/RawhideKernelNodebug

as well as:

  https://fedoraproject.org/wiki/Kernel_Vanilla_Repositories
  https://docs.fedoraproject.org/en-US/releases/rawhide/#_questions_and_answers

and kernel-6.12.0-0.rc3.20241015giteca631b8fe80.32.fc42 surely includes
that commit:

  https://bodhi.fedoraproject.org/updates/FEDORA-2024-4f8beaeee0

-- 
Stefano