Virtualization - Jul 2019 - [PATCH v4 4/5] vhost/vsock: split packets to send using multiple buffers

On Thu, Jul 18, 2019 at 09:50:14AM +0200, Stefano Garzarella
wrote:
> On Wed, Jul 17, 2019 at 4:55 PM Michael S. Tsirkin <mst at
redhat.com> wrote:
> >
> > On Wed, Jul 17, 2019 at 01:30:29PM +0200, Stefano Garzarella wrote:
> > > If the packets to sent to the guest are bigger than the buffer
> > > available, we can split them, using multiple buffers and fixing
> > > the length in the packet header.
> > > This is safe since virtio-vsock supports only stream sockets.
> > >
> > > Signed-off-by: Stefano Garzarella <sgarzare at redhat.com>
> >
> > So how does it work right now? If an app
> > does sendmsg with a 64K buffer and the other
> > side publishes 4K buffers - does it just stall?
> 
> Before this series, the 64K (or bigger) user messages was split in 4K
packets
> (fixed in the code) and queued in an internal list for the TX worker.
> 
> After this series, we will queue up to 64K packets and then it will be
split in
> the TX worker, depending on the size of the buffers available in the
> vring. (The idea was to allow EWMA or a configuration of the buffers size,
but
> for now we postponed it)
Got it. Using workers for xmit is IMHO a bad idea btw.
Why is it done like this?
> Note: virtio-vsock only supports stream socket for now.
> 
> Thanks,
> Stefano

On Thu, Jul 18, 2019 at 10:13 AM Michael S. Tsirkin <mst at redhat.com>
wrote:
> On Thu, Jul 18, 2019 at 09:50:14AM +0200, Stefano Garzarella wrote:
> > On Wed, Jul 17, 2019 at 4:55 PM Michael S. Tsirkin <mst at
redhat.com> wrote:
> > > On Wed, Jul 17, 2019 at 01:30:29PM +0200, Stefano Garzarella
wrote:
> > > > If the packets to sent to the guest are bigger than the
buffer
> > > > available, we can split them, using multiple buffers and
fixing
> > > > the length in the packet header.
> > > > This is safe since virtio-vsock supports only stream
sockets.
> > > >
> > > > Signed-off-by: Stefano Garzarella <sgarzare at
redhat.com>
> > >
> > > So how does it work right now? If an app
> > > does sendmsg with a 64K buffer and the other
> > > side publishes 4K buffers - does it just stall?
> >
> > Before this series, the 64K (or bigger) user messages was split in 4K
packets
> > (fixed in the code) and queued in an internal list for the TX worker.
> >
> > After this series, we will queue up to 64K packets and then it will be
split in
> > the TX worker, depending on the size of the buffers available in the
> > vring. (The idea was to allow EWMA or a configuration of the buffers
size, but
> > for now we postponed it)
>
> Got it. Using workers for xmit is IMHO a bad idea btw.
> Why is it done like this?
Honestly, I don't know the exact reasons for this design, but I suppose
that the idea was to have only one worker that uses the vring, and
multiple user threads that enqueue packets in the list.
This can simplify the code and we can put the user threads to sleep if
we don't have "credit" available (this means that the receiver
doesn't
have space to receive the packet).

What are the drawbacks in your opinion?


Thanks,
Stefano

On Thu, Jul 18, 2019 at 11:37:30AM +0200, Stefano Garzarella
wrote:
> On Thu, Jul 18, 2019 at 10:13 AM Michael S. Tsirkin <mst at
redhat.com> wrote:
> > On Thu, Jul 18, 2019 at 09:50:14AM +0200, Stefano Garzarella wrote:
> > > On Wed, Jul 17, 2019 at 4:55 PM Michael S. Tsirkin <mst at
redhat.com> wrote:
> > > > On Wed, Jul 17, 2019 at 01:30:29PM +0200, Stefano Garzarella
wrote:
> > > > > If the packets to sent to the guest are bigger than the
buffer
> > > > > available, we can split them, using multiple buffers
and fixing
> > > > > the length in the packet header.
> > > > > This is safe since virtio-vsock supports only stream
sockets.
> > > > >
> > > > > Signed-off-by: Stefano Garzarella <sgarzare at
redhat.com>
> > > >
> > > > So how does it work right now? If an app
> > > > does sendmsg with a 64K buffer and the other
> > > > side publishes 4K buffers - does it just stall?
> > >
> > > Before this series, the 64K (or bigger) user messages was split
in 4K packets
> > > (fixed in the code) and queued in an internal list for the TX
worker.
> > >
> > > After this series, we will queue up to 64K packets and then it
will be split in
> > > the TX worker, depending on the size of the buffers available in
the
> > > vring. (The idea was to allow EWMA or a configuration of the
buffers size, but
> > > for now we postponed it)
> >
> > Got it. Using workers for xmit is IMHO a bad idea btw.
> > Why is it done like this?
> 
> Honestly, I don't know the exact reasons for this design, but I suppose
> that the idea was to have only one worker that uses the vring, and
> multiple user threads that enqueue packets in the list.
> This can simplify the code and we can put the user threads to sleep if
> we don't have "credit" available (this means that the
receiver doesn't
> have space to receive the packet).

I think you mean the reverse: even without credits you can copy from
user and queue up data, then process it without waking up the user
thread.
Does it help though? It certainly adds up work outside of
user thread context which means it's not accounted for
correctly.

Maybe we want more VQs. Would help improve parallelism. The question
would then become how to map sockets to VQs. With a simple hash
it's easy to create collisions ...

> 
> What are the drawbacks in your opinion?
> 
> 
> Thanks,
> Stefano
- More pressure on scheduler
- Increased latency


-- 
MST