Re: [dpdk-dev] [RFC 00/24] vhost: add virtio-vhost-user transport

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi Stefan,

I went through the series, and I really like how it fits with existing
vhost-user over af_unix socket.

I will post the change I suggested to fix the regression met with SPDK
and multiqueue.

Thanks,
Maxime

On 01/19/2018 02:44 PM, Stefan Hajnoczi wrote:
> This patch series implements the virtio-vhost-user device, which tunnels
> vhost-user protocol messages over virtio.  This lets guests act as vhost device
> backends for other guests.
> 
> The virtio-vhost-user device is the result of discussion about Wei Wang and
> Zhiyong Yang's vhost-pci device.  This patch series demonstrates that vhost
> device backends, such as the vhost vdev driver, can work over both AF_UNIX and
> virtio-vhost-user without significant modifications.  This allows a lot of code
> to be shared between traditional AF_UNIX vhost-user and virtio-vhost-user.  The
> vhost-pci patches duplicated the vhost net device backend and didn't reuse
> librte_vhost:
> 
>    http://dpdk.org/ml/archives/dev/2017-November/082615.html
> 
> User-visible changes
> --------------------
> The vhost vdev can now be used when DPDK runs inside a guest with a
> virtio-vhost-user PCI device:
> 
>    --vdev net_vhost0,iface="0000:00:04.0",virtio-transport=1
> 
> The vhost-scsi example has also been extended to support virtio-vhost-user:
> 
>    ./vhost-scsi ... -- --virtio-vhost-user-pci "0000:00:04.0"
> 
> For more information (including instructions for running the code), see
> https://wiki.qemu.org/Features/VirtioVhostUser
> 
> Virtio device design
> --------------------
> The virtio-vhost-user device is a new virtio device type.  It acts as a
> vhost-user transport and is an alternative for the traditional AF_UNIX
> transport.
> 
> You can find the virtio-vhost-user VIRTIO device specification here:
> https://stefanha.github.io/virtio/vhost-user-slave.html#x1-2830007
> 
> librte_vhost API changes
> ------------------------
> This patch series extends librte_vhost so that it now accepts:
> 
>    rte_vhost_driver_register("0000:00:04.0",
>                              RTE_VHOST_USER_VIRTIO_TRANSPORT);
> 
> All other librte_vhost API usage remains unchanged, except that the file
> descriptors exposed in some <rte_vhost.h> structs will be -1 since there is no
> file descriptor passing involved.
> 
> This makes it extremely easy to support virtio-vhost-user in existing vhost
> device backends!  I have extended the vhost vdev driver and the vhost-scsi
> example application in this patch series.
> 
> Patch series overview
> ---------------------
> This series is based on commit 814339ba7eea13d132508af2cccec2f73568e2d0 from
> dpdk-next-virtio/master.  You can also get my git branch here:
> 
>    https://github.com/stefanha/dpdk/tree/virtio-vhost-user
> 
> The initial patches refactor librte_vhost so that AF_UNIX-specific code is moved
> to a new trans_af_unix.c file.  This also introduces a struct
> vhost_transport_ops interface that all transports will implement:
> 
>    adf412c3c vhost: move vring_call() into trans_af_unix.c
>    04079a077 vhost: move AF_UNIX code from socket.c to trans_af_unix.c
>    885642091 vhost: allocate per-socket transport state
>    9c48377df vhost: move socket_fd and un sockaddr into trans_af_unix.c
>    c646e6292 vhost: move start_server/client() calls to trans_af_unix.c
>    db07ef7a8 vhost: move vhost_user_connection to trans_af_unix.c
>    d10b80163 vhost: move vhost_user_reconnect_init() into trans_af_unix.c
>    1258bcd68 vhost: move vhost_user.fdset to trans_af_unix.c
>    bc7f6d7ab vhost: pass vhost_transport_ops through vhost_new_device()
>    b82187a29 vhost: embed struct virtio_net inside struct vhost_user_connection
>    abbd544f2 vhost: extract vhost_user.c socket I/O into transport
>    0658d711b vhost: move slave_req_fd field to AF_UNIX transport
>    e2ecf78ed vhost: move mmap/munmap to AF_UNIX transport
> 
> I was about to add the virtio-vhost-user PCI driver when I realized that
> lib/librte_vhost/ cannot have a dependency on rte_bus_pci.h (it lives in
> drivers/).  The solution I chose is to move all of librte_vhost to
> drivers/librte_vhost/, but I'm open to suggestions if this is undesirable.
> 
>    8615c2140 vhost: move librte_vhost to drivers/
> 
> Since virtio-vhost-user is a virtio device it's necessary to perform a sequence
> of device initialization steps and set up virtqueues.  I didn't see a virtio
> API in DPDK and didn't have time to create one myself.  I copied the virtio
> code from drivers/net/virtio/ as a quick hack, but really there needs to be a
> librte_virtio that is shared.
> 
>    c26937c66 vhost: add virtio pci framework
> 
> Next the virtio-vhost-user transport is added along with the
> RTE_VHOST_USER_VIRTIO_TRANSPORT flag:
> 
>    7fad5adc4 vhost: remember a vhost_virtqueue's queue index
>    d26922892 vhost: add virtio-vhost-user transport
>    f562a70ab vhost: add RTE_VHOST_USER_VIRTIO_TRANSPORT flag
> 
> Then I extended the vhost vdev and vhost-scsi example application to support
> virtio-vhost-user:
> 
>    47434f2a2 net/vhost: add virtio-vhost-user support
>    22ca05d1b examples/vhost_scsi: add --socket-file argument
>    db3b391dc examples/vhost_scsi: add virtio-vhost-user support
> 
> It was also necessary to tweak dpdk-devbind.py to support virtio-vhost-user:
> 
>    4aee6f653 usertools: add virtio-vhost-user devices to dpdk-devbind.py
> 
> Finally, vhost-scsi seems broken to me so two workarounds were needed so it can
> be tested again:
> 
>    b9d17bfaf WORKAROUND revert virtio-net mq vring deletion
>    cadb25e7d WORKAROUND examples/vhost_scsi: avoid broken EVENT_IDX
> 
> Stefan Hajnoczi (24):
>    vhost: move vring_call() into trans_af_unix.c
>    vhost: move AF_UNIX code from socket.c to trans_af_unix.c
>    vhost: allocate per-socket transport state
>    vhost: move socket_fd and un sockaddr into trans_af_unix.c
>    vhost: move start_server/client() calls to trans_af_unix.c
>    vhost: move vhost_user_connection to trans_af_unix.c
>    vhost: move vhost_user_reconnect_init() into trans_af_unix.c
>    vhost: move vhost_user.fdset to trans_af_unix.c
>    vhost: pass vhost_transport_ops through vhost_new_device()
>    vhost: embed struct virtio_net inside struct vhost_user_connection
>    vhost: extract vhost_user.c socket I/O into transport
>    vhost: move slave_req_fd field to AF_UNIX transport
>    vhost: move mmap/munmap to AF_UNIX transport
>    vhost: move librte_vhost to drivers/
>    vhost: add virtio pci framework
>    vhost: remember a vhost_virtqueue's queue index
>    vhost: add virtio-vhost-user transport
>    vhost: add RTE_VHOST_USER_VIRTIO_TRANSPORT flag
>    net/vhost: add virtio-vhost-user support
>    examples/vhost_scsi: add --socket-file argument
>    examples/vhost_scsi: add virtio-vhost-user support
>    usertools: add virtio-vhost-user devices to dpdk-devbind.py
>    WORKAROUND revert virtio-net mq vring deletion
>    WORKAROUND examples/vhost_scsi: avoid broken EVENT_IDX
> 
>   drivers/Makefile                                   |    2 +
>   {lib => drivers}/librte_vhost/Makefile             |    5 +-
>   lib/Makefile                                       |    3 -
>   {lib => drivers}/librte_vhost/fd_man.h             |    0
>   {lib => drivers}/librte_vhost/iotlb.h              |    0
>   {lib => drivers}/librte_vhost/rte_vhost.h          |    1 +
>   {lib => drivers}/librte_vhost/vhost.h              |  193 +++-
>   {lib => drivers}/librte_vhost/vhost_user.h         |    9 +-
>   drivers/librte_vhost/virtio_pci.h                  |  267 +++++
>   drivers/librte_vhost/virtio_vhost_user.h           |   18 +
>   drivers/librte_vhost/virtqueue.h                   |  181 ++++
>   {lib => drivers}/librte_vhost/fd_man.c             |    0
>   {lib => drivers}/librte_vhost/iotlb.c              |    0
>   drivers/librte_vhost/socket.c                      |  282 ++++++
>   drivers/librte_vhost/trans_af_unix.c               |  795 +++++++++++++++
>   drivers/librte_vhost/trans_virtio_vhost_user.c     | 1050 ++++++++++++++++++++
>   {lib => drivers}/librte_vhost/vhost.c              |   18 +-
>   {lib => drivers}/librte_vhost/vhost_user.c         |  200 +---
>   {lib => drivers}/librte_vhost/virtio_net.c         |    0
>   drivers/librte_vhost/virtio_pci.c                  |  504 ++++++++++
>   drivers/net/vhost/rte_eth_vhost.c                  |   13 +
>   examples/vhost_scsi/vhost_scsi.c                   |  104 +-
>   lib/librte_vhost/socket.c                          |  828 ---------------
>   .../librte_vhost/rte_vhost_version.map             |    0
>   usertools/dpdk-devbind.py                          |    8 +
>   25 files changed, 3455 insertions(+), 1026 deletions(-)
>   rename {lib => drivers}/librte_vhost/Makefile (86%)
>   rename {lib => drivers}/librte_vhost/fd_man.h (100%)
>   rename {lib => drivers}/librte_vhost/iotlb.h (100%)
>   rename {lib => drivers}/librte_vhost/rte_vhost.h (99%)
>   rename {lib => drivers}/librte_vhost/vhost.h (68%)
>   rename {lib => drivers}/librte_vhost/vhost_user.h (93%)
>   create mode 100644 drivers/librte_vhost/virtio_pci.h
>   create mode 100644 drivers/librte_vhost/virtio_vhost_user.h
>   create mode 100644 drivers/librte_vhost/virtqueue.h
>   rename {lib => drivers}/librte_vhost/fd_man.c (100%)
>   rename {lib => drivers}/librte_vhost/iotlb.c (100%)
>   create mode 100644 drivers/librte_vhost/socket.c
>   create mode 100644 drivers/librte_vhost/trans_af_unix.c
>   create mode 100644 drivers/librte_vhost/trans_virtio_vhost_user.c
>   rename {lib => drivers}/librte_vhost/vhost.c (97%)
>   rename {lib => drivers}/librte_vhost/vhost_user.c (89%)
>   rename {lib => drivers}/librte_vhost/virtio_net.c (100%)
>   create mode 100644 drivers/librte_vhost/virtio_pci.c
>   delete mode 100644 lib/librte_vhost/socket.c
>   rename {lib => drivers}/librte_vhost/rte_vhost_version.map (100%)
>