mbox series

[RFC,v1,0/6] virtio/vsock: introduce SOCK_DGRAM support

Message ID 20210609232501.171257-1-jiang.wang@bytedance.com (mailing list archive)
Headers show
Series virtio/vsock: introduce SOCK_DGRAM support | expand

Message

Jiang Wang . June 9, 2021, 11:24 p.m. UTC
This patchset implements support of SOCK_DGRAM for virtio
transport.

Datagram sockets are connectionless and unreliable. To avoid unfair contention
with stream and other sockets, add two more virtqueues and
a new feature bit to indicate if those two new queues exist or not.

Dgram does not use the existing credit update mechanism for
stream sockets. When sending from the guest/driver, sending packets 
synchronously, so the sender will get an error when the virtqueue is full.
When sending from the host/device, send packets asynchronously
because the descriptor memory belongs to the corresponding QEMU
process.

The virtio spec patch is here: 
https://www.spinics.net/lists/linux-virtualization/msg50027.html

For those who prefer git repo, here is the link for the linux kernel:
https://github.com/Jiang1155/linux/tree/vsock-dgram-v1

qemu patch link:
https://github.com/Jiang1155/qemu/tree/vsock-dgram-v1


To do:
1. use skb when receiving packets
2. support multiple transport
3. support mergeable rx buffer


Jiang Wang (6):
  virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
  virtio/vsock: add support for virtio datagram
  vhost/vsock: add support for vhost dgram.
  vsock_test: add tests for vsock dgram
  vhost/vsock: add kconfig for vhost dgram support
  virtio/vsock: add sysfs for rx buf len for dgram

 drivers/vhost/Kconfig                              |   8 +
 drivers/vhost/vsock.c                              | 207 ++++++++--
 include/linux/virtio_vsock.h                       |   9 +
 include/net/af_vsock.h                             |   1 +
 .../trace/events/vsock_virtio_transport_common.h   |   5 +-
 include/uapi/linux/virtio_vsock.h                  |   4 +
 net/vmw_vsock/af_vsock.c                           |  12 +
 net/vmw_vsock/virtio_transport.c                   | 433 ++++++++++++++++++---
 net/vmw_vsock/virtio_transport_common.c            | 184 ++++++++-
 tools/testing/vsock/util.c                         | 105 +++++
 tools/testing/vsock/util.h                         |   4 +
 tools/testing/vsock/vsock_test.c                   | 195 ++++++++++
 12 files changed, 1070 insertions(+), 97 deletions(-)

Comments

Jason Wang June 10, 2021, 1:50 a.m. UTC | #1
在 2021/6/10 上午7:24, Jiang Wang 写道:
> This patchset implements support of SOCK_DGRAM for virtio
> transport.
>
> Datagram sockets are connectionless and unreliable. To avoid unfair contention
> with stream and other sockets, add two more virtqueues and
> a new feature bit to indicate if those two new queues exist or not.
>
> Dgram does not use the existing credit update mechanism for
> stream sockets. When sending from the guest/driver, sending packets
> synchronously, so the sender will get an error when the virtqueue is full.
> When sending from the host/device, send packets asynchronously
> because the descriptor memory belongs to the corresponding QEMU
> process.


What's the use case for the datagram vsock?


>
> The virtio spec patch is here:
> https://www.spinics.net/lists/linux-virtualization/msg50027.html


Have a quick glance, I suggest to split mergeable rx buffer into an 
separate patch.

But I think it's time to revisit the idea of unifying the virtio-net and 
virtio-vsock. Otherwise we're duplicating features and bugs.

Thanks


>
> For those who prefer git repo, here is the link for the linux kernel:
> https://github.com/Jiang1155/linux/tree/vsock-dgram-v1
>
> qemu patch link:
> https://github.com/Jiang1155/qemu/tree/vsock-dgram-v1
>
>
> To do:
> 1. use skb when receiving packets
> 2. support multiple transport
> 3. support mergeable rx buffer
>
>
> Jiang Wang (6):
>    virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
>    virtio/vsock: add support for virtio datagram
>    vhost/vsock: add support for vhost dgram.
>    vsock_test: add tests for vsock dgram
>    vhost/vsock: add kconfig for vhost dgram support
>    virtio/vsock: add sysfs for rx buf len for dgram
>
>   drivers/vhost/Kconfig                              |   8 +
>   drivers/vhost/vsock.c                              | 207 ++++++++--
>   include/linux/virtio_vsock.h                       |   9 +
>   include/net/af_vsock.h                             |   1 +
>   .../trace/events/vsock_virtio_transport_common.h   |   5 +-
>   include/uapi/linux/virtio_vsock.h                  |   4 +
>   net/vmw_vsock/af_vsock.c                           |  12 +
>   net/vmw_vsock/virtio_transport.c                   | 433 ++++++++++++++++++---
>   net/vmw_vsock/virtio_transport_common.c            | 184 ++++++++-
>   tools/testing/vsock/util.c                         | 105 +++++
>   tools/testing/vsock/util.h                         |   4 +
>   tools/testing/vsock/vsock_test.c                   | 195 ++++++++++
>   12 files changed, 1070 insertions(+), 97 deletions(-)
>
Jiang Wang . June 10, 2021, 3:43 a.m. UTC | #2
On Wed, Jun 9, 2021 at 6:51 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/6/10 上午7:24, Jiang Wang 写道:
> > This patchset implements support of SOCK_DGRAM for virtio
> > transport.
> >
> > Datagram sockets are connectionless and unreliable. To avoid unfair contention
> > with stream and other sockets, add two more virtqueues and
> > a new feature bit to indicate if those two new queues exist or not.
> >
> > Dgram does not use the existing credit update mechanism for
> > stream sockets. When sending from the guest/driver, sending packets
> > synchronously, so the sender will get an error when the virtqueue is full.
> > When sending from the host/device, send packets asynchronously
> > because the descriptor memory belongs to the corresponding QEMU
> > process.
>
>
> What's the use case for the datagram vsock?
>
One use case is for non critical info logging from the guest
to the host, such as the performance data of some applications.

It can also be used to replace UDP communications between
the guest and the host.

> >
> > The virtio spec patch is here:
> > https://www.spinics.net/lists/linux-virtualization/msg50027.html
>
>
> Have a quick glance, I suggest to split mergeable rx buffer into an
> separate patch.

Sure.

> But I think it's time to revisit the idea of unifying the virtio-net and
> virtio-vsock. Otherwise we're duplicating features and bugs.

For mergeable rxbuf related code, I think a set of common helper
functions can be used by both virtio-net and virtio-vsock. For other
parts, that may not be very beneficial. I will think about more.

If there is a previous email discussion about this topic, could you send me
some links? I did a quick web search but did not find any related
info. Thanks.

> Thanks
>
>
> >
> > For those who prefer git repo, here is the link for the linux kernel:
> > https://github.com/Jiang1155/linux/tree/vsock-dgram-v1
> >
> > qemu patch link:
> > https://github.com/Jiang1155/qemu/tree/vsock-dgram-v1
> >
> >
> > To do:
> > 1. use skb when receiving packets
> > 2. support multiple transport
> > 3. support mergeable rx buffer
> >
> >
> > Jiang Wang (6):
> >    virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
> >    virtio/vsock: add support for virtio datagram
> >    vhost/vsock: add support for vhost dgram.
> >    vsock_test: add tests for vsock dgram
> >    vhost/vsock: add kconfig for vhost dgram support
> >    virtio/vsock: add sysfs for rx buf len for dgram
> >
> >   drivers/vhost/Kconfig                              |   8 +
> >   drivers/vhost/vsock.c                              | 207 ++++++++--
> >   include/linux/virtio_vsock.h                       |   9 +
> >   include/net/af_vsock.h                             |   1 +
> >   .../trace/events/vsock_virtio_transport_common.h   |   5 +-
> >   include/uapi/linux/virtio_vsock.h                  |   4 +
> >   net/vmw_vsock/af_vsock.c                           |  12 +
> >   net/vmw_vsock/virtio_transport.c                   | 433 ++++++++++++++++++---
> >   net/vmw_vsock/virtio_transport_common.c            | 184 ++++++++-
> >   tools/testing/vsock/util.c                         | 105 +++++
> >   tools/testing/vsock/util.h                         |   4 +
> >   tools/testing/vsock/vsock_test.c                   | 195 ++++++++++
> >   12 files changed, 1070 insertions(+), 97 deletions(-)
> >
>
Jason Wang June 10, 2021, 4:02 a.m. UTC | #3
在 2021/6/10 上午11:43, Jiang Wang . 写道:
> On Wed, Jun 9, 2021 at 6:51 PM Jason Wang <jasowang@redhat.com> wrote:
>>
>> 在 2021/6/10 上午7:24, Jiang Wang 写道:
>>> This patchset implements support of SOCK_DGRAM for virtio
>>> transport.
>>>
>>> Datagram sockets are connectionless and unreliable. To avoid unfair contention
>>> with stream and other sockets, add two more virtqueues and
>>> a new feature bit to indicate if those two new queues exist or not.
>>>
>>> Dgram does not use the existing credit update mechanism for
>>> stream sockets. When sending from the guest/driver, sending packets
>>> synchronously, so the sender will get an error when the virtqueue is full.
>>> When sending from the host/device, send packets asynchronously
>>> because the descriptor memory belongs to the corresponding QEMU
>>> process.
>>
>> What's the use case for the datagram vsock?
>>
> One use case is for non critical info logging from the guest
> to the host, such as the performance data of some applications.


Anything that prevents you from using the stream socket?


>
> It can also be used to replace UDP communications between
> the guest and the host.


Any advantage for VSOCK in this case? Is it for performance (I guess not 
since I don't exepct vsock will be faster).

An obvious drawback is that it breaks the migration. Using UDP you can 
have a very rich features support from the kernel where vsock can't.


>
>>> The virtio spec patch is here:
>>> https://www.spinics.net/lists/linux-virtualization/msg50027.html
>>
>> Have a quick glance, I suggest to split mergeable rx buffer into an
>> separate patch.
> Sure.
>
>> But I think it's time to revisit the idea of unifying the virtio-net and
>> virtio-vsock. Otherwise we're duplicating features and bugs.
> For mergeable rxbuf related code, I think a set of common helper
> functions can be used by both virtio-net and virtio-vsock. For other
> parts, that may not be very beneficial. I will think about more.
>
> If there is a previous email discussion about this topic, could you send me
> some links? I did a quick web search but did not find any related
> info. Thanks.


We had a lot:

[1] 
https://patchwork.kernel.org/project/kvm/patch/5BDFF537.3050806@huawei.com/
[2] 
https://lists.linuxfoundation.org/pipermail/virtualization/2018-November/039798.html
[3] https://www.lkml.org/lkml/2020/1/16/2043

Thanks

>
>> Thanks
>>
>>
>>> For those who prefer git repo, here is the link for the linux kernel:
>>> https://github.com/Jiang1155/linux/tree/vsock-dgram-v1
>>>
>>> qemu patch link:
>>> https://github.com/Jiang1155/qemu/tree/vsock-dgram-v1
>>>
>>>
>>> To do:
>>> 1. use skb when receiving packets
>>> 2. support multiple transport
>>> 3. support mergeable rx buffer
>>>
>>>
>>> Jiang Wang (6):
>>>     virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
>>>     virtio/vsock: add support for virtio datagram
>>>     vhost/vsock: add support for vhost dgram.
>>>     vsock_test: add tests for vsock dgram
>>>     vhost/vsock: add kconfig for vhost dgram support
>>>     virtio/vsock: add sysfs for rx buf len for dgram
>>>
>>>    drivers/vhost/Kconfig                              |   8 +
>>>    drivers/vhost/vsock.c                              | 207 ++++++++--
>>>    include/linux/virtio_vsock.h                       |   9 +
>>>    include/net/af_vsock.h                             |   1 +
>>>    .../trace/events/vsock_virtio_transport_common.h   |   5 +-
>>>    include/uapi/linux/virtio_vsock.h                  |   4 +
>>>    net/vmw_vsock/af_vsock.c                           |  12 +
>>>    net/vmw_vsock/virtio_transport.c                   | 433 ++++++++++++++++++---
>>>    net/vmw_vsock/virtio_transport_common.c            | 184 ++++++++-
>>>    tools/testing/vsock/util.c                         | 105 +++++
>>>    tools/testing/vsock/util.h                         |   4 +
>>>    tools/testing/vsock/vsock_test.c                   | 195 ++++++++++
>>>    12 files changed, 1070 insertions(+), 97 deletions(-)
>>>
Stefano Garzarella June 10, 2021, 7:23 a.m. UTC | #4
On Thu, Jun 10, 2021 at 12:02:35PM +0800, Jason Wang wrote:
>
>在 2021/6/10 上午11:43, Jiang Wang . 写道:
>>On Wed, Jun 9, 2021 at 6:51 PM Jason Wang <jasowang@redhat.com> wrote:
>>>
>>>在 2021/6/10 上午7:24, Jiang Wang 写道:
>>>>This patchset implements support of SOCK_DGRAM for virtio
>>>>transport.
>>>>
>>>>Datagram sockets are connectionless and unreliable. To avoid unfair contention
>>>>with stream and other sockets, add two more virtqueues and
>>>>a new feature bit to indicate if those two new queues exist or not.
>>>>
>>>>Dgram does not use the existing credit update mechanism for
>>>>stream sockets. When sending from the guest/driver, sending packets
>>>>synchronously, so the sender will get an error when the virtqueue is 
>>>>full.
>>>>When sending from the host/device, send packets asynchronously
>>>>because the descriptor memory belongs to the corresponding QEMU
>>>>process.
>>>
>>>What's the use case for the datagram vsock?
>>>
>>One use case is for non critical info logging from the guest
>>to the host, such as the performance data of some applications.
>
>
>Anything that prevents you from using the stream socket?
>
>
>>
>>It can also be used to replace UDP communications between
>>the guest and the host.
>
>
>Any advantage for VSOCK in this case? Is it for performance (I guess 
>not since I don't exepct vsock will be faster).

I think the general advantage to using vsock are for the guest agents 
that potentially don't need any configuration.

>
>An obvious drawback is that it breaks the migration. Using UDP you can 
>have a very rich features support from the kernel where vsock can't.
>

Thanks for bringing this up!
What features does UDP support and datagram on vsock could not support?

>
>>
>>>>The virtio spec patch is here:
>>>>https://www.spinics.net/lists/linux-virtualization/msg50027.html
>>>
>>>Have a quick glance, I suggest to split mergeable rx buffer into an
>>>separate patch.
>>Sure.
>>
>>>But I think it's time to revisit the idea of unifying the virtio-net 
>>>and
>>>virtio-vsock. Otherwise we're duplicating features and bugs.
>>For mergeable rxbuf related code, I think a set of common helper
>>functions can be used by both virtio-net and virtio-vsock. For other
>>parts, that may not be very beneficial. I will think about more.
>>
>>If there is a previous email discussion about this topic, could you 
>>send me
>>some links? I did a quick web search but did not find any related
>>info. Thanks.
>
>
>We had a lot:
>
>[1] 
>https://patchwork.kernel.org/project/kvm/patch/5BDFF537.3050806@huawei.com/
>[2] 
>https://lists.linuxfoundation.org/pipermail/virtualization/2018-November/039798.html
>[3] https://www.lkml.org/lkml/2020/1/16/2043
>

When I tried it, the biggest problem that blocked me were all the 
features strictly related to TCP/IP stack and ethernet devices that 
vsock device doesn't know how to handle: TSO, GSO, checksums, MAC, napi, 
xdp, min ethernet frame size, MTU, etc.

So in my opinion to unify them is not so simple, because vsock is not 
really an ethernet device, but simply a socket.

But I fully agree that we shouldn't duplicate functionality and code, so 
maybe we could find those common parts and create helpers to be used by 
both.

Thanks,
Stefano
Jason Wang June 10, 2021, 7:46 a.m. UTC | #5
在 2021/6/10 下午3:23, Stefano Garzarella 写道:
> On Thu, Jun 10, 2021 at 12:02:35PM +0800, Jason Wang wrote:
>>
>> 在 2021/6/10 上午11:43, Jiang Wang . 写道:
>>> On Wed, Jun 9, 2021 at 6:51 PM Jason Wang <jasowang@redhat.com> wrote:
>>>>
>>>> 在 2021/6/10 上午7:24, Jiang Wang 写道:
>>>>> This patchset implements support of SOCK_DGRAM for virtio
>>>>> transport.
>>>>>
>>>>> Datagram sockets are connectionless and unreliable. To avoid 
>>>>> unfair contention
>>>>> with stream and other sockets, add two more virtqueues and
>>>>> a new feature bit to indicate if those two new queues exist or not.
>>>>>
>>>>> Dgram does not use the existing credit update mechanism for
>>>>> stream sockets. When sending from the guest/driver, sending packets
>>>>> synchronously, so the sender will get an error when the virtqueue 
>>>>> is full.
>>>>> When sending from the host/device, send packets asynchronously
>>>>> because the descriptor memory belongs to the corresponding QEMU
>>>>> process.
>>>>
>>>> What's the use case for the datagram vsock?
>>>>
>>> One use case is for non critical info logging from the guest
>>> to the host, such as the performance data of some applications.
>>
>>
>> Anything that prevents you from using the stream socket?
>>
>>
>>>
>>> It can also be used to replace UDP communications between
>>> the guest and the host.
>>
>>
>> Any advantage for VSOCK in this case? Is it for performance (I guess 
>> not since I don't exepct vsock will be faster).
>
> I think the general advantage to using vsock are for the guest agents 
> that potentially don't need any configuration.


Right, I wonder if we really need datagram consider the host to guest 
communication is reliable.

(Note that I don't object it since vsock has already supported that, 
just wonder its use cases)


>
>>
>> An obvious drawback is that it breaks the migration. Using UDP you 
>> can have a very rich features support from the kernel where vsock can't.
>>
>
> Thanks for bringing this up!
> What features does UDP support and datagram on vsock could not support?


E.g the sendpage() and busy polling. And using UDP means qdiscs and eBPF 
can work.


>
>>
>>>
>>>>> The virtio spec patch is here:
>>>>> https://www.spinics.net/lists/linux-virtualization/msg50027.html
>>>>
>>>> Have a quick glance, I suggest to split mergeable rx buffer into an
>>>> separate patch.
>>> Sure.
>>>
>>>> But I think it's time to revisit the idea of unifying the 
>>>> virtio-net and
>>>> virtio-vsock. Otherwise we're duplicating features and bugs.
>>> For mergeable rxbuf related code, I think a set of common helper
>>> functions can be used by both virtio-net and virtio-vsock. For other
>>> parts, that may not be very beneficial. I will think about more.
>>>
>>> If there is a previous email discussion about this topic, could you 
>>> send me
>>> some links? I did a quick web search but did not find any related
>>> info. Thanks.
>>
>>
>> We had a lot:
>>
>> [1] 
>> https://patchwork.kernel.org/project/kvm/patch/5BDFF537.3050806@huawei.com/
>> [2] 
>> https://lists.linuxfoundation.org/pipermail/virtualization/2018-November/039798.html
>> [3] https://www.lkml.org/lkml/2020/1/16/2043
>>
>
> When I tried it, the biggest problem that blocked me were all the 
> features strictly related to TCP/IP stack and ethernet devices that 
> vsock device doesn't know how to handle: TSO, GSO, checksums, MAC, 
> napi, xdp, min ethernet frame size, MTU, etc.


It depends on which level we want to share:

1) sharing codes
2) sharing devices
3) make vsock a protocol that is understood by the network core

We can start from 1), the low level tx/rx logic can be shared at both 
virtio-net and vhost-net. For 2) we probably need some work on the spec, 
probably with a new feature bit to demonstrate that it's a vsock device 
not a ethernet device. Then if it is probed as a vsock device we won't 
let packet to be delivered in the TCP/IP stack. For 3), it would be even 
harder and I'm not sure it's worth to do that.


>
> So in my opinion to unify them is not so simple, because vsock is not 
> really an ethernet device, but simply a socket.


We can start from sharing codes.


>
> But I fully agree that we shouldn't duplicate functionality and code, 
> so maybe we could find those common parts and create helpers to be 
> used by both.


Yes.

Thanks


>
> Thanks,
> Stefano
>
Stefano Garzarella June 10, 2021, 9:51 a.m. UTC | #6
On Thu, Jun 10, 2021 at 03:46:55PM +0800, Jason Wang wrote:
>
>在 2021/6/10 下午3:23, Stefano Garzarella 写道:
>>On Thu, Jun 10, 2021 at 12:02:35PM +0800, Jason Wang wrote:
>>>
>>>在 2021/6/10 上午11:43, Jiang Wang . 写道:
>>>>On Wed, Jun 9, 2021 at 6:51 PM Jason Wang <jasowang@redhat.com> wrote:
>>>>>
>>>>>在 2021/6/10 上午7:24, Jiang Wang 写道:
>>>>>>This patchset implements support of SOCK_DGRAM for virtio
>>>>>>transport.
>>>>>>
>>>>>>Datagram sockets are connectionless and unreliable. To avoid 
>>>>>>unfair contention
>>>>>>with stream and other sockets, add two more virtqueues and
>>>>>>a new feature bit to indicate if those two new queues exist or not.
>>>>>>
>>>>>>Dgram does not use the existing credit update mechanism for
>>>>>>stream sockets. When sending from the guest/driver, sending packets
>>>>>>synchronously, so the sender will get an error when the 
>>>>>>virtqueue is full.
>>>>>>When sending from the host/device, send packets asynchronously
>>>>>>because the descriptor memory belongs to the corresponding QEMU
>>>>>>process.
>>>>>
>>>>>What's the use case for the datagram vsock?
>>>>>
>>>>One use case is for non critical info logging from the guest
>>>>to the host, such as the performance data of some applications.
>>>
>>>
>>>Anything that prevents you from using the stream socket?
>>>
>>>
>>>>
>>>>It can also be used to replace UDP communications between
>>>>the guest and the host.
>>>
>>>
>>>Any advantage for VSOCK in this case? Is it for performance (I 
>>>guess not since I don't exepct vsock will be faster).
>>
>>I think the general advantage to using vsock are for the guest 
>>agents that potentially don't need any configuration.
>
>
>Right, I wonder if we really need datagram consider the host to guest 
>communication is reliable.
>
>(Note that I don't object it since vsock has already supported that, 
>just wonder its use cases)

Yep, it was the same concern I had :-)
Also because we're now adding SEQPACKET, which provides reliable 
datagram support.

But IIUC the use case is the logging where you don't need a reliable 
communication and you want to avoid to keep more open connections with 
different guests.

So the server in the host can be pretty simple and doesn't have to 
handle connections. It just waits for datagrams on a port.

>
>
>>
>>>
>>>An obvious drawback is that it breaks the migration. Using UDP you 
>>>can have a very rich features support from the kernel where vsock 
>>>can't.
>>>
>>
>>Thanks for bringing this up!
>>What features does UDP support and datagram on vsock could not support?
>
>
>E.g the sendpage() and busy polling. And using UDP means qdiscs and 
>eBPF can work.

Thanks, I see!

>
>
>>
>>>
>>>>
>>>>>>The virtio spec patch is here:
>>>>>>https://www.spinics.net/lists/linux-virtualization/msg50027.html
>>>>>
>>>>>Have a quick glance, I suggest to split mergeable rx buffer into an
>>>>>separate patch.
>>>>Sure.
>>>>
>>>>>But I think it's time to revisit the idea of unifying the 
>>>>>virtio-net and
>>>>>virtio-vsock. Otherwise we're duplicating features and bugs.
>>>>For mergeable rxbuf related code, I think a set of common helper
>>>>functions can be used by both virtio-net and virtio-vsock. For other
>>>>parts, that may not be very beneficial. I will think about more.
>>>>
>>>>If there is a previous email discussion about this topic, could 
>>>>you send me
>>>>some links? I did a quick web search but did not find any related
>>>>info. Thanks.
>>>
>>>
>>>We had a lot:
>>>
>>>[1] https://patchwork.kernel.org/project/kvm/patch/5BDFF537.3050806@huawei.com/
>>>[2] https://lists.linuxfoundation.org/pipermail/virtualization/2018-November/039798.html
>>>[3] https://www.lkml.org/lkml/2020/1/16/2043
>>>
>>
>>When I tried it, the biggest problem that blocked me were all the 
>>features strictly related to TCP/IP stack and ethernet devices that 
>>vsock device doesn't know how to handle: TSO, GSO, checksums, MAC, 
>>napi, xdp, min ethernet frame size, MTU, etc.
>
>
>It depends on which level we want to share:
>
>1) sharing codes
>2) sharing devices
>3) make vsock a protocol that is understood by the network core
>
>We can start from 1), the low level tx/rx logic can be shared at both 
>virtio-net and vhost-net. For 2) we probably need some work on the 
>spec, probably with a new feature bit to demonstrate that it's a vsock 
>device not a ethernet device. Then if it is probed as a vsock device we 
>won't let packet to be delivered in the TCP/IP stack. For 3), it would 
>be even harder and I'm not sure it's worth to do that.
>
>
>>
>>So in my opinion to unify them is not so simple, because vsock is not 
>>really an ethernet device, but simply a socket.
>
>
>We can start from sharing codes.

Yep, I agree, and maybe the mergeable buffer is a good starting point to 
share code!

@Jiang, do you want to take a look of this possibility?

Thanks,
Stefano
Jiang Wang . June 10, 2021, 4:44 p.m. UTC | #7
On Thu, Jun 10, 2021 at 2:52 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Thu, Jun 10, 2021 at 03:46:55PM +0800, Jason Wang wrote:
> >
> >在 2021/6/10 下午3:23, Stefano Garzarella 写道:
> >>On Thu, Jun 10, 2021 at 12:02:35PM +0800, Jason Wang wrote:
> >>>
> >>>在 2021/6/10 上午11:43, Jiang Wang . 写道:
> >>>>On Wed, Jun 9, 2021 at 6:51 PM Jason Wang <jasowang@redhat.com> wrote:
> >>>>>
> >>>>>在 2021/6/10 上午7:24, Jiang Wang 写道:
> >>>>>>This patchset implements support of SOCK_DGRAM for virtio
> >>>>>>transport.
> >>>>>>
> >>>>>>Datagram sockets are connectionless and unreliable. To avoid
> >>>>>>unfair contention
> >>>>>>with stream and other sockets, add two more virtqueues and
> >>>>>>a new feature bit to indicate if those two new queues exist or not.
> >>>>>>
> >>>>>>Dgram does not use the existing credit update mechanism for
> >>>>>>stream sockets. When sending from the guest/driver, sending packets
> >>>>>>synchronously, so the sender will get an error when the
> >>>>>>virtqueue is full.
> >>>>>>When sending from the host/device, send packets asynchronously
> >>>>>>because the descriptor memory belongs to the corresponding QEMU
> >>>>>>process.
> >>>>>
> >>>>>What's the use case for the datagram vsock?
> >>>>>
> >>>>One use case is for non critical info logging from the guest
> >>>>to the host, such as the performance data of some applications.
> >>>
> >>>
> >>>Anything that prevents you from using the stream socket?
> >>>
> >>>
> >>>>
> >>>>It can also be used to replace UDP communications between
> >>>>the guest and the host.
> >>>
> >>>
> >>>Any advantage for VSOCK in this case? Is it for performance (I
> >>>guess not since I don't exepct vsock will be faster).
> >>
> >>I think the general advantage to using vsock are for the guest
> >>agents that potentially don't need any configuration.
> >
> >
> >Right, I wonder if we really need datagram consider the host to guest
> >communication is reliable.
> >
> >(Note that I don't object it since vsock has already supported that,
> >just wonder its use cases)
>
> Yep, it was the same concern I had :-)
> Also because we're now adding SEQPACKET, which provides reliable
> datagram support.
>
> But IIUC the use case is the logging where you don't need a reliable
> communication and you want to avoid to keep more open connections with
> different guests.
>
> So the server in the host can be pretty simple and doesn't have to
> handle connections. It just waits for datagrams on a port.

Yes. With datagram sockets, the application code is simpler than the stream
sockets. Also, it will be easier to port existing applications written
for dgram,
such as UDP or unix domain socket with datagram types to the vsock
dgram sockets.

Compared to UDP, the vsock dgram has a minimum configuration. When
sending data from the guest to the host, the client in the guest knows
the host CID will always be 2. For UDP, the host IP may change depending
on the configuration.

The advantage over UNIX domain sockets is more obvious. We
have some applications talking to each other with UNIX domain sockets,
but now the applications are running inside VMs, so we will need to
use vsock and those applications use datagram types, so it is natural
and simpler if vsock has datagram types too.

And we can also run applications for vmware vsock dgram on
the QEMU directly.

btw, SEQPACKET also supports datagram, but the application code
logic is similar to stream sockets and the server needs to maintain
connections.

> >
> >
> >>
> >>>
> >>>An obvious drawback is that it breaks the migration. Using UDP you
> >>>can have a very rich features support from the kernel where vsock
> >>>can't.
> >>>
> >>
> >>Thanks for bringing this up!
> >>What features does UDP support and datagram on vsock could not support?
> >
> >
> >E.g the sendpage() and busy polling. And using UDP means qdiscs and
> >eBPF can work.
>
> Thanks, I see!
>
> >
> >
> >>
> >>>
> >>>>
> >>>>>>The virtio spec patch is here:
> >>>>>>https://www.spinics.net/lists/linux-virtualization/msg50027.html
> >>>>>
> >>>>>Have a quick glance, I suggest to split mergeable rx buffer into an
> >>>>>separate patch.
> >>>>Sure.
> >>>>
> >>>>>But I think it's time to revisit the idea of unifying the
> >>>>>virtio-net and
> >>>>>virtio-vsock. Otherwise we're duplicating features and bugs.
> >>>>For mergeable rxbuf related code, I think a set of common helper
> >>>>functions can be used by both virtio-net and virtio-vsock. For other
> >>>>parts, that may not be very beneficial. I will think about more.
> >>>>
> >>>>If there is a previous email discussion about this topic, could
> >>>>you send me
> >>>>some links? I did a quick web search but did not find any related
> >>>>info. Thanks.
> >>>
> >>>
> >>>We had a lot:
> >>>
> >>>[1] https://patchwork.kernel.org/project/kvm/patch/5BDFF537.3050806@huawei.com/
> >>>[2] https://lists.linuxfoundation.org/pipermail/virtualization/2018-November/039798.html
> >>>[3] https://www.lkml.org/lkml/2020/1/16/2043
> >>>
Got it. I will check, thanks.

> >>When I tried it, the biggest problem that blocked me were all the
> >>features strictly related to TCP/IP stack and ethernet devices that
> >>vsock device doesn't know how to handle: TSO, GSO, checksums, MAC,
> >>napi, xdp, min ethernet frame size, MTU, etc.
> >
> >
> >It depends on which level we want to share:
> >
> >1) sharing codes
> >2) sharing devices
> >3) make vsock a protocol that is understood by the network core
> >
> >We can start from 1), the low level tx/rx logic can be shared at both
> >virtio-net and vhost-net. For 2) we probably need some work on the
> >spec, probably with a new feature bit to demonstrate that it's a vsock
> >device not a ethernet device. Then if it is probed as a vsock device we
> >won't let packet to be delivered in the TCP/IP stack. For 3), it would
> >be even harder and I'm not sure it's worth to do that.
> >
> >
> >>
> >>So in my opinion to unify them is not so simple, because vsock is not
> >>really an ethernet device, but simply a socket.
> >
> >
> >We can start from sharing codes.
>
> Yep, I agree, and maybe the mergeable buffer is a good starting point to
> share code!
>
> @Jiang, do you want to take a look of this possibility?

Yes. I already read code about mergeable buffer in virtio-net, which I think
is the only place so far to use it. I will check how to share the code.

Thanks for all the comments.

> Thanks,
> Stefano
>
Stefano Garzarella June 18, 2021, 9:35 a.m. UTC | #8
On Wed, Jun 09, 2021 at 11:24:52PM +0000, Jiang Wang wrote:
>This patchset implements support of SOCK_DGRAM for virtio
>transport.
>
>Datagram sockets are connectionless and unreliable. To avoid unfair contention
>with stream and other sockets, add two more virtqueues and
>a new feature bit to indicate if those two new queues exist or not.
>
>Dgram does not use the existing credit update mechanism for
>stream sockets. When sending from the guest/driver, sending packets
>synchronously, so the sender will get an error when the virtqueue is full.
>When sending from the host/device, send packets asynchronously
>because the descriptor memory belongs to the corresponding QEMU
>process.
>
>The virtio spec patch is here:
>https://www.spinics.net/lists/linux-virtualization/msg50027.html
>
>For those who prefer git repo, here is the link for the linux kernel:
>https://github.com/Jiang1155/linux/tree/vsock-dgram-v1
>
>qemu patch link:
>https://github.com/Jiang1155/qemu/tree/vsock-dgram-v1
>
>
>To do:
>1. use skb when receiving packets
>2. support multiple transport
>3. support mergeable rx buffer

Jiang, I'll do a fast review, but I think is better to rebase on 
net-next since SEQPACKET support is now merged.

Please also run ./scripts/checkpatch.pl, there are a lot of issues.

I'll leave some simple comments in the patches, but I prefer to do a 
deep review after the rebase and the dynamic handling of DGRAM.

Thanks,
Stefano
Jiang Wang . June 21, 2021, 5:21 p.m. UTC | #9
On Fri, Jun 18, 2021 at 2:35 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Wed, Jun 09, 2021 at 11:24:52PM +0000, Jiang Wang wrote:
> >This patchset implements support of SOCK_DGRAM for virtio
> >transport.
> >
> >Datagram sockets are connectionless and unreliable. To avoid unfair contention
> >with stream and other sockets, add two more virtqueues and
> >a new feature bit to indicate if those two new queues exist or not.
> >
> >Dgram does not use the existing credit update mechanism for
> >stream sockets. When sending from the guest/driver, sending packets
> >synchronously, so the sender will get an error when the virtqueue is full.
> >When sending from the host/device, send packets asynchronously
> >because the descriptor memory belongs to the corresponding QEMU
> >process.
> >
> >The virtio spec patch is here:
> >https://www.spinics.net/lists/linux-virtualization/msg50027.html
> >
> >For those who prefer git repo, here is the link for the linux kernel:
> >https://github.com/Jiang1155/linux/tree/vsock-dgram-v1
> >
> >qemu patch link:
> >https://github.com/Jiang1155/qemu/tree/vsock-dgram-v1
> >
> >
> >To do:
> >1. use skb when receiving packets
> >2. support multiple transport
> >3. support mergeable rx buffer
>
> Jiang, I'll do a fast review, but I think is better to rebase on
> net-next since SEQPACKET support is now merged.
>
> Please also run ./scripts/checkpatch.pl, there are a lot of issues.
>
> I'll leave some simple comments in the patches, but I prefer to do a
> deep review after the rebase and the dynamic handling of DGRAM.

Hi Stefano,

Sure. I will rebase and add dynamic handling of DGRAM. I run checkpatch.pl
at some point but I will make sure to run it again before submitting. Thanks.

Regards,

Jiang


> Thanks,
> Stefano
>