Re: [Qemu-devel] [PATCH v10 00/18] Vhost and vhost-net support for userspace based backends

Re: [Qemu-devel] [PATCH v10 00/18] Vhost and vhost-net support for userspace based backends

[Michael S. Tsirkin]

On Tue, May 27, 2014 at 03:03:21PM +0300, Nikolay Nikolaev wrote:
> In this patch series we would like to introduce our approach for putting a
> virtio-net backend in an external userspace process. Our eventual target is to
> run the network backend in the Snabbswitch ethernet switch, while receiving
> traffic from a guest inside QEMU/KVM which runs an unmodified virtio-net
> implementation.
> 
> For this, we are working into extending vhost to allow equivalent functionality
> for userspace. Vhost already passes control of the data plane of virtio-net to
> the host kernel; we want to realize a similar model, but for userspace.
> 
> In this patch series the concept of a vhost-backend is introduced.
> 
> We define two vhost backend types - vhost-kernel and vhost-user. The former is
> the interface to the current kernel module implementation. Its control plane is
> ioctl based. The data plane is realized by the kernel directly accessing the
> QEMU allocated, guest memory.
> 
> In the new vhost-user backend, the control plane is based on communication
> between QEMU and another userspace process using a unix domain socket. This
> allows to implement a virtio backend for a guest running in QEMU, inside the
> other userspace process. For this communication we use a chardev with a Unix
> domain socket backend. Vhost-user is client/server agnostic regarding the
> chardev, however it does not support the 'nowait' and 'telnet' options.
> 
> We rely on the memdev with a memory-file backend. The backend's share=on option
> should be used. HugeTLBFS is required for this option to work.
> 
> The data path is realized by directly accessing the vrings and the buffer data
> off the guest's memory.
> 
> The current user of vhost-user is only vhost-net. We add a new netdev backend
> that is intended to initialize vhost-net with vhost-user backend.
> 
> Example usage:
> 
> qemu -m 512 \
>      -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
>      -numa node,memdev=mem \
>      -chardev socket,id=chr0,path=/path/to/socket \
>      -netdev type=vhost-user,id=net0,chardev=chr0 \
>      -device virtio-net-pci,netdev=net0
> 
> On non-MSIX guests the vhost feature can be forced using a special option:
> 
> ...
>      -netdev type=vhost-user,id=net0,chardev=chr0,vhostforce
> ...
> 
> In order to use ioeventfds, kvm should be enabled.
> 
> The work is made on top of the NUMA patch series v3.2
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg02706.html
> 
> This code can be pulled from git@github.com:virtualopensystems/qemu.git vhost-user-v10
> A simple functional test is available in tests/vhost-user-test.c
> 
> A reference vhost-user slave for testing is also available from git@github.com:virtualopensystems/vapp.git
> 
> Changes from v9:
>  - Rebased on the NUMA memdev patchseries and reworked to use memdev

OK so I should wait until NUMA memdev is merged
before merging this one?

>  - Removed -mem-path refactoring
>  - Removed all reconnection code
>  - Fixed 100% CPU usage in the G_IO_HUP handler after disconnect
>  - Reworked vhost feature bits handling so vhost-user has better control in the negotiation
> 
> Changes from v8:
>  - Removed prealloc property from the -mem-path refactoring
>  - Added and use new function - kvm_eventfds_enabled
>  - Add virtio_queue_get_avail_idx used in vhost_virtqueue_stop to
>    get a sane value in case of VHOST_GET_VRING_BASE failure
>  - vhost user uses kvm_eventfds_enabled to check whether the ioeventfd
>    capability of KVM is available
>  - Added flag VHOST_USER_VRING_NOFD_MASK to be set when KICK, CALL or ERR file
>    descriptor is invalid or ioeventfd is not available
> 
> Changes from v7:
>  - Slave reconnection when using chardev in server mode
>  - qtest vhost-user-test added
>  - New qemu_chr_fe_get_msgfds for reading multiple fds from the chardev
>  - Mandatory features in vhost_dev, used on reconnect to verify for conflicts
>  - Add vhostforce parameter to -netdev vhost-user (for non-MSIX guests)
>  - Extend libqemustub.a to support qemu-char.c
> 
> Changes from v6:
>  - Remove the 'unlink' property of '-mem-path'
>  - Extend qemu-char: blocking read, send fds, monitor for connection close
>  - Vhost-user uses chardev as a backend
>  - Poll and reconnect removed (no VHOST_USER_ECHO).
>  - Disconnect is deteced by the chardev (G_IO_HUP event)
>  - vhost-backend.c split to vhost-user.c
> 
> Changes from v5:
>  - Split -mem-path unlink option to a separate patch
>  - Fds are passed only in the ancillary data
>  - Stricter message size checks on receive/send
>  - Netdev vhost-user now includes path and poll_time options
>  - The connection probing interval is configurable
> 
> Changes from v4:
>  - Use error_report for errors
>  - VhostUserMsg has new field `size` indicating the following payload length.
>    Field `flags` now has version and reply bits. The structure is packed.
>  - Send data is of variable length (`size` field in message)
>  - Receive in 2 steps, header and payload
>  - Add new message type VHOST_USER_ECHO, to check connection status
> 
> Changes from v3:
>  - Convert -mem-path to QemuOpts with prealloc, share and unlink properties
>  - Set 1 sec timeout when read/write to the unix domain socket
>  - Fix file descriptor leak
> 
> Changes from v2:
>  - Reconnect when the backend disappears
> 
> Changes from v1:
>  - Implementation of vhost-user netdev backend
>  - Code improvements
> 
> 
> ---
> 
> Nikolay Nikolaev (18):
>       Add kvm_eventfds_enabled function
>       Add chardev API qemu_chr_fe_read_all
>       Add chardev API qemu_chr_fe_set_msgfds
>       Add chardev API qemu_chr_fe_get_msgfds
>       Add G_IO_HUP handler for socket chardev
>       vhost: add vhost_get_features and vhost_ack_features
>       vhost_net should call the poll callback only when it is set
>       Refactor virtio-net to use generic get_vhost_net
>       vhost_net_init will use VhostNetOptions to get all its arguments
>       Add vhost_ops to vhost_dev struct and replace all relevant ioctls
>       Add vhost-backend and VhostBackendType
>       Add vhost-user as a vhost backend.
>       vhost-net: vhost-user feature bits support
>       Add new vhost-user netdev backend
>       Add the vhost-user netdev backend to the command line
>       Add vhost-user protocol documentation
>       libqemustub: add stubs to be able to use qemu-char.c
>       Add qtest for vhost-user
> 
> 
>  docs/specs/vhost-user.txt         |  261 ++++++++++++++++++++++++++++
>  hmp-commands.hx                   |    4 
>  hw/net/vhost_net.c                |  228 +++++++++++++++++--------
>  hw/net/virtio-net.c               |   29 +--
>  hw/scsi/vhost-scsi.c              |   45 +++--
>  hw/virtio/Makefile.objs           |    2 
>  hw/virtio/vhost-backend.c         |   71 ++++++++
>  hw/virtio/vhost-user.c            |  342 +++++++++++++++++++++++++++++++++++++
>  hw/virtio/vhost.c                 |   82 ++++++---
>  include/hw/virtio/vhost-backend.h |   38 ++++
>  include/hw/virtio/vhost.h         |   13 +
>  include/net/vhost-user.h          |   17 ++
>  include/net/vhost_net.h           |   11 +
>  include/sysemu/char.h             |   44 +++++
>  include/sysemu/kvm.h              |   11 +
>  kvm-all.c                         |    4 
>  kvm-stub.c                        |    1 
>  net/Makefile.objs                 |    2 
>  net/clients.h                     |    3 
>  net/hub.c                         |    1 
>  net/net.c                         |   25 ++-
>  net/tap.c                         |   18 ++
>  net/vhost-user.c                  |  265 +++++++++++++++++++++++++++++
>  qapi-schema.json                  |   19 ++
>  qemu-char.c                       |  277 +++++++++++++++++++++++++++---
>  qemu-options.hx                   |   18 ++
>  stubs/Makefile.objs               |    8 +
>  stubs/bdrv-commit-all.c           |    7 +
>  stubs/chr-msmouse.c               |    7 +
>  stubs/get-next-serial.c           |    3 
>  stubs/is-daemonized.c             |    7 +
>  stubs/machine-init-done.c         |    6 +
>  stubs/monitor-init.c              |    6 +
>  stubs/notify-event.c              |    6 +
>  stubs/vc-init.c                   |    7 +
>  tests/Makefile                    |    4 
>  tests/vhost-user-test.c           |  312 ++++++++++++++++++++++++++++++++++
>  37 files changed, 2011 insertions(+), 193 deletions(-)
>  create mode 100644 docs/specs/vhost-user.txt
>  create mode 100644 hw/virtio/vhost-backend.c
>  create mode 100644 hw/virtio/vhost-user.c
>  create mode 100644 include/hw/virtio/vhost-backend.h
>  create mode 100644 include/net/vhost-user.h
>  create mode 100644 net/vhost-user.c
>  create mode 100644 stubs/bdrv-commit-all.c
>  create mode 100644 stubs/chr-msmouse.c
>  create mode 100644 stubs/get-next-serial.c
>  create mode 100644 stubs/is-daemonized.c
>  create mode 100644 stubs/machine-init-done.c
>  create mode 100644 stubs/monitor-init.c
>  create mode 100644 stubs/notify-event.c
>  create mode 100644 stubs/vc-init.c
>  create mode 100644 tests/vhost-user-test.c
> 
> --
> Signature

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 00/18] Vhost and vhost-net support for userspace based backends

[Nikolay Nikolaev]

[-- Attachment #1: Type: text/plain, Size: 10146 bytes --]

On Wed, Jun 4, 2014 at 10:30 PM, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Tue, May 27, 2014 at 03:03:21PM +0300, Nikolay Nikolaev wrote:
> > In this patch series we would like to introduce our approach for putting
> a
> > virtio-net backend in an external userspace process. Our eventual target
> is to
> > run the network backend in the Snabbswitch ethernet switch, while
> receiving
> > traffic from a guest inside QEMU/KVM which runs an unmodified virtio-net
> > implementation.
> >
> > For this, we are working into extending vhost to allow equivalent
> functionality
> > for userspace. Vhost already passes control of the data plane of
> virtio-net to
> > the host kernel; we want to realize a similar model, but for userspace.
> >
> > In this patch series the concept of a vhost-backend is introduced.
> >
> > We define two vhost backend types - vhost-kernel and vhost-user. The
> former is
> > the interface to the current kernel module implementation. Its control
> plane is
> > ioctl based. The data plane is realized by the kernel directly accessing
> the
> > QEMU allocated, guest memory.
> >
> > In the new vhost-user backend, the control plane is based on
> communication
> > between QEMU and another userspace process using a unix domain socket.
> This
> > allows to implement a virtio backend for a guest running in QEMU, inside
> the
> > other userspace process. For this communication we use a chardev with a
> Unix
> > domain socket backend. Vhost-user is client/server agnostic regarding the
> > chardev, however it does not support the 'nowait' and 'telnet' options.
> >
> > We rely on the memdev with a memory-file backend. The backend's share=on
> option
> > should be used. HugeTLBFS is required for this option to work.
> >
> > The data path is realized by directly accessing the vrings and the
> buffer data
> > off the guest's memory.
> >
> > The current user of vhost-user is only vhost-net. We add a new netdev
> backend
> > that is intended to initialize vhost-net with vhost-user backend.
> >
> > Example usage:
> >
> > qemu -m 512 \
> >      -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
> >      -numa node,memdev=mem \
> >      -chardev socket,id=chr0,path=/path/to/socket \
> >      -netdev type=vhost-user,id=net0,chardev=chr0 \
> >      -device virtio-net-pci,netdev=net0
> >
> > On non-MSIX guests the vhost feature can be forced using a special
> option:
> >
> > ...
> >      -netdev type=vhost-user,id=net0,chardev=chr0,vhostforce
> > ...
> >
> > In order to use ioeventfds, kvm should be enabled.
> >
> > The work is made on top of the NUMA patch series v3.2
> > http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg02706.html
> >
> > This code can be pulled from git@github.com:virtualopensystems/qemu.git
> vhost-user-v10
> > A simple functional test is available in tests/vhost-user-test.c
> >
> > A reference vhost-user slave for testing is also available from
> git@github.com:virtualopensystems/vapp.git
> >
> > Changes from v9:
> >  - Rebased on the NUMA memdev patchseries and reworked to use memdev
>
> OK so I should wait until NUMA memdev is merged
> before merging this one?
>

I guess it is still planned for 2.1 as was said by Paolo:
https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00582.html


>
> >  - Removed -mem-path refactoring
> >  - Removed all reconnection code
> >  - Fixed 100% CPU usage in the G_IO_HUP handler after disconnect
> >  - Reworked vhost feature bits handling so vhost-user has better control
> in the negotiation
> >
> > Changes from v8:
> >  - Removed prealloc property from the -mem-path refactoring
> >  - Added and use new function - kvm_eventfds_enabled
> >  - Add virtio_queue_get_avail_idx used in vhost_virtqueue_stop to
> >    get a sane value in case of VHOST_GET_VRING_BASE failure
> >  - vhost user uses kvm_eventfds_enabled to check whether the ioeventfd
> >    capability of KVM is available
> >  - Added flag VHOST_USER_VRING_NOFD_MASK to be set when KICK, CALL or
> ERR file
> >    descriptor is invalid or ioeventfd is not available
> >
> > Changes from v7:
> >  - Slave reconnection when using chardev in server mode
> >  - qtest vhost-user-test added
> >  - New qemu_chr_fe_get_msgfds for reading multiple fds from the chardev
> >  - Mandatory features in vhost_dev, used on reconnect to verify for
> conflicts
> >  - Add vhostforce parameter to -netdev vhost-user (for non-MSIX guests)
> >  - Extend libqemustub.a to support qemu-char.c
> >
> > Changes from v6:
> >  - Remove the 'unlink' property of '-mem-path'
> >  - Extend qemu-char: blocking read, send fds, monitor for connection
> close
> >  - Vhost-user uses chardev as a backend
> >  - Poll and reconnect removed (no VHOST_USER_ECHO).
> >  - Disconnect is deteced by the chardev (G_IO_HUP event)
> >  - vhost-backend.c split to vhost-user.c
> >
> > Changes from v5:
> >  - Split -mem-path unlink option to a separate patch
> >  - Fds are passed only in the ancillary data
> >  - Stricter message size checks on receive/send
> >  - Netdev vhost-user now includes path and poll_time options
> >  - The connection probing interval is configurable
> >
> > Changes from v4:
> >  - Use error_report for errors
> >  - VhostUserMsg has new field `size` indicating the following payload
> length.
> >    Field `flags` now has version and reply bits. The structure is packed.
> >  - Send data is of variable length (`size` field in message)
> >  - Receive in 2 steps, header and payload
> >  - Add new message type VHOST_USER_ECHO, to check connection status
> >
> > Changes from v3:
> >  - Convert -mem-path to QemuOpts with prealloc, share and unlink
> properties
> >  - Set 1 sec timeout when read/write to the unix domain socket
> >  - Fix file descriptor leak
> >
> > Changes from v2:
> >  - Reconnect when the backend disappears
> >
> > Changes from v1:
> >  - Implementation of vhost-user netdev backend
> >  - Code improvements
> >
> >
> > ---
> >
> > Nikolay Nikolaev (18):
> >       Add kvm_eventfds_enabled function
> >       Add chardev API qemu_chr_fe_read_all
> >       Add chardev API qemu_chr_fe_set_msgfds
> >       Add chardev API qemu_chr_fe_get_msgfds
> >       Add G_IO_HUP handler for socket chardev
> >       vhost: add vhost_get_features and vhost_ack_features
> >       vhost_net should call the poll callback only when it is set
> >       Refactor virtio-net to use generic get_vhost_net
> >       vhost_net_init will use VhostNetOptions to get all its arguments
> >       Add vhost_ops to vhost_dev struct and replace all relevant ioctls
> >       Add vhost-backend and VhostBackendType
> >       Add vhost-user as a vhost backend.
> >       vhost-net: vhost-user feature bits support
> >       Add new vhost-user netdev backend
> >       Add the vhost-user netdev backend to the command line
> >       Add vhost-user protocol documentation
> >       libqemustub: add stubs to be able to use qemu-char.c
> >       Add qtest for vhost-user
> >
> >
> >  docs/specs/vhost-user.txt         |  261 ++++++++++++++++++++++++++++
> >  hmp-commands.hx                   |    4
> >  hw/net/vhost_net.c                |  228 +++++++++++++++++--------
> >  hw/net/virtio-net.c               |   29 +--
> >  hw/scsi/vhost-scsi.c              |   45 +++--
> >  hw/virtio/Makefile.objs           |    2
> >  hw/virtio/vhost-backend.c         |   71 ++++++++
> >  hw/virtio/vhost-user.c            |  342
> +++++++++++++++++++++++++++++++++++++
> >  hw/virtio/vhost.c                 |   82 ++++++---
> >  include/hw/virtio/vhost-backend.h |   38 ++++
> >  include/hw/virtio/vhost.h         |   13 +
> >  include/net/vhost-user.h          |   17 ++
> >  include/net/vhost_net.h           |   11 +
> >  include/sysemu/char.h             |   44 +++++
> >  include/sysemu/kvm.h              |   11 +
> >  kvm-all.c                         |    4
> >  kvm-stub.c                        |    1
> >  net/Makefile.objs                 |    2
> >  net/clients.h                     |    3
> >  net/hub.c                         |    1
> >  net/net.c                         |   25 ++-
> >  net/tap.c                         |   18 ++
> >  net/vhost-user.c                  |  265 +++++++++++++++++++++++++++++
> >  qapi-schema.json                  |   19 ++
> >  qemu-char.c                       |  277 +++++++++++++++++++++++++++---
> >  qemu-options.hx                   |   18 ++
> >  stubs/Makefile.objs               |    8 +
> >  stubs/bdrv-commit-all.c           |    7 +
> >  stubs/chr-msmouse.c               |    7 +
> >  stubs/get-next-serial.c           |    3
> >  stubs/is-daemonized.c             |    7 +
> >  stubs/machine-init-done.c         |    6 +
> >  stubs/monitor-init.c              |    6 +
> >  stubs/notify-event.c              |    6 +
> >  stubs/vc-init.c                   |    7 +
> >  tests/Makefile                    |    4
> >  tests/vhost-user-test.c           |  312
> ++++++++++++++++++++++++++++++++++
> >  37 files changed, 2011 insertions(+), 193 deletions(-)
> >  create mode 100644 docs/specs/vhost-user.txt
> >  create mode 100644 hw/virtio/vhost-backend.c
> >  create mode 100644 hw/virtio/vhost-user.c
> >  create mode 100644 include/hw/virtio/vhost-backend.h
> >  create mode 100644 include/net/vhost-user.h
> >  create mode 100644 net/vhost-user.c
> >  create mode 100644 stubs/bdrv-commit-all.c
> >  create mode 100644 stubs/chr-msmouse.c
> >  create mode 100644 stubs/get-next-serial.c
> >  create mode 100644 stubs/is-daemonized.c
> >  create mode 100644 stubs/machine-init-done.c
> >  create mode 100644 stubs/monitor-init.c
> >  create mode 100644 stubs/notify-event.c
> >  create mode 100644 stubs/vc-init.c
> >  create mode 100644 tests/vhost-user-test.c
> >
> > --
> > Signature
>
> --
> You received this message because you are subscribed to the Google Groups
> "Snabb Switch development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to snabb-devel+unsubscribe@googlegroups.com.
> To post to this group, send an email to snabb-devel@googlegroups.com.
> Visit this group at http://groups.google.com/group/snabb-devel.
>

regards,
Nikolay Nikolaev

[-- Attachment #2: Type: text/html, Size: 12930 bytes --]

Re: [Qemu-devel] [PATCH v10 01/18] Add kvm_eventfds_enabled function

[Paolo Bonzini]

Il 27/05/2014 14:03, Nikolay Nikolaev ha scritto:
> Add a function to check if the eventfd capability is present in KVM in
> the host kernel.
>
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> ---
>  include/sysemu/kvm.h |   11 +++++++++++
>  kvm-all.c            |    4 ++++
>  kvm-stub.c           |    1 +
>  3 files changed, 16 insertions(+)
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index e7ad9d1..1c7f5f6 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -43,6 +43,7 @@ extern bool kvm_allowed;
>  extern bool kvm_kernel_irqchip;
>  extern bool kvm_async_interrupts_allowed;
>  extern bool kvm_halt_in_kernel_allowed;
> +extern bool kvm_eventfds_allowed;
>  extern bool kvm_irqfds_allowed;
>  extern bool kvm_msi_via_irqfd_allowed;
>  extern bool kvm_gsi_routing_allowed;
> @@ -83,6 +84,15 @@ extern bool kvm_readonly_mem_allowed;
>  #define kvm_halt_in_kernel() (kvm_halt_in_kernel_allowed)
>
>  /**
> + * kvm_eventfds_enabled:
> + *
> + * Returns: true if we can use eventfds to receive notifications
> + * from a KVM CPU (ie the kernel supports eventds and we are running
> + * with a configuration where it is meaningful to use them).
> + */
> +#define kvm_eventfds_enabled() (kvm_eventfds_allowed)
> +
> +/**
>   * kvm_irqfds_enabled:
>   *
>   * Returns: true if we can use irqfds to inject interrupts into
> @@ -128,6 +138,7 @@ extern bool kvm_readonly_mem_allowed;
>  #define kvm_irqchip_in_kernel() (false)
>  #define kvm_async_interrupts_enabled() (false)
>  #define kvm_halt_in_kernel() (false)
> +#define kvm_eventfds_enabled() (false)
>  #define kvm_irqfds_enabled() (false)
>  #define kvm_msi_via_irqfd_enabled() (false)
>  #define kvm_gsi_routing_allowed() (false)
> diff --git a/kvm-all.c b/kvm-all.c
> index a343ede..9649cd3 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -112,6 +112,7 @@ KVMState *kvm_state;
>  bool kvm_kernel_irqchip;
>  bool kvm_async_interrupts_allowed;
>  bool kvm_halt_in_kernel_allowed;
> +bool kvm_eventfds_allowed;
>  bool kvm_irqfds_allowed;
>  bool kvm_msi_via_irqfd_allowed;
>  bool kvm_gsi_routing_allowed;
> @@ -1504,6 +1505,9 @@ int kvm_init(MachineClass *mc)
>          (kvm_check_extension(s, KVM_CAP_READONLY_MEM) > 0);
>  #endif
>
> +    kvm_eventfds_allowed =
> +        (kvm_check_extension(s, KVM_CAP_IOEVENTFD) > 0);
> +
>      ret = kvm_arch_init(s);
>      if (ret < 0) {
>          goto err;
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 8acda86..3fb17f2 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -22,6 +22,7 @@
>  KVMState *kvm_state;
>  bool kvm_kernel_irqchip;
>  bool kvm_async_interrupts_allowed;
> +bool kvm_eventfds_allowed;
>  bool kvm_irqfds_allowed;
>  bool kvm_msi_via_irqfd_allowed;
>  bool kvm_gsi_routing_allowed;
>

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v10 15/18] Add the vhost-user netdev backend to the command line

[Luiz Capitulino]

On Tue, 27 May 2014 15:06:43 +0300
Nikolay Nikolaev <n.nikolaev@virtualopensystems.com> wrote:

> The supplied chardev id will be inspected for supported options. Only
> a socket backend, with a set path (i.e. a Unix socket) and optionally
> the server parameter set, will be allowed. Other options (nowait, telnet)
> will make the chardev unusable and the netdev will not be initialised.
> 
> Additional checks for validity:
>   - requires `-numa node,memdev=..`
>   - requires `-device virtio-net-*`
> 
> The `vhostforce` option is used to force vhost-net when we deal with
> non-MSIX guests.
> 
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>

I gave a quick review and apart from some minor comments below it seems
good to me, but I think it would be good to have Eric's review too:

Acked-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  hmp-commands.hx    |    4 +-
>  hw/net/vhost_net.c |    4 ++
>  net/hub.c          |    1 
>  net/net.c          |   25 ++++++-----
>  net/vhost-user.c   |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json   |   19 ++++++++-
>  qemu-options.hx    |   18 ++++++++
>  7 files changed, 169 insertions(+), 16 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8971f1b..ef3782c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1205,7 +1205,7 @@ ETEXI
>      {
>          .name       = "host_net_add",
>          .args_type  = "device:s,opts:s?",
> -        .params     = "tap|user|socket|vde|netmap|dump [options]",
> +        .params     = "tap|user|socket|vde|netmap|vhost-user|dump [options]",
>          .help       = "add host VLAN client",
>          .mhandler.cmd = net_host_device_add,
>      },
> @@ -1233,7 +1233,7 @@ ETEXI
>      {
>          .name       = "netdev_add",
>          .args_type  = "netdev:O",
> -        .params     = "[user|tap|socket|hubport|netmap],id=str[,prop=value][,...]",
> +        .params     = "[user|tap|socket|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
>          .help       = "add host network device",
>          .mhandler.cmd = hmp_netdev_add,
>      },
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 5f06736..7ac7c21 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -15,6 +15,7 @@
>  
>  #include "net/net.h"
>  #include "net/tap.h"
> +#include "net/vhost-user.h"
>  
>  #include "hw/virtio/virtio-net.h"
>  #include "net/vhost_net.h"
> @@ -360,6 +361,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>      case NET_CLIENT_OPTIONS_KIND_TAP:
>          vhost_net = tap_get_vhost_net(nc);
>          break;
> +    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> +        vhost_net = vhost_user_get_vhost_net(nc);
> +        break;
>      default:
>          break;
>      }
> diff --git a/net/hub.c b/net/hub.c
> index 33a99c9..7e0f2d6 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -322,6 +322,7 @@ void net_hub_check_clients(void)
>              case NET_CLIENT_OPTIONS_KIND_TAP:
>              case NET_CLIENT_OPTIONS_KIND_SOCKET:
>              case NET_CLIENT_OPTIONS_KIND_VDE:
> +            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>                  has_host_dev = 1;
>                  break;
>              default:
> diff --git a/net/net.c b/net/net.c
> index 9db4dba..907f679 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -769,23 +769,24 @@ static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
>      const NetClientOptions *opts,
>      const char *name,
>      NetClientState *peer) = {
> -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
> +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
>  #ifdef CONFIG_SLIRP
> -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
> +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
> -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
> +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
> +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
>  #ifdef CONFIG_VDE
> -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
> +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
>  #endif
>  #ifdef CONFIG_NETMAP
> -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
> +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
> +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
>  #ifdef CONFIG_NET_BRIDGE
> -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,

These changes are unrelated.

>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> +        [NET_CLIENT_OPTIONS_KIND_HUBPORT]       = net_init_hubport,
> +        [NET_CLIENT_OPTIONS_KIND_VHOST_USER]    = net_init_vhost_user,
>  };
>  
>  
> @@ -819,6 +820,7 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          case NET_CLIENT_OPTIONS_KIND_BRIDGE:
>  #endif
>          case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> +        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>              break;
>  
>          default:
> @@ -902,11 +904,12 @@ static int net_host_check_device(const char *device)
>                                         , "bridge"
>  #endif
>  #ifdef CONFIG_SLIRP
> -                                       ,"user"
> +                                       , "user"
>  #endif
>  #ifdef CONFIG_VDE
> -                                       ,"vde"
> +                                       , "vde"
>  #endif
> +                                       , "vhost-user"
>      };
>      for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
>          if (!strncmp(valid_param_list[i], device,
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 4bdd19d..69a5eb4 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -12,6 +12,7 @@
>  #include "net/vhost_net.h"
>  #include "net/vhost-user.h"
>  #include "sysemu/char.h"
> +#include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  
>  typedef struct VhostUserState {
> @@ -21,9 +22,17 @@ typedef struct VhostUserState {
>      VHostNetState *vhost_net;
>  } VhostUserState;
>  
> +typedef struct VhostUserChardevProps {
> +    bool is_socket;
> +    bool is_unix;
> +    bool is_server;
> +    bool has_unsupported;
> +} VhostUserChardevProps;
> +
>  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
>  {
>      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> +    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
>      return s->vhost_net;
>  }
>  
> @@ -82,7 +91,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
>  }
>  
>  static NetClientInfo net_vhost_user_info = {
> -        .type = 0,
> +        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
>          .size = sizeof(VhostUserState),
>          .cleanup = vhost_user_cleanup,
>          .has_vnet_hdr = vhost_user_has_vnet_hdr,
> @@ -148,8 +157,109 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
>      return 0;
>  }
>  
> +static int net_vhost_chardev_opts(const char *name, const char *value,
> +        void *opaque)
> +{
> +    VhostUserChardevProps *props = opaque;
> +
> +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> +        props->is_socket = 1;
> +    } else if (strcmp(name, "path") == 0) {
> +        props->is_unix = 1;
> +    } else if (strcmp(name, "server") == 0) {
> +        props->is_server = 1;
> +    } else {
> +        error_report("vhost-user does not support a chardev"
> +                     " with the following option:\n %s = %s",
> +                     name, value);
> +        props->has_unsupported = 1;
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static CharDriverState *net_vhost_parse_chardev(
> +        const NetdevVhostUserOptions *opts)
> +{
> +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> +    VhostUserChardevProps props;
> +
> +    if (chr == NULL) {
> +        error_report("chardev \"%s\" not found\n", opts->chardev);
> +        return 0;
> +    }
> +
> +    /* inspect chardev opts */
> +    memset(&props, 0, sizeof(props));
> +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> +
> +    if (!props.is_socket || !props.is_unix) {
> +        error_report("chardev \"%s\" is not a unix socket\n",
> +                     opts->chardev);
> +        return 0;
> +    }
> +
> +    if (props.has_unsupported) {
> +        error_report("chardev \"%s\" has an unsupported option\n",
> +                opts->chardev);
> +        return 0;
> +    }
> +
> +    qemu_chr_fe_claim_no_fail(chr);
> +
> +    return chr;
> +}
> +
> +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> +{
> +    const char *name = opaque;
> +    const char *driver, *netdev;
> +    const char virtio_name[] = "virtio-net-";
> +
> +    driver = qemu_opt_get(opts, "driver");
> +    netdev = qemu_opt_get(opts, "netdev");
> +
> +    if (!driver || !netdev) {
> +        return 0;
> +    }
> +
> +    if ((strcmp(netdev, name) == 0)
> +            && (strncmp(driver, virtio_name, strlen(virtio_name)) != 0)) {
> +        error_report("vhost-user requires frontend driver virtio-net-*");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
>                     NetClientState *peer)
>  {
> -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> +    const NetdevVhostUserOptions *vhost_user_opts;
> +    CharDriverState *chr;
> +    bool vhostforce;
> +
> +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> +    vhost_user_opts = opts->vhost_user;
> +
> +    chr = net_vhost_parse_chardev(vhost_user_opts);
> +    if (!chr) {
> +        error_report("No suitable chardev found\n");
> +        return -1;
> +    }
> +
> +    /* verify net frontend */
> +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> +                          (void *)name, true) == -1) {
> +        return -1;
> +    }
> +
> +    /* vhostforce for non-MSIX */
> +    if (vhost_user_opts->has_vhostforce) {
> +        vhostforce = vhost_user_opts->vhostforce;
> +    } else {
> +        vhostforce = false;
> +    }
> +
> +    return net_vhost_user_init(peer, "vhost_user", name, chr, vhostforce);
>  }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 1f28177..f458dd8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3264,6 +3264,22 @@
>      '*devname':    'str' } }
>  
>  ##
> +# @NetdevVhostUserOptions
> +#
> +# Vhost-user network backend
> +#
> +# @path: control socket path
> +#
> +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
> +#
> +# Since 2.1
> +##
> +{ 'type': 'NetdevVhostUserOptions',
> +  'data': {
> +    'chardev':        'str',

chardev or path?

> +    '*vhostforce':    'bool' } }
> +
> +##
>  # @NetClientOptions
>  #
>  # A discriminated record of network device traits.
> @@ -3281,7 +3297,8 @@
>      'dump':     'NetdevDumpOptions',
>      'bridge':   'NetdevBridgeOptions',
>      'hubport':  'NetdevHubPortOptions',
> -    'netmap':   'NetdevNetmapOptions' } }
> +    'netmap':   'NetdevNetmapOptions',
> +    'vhost-user': 'NetdevVhostUserOptions' } }
>  
>  ##
>  # @NetLegacy
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 7f4ab83..2514264 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1459,6 +1459,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>  #ifdef CONFIG_NETMAP
>      "netmap|"
>  #endif
> +    "vhost-user|"
>      "socket|"
>      "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
>  STEXI
> @@ -1790,6 +1791,23 @@ The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
>  netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
>  required hub automatically.
>  
> +@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
> +
> +Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
> +be a unix domain socket backed one. The vhost-user uses a specifically defined
> +protocol to pass vhost ioctl replacement messages to an application on the other
> +end of the socket. On non-MSIX guests, the feature can be forced with
> +@var{vhostforce}.
> +
> +Example:
> +@example
> +qemu -m 512 -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
> +     -numa node,memdev=mem \
> +     -chardev socket,path=/path/to/socket \
> +     -netdev type=vhost-user,id=net0,chardev=chr0 \
> +     -device virtio-net-pci,netdev=net0
> +@end example
> +
>  @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
>  Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu-vlan0.pcap} by default).
>  At most @var{len} bytes (64k by default) per packet are stored. The file format is
> 
> 

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 15/18] Add the vhost-user netdev backend to the command line

[Nikolay Nikolaev]

[-- Attachment #1: Type: text/plain, Size: 14415 bytes --]

Hello,


On Thu, Jun 5, 2014 at 5:37 PM, Luiz Capitulino <lcapitulino@redhat.com>
wrote:

> On Tue, 27 May 2014 15:06:43 +0300
> Nikolay Nikolaev <n.nikolaev@virtualopensystems.com> wrote:
>
> > The supplied chardev id will be inspected for supported options. Only
> > a socket backend, with a set path (i.e. a Unix socket) and optionally
> > the server parameter set, will be allowed. Other options (nowait, telnet)
> > will make the chardev unusable and the netdev will not be initialised.
> >
> > Additional checks for validity:
> >   - requires `-numa node,memdev=..`
> >   - requires `-device virtio-net-*`
> >
> > The `vhostforce` option is used to force vhost-net when we deal with
> > non-MSIX guests.
> >
> > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
>
> I gave a quick review and apart from some minor comments below it seems
> good to me, but I think it would be good to have Eric's review too:
>
> Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
>
Thanks!

>
> > ---
> >  hmp-commands.hx    |    4 +-
> >  hw/net/vhost_net.c |    4 ++
> >  net/hub.c          |    1
> >  net/net.c          |   25 ++++++-----
> >  net/vhost-user.c   |  114
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  qapi-schema.json   |   19 ++++++++-
> >  qemu-options.hx    |   18 ++++++++
> >  7 files changed, 169 insertions(+), 16 deletions(-)
> >
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 8971f1b..ef3782c 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1205,7 +1205,7 @@ ETEXI
> >      {
> >          .name       = "host_net_add",
> >          .args_type  = "device:s,opts:s?",
> > -        .params     = "tap|user|socket|vde|netmap|dump [options]",
> > +        .params     = "tap|user|socket|vde|netmap|vhost-user|dump
> [options]",
> >          .help       = "add host VLAN client",
> >          .mhandler.cmd = net_host_device_add,
> >      },
> > @@ -1233,7 +1233,7 @@ ETEXI
> >      {
> >          .name       = "netdev_add",
> >          .args_type  = "netdev:O",
> > -        .params     =
> "[user|tap|socket|hubport|netmap],id=str[,prop=value][,...]",
> > +        .params     =
> "[user|tap|socket|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
> >          .help       = "add host network device",
> >          .mhandler.cmd = hmp_netdev_add,
> >      },
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 5f06736..7ac7c21 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -15,6 +15,7 @@
> >
> >  #include "net/net.h"
> >  #include "net/tap.h"
> > +#include "net/vhost-user.h"
> >
> >  #include "hw/virtio/virtio-net.h"
> >  #include "net/vhost_net.h"
> > @@ -360,6 +361,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
> >      case NET_CLIENT_OPTIONS_KIND_TAP:
> >          vhost_net = tap_get_vhost_net(nc);
> >          break;
> > +    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> > +        vhost_net = vhost_user_get_vhost_net(nc);
> > +        break;
> >      default:
> >          break;
> >      }
> > diff --git a/net/hub.c b/net/hub.c
> > index 33a99c9..7e0f2d6 100644
> > --- a/net/hub.c
> > +++ b/net/hub.c
> > @@ -322,6 +322,7 @@ void net_hub_check_clients(void)
> >              case NET_CLIENT_OPTIONS_KIND_TAP:
> >              case NET_CLIENT_OPTIONS_KIND_SOCKET:
> >              case NET_CLIENT_OPTIONS_KIND_VDE:
> > +            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> >                  has_host_dev = 1;
> >                  break;
> >              default:
> > diff --git a/net/net.c b/net/net.c
> > index 9db4dba..907f679 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -769,23 +769,24 @@ static int (* const
> net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
> >      const NetClientOptions *opts,
> >      const char *name,
> >      NetClientState *peer) = {
> > -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
> > +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
> >  #ifdef CONFIG_SLIRP
> > -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
> > +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
> >  #endif
> > -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
> > -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
> > +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
> > +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
> >  #ifdef CONFIG_VDE
> > -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
> > +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
> >  #endif
> >  #ifdef CONFIG_NETMAP
> > -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
> > +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
> >  #endif
> > -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
> > +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
> >  #ifdef CONFIG_NET_BRIDGE
> > -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> > +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,
>
> These changes are unrelated.
>
OK. Removing them.

>
> >  #endif
> > -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> > +        [NET_CLIENT_OPTIONS_KIND_HUBPORT]       = net_init_hubport,
> > +        [NET_CLIENT_OPTIONS_KIND_VHOST_USER]    = net_init_vhost_user,
> >  };
> >
> >
> > @@ -819,6 +820,7 @@ static int net_client_init1(const void *object, int
> is_netdev, Error **errp)
> >          case NET_CLIENT_OPTIONS_KIND_BRIDGE:
> >  #endif
> >          case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> > +        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> >              break;
> >
> >          default:
> > @@ -902,11 +904,12 @@ static int net_host_check_device(const char
> *device)
> >                                         , "bridge"
> >  #endif
> >  #ifdef CONFIG_SLIRP
> > -                                       ,"user"
> > +                                       , "user"
> >  #endif
> >  #ifdef CONFIG_VDE
> > -                                       ,"vde"
> > +                                       , "vde"
> >  #endif
> > +                                       , "vhost-user"
> >      };
> >      for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
> >          if (!strncmp(valid_param_list[i], device,
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 4bdd19d..69a5eb4 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -12,6 +12,7 @@
> >  #include "net/vhost_net.h"
> >  #include "net/vhost-user.h"
> >  #include "sysemu/char.h"
> > +#include "qemu/config-file.h"
> >  #include "qemu/error-report.h"
> >
> >  typedef struct VhostUserState {
> > @@ -21,9 +22,17 @@ typedef struct VhostUserState {
> >      VHostNetState *vhost_net;
> >  } VhostUserState;
> >
> > +typedef struct VhostUserChardevProps {
> > +    bool is_socket;
> > +    bool is_unix;
> > +    bool is_server;
> > +    bool has_unsupported;
> > +} VhostUserChardevProps;
> > +
> >  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
> >  {
> >      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> > +    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> >      return s->vhost_net;
> >  }
> >
> > @@ -82,7 +91,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
> >  }
> >
> >  static NetClientInfo net_vhost_user_info = {
> > -        .type = 0,
> > +        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
> >          .size = sizeof(VhostUserState),
> >          .cleanup = vhost_user_cleanup,
> >          .has_vnet_hdr = vhost_user_has_vnet_hdr,
> > @@ -148,8 +157,109 @@ static int net_vhost_user_init(NetClientState
> *peer, const char *device,
> >      return 0;
> >  }
> >
> > +static int net_vhost_chardev_opts(const char *name, const char *value,
> > +        void *opaque)
> > +{
> > +    VhostUserChardevProps *props = opaque;
> > +
> > +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> > +        props->is_socket = 1;
> > +    } else if (strcmp(name, "path") == 0) {
> > +        props->is_unix = 1;
> > +    } else if (strcmp(name, "server") == 0) {
> > +        props->is_server = 1;
> > +    } else {
> > +        error_report("vhost-user does not support a chardev"
> > +                     " with the following option:\n %s = %s",
> > +                     name, value);
> > +        props->has_unsupported = 1;
> > +        return -1;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static CharDriverState *net_vhost_parse_chardev(
> > +        const NetdevVhostUserOptions *opts)
> > +{
> > +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> > +    VhostUserChardevProps props;
> > +
> > +    if (chr == NULL) {
> > +        error_report("chardev \"%s\" not found\n", opts->chardev);
> > +        return 0;
> > +    }
> > +
> > +    /* inspect chardev opts */
> > +    memset(&props, 0, sizeof(props));
> > +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> > +
> > +    if (!props.is_socket || !props.is_unix) {
> > +        error_report("chardev \"%s\" is not a unix socket\n",
> > +                     opts->chardev);
> > +        return 0;
> > +    }
> > +
> > +    if (props.has_unsupported) {
> > +        error_report("chardev \"%s\" has an unsupported option\n",
> > +                opts->chardev);
> > +        return 0;
> > +    }
> > +
> > +    qemu_chr_fe_claim_no_fail(chr);
> > +
> > +    return chr;
> > +}
> > +
> > +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> > +{
> > +    const char *name = opaque;
> > +    const char *driver, *netdev;
> > +    const char virtio_name[] = "virtio-net-";
> > +
> > +    driver = qemu_opt_get(opts, "driver");
> > +    netdev = qemu_opt_get(opts, "netdev");
> > +
> > +    if (!driver || !netdev) {
> > +        return 0;
> > +    }
> > +
> > +    if ((strcmp(netdev, name) == 0)
> > +            && (strncmp(driver, virtio_name, strlen(virtio_name)) !=
> 0)) {
> > +        error_report("vhost-user requires frontend driver
> virtio-net-*");
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
> >                     NetClientState *peer)
> >  {
> > -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> > +    const NetdevVhostUserOptions *vhost_user_opts;
> > +    CharDriverState *chr;
> > +    bool vhostforce;
> > +
> > +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> > +    vhost_user_opts = opts->vhost_user;
> > +
> > +    chr = net_vhost_parse_chardev(vhost_user_opts);
> > +    if (!chr) {
> > +        error_report("No suitable chardev found\n");
> > +        return -1;
> > +    }
> > +
> > +    /* verify net frontend */
> > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> > +                          (void *)name, true) == -1) {
> > +        return -1;
> > +    }
> > +
> > +    /* vhostforce for non-MSIX */
> > +    if (vhost_user_opts->has_vhostforce) {
> > +        vhostforce = vhost_user_opts->vhostforce;
> > +    } else {
> > +        vhostforce = false;
> > +    }
> > +
> > +    return net_vhost_user_init(peer, "vhost_user", name, chr,
> vhostforce);
> >  }
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 1f28177..f458dd8 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3264,6 +3264,22 @@
> >      '*devname':    'str' } }
> >
> >  ##
> > +# @NetdevVhostUserOptions
> > +#
> > +# Vhost-user network backend
> > +#
> > +# @path: control socket path
> > +#
> > +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default:
> false).
> > +#
> > +# Since 2.1
> > +##
> > +{ 'type': 'NetdevVhostUserOptions',
> > +  'data': {
> > +    'chardev':        'str',
>
> chardev or path?
>
Right, it's chardev.

>
> > +    '*vhostforce':    'bool' } }
> > +
> > +##
> >  # @NetClientOptions
> >  #
> >  # A discriminated record of network device traits.
> > @@ -3281,7 +3297,8 @@
> >      'dump':     'NetdevDumpOptions',
> >      'bridge':   'NetdevBridgeOptions',
> >      'hubport':  'NetdevHubPortOptions',
> > -    'netmap':   'NetdevNetmapOptions' } }
> > +    'netmap':   'NetdevNetmapOptions',
> > +    'vhost-user': 'NetdevVhostUserOptions' } }
> >
> >  ##
> >  # @NetLegacy
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 7f4ab83..2514264 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1459,6 +1459,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
> >  #ifdef CONFIG_NETMAP
> >      "netmap|"
> >  #endif
> > +    "vhost-user|"
> >      "socket|"
> >      "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
> >  STEXI
> > @@ -1790,6 +1791,23 @@ The hubport netdev lets you connect a NIC to a
> QEMU "vlan" instead of a single
> >  netdev.  @code{-net} and @code{-device} with parameter @option{vlan}
> create the
> >  required hub automatically.
> >
> > +@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
> > +
> > +Establish a vhost-user netdev, backed by a chardev @var{id}. The
> chardev should
> > +be a unix domain socket backed one. The vhost-user uses a specifically
> defined
> > +protocol to pass vhost ioctl replacement messages to an application on
> the other
> > +end of the socket. On non-MSIX guests, the feature can be forced with
> > +@var{vhostforce}.
> > +
> > +Example:
> > +@example
> > +qemu -m 512 -object
> memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
> > +     -numa node,memdev=mem \
> > +     -chardev socket,path=/path/to/socket \
> > +     -netdev type=vhost-user,id=net0,chardev=chr0 \
> > +     -device virtio-net-pci,netdev=net0
> > +@end example
> > +
> >  @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
> >  Dump network traffic on VLAN @var{n} to file @var{file}
> (@file{qemu-vlan0.pcap} by default).
> >  At most @var{len} bytes (64k by default) per packet are stored. The
> file format is
> >
> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "Snabb Switch development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to snabb-devel+unsubscribe@googlegroups.com.
> To post to this group, send an email to snabb-devel@googlegroups.com.
> Visit this group at http://groups.google.com/group/snabb-devel.
>

regards,
Nikolay Nikolaev

[-- Attachment #2: Type: text/html, Size: 19699 bytes --]

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 15/18] Add the vhost-user netdev backend to the command line

[Michael S. Tsirkin]

On Mon, Jun 09, 2014 at 04:28:23PM +0300, Nikolay Nikolaev wrote:
> Hello,
> 
> 
> On Thu, Jun 5, 2014 at 5:37 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> 
>     On Tue, 27 May 2014 15:06:43 +0300
>     Nikolay Nikolaev <n.nikolaev@virtualopensystems.com> wrote:
> 
>     > The supplied chardev id will be inspected for supported options. Only
>     > a socket backend, with a set path (i.e. a Unix socket) and optionally
>     > the server parameter set, will be allowed. Other options (nowait, telnet)
>     > will make the chardev unusable and the netdev will not be initialised.
>     >
>     > Additional checks for validity:
>     >   - requires `-numa node,memdev=..`
>     >   - requires `-device virtio-net-*`
>     >
>     > The `vhostforce` option is used to force vhost-net when we deal with
>     > non-MSIX guests.
>     >
>     > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
>     > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> 
>     I gave a quick review and apart from some minor comments below it seems
>     good to me, but I think it would be good to have Eric's review too:
> 
>     Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
> 
> Thanks!
> 
> 
>     > ---
>     >  hmp-commands.hx    |    4 +-
>     >  hw/net/vhost_net.c |    4 ++
>     >  net/hub.c          |    1
>     >  net/net.c          |   25 ++++++-----
>     >  net/vhost-user.c   |  114
>     +++++++++++++++++++++++++++++++++++++++++++++++++++-
>     >  qapi-schema.json   |   19 ++++++++-
>     >  qemu-options.hx    |   18 ++++++++
>     >  7 files changed, 169 insertions(+), 16 deletions(-)
>     >
>     > diff --git a/hmp-commands.hx b/hmp-commands.hx
>     > index 8971f1b..ef3782c 100644
>     > --- a/hmp-commands.hx
>     > +++ b/hmp-commands.hx
>     > @@ -1205,7 +1205,7 @@ ETEXI
>     >      {
>     >          .name       = "host_net_add",
>     >          .args_type  = "device:s,opts:s?",
>     > -        .params     = "tap|user|socket|vde|netmap|dump [options]",
>     > +        .params     = "tap|user|socket|vde|netmap|vhost-user|dump
>     [options]",
>     >          .help       = "add host VLAN client",
>     >          .mhandler.cmd = net_host_device_add,
>     >      },
>     > @@ -1233,7 +1233,7 @@ ETEXI
>     >      {
>     >          .name       = "netdev_add",
>     >          .args_type  = "netdev:O",
>     > -        .params     = "[user|tap|socket|hubport|netmap],id=str[,prop=
>     value][,...]",
>     > +        .params     = "[user|tap|socket|hubport|netmap|vhost-user],id=
>     str[,prop=value][,...]",
>     >          .help       = "add host network device",
>     >          .mhandler.cmd = hmp_netdev_add,
>     >      },
>     > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>     > index 5f06736..7ac7c21 100644
>     > --- a/hw/net/vhost_net.c
>     > +++ b/hw/net/vhost_net.c
>     > @@ -15,6 +15,7 @@
>     >
>     >  #include "net/net.h"
>     >  #include "net/tap.h"
>     > +#include "net/vhost-user.h"
>     >
>     >  #include "hw/virtio/virtio-net.h"
>     >  #include "net/vhost_net.h"
>     > @@ -360,6 +361,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>     >      case NET_CLIENT_OPTIONS_KIND_TAP:
>     >          vhost_net = tap_get_vhost_net(nc);
>     >          break;
>     > +    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>     > +        vhost_net = vhost_user_get_vhost_net(nc);
>     > +        break;
>     >      default:
>     >          break;
>     >      }
>     > diff --git a/net/hub.c b/net/hub.c
>     > index 33a99c9..7e0f2d6 100644
>     > --- a/net/hub.c
>     > +++ b/net/hub.c
>     > @@ -322,6 +322,7 @@ void net_hub_check_clients(void)
>     >              case NET_CLIENT_OPTIONS_KIND_TAP:
>     >              case NET_CLIENT_OPTIONS_KIND_SOCKET:
>     >              case NET_CLIENT_OPTIONS_KIND_VDE:
>     > +            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>     >                  has_host_dev = 1;
>     >                  break;
>     >              default:
>     > diff --git a/net/net.c b/net/net.c
>     > index 9db4dba..907f679 100644
>     > --- a/net/net.c
>     > +++ b/net/net.c
>     > @@ -769,23 +769,24 @@ static int (* const net_client_init_fun
>     [NET_CLIENT_OPTIONS_KIND_MAX])(
>     >      const NetClientOptions *opts,
>     >      const char *name,
>     >      NetClientState *peer) = {
>     > -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
>     > +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
>     >  #ifdef CONFIG_SLIRP
>     > -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
>     > +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
>     >  #endif
>     > -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
>     > -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
>     > +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
>     > +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
>     >  #ifdef CONFIG_VDE
>     > -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
>     > +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
>     >  #endif
>     >  #ifdef CONFIG_NETMAP
>     > -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
>     > +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
>     >  #endif
>     > -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
>     > +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
>     >  #ifdef CONFIG_NET_BRIDGE
>     > -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
>     > +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,
> 
>     These changes are unrelated.
> 
> OK. Removing them.
> 
> 
>     >  #endif
>     > -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
>     > +        [NET_CLIENT_OPTIONS_KIND_HUBPORT]       = net_init_hubport,
>     > +        [NET_CLIENT_OPTIONS_KIND_VHOST_USER]    = net_init_vhost_user,
>     >  };
>     >
>     >
>     > @@ -819,6 +820,7 @@ static int net_client_init1(const void *object, int
>     is_netdev, Error **errp)
>     >          case NET_CLIENT_OPTIONS_KIND_BRIDGE:
>     >  #endif
>     >          case NET_CLIENT_OPTIONS_KIND_HUBPORT:
>     > +        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>     >              break;
>     >
>     >          default:
>     > @@ -902,11 +904,12 @@ static int net_host_check_device(const char
>     *device)
>     >                                         , "bridge"
>     >  #endif
>     >  #ifdef CONFIG_SLIRP
>     > -                                       ,"user"
>     > +                                       , "user"
>     >  #endif
>     >  #ifdef CONFIG_VDE
>     > -                                       ,"vde"
>     > +                                       , "vde"
>     >  #endif
>     > +                                       , "vhost-user"
>     >      };
>     >      for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
>     >          if (!strncmp(valid_param_list[i], device,
>     > diff --git a/net/vhost-user.c b/net/vhost-user.c
>     > index 4bdd19d..69a5eb4 100644
>     > --- a/net/vhost-user.c
>     > +++ b/net/vhost-user.c
>     > @@ -12,6 +12,7 @@
>     >  #include "net/vhost_net.h"
>     >  #include "net/vhost-user.h"
>     >  #include "sysemu/char.h"
>     > +#include "qemu/config-file.h"
>     >  #include "qemu/error-report.h"
>     >
>     >  typedef struct VhostUserState {
>     > @@ -21,9 +22,17 @@ typedef struct VhostUserState {
>     >      VHostNetState *vhost_net;
>     >  } VhostUserState;
>     >
>     > +typedef struct VhostUserChardevProps {
>     > +    bool is_socket;
>     > +    bool is_unix;
>     > +    bool is_server;
>     > +    bool has_unsupported;
>     > +} VhostUserChardevProps;
>     > +
>     >  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
>     >  {
>     >      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
>     > +    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
>     >      return s->vhost_net;
>     >  }
>     >
>     > @@ -82,7 +91,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
>     >  }
>     >
>     >  static NetClientInfo net_vhost_user_info = {
>     > -        .type = 0,
>     > +        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
>     >          .size = sizeof(VhostUserState),
>     >          .cleanup = vhost_user_cleanup,
>     >          .has_vnet_hdr = vhost_user_has_vnet_hdr,
>     > @@ -148,8 +157,109 @@ static int net_vhost_user_init(NetClientState
>     *peer, const char *device,
>     >      return 0;
>     >  }
>     >
>     > +static int net_vhost_chardev_opts(const char *name, const char *value,
>     > +        void *opaque)
>     > +{
>     > +    VhostUserChardevProps *props = opaque;
>     > +
>     > +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
>     > +        props->is_socket = 1;
>     > +    } else if (strcmp(name, "path") == 0) {
>     > +        props->is_unix = 1;
>     > +    } else if (strcmp(name, "server") == 0) {
>     > +        props->is_server = 1;
>     > +    } else {
>     > +        error_report("vhost-user does not support a chardev"
>     > +                     " with the following option:\n %s = %s",
>     > +                     name, value);
>     > +        props->has_unsupported = 1;
>     > +        return -1;
>     > +    }
>     > +    return 0;
>     > +}
>     > +
>     > +static CharDriverState *net_vhost_parse_chardev(
>     > +        const NetdevVhostUserOptions *opts)
>     > +{
>     > +    CharDriverState *chr = qemu_chr_find(opts->chardev);
>     > +    VhostUserChardevProps props;
>     > +
>     > +    if (chr == NULL) {
>     > +        error_report("chardev \"%s\" not found\n", opts->chardev);
>     > +        return 0;
>     > +    }
>     > +
>     > +    /* inspect chardev opts */
>     > +    memset(&props, 0, sizeof(props));
>     > +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
>     > +
>     > +    if (!props.is_socket || !props.is_unix) {
>     > +        error_report("chardev \"%s\" is not a unix socket\n",
>     > +                     opts->chardev);
>     > +        return 0;
>     > +    }
>     > +
>     > +    if (props.has_unsupported) {
>     > +        error_report("chardev \"%s\" has an unsupported option\n",
>     > +                opts->chardev);
>     > +        return 0;
>     > +    }
>     > +
>     > +    qemu_chr_fe_claim_no_fail(chr);
>     > +
>     > +    return chr;
>     > +}
>     > +
>     > +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
>     > +{
>     > +    const char *name = opaque;
>     > +    const char *driver, *netdev;
>     > +    const char virtio_name[] = "virtio-net-";
>     > +
>     > +    driver = qemu_opt_get(opts, "driver");
>     > +    netdev = qemu_opt_get(opts, "netdev");
>     > +
>     > +    if (!driver || !netdev) {
>     > +        return 0;
>     > +    }
>     > +
>     > +    if ((strcmp(netdev, name) == 0)
>     > +            && (strncmp(driver, virtio_name, strlen(virtio_name)) != 0))
>     {
>     > +        error_report("vhost-user requires frontend driver
>     virtio-net-*");
>     > +        return -1;
>     > +    }
>     > +
>     > +    return 0;
>     > +}
>     > +
>     >  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
>     >                     NetClientState *peer)
>     >  {
>     > -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
>     > +    const NetdevVhostUserOptions *vhost_user_opts;
>     > +    CharDriverState *chr;
>     > +    bool vhostforce;
>     > +
>     > +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
>     > +    vhost_user_opts = opts->vhost_user;
>     > +
>     > +    chr = net_vhost_parse_chardev(vhost_user_opts);
>     > +    if (!chr) {
>     > +        error_report("No suitable chardev found\n");
>     > +        return -1;
>     > +    }
>     > +
>     > +    /* verify net frontend */
>     > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
>     > +                          (void *)name, true) == -1) {
>     > +        return -1;
>     > +    }
>     > +
>     > +    /* vhostforce for non-MSIX */
>     > +    if (vhost_user_opts->has_vhostforce) {
>     > +        vhostforce = vhost_user_opts->vhostforce;
>     > +    } else {
>     > +        vhostforce = false;
>     > +    }
>     > +
>     > +    return net_vhost_user_init(peer, "vhost_user", name, chr,
>     vhostforce);
>     >  }
>     > diff --git a/qapi-schema.json b/qapi-schema.json
>     > index 1f28177..f458dd8 100644
>     > --- a/qapi-schema.json
>     > +++ b/qapi-schema.json
>     > @@ -3264,6 +3264,22 @@
>     >      '*devname':    'str' } }
>     >
>     >  ##
>     > +# @NetdevVhostUserOptions
>     > +#
>     > +# Vhost-user network backend
>     > +#
>     > +# @path: control socket path
>     > +#
>     > +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default:
>     false).
>     > +#
>     > +# Since 2.1
>     > +##
>     > +{ 'type': 'NetdevVhostUserOptions',
>     > +  'data': {
>     > +    'chardev':        'str',
> 
>     chardev or path?
> 
> Right, it's chardev.
> 
> 
>     > +    '*vhostforce':    'bool' } }
>     > +
>     > +##
>     >  # @NetClientOptions
>     >  #
>     >  # A discriminated record of network device traits.
>     > @@ -3281,7 +3297,8 @@
>     >      'dump':     'NetdevDumpOptions',
>     >      'bridge':   'NetdevBridgeOptions',
>     >      'hubport':  'NetdevHubPortOptions',
>     > -    'netmap':   'NetdevNetmapOptions' } }
>     > +    'netmap':   'NetdevNetmapOptions',
>     > +    'vhost-user': 'NetdevVhostUserOptions' } }
>     >
>     >  ##
>     >  # @NetLegacy
>     > diff --git a/qemu-options.hx b/qemu-options.hx
>     > index 7f4ab83..2514264 100644
>     > --- a/qemu-options.hx
>     > +++ b/qemu-options.hx
>     > @@ -1459,6 +1459,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>     >  #ifdef CONFIG_NETMAP
>     >      "netmap|"
>     >  #endif
>     > +    "vhost-user|"
>     >      "socket|"
>     >      "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
>     >  STEXI
>     > @@ -1790,6 +1791,23 @@ The hubport netdev lets you connect a NIC to a
>     QEMU "vlan" instead of a single
>     >  netdev.  @code{-net} and @code{-device} with parameter @option{vlan}
>     create the
>     >  required hub automatically.
>     >
>     > +@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
>     > +
>     > +Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev
>     should
>     > +be a unix domain socket backed one. The vhost-user uses a specifically
>     defined
>     > +protocol to pass vhost ioctl replacement messages to an application on
>     the other
>     > +end of the socket. On non-MSIX guests, the feature can be forced with
>     > +@var{vhostforce}.
>     > +
>     > +Example:
>     > +@example
>     > +qemu -m 512 -object memory-file,id=mem,size=512M,mem-path=/
>     hugetlbfs,share=on \
>     > +     -numa node,memdev=mem \
>     > +     -chardev socket,path=/path/to/socket \
>     > +     -netdev type=vhost-user,id=net0,chardev=chr0 \
>     > +     -device virtio-net-pci,netdev=net0
>     > +@end example
>     > +
>     >  @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
>     >  Dump network traffic on VLAN @var{n} to file @var{file} (@file
>     {qemu-vlan0.pcap} by default).
>     >  At most @var{len} bytes (64k by default) per packet are stored. The file
>     format is
>     >
>     >
> 
>     --
>     You received this message because you are subscribed to the Google Groups
>     "Snabb Switch development" group.
>     To unsubscribe from this group and stop receiving emails from it, send an
>     email to snabb-devel+unsubscribe@googlegroups.com.
>     To post to this group, send an email to snabb-devel@googlegroups.com.
>     Visit this group at http://groups.google.com/group/snabb-devel.
> 
> 
> regards,
> Nikolay Nikolaev


Pls remember to base on top of vhost branch in my tree,
this way you will not need to re-post merged patches.

If you want me to drop some merged patch from my tree,
include a revert and I'll figure it out.

-- 
MST

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 15/18] Add the vhost-user netdev backend to the command line

[Nikolay Nikolaev]

[-- Attachment #1: Type: text/plain, Size: 18459 bytes --]

On Mon, Jun 9, 2014 at 4:31 PM, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Mon, Jun 09, 2014 at 04:28:23PM +0300, Nikolay Nikolaev wrote:
> > Hello,
> >
> >
> > On Thu, Jun 5, 2014 at 5:37 PM, Luiz Capitulino <lcapitulino@redhat.com>
> wrote:
> >
> >     On Tue, 27 May 2014 15:06:43 +0300
> >     Nikolay Nikolaev <n.nikolaev@virtualopensystems.com> wrote:
> >
> >     > The supplied chardev id will be inspected for supported options.
> Only
> >     > a socket backend, with a set path (i.e. a Unix socket) and
> optionally
> >     > the server parameter set, will be allowed. Other options (nowait,
> telnet)
> >     > will make the chardev unusable and the netdev will not be
> initialised.
> >     >
> >     > Additional checks for validity:
> >     >   - requires `-numa node,memdev=..`
> >     >   - requires `-device virtio-net-*`
> >     >
> >     > The `vhostforce` option is used to force vhost-net when we deal
> with
> >     > non-MSIX guests.
> >     >
> >     > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> >     > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com
> >
> >
> >     I gave a quick review and apart from some minor comments below it
> seems
> >     good to me, but I think it would be good to have Eric's review too:
> >
> >     Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
> >
> > Thanks!
> >
> >
> >     > ---
> >     >  hmp-commands.hx    |    4 +-
> >     >  hw/net/vhost_net.c |    4 ++
> >     >  net/hub.c          |    1
> >     >  net/net.c          |   25 ++++++-----
> >     >  net/vhost-user.c   |  114
> >     +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >     >  qapi-schema.json   |   19 ++++++++-
> >     >  qemu-options.hx    |   18 ++++++++
> >     >  7 files changed, 169 insertions(+), 16 deletions(-)
> >     >
> >     > diff --git a/hmp-commands.hx b/hmp-commands.hx
> >     > index 8971f1b..ef3782c 100644
> >     > --- a/hmp-commands.hx
> >     > +++ b/hmp-commands.hx
> >     > @@ -1205,7 +1205,7 @@ ETEXI
> >     >      {
> >     >          .name       = "host_net_add",
> >     >          .args_type  = "device:s,opts:s?",
> >     > -        .params     = "tap|user|socket|vde|netmap|dump [options]",
> >     > +        .params     = "tap|user|socket|vde|netmap|vhost-user|dump
> >     [options]",
> >     >          .help       = "add host VLAN client",
> >     >          .mhandler.cmd = net_host_device_add,
> >     >      },
> >     > @@ -1233,7 +1233,7 @@ ETEXI
> >     >      {
> >     >          .name       = "netdev_add",
> >     >          .args_type  = "netdev:O",
> >     > -        .params     =
> "[user|tap|socket|hubport|netmap],id=str[,prop=
> >     value][,...]",
> >     > +        .params     =
> "[user|tap|socket|hubport|netmap|vhost-user],id=
> >     str[,prop=value][,...]",
> >     >          .help       = "add host network device",
> >     >          .mhandler.cmd = hmp_netdev_add,
> >     >      },
> >     > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> >     > index 5f06736..7ac7c21 100644
> >     > --- a/hw/net/vhost_net.c
> >     > +++ b/hw/net/vhost_net.c
> >     > @@ -15,6 +15,7 @@
> >     >
> >     >  #include "net/net.h"
> >     >  #include "net/tap.h"
> >     > +#include "net/vhost-user.h"
> >     >
> >     >  #include "hw/virtio/virtio-net.h"
> >     >  #include "net/vhost_net.h"
> >     > @@ -360,6 +361,9 @@ VHostNetState *get_vhost_net(NetClientState
> *nc)
> >     >      case NET_CLIENT_OPTIONS_KIND_TAP:
> >     >          vhost_net = tap_get_vhost_net(nc);
> >     >          break;
> >     > +    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> >     > +        vhost_net = vhost_user_get_vhost_net(nc);
> >     > +        break;
> >     >      default:
> >     >          break;
> >     >      }
> >     > diff --git a/net/hub.c b/net/hub.c
> >     > index 33a99c9..7e0f2d6 100644
> >     > --- a/net/hub.c
> >     > +++ b/net/hub.c
> >     > @@ -322,6 +322,7 @@ void net_hub_check_clients(void)
> >     >              case NET_CLIENT_OPTIONS_KIND_TAP:
> >     >              case NET_CLIENT_OPTIONS_KIND_SOCKET:
> >     >              case NET_CLIENT_OPTIONS_KIND_VDE:
> >     > +            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> >     >                  has_host_dev = 1;
> >     >                  break;
> >     >              default:
> >     > diff --git a/net/net.c b/net/net.c
> >     > index 9db4dba..907f679 100644
> >     > --- a/net/net.c
> >     > +++ b/net/net.c
> >     > @@ -769,23 +769,24 @@ static int (* const net_client_init_fun
> >     [NET_CLIENT_OPTIONS_KIND_MAX])(
> >     >      const NetClientOptions *opts,
> >     >      const char *name,
> >     >      NetClientState *peer) = {
> >     > -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
> >     > +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
> >     >  #ifdef CONFIG_SLIRP
> >     > -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
> >     > +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
> >     >  #endif
> >     > -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
> >     > -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
> >     > +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
> >     > +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
> >     >  #ifdef CONFIG_VDE
> >     > -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
> >     > +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
> >     >  #endif
> >     >  #ifdef CONFIG_NETMAP
> >     > -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
> >     > +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
> >     >  #endif
> >     > -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
> >     > +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
> >     >  #ifdef CONFIG_NET_BRIDGE
> >     > -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> >     > +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,
> >
> >     These changes are unrelated.
> >
> > OK. Removing them.
> >
> >
> >     >  #endif
> >     > -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> >     > +        [NET_CLIENT_OPTIONS_KIND_HUBPORT]       =
> net_init_hubport,
> >     > +        [NET_CLIENT_OPTIONS_KIND_VHOST_USER]    =
> net_init_vhost_user,
> >     >  };
> >     >
> >     >
> >     > @@ -819,6 +820,7 @@ static int net_client_init1(const void
> *object, int
> >     is_netdev, Error **errp)
> >     >          case NET_CLIENT_OPTIONS_KIND_BRIDGE:
> >     >  #endif
> >     >          case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> >     > +        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> >     >              break;
> >     >
> >     >          default:
> >     > @@ -902,11 +904,12 @@ static int net_host_check_device(const char
> >     *device)
> >     >                                         , "bridge"
> >     >  #endif
> >     >  #ifdef CONFIG_SLIRP
> >     > -                                       ,"user"
> >     > +                                       , "user"
> >     >  #endif
> >     >  #ifdef CONFIG_VDE
> >     > -                                       ,"vde"
> >     > +                                       , "vde"
> >     >  #endif
> >     > +                                       , "vhost-user"
> >     >      };
> >     >      for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
> >     >          if (!strncmp(valid_param_list[i], device,
> >     > diff --git a/net/vhost-user.c b/net/vhost-user.c
> >     > index 4bdd19d..69a5eb4 100644
> >     > --- a/net/vhost-user.c
> >     > +++ b/net/vhost-user.c
> >     > @@ -12,6 +12,7 @@
> >     >  #include "net/vhost_net.h"
> >     >  #include "net/vhost-user.h"
> >     >  #include "sysemu/char.h"
> >     > +#include "qemu/config-file.h"
> >     >  #include "qemu/error-report.h"
> >     >
> >     >  typedef struct VhostUserState {
> >     > @@ -21,9 +22,17 @@ typedef struct VhostUserState {
> >     >      VHostNetState *vhost_net;
> >     >  } VhostUserState;
> >     >
> >     > +typedef struct VhostUserChardevProps {
> >     > +    bool is_socket;
> >     > +    bool is_unix;
> >     > +    bool is_server;
> >     > +    bool has_unsupported;
> >     > +} VhostUserChardevProps;
> >     > +
> >     >  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
> >     >  {
> >     >      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> >     > +    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> >     >      return s->vhost_net;
> >     >  }
> >     >
> >     > @@ -82,7 +91,7 @@ static bool vhost_user_has_ufo(NetClientState
> *nc)
> >     >  }
> >     >
> >     >  static NetClientInfo net_vhost_user_info = {
> >     > -        .type = 0,
> >     > +        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
> >     >          .size = sizeof(VhostUserState),
> >     >          .cleanup = vhost_user_cleanup,
> >     >          .has_vnet_hdr = vhost_user_has_vnet_hdr,
> >     > @@ -148,8 +157,109 @@ static int net_vhost_user_init(NetClientState
> >     *peer, const char *device,
> >     >      return 0;
> >     >  }
> >     >
> >     > +static int net_vhost_chardev_opts(const char *name, const char
> *value,
> >     > +        void *opaque)
> >     > +{
> >     > +    VhostUserChardevProps *props = opaque;
> >     > +
> >     > +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket")
> == 0) {
> >     > +        props->is_socket = 1;
> >     > +    } else if (strcmp(name, "path") == 0) {
> >     > +        props->is_unix = 1;
> >     > +    } else if (strcmp(name, "server") == 0) {
> >     > +        props->is_server = 1;
> >     > +    } else {
> >     > +        error_report("vhost-user does not support a chardev"
> >     > +                     " with the following option:\n %s = %s",
> >     > +                     name, value);
> >     > +        props->has_unsupported = 1;
> >     > +        return -1;
> >     > +    }
> >     > +    return 0;
> >     > +}
> >     > +
> >     > +static CharDriverState *net_vhost_parse_chardev(
> >     > +        const NetdevVhostUserOptions *opts)
> >     > +{
> >     > +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> >     > +    VhostUserChardevProps props;
> >     > +
> >     > +    if (chr == NULL) {
> >     > +        error_report("chardev \"%s\" not found\n", opts->chardev);
> >     > +        return 0;
> >     > +    }
> >     > +
> >     > +    /* inspect chardev opts */
> >     > +    memset(&props, 0, sizeof(props));
> >     > +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props,
> false);
> >     > +
> >     > +    if (!props.is_socket || !props.is_unix) {
> >     > +        error_report("chardev \"%s\" is not a unix socket\n",
> >     > +                     opts->chardev);
> >     > +        return 0;
> >     > +    }
> >     > +
> >     > +    if (props.has_unsupported) {
> >     > +        error_report("chardev \"%s\" has an unsupported option\n",
> >     > +                opts->chardev);
> >     > +        return 0;
> >     > +    }
> >     > +
> >     > +    qemu_chr_fe_claim_no_fail(chr);
> >     > +
> >     > +    return chr;
> >     > +}
> >     > +
> >     > +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> >     > +{
> >     > +    const char *name = opaque;
> >     > +    const char *driver, *netdev;
> >     > +    const char virtio_name[] = "virtio-net-";
> >     > +
> >     > +    driver = qemu_opt_get(opts, "driver");
> >     > +    netdev = qemu_opt_get(opts, "netdev");
> >     > +
> >     > +    if (!driver || !netdev) {
> >     > +        return 0;
> >     > +    }
> >     > +
> >     > +    if ((strcmp(netdev, name) == 0)
> >     > +            && (strncmp(driver, virtio_name, strlen(virtio_name))
> != 0))
> >     {
> >     > +        error_report("vhost-user requires frontend driver
> >     virtio-net-*");
> >     > +        return -1;
> >     > +    }
> >     > +
> >     > +    return 0;
> >     > +}
> >     > +
> >     >  int net_init_vhost_user(const NetClientOptions *opts, const char
> *name,
> >     >                     NetClientState *peer)
> >     >  {
> >     > -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> >     > +    const NetdevVhostUserOptions *vhost_user_opts;
> >     > +    CharDriverState *chr;
> >     > +    bool vhostforce;
> >     > +
> >     > +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> >     > +    vhost_user_opts = opts->vhost_user;
> >     > +
> >     > +    chr = net_vhost_parse_chardev(vhost_user_opts);
> >     > +    if (!chr) {
> >     > +        error_report("No suitable chardev found\n");
> >     > +        return -1;
> >     > +    }
> >     > +
> >     > +    /* verify net frontend */
> >     > +    if (qemu_opts_foreach(qemu_find_opts("device"),
> net_vhost_check_net,
> >     > +                          (void *)name, true) == -1) {
> >     > +        return -1;
> >     > +    }
> >     > +
> >     > +    /* vhostforce for non-MSIX */
> >     > +    if (vhost_user_opts->has_vhostforce) {
> >     > +        vhostforce = vhost_user_opts->vhostforce;
> >     > +    } else {
> >     > +        vhostforce = false;
> >     > +    }
> >     > +
> >     > +    return net_vhost_user_init(peer, "vhost_user", name, chr,
> >     vhostforce);
> >     >  }
> >     > diff --git a/qapi-schema.json b/qapi-schema.json
> >     > index 1f28177..f458dd8 100644
> >     > --- a/qapi-schema.json
> >     > +++ b/qapi-schema.json
> >     > @@ -3264,6 +3264,22 @@
> >     >      '*devname':    'str' } }
> >     >
> >     >  ##
> >     > +# @NetdevVhostUserOptions
> >     > +#
> >     > +# Vhost-user network backend
> >     > +#
> >     > +# @path: control socket path
> >     > +#
> >     > +# @vhostforce: #optional vhost on for non-MSIX virtio guests
> (default:
> >     false).
> >     > +#
> >     > +# Since 2.1
> >     > +##
> >     > +{ 'type': 'NetdevVhostUserOptions',
> >     > +  'data': {
> >     > +    'chardev':        'str',
> >
> >     chardev or path?
> >
> > Right, it's chardev.
> >
> >
> >     > +    '*vhostforce':    'bool' } }
> >     > +
> >     > +##
> >     >  # @NetClientOptions
> >     >  #
> >     >  # A discriminated record of network device traits.
> >     > @@ -3281,7 +3297,8 @@
> >     >      'dump':     'NetdevDumpOptions',
> >     >      'bridge':   'NetdevBridgeOptions',
> >     >      'hubport':  'NetdevHubPortOptions',
> >     > -    'netmap':   'NetdevNetmapOptions' } }
> >     > +    'netmap':   'NetdevNetmapOptions',
> >     > +    'vhost-user': 'NetdevVhostUserOptions' } }
> >     >
> >     >  ##
> >     >  # @NetLegacy
> >     > diff --git a/qemu-options.hx b/qemu-options.hx
> >     > index 7f4ab83..2514264 100644
> >     > --- a/qemu-options.hx
> >     > +++ b/qemu-options.hx
> >     > @@ -1459,6 +1459,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
> >     >  #ifdef CONFIG_NETMAP
> >     >      "netmap|"
> >     >  #endif
> >     > +    "vhost-user|"
> >     >      "socket|"
> >     >      "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
> >     >  STEXI
> >     > @@ -1790,6 +1791,23 @@ The hubport netdev lets you connect a NIC
> to a
> >     QEMU "vlan" instead of a single
> >     >  netdev.  @code{-net} and @code{-device} with parameter
> @option{vlan}
> >     create the
> >     >  required hub automatically.
> >     >
> >     > +@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
> >     > +
> >     > +Establish a vhost-user netdev, backed by a chardev @var{id}. The
> chardev
> >     should
> >     > +be a unix domain socket backed one. The vhost-user uses a
> specifically
> >     defined
> >     > +protocol to pass vhost ioctl replacement messages to an
> application on
> >     the other
> >     > +end of the socket. On non-MSIX guests, the feature can be forced
> with
> >     > +@var{vhostforce}.
> >     > +
> >     > +Example:
> >     > +@example
> >     > +qemu -m 512 -object memory-file,id=mem,size=512M,mem-path=/
> >     hugetlbfs,share=on \
> >     > +     -numa node,memdev=mem \
> >     > +     -chardev socket,path=/path/to/socket \
> >     > +     -netdev type=vhost-user,id=net0,chardev=chr0 \
> >     > +     -device virtio-net-pci,netdev=net0
> >     > +@end example
> >     > +
> >     >  @item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
> >     >  Dump network traffic on VLAN @var{n} to file @var{file} (@file
> >     {qemu-vlan0.pcap} by default).
> >     >  At most @var{len} bytes (64k by default) per packet are stored.
> The file
> >     format is
> >     >
> >     >
> >
> >     --
> >     You received this message because you are subscribed to the Google
> Groups
> >     "Snabb Switch development" group.
> >     To unsubscribe from this group and stop receiving emails from it,
> send an
> >     email to snabb-devel+unsubscribe@googlegroups.com.
> >     To post to this group, send an email to snabb-devel@googlegroups.com
> .
> >     Visit this group at http://groups.google.com/group/snabb-devel.
> >
> >
> > regards,
> > Nikolay Nikolaev
>
>
> Pls remember to base on top of vhost branch in my tree,
> this way you will not need to re-post merged patches.
>

OK - that's what we're doing. There are two questions though:

 - should we post with separate numbering - 15,16,18 will become 1/3, 2/3,
3/3? Or do you prefer to preserve the original numbering?

 - We can not test it. Probably we get something wrong, but vhost-user-v10
was rebased on Hu Tao's NUMA v3.2 series. And it uses
this on the command line to enalbe mmaped memory share flag (on HUGETLBFS):

     -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
      -numa node,memdev=mem \

Now, this does not work, which will break the qtest (patch 18). Maybe we
misled you by saying it is rebased on memdev instead of naming it NUMA
patchseries.


>
> If you want me to drop some merged patch from my tree,
> include a revert and I'll figure it out.
>
> Will have this in mind - thanks.

>  --
> MST
>
> --
> You received this message because you are subscribed to the Google Groups
> "Snabb Switch development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to snabb-devel+unsubscribe@googlegroups.com.
> To post to this group, send an email to snabb-devel@googlegroups.com.
> Visit this group at http://groups.google.com/group/snabb-devel.
>

regards,
Nikolay Nikolaev

[-- Attachment #2: Type: text/html, Size: 26263 bytes --]

Re: [Qemu-devel] [PATCH v10 15/18] Add the vhost-user netdev backend to the command line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 6006 bytes --]

On 05/27/2014 06:06 AM, Nikolay Nikolaev wrote:
> The supplied chardev id will be inspected for supported options. Only
> a socket backend, with a set path (i.e. a Unix socket) and optionally
> the server parameter set, will be allowed. Other options (nowait, telnet)
> will make the chardev unusable and the netdev will not be initialised.
> 
> Additional checks for validity:
>   - requires `-numa node,memdev=..`
>   - requires `-device virtio-net-*`
> 
> The `vhostforce` option is used to force vhost-net when we deal with
> non-MSIX guests.
> 
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> ---
>  hmp-commands.hx    |    4 +-
>  hw/net/vhost_net.c |    4 ++
>  net/hub.c          |    1 
>  net/net.c          |   25 ++++++-----
>  net/vhost-user.c   |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json   |   19 ++++++++-
>  qemu-options.hx    |   18 ++++++++
>  7 files changed, 169 insertions(+), 16 deletions(-)
> 


>  
> +static int net_vhost_chardev_opts(const char *name, const char *value,
> +        void *opaque)

Indentation is off (second line should be aligned to the ( of the first).

> +{
> +    VhostUserChardevProps *props = opaque;
> +
> +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> +        props->is_socket = 1;

s/1/true/ - when a field is a boolean, assign it boolean constants.

> +    } else if (strcmp(name, "path") == 0) {
> +        props->is_unix = 1;
> +    } else if (strcmp(name, "server") == 0) {
> +        props->is_server = 1;
> +    } else {
> +        error_report("vhost-user does not support a chardev"
> +                     " with the following option:\n %s = %s",
> +                     name, value);
> +        props->has_unsupported = 1;

and 3 more times.

> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static CharDriverState *net_vhost_parse_chardev(
> +        const NetdevVhostUserOptions *opts)

Unusual indentation, but I see your dilemma of fitting 80 columns.

> +{
> +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> +    VhostUserChardevProps props;
> +
> +    if (chr == NULL) {
> +        error_report("chardev \"%s\" not found\n", opts->chardev);
> +        return 0;

s/0/NULL/ - much nicer to use the named constant when referring to a
null pointer.

> +    }
> +
> +    /* inspect chardev opts */
> +    memset(&props, 0, sizeof(props));
> +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> +
> +    if (!props.is_socket || !props.is_unix) {
> +        error_report("chardev \"%s\" is not a unix socket\n",
> +                     opts->chardev);
> +        return 0;
> +    }
> +
> +    if (props.has_unsupported) {
> +        error_report("chardev \"%s\" has an unsupported option\n",
> +                opts->chardev);
> +        return 0;

2 more instances

> +    }
> +
> +    qemu_chr_fe_claim_no_fail(chr);
> +
> +    return chr;
> +}
> +
> +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> +{
> +    const char *name = opaque;
> +    const char *driver, *netdev;
> +    const char virtio_name[] = "virtio-net-";
> +
> +    driver = qemu_opt_get(opts, "driver");
> +    netdev = qemu_opt_get(opts, "netdev");
> +
> +    if (!driver || !netdev) {
> +        return 0;
> +    }
> +
> +    if ((strcmp(netdev, name) == 0)
> +            && (strncmp(driver, virtio_name, strlen(virtio_name)) != 0)) {

Unusual indentation and spurious ():

if (strcmp(netdev, name) == 0 &&
    strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {

> +        error_report("vhost-user requires frontend driver virtio-net-*");

How many of these error_report() should be using Error **errp logistics
instead?

> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
>                     NetClientState *peer)

Pre-existing indentation problem, but you could fix it here (or if you
introduced it earlier in the series, fix it there)

>  {
> -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> +    const NetdevVhostUserOptions *vhost_user_opts;
> +    CharDriverState *chr;
> +    bool vhostforce;
> +
> +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> +    vhost_user_opts = opts->vhost_user;
> +
> +    chr = net_vhost_parse_chardev(vhost_user_opts);
> +    if (!chr) {
> +        error_report("No suitable chardev found\n");

No \n in error_report() messages.

> +        return -1;
> +    }
> +
> +    /* verify net frontend */
> +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> +                          (void *)name, true) == -1) {

This is C; why do you need a cast to (void*)?

> +        return -1;
> +    }
> +
> +    /* vhostforce for non-MSIX */
> +    if (vhost_user_opts->has_vhostforce) {
> +        vhostforce = vhost_user_opts->vhostforce;
> +    } else {
> +        vhostforce = false;
> +    }
> +
> +    return net_vhost_user_init(peer, "vhost_user", name, chr, vhostforce);
>  }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 1f28177..f458dd8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3264,6 +3264,22 @@
>      '*devname':    'str' } }
>  
>  ##
> +# @NetdevVhostUserOptions
> +#
> +# Vhost-user network backend
> +#
> +# @path: control socket path
> +#
> +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
> +#
> +# Since 2.1
> +##
> +{ 'type': 'NetdevVhostUserOptions',
> +  'data': {
> +    'chardev':        'str',
> +    '*vhostforce':    'bool' } }

Bikeshedding - is 'vhost-force' any more legible?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 15/18] Add the vhost-user netdev backend to the command line

[Nikolay Nikolaev]

[-- Attachment #1: Type: text/plain, Size: 7171 bytes --]

On Thu, Jun 5, 2014 at 7:08 PM, Eric Blake <eblake@redhat.com> wrote:

> On 05/27/2014 06:06 AM, Nikolay Nikolaev wrote:
> > The supplied chardev id will be inspected for supported options. Only
> > a socket backend, with a set path (i.e. a Unix socket) and optionally
> > the server parameter set, will be allowed. Other options (nowait, telnet)
> > will make the chardev unusable and the netdev will not be initialised.
> >
> > Additional checks for validity:
> >   - requires `-numa node,memdev=..`
> >   - requires `-device virtio-net-*`
> >
> > The `vhostforce` option is used to force vhost-net when we deal with
> > non-MSIX guests.
> >
> > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> > ---
> >  hmp-commands.hx    |    4 +-
> >  hw/net/vhost_net.c |    4 ++
> >  net/hub.c          |    1
> >  net/net.c          |   25 ++++++-----
> >  net/vhost-user.c   |  114
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  qapi-schema.json   |   19 ++++++++-
> >  qemu-options.hx    |   18 ++++++++
> >  7 files changed, 169 insertions(+), 16 deletions(-)
> >
>
>
> >
> > +static int net_vhost_chardev_opts(const char *name, const char *value,
> > +        void *opaque)
>
> Indentation is off (second line should be aligned to the ( of the first).
>
OK, will fix it.

>
> > +{
> > +    VhostUserChardevProps *props = opaque;
> > +
> > +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> > +        props->is_socket = 1;
>
> s/1/true/ - when a field is a boolean, assign it boolean constants.
>
OK, will fix all of them.

>
> > +    } else if (strcmp(name, "path") == 0) {
> > +        props->is_unix = 1;
> > +    } else if (strcmp(name, "server") == 0) {
> > +        props->is_server = 1;
> > +    } else {
> > +        error_report("vhost-user does not support a chardev"
> > +                     " with the following option:\n %s = %s",
> > +                     name, value);
> > +        props->has_unsupported = 1;
>
> and 3 more times.
>
> > +        return -1;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static CharDriverState *net_vhost_parse_chardev(
> > +        const NetdevVhostUserOptions *opts)
>
> Unusual indentation, but I see your dilemma of fitting 80 columns.
>
What woudl be the right approach here, is leaving it 84 colums acceptable:

static CharDriverState *net_vhost_parse_chardev(const
NetdevVhostUserOptions *opts)

Or split it like this:
static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions

*opts)


>
> > +{
> > +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> > +    VhostUserChardevProps props;
> > +
> > +    if (chr == NULL) {
> > +        error_report("chardev \"%s\" not found\n", opts->chardev);
> > +        return 0;
>
> s/0/NULL/ - much nicer to use the named constant when referring to a
> null pointer.
>
OK - fixed on all places.


> > +    }
> > +
> > +    /* inspect chardev opts */
> > +    memset(&props, 0, sizeof(props));
> > +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> > +
> > +    if (!props.is_socket || !props.is_unix) {
> > +        error_report("chardev \"%s\" is not a unix socket\n",
> > +                     opts->chardev);
> > +        return 0;
> > +    }
> > +
> > +    if (props.has_unsupported) {
> > +        error_report("chardev \"%s\" has an unsupported option\n",
> > +                opts->chardev);
> > +        return 0;
>
> 2 more instances
>
> > +    }
> > +
> > +    qemu_chr_fe_claim_no_fail(chr);
> > +
> > +    return chr;
> > +}
> > +
> > +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> > +{
> > +    const char *name = opaque;
> > +    const char *driver, *netdev;
> > +    const char virtio_name[] = "virtio-net-";
> > +
> > +    driver = qemu_opt_get(opts, "driver");
> > +    netdev = qemu_opt_get(opts, "netdev");
> > +
> > +    if (!driver || !netdev) {
> > +        return 0;
> > +    }
> > +
> > +    if ((strcmp(netdev, name) == 0)
> > +            && (strncmp(driver, virtio_name, strlen(virtio_name)) !=
> 0)) {
>
> Unusual indentation and spurious ():
>
> if (strcmp(netdev, name) == 0 &&
>     strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
>
> OK - thanks for the providing a solution.

> > +        error_report("vhost-user requires frontend driver
> virtio-net-*");
>
> How many of these error_report() should be using Error **errp logistics
> instead?
>
> I don't see this 'errp' logic applied here. These are static functions
called in the init function to check for compatible command line parameters.
This init fucntion is part of 'net_client_init_fun[]" which does not
provide 'Error **errp' argument. The way I see it, there is no way to
propagate the 'errp' up. Or am I getting this wrong?


>  > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
> >                     NetClientState *peer)
>
> Pre-existing indentation problem, but you could fix it here (or if you
> introduced it earlier in the series, fix it there)
>
Will fix it.

>
> >  {
> > -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> > +    const NetdevVhostUserOptions *vhost_user_opts;
> > +    CharDriverState *chr;
> > +    bool vhostforce;
> > +
> > +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> > +    vhost_user_opts = opts->vhost_user;
> > +
> > +    chr = net_vhost_parse_chardev(vhost_user_opts);
> > +    if (!chr) {
> > +        error_report("No suitable chardev found\n");
>
> No \n in error_report() messages.
>
OK.

>
> > +        return -1;
> > +    }
> > +
> > +    /* verify net frontend */
> > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> > +                          (void *)name, true) == -1) {
>
> This is C; why do you need a cast to (void*)?
>
Because of the constness in 'const char *name'. I am casting to 'char *
now', is it better?

>
> > +        return -1;
> > +    }
> > +
> > +    /* vhostforce for non-MSIX */
> > +    if (vhost_user_opts->has_vhostforce) {
> > +        vhostforce = vhost_user_opts->vhostforce;
> > +    } else {
> > +        vhostforce = false;
> > +    }
> > +
> > +    return net_vhost_user_init(peer, "vhost_user", name, chr,
> vhostforce);
> >  }
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 1f28177..f458dd8 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3264,6 +3264,22 @@
> >      '*devname':    'str' } }
> >
> >  ##
> > +# @NetdevVhostUserOptions
> > +#
> > +# Vhost-user network backend
> > +#
> > +# @path: control socket path
> > +#
> > +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default:
> false).
> > +#
> > +# Since 2.1
> > +##
> > +{ 'type': 'NetdevVhostUserOptions',
> > +  'data': {
> > +    'chardev':        'str',
> > +    '*vhostforce':    'bool' } }
>
> Bikeshedding - is 'vhost-force' any more legible?
>
'vhost-force' it is.

>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
> Thanks!

regards,
Nikolay Nikolaev

[-- Attachment #2: Type: text/html, Size: 11614 bytes --]

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 15/18] Add the vhost-user netdev backend to the command line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2514 bytes --]

On 06/09/2014 03:19 PM, Nikolay Nikolaev wrote:

>>> +static CharDriverState *net_vhost_parse_chardev(
>>> +        const NetdevVhostUserOptions *opts)
>>
>> Unusual indentation, but I see your dilemma of fitting 80 columns.
>>
> What woudl be the right approach here, is leaving it 84 colums acceptable:

checkpatch.pl is for guidance; it is okay to have a patch that doesn't
pass. At any rate, I won't reject a patch for long lines, or for unusual
style, where it is obvious that there is no way to fit things into 80
columns while still complying with all other style issues.


>>> +        error_report("vhost-user requires frontend driver
>> virtio-net-*");
>>
>> How many of these error_report() should be using Error **errp logistics
>> instead?
>>
>> I don't see this 'errp' logic applied here. These are static functions
> called in the init function to check for compatible command line parameters.
> This init fucntion is part of 'net_client_init_fun[]" which does not
> provide 'Error **errp' argument. The way I see it, there is no way to
> propagate the 'errp' up. Or am I getting this wrong?

It was more of a 'food for thought' question - we may eventually want to
enhance the code base to be able to propagate errp up; but since that is
a more invasive patch, and your series isn't making the situation worse,
it does not necessarily mean you have to do that additional work.  On
the other hand, if the conversion is worth doing, then getting it done
sooner makes it easier to take advantage of the improved error handling
for other new code in the same area.


>>> +
>>> +    /* verify net frontend */
>>> +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
>>> +                          (void *)name, true) == -1) {
>>
>> This is C; why do you need a cast to (void*)?
>>
> Because of the constness in 'const char *name'. I am casting to 'char *
> now', is it better?

Okay, I see.  Sometimes, I like to put in a comment /* cast away const
*/ to make it obvious why I'm doing things like that.  It also serves as
a spur to investigate a couple of things: 1) should the callback
signature should have allowed a const in the first place, rather than
making callers have to cast away const, 2) if the callback can modify
the pointer but I'm casting away const, am I going to cause data
corruption and/or a crash

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v10 16/18] Add vhost-user protocol documentation

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

On 05/27/2014 06:06 AM, Nikolay Nikolaev wrote:
> This document describes the basic message format used by vhost-user
> for communication over a unix domain socket. The protocol is based
> on the existing ioctl interface used for the kernel version of vhost.
> 
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> ---
>  docs/specs/vhost-user.txt |  261 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 261 insertions(+)
>  create mode 100644 docs/specs/vhost-user.txt
> 
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> new file mode 100644
> index 0000000..e7f43f5
> --- /dev/null
> +++ b/docs/specs/vhost-user.txt
> @@ -0,0 +1,261 @@
> +Vhost-user Protocol

Pre-existing problem throughout this directory, but it might be nice to
assert copyright and assign a license to this file.


> +
> +The protocol defines 2 sides of the communication, master and slave. Master is
> +the application that shares it's virtqueues, in our case QEMU. Slave is the

s/it's/its/ (remember, "it's" only works if "it is" could be used in the
same context)

> +consumer of the virtqueues.
> +
> +In the current implementation QEMU is the Master, and the Slave is intended to
> +be a software ethernet switch running in user space, such as Snabbswitch.

s/ethernet/Ethernet/

> +Communication
> +-------------
> +
> +The protocol for vhost-user is based on the existing implementation of vhost
> +for the Linux Kernel. Most messages that can be send via the Unix domain socket

s/send/sent/

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v10 16/18] Add vhost-user protocol documentation

[Michael S. Tsirkin]

On Thu, Jun 05, 2014 at 10:17:35AM -0600, Eric Blake wrote:
> On 05/27/2014 06:06 AM, Nikolay Nikolaev wrote:
> > This document describes the basic message format used by vhost-user
> > for communication over a unix domain socket. The protocol is based
> > on the existing ioctl interface used for the kernel version of vhost.
> > 
> > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> > ---
> >  docs/specs/vhost-user.txt |  261 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 261 insertions(+)
> >  create mode 100644 docs/specs/vhost-user.txt
> > 
> > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > new file mode 100644
> > index 0000000..e7f43f5
> > --- /dev/null
> > +++ b/docs/specs/vhost-user.txt
> > @@ -0,0 +1,261 @@
> > +Vhost-user Protocol
> 
> Pre-existing problem throughout this directory, but it might be nice to
> assert copyright and assign a license to this file.
> 
> 
> > +
> > +The protocol defines 2 sides of the communication, master and slave. Master is
> > +the application that shares it's virtqueues, in our case QEMU. Slave is the
> 
> s/it's/its/ (remember, "it's" only works if "it is" could be used in the
> same context)
> 
> > +consumer of the virtqueues.
> > +
> > +In the current implementation QEMU is the Master, and the Slave is intended to
> > +be a software ethernet switch running in user space, such as Snabbswitch.
> 
> s/ethernet/Ethernet/
> 
> > +Communication
> > +-------------
> > +
> > +The protocol for vhost-user is based on the existing implementation of vhost
> > +for the Linux Kernel. Most messages that can be send via the Unix domain socket
> 
> s/send/sent/

I made these changes after applying the patch, thanks!

> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

Re: [Qemu-devel] [PATCH v10 00/18] Vhost and vhost-net support for userspace based backends

[Michael S. Tsirkin]

On Tue, May 27, 2014 at 03:03:21PM +0300, Nikolay Nikolaev wrote:
> In this patch series we would like to introduce our approach for putting a
> virtio-net backend in an external userspace process. Our eventual target is to
> run the network backend in the Snabbswitch ethernet switch, while receiving
> traffic from a guest inside QEMU/KVM which runs an unmodified virtio-net
> implementation.
> 
> For this, we are working into extending vhost to allow equivalent functionality
> for userspace. Vhost already passes control of the data plane of virtio-net to
> the host kernel; we want to realize a similar model, but for userspace.
> 
> In this patch series the concept of a vhost-backend is introduced.
> 
> We define two vhost backend types - vhost-kernel and vhost-user. The former is
> the interface to the current kernel module implementation. Its control plane is
> ioctl based. The data plane is realized by the kernel directly accessing the
> QEMU allocated, guest memory.
> 
> In the new vhost-user backend, the control plane is based on communication
> between QEMU and another userspace process using a unix domain socket. This
> allows to implement a virtio backend for a guest running in QEMU, inside the
> other userspace process. For this communication we use a chardev with a Unix
> domain socket backend. Vhost-user is client/server agnostic regarding the
> chardev, however it does not support the 'nowait' and 'telnet' options.
> 
> We rely on the memdev with a memory-file backend. The backend's share=on option
> should be used. HugeTLBFS is required for this option to work.
> 
> The data path is realized by directly accessing the vrings and the buffer data
> off the guest's memory.
> 
> The current user of vhost-user is only vhost-net. We add a new netdev backend
> that is intended to initialize vhost-net with vhost-user backend.
> 
> Example usage:
> 
> qemu -m 512 \
>      -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
>      -numa node,memdev=mem \
>      -chardev socket,id=chr0,path=/path/to/socket \
>      -netdev type=vhost-user,id=net0,chardev=chr0 \
>      -device virtio-net-pci,netdev=net0
> 
> On non-MSIX guests the vhost feature can be forced using a special option:
> 
> ...
>      -netdev type=vhost-user,id=net0,chardev=chr0,vhostforce
> ...
> 
> In order to use ioeventfds, kvm should be enabled.
> 
> The work is made on top of the NUMA patch series v3.2
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg02706.html
> 
> This code can be pulled from git@github.com:virtualopensystems/qemu.git vhost-user-v10
> A simple functional test is available in tests/vhost-user-test.c
> 
> A reference vhost-user slave for testing is also available from git@github.com:virtualopensystems/vapp.git

I put patches 1-14 and 17 on my tree, branch vhost:
    git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git vhost
please rebase 15,16 and 18 on top, addressing comments that were sent
since, and post just these, then I'll be able to merge the
feature.

Thanks!


> Changes from v9:
>  - Rebased on the NUMA memdev patchseries and reworked to use memdev

memdev is now merged though not the numa bits.
will still work right?

>  - Removed -mem-path refactoring
>  - Removed all reconnection code
>  - Fixed 100% CPU usage in the G_IO_HUP handler after disconnect
>  - Reworked vhost feature bits handling so vhost-user has better control in the negotiation
> 
> Changes from v8:
>  - Removed prealloc property from the -mem-path refactoring
>  - Added and use new function - kvm_eventfds_enabled
>  - Add virtio_queue_get_avail_idx used in vhost_virtqueue_stop to
>    get a sane value in case of VHOST_GET_VRING_BASE failure
>  - vhost user uses kvm_eventfds_enabled to check whether the ioeventfd
>    capability of KVM is available
>  - Added flag VHOST_USER_VRING_NOFD_MASK to be set when KICK, CALL or ERR file
>    descriptor is invalid or ioeventfd is not available
> 
> Changes from v7:
>  - Slave reconnection when using chardev in server mode
>  - qtest vhost-user-test added
>  - New qemu_chr_fe_get_msgfds for reading multiple fds from the chardev
>  - Mandatory features in vhost_dev, used on reconnect to verify for conflicts
>  - Add vhostforce parameter to -netdev vhost-user (for non-MSIX guests)
>  - Extend libqemustub.a to support qemu-char.c
> 
> Changes from v6:
>  - Remove the 'unlink' property of '-mem-path'
>  - Extend qemu-char: blocking read, send fds, monitor for connection close
>  - Vhost-user uses chardev as a backend
>  - Poll and reconnect removed (no VHOST_USER_ECHO).
>  - Disconnect is deteced by the chardev (G_IO_HUP event)
>  - vhost-backend.c split to vhost-user.c
> 
> Changes from v5:
>  - Split -mem-path unlink option to a separate patch
>  - Fds are passed only in the ancillary data
>  - Stricter message size checks on receive/send
>  - Netdev vhost-user now includes path and poll_time options
>  - The connection probing interval is configurable
> 
> Changes from v4:
>  - Use error_report for errors
>  - VhostUserMsg has new field `size` indicating the following payload length.
>    Field `flags` now has version and reply bits. The structure is packed.
>  - Send data is of variable length (`size` field in message)
>  - Receive in 2 steps, header and payload
>  - Add new message type VHOST_USER_ECHO, to check connection status
> 
> Changes from v3:
>  - Convert -mem-path to QemuOpts with prealloc, share and unlink properties
>  - Set 1 sec timeout when read/write to the unix domain socket
>  - Fix file descriptor leak
> 
> Changes from v2:
>  - Reconnect when the backend disappears
> 
> Changes from v1:
>  - Implementation of vhost-user netdev backend
>  - Code improvements
> 
> 
> ---
> 
> Nikolay Nikolaev (18):
>       Add kvm_eventfds_enabled function
>       Add chardev API qemu_chr_fe_read_all
>       Add chardev API qemu_chr_fe_set_msgfds
>       Add chardev API qemu_chr_fe_get_msgfds
>       Add G_IO_HUP handler for socket chardev
>       vhost: add vhost_get_features and vhost_ack_features
>       vhost_net should call the poll callback only when it is set
>       Refactor virtio-net to use generic get_vhost_net
>       vhost_net_init will use VhostNetOptions to get all its arguments
>       Add vhost_ops to vhost_dev struct and replace all relevant ioctls
>       Add vhost-backend and VhostBackendType
>       Add vhost-user as a vhost backend.
>       vhost-net: vhost-user feature bits support
>       Add new vhost-user netdev backend
>       Add the vhost-user netdev backend to the command line
>       Add vhost-user protocol documentation
>       libqemustub: add stubs to be able to use qemu-char.c
>       Add qtest for vhost-user
> 
> 
>  docs/specs/vhost-user.txt         |  261 ++++++++++++++++++++++++++++
>  hmp-commands.hx                   |    4 
>  hw/net/vhost_net.c                |  228 +++++++++++++++++--------
>  hw/net/virtio-net.c               |   29 +--
>  hw/scsi/vhost-scsi.c              |   45 +++--
>  hw/virtio/Makefile.objs           |    2 
>  hw/virtio/vhost-backend.c         |   71 ++++++++
>  hw/virtio/vhost-user.c            |  342 +++++++++++++++++++++++++++++++++++++
>  hw/virtio/vhost.c                 |   82 ++++++---
>  include/hw/virtio/vhost-backend.h |   38 ++++
>  include/hw/virtio/vhost.h         |   13 +
>  include/net/vhost-user.h          |   17 ++
>  include/net/vhost_net.h           |   11 +
>  include/sysemu/char.h             |   44 +++++
>  include/sysemu/kvm.h              |   11 +
>  kvm-all.c                         |    4 
>  kvm-stub.c                        |    1 
>  net/Makefile.objs                 |    2 
>  net/clients.h                     |    3 
>  net/hub.c                         |    1 
>  net/net.c                         |   25 ++-
>  net/tap.c                         |   18 ++
>  net/vhost-user.c                  |  265 +++++++++++++++++++++++++++++
>  qapi-schema.json                  |   19 ++
>  qemu-char.c                       |  277 +++++++++++++++++++++++++++---
>  qemu-options.hx                   |   18 ++
>  stubs/Makefile.objs               |    8 +
>  stubs/bdrv-commit-all.c           |    7 +
>  stubs/chr-msmouse.c               |    7 +
>  stubs/get-next-serial.c           |    3 
>  stubs/is-daemonized.c             |    7 +
>  stubs/machine-init-done.c         |    6 +
>  stubs/monitor-init.c              |    6 +
>  stubs/notify-event.c              |    6 +
>  stubs/vc-init.c                   |    7 +
>  tests/Makefile                    |    4 
>  tests/vhost-user-test.c           |  312 ++++++++++++++++++++++++++++++++++
>  37 files changed, 2011 insertions(+), 193 deletions(-)
>  create mode 100644 docs/specs/vhost-user.txt
>  create mode 100644 hw/virtio/vhost-backend.c
>  create mode 100644 hw/virtio/vhost-user.c
>  create mode 100644 include/hw/virtio/vhost-backend.h
>  create mode 100644 include/net/vhost-user.h
>  create mode 100644 net/vhost-user.c
>  create mode 100644 stubs/bdrv-commit-all.c
>  create mode 100644 stubs/chr-msmouse.c
>  create mode 100644 stubs/get-next-serial.c
>  create mode 100644 stubs/is-daemonized.c
>  create mode 100644 stubs/machine-init-done.c
>  create mode 100644 stubs/monitor-init.c
>  create mode 100644 stubs/notify-event.c
>  create mode 100644 stubs/vc-init.c
>  create mode 100644 tests/vhost-user-test.c
> 
> --
> Signature

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 00/18] Vhost and vhost-net support for userspace based backends

[Nikolay Nikolaev]

[-- Attachment #1: Type: text/plain, Size: 10760 bytes --]

Hello Michael,


On Sun, Jun 8, 2014 at 6:12 PM, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Tue, May 27, 2014 at 03:03:21PM +0300, Nikolay Nikolaev wrote:
> > In this patch series we would like to introduce our approach for putting
> a
> > virtio-net backend in an external userspace process. Our eventual target
> is to
> > run the network backend in the Snabbswitch ethernet switch, while
> receiving
> > traffic from a guest inside QEMU/KVM which runs an unmodified virtio-net
> > implementation.
> >
> > For this, we are working into extending vhost to allow equivalent
> functionality
> > for userspace. Vhost already passes control of the data plane of
> virtio-net to
> > the host kernel; we want to realize a similar model, but for userspace.
> >
> > In this patch series the concept of a vhost-backend is introduced.
> >
> > We define two vhost backend types - vhost-kernel and vhost-user. The
> former is
> > the interface to the current kernel module implementation. Its control
> plane is
> > ioctl based. The data plane is realized by the kernel directly accessing
> the
> > QEMU allocated, guest memory.
> >
> > In the new vhost-user backend, the control plane is based on
> communication
> > between QEMU and another userspace process using a unix domain socket.
> This
> > allows to implement a virtio backend for a guest running in QEMU, inside
> the
> > other userspace process. For this communication we use a chardev with a
> Unix
> > domain socket backend. Vhost-user is client/server agnostic regarding the
> > chardev, however it does not support the 'nowait' and 'telnet' options.
> >
> > We rely on the memdev with a memory-file backend. The backend's share=on
> option
> > should be used. HugeTLBFS is required for this option to work.
> >
> > The data path is realized by directly accessing the vrings and the
> buffer data
> > off the guest's memory.
> >
> > The current user of vhost-user is only vhost-net. We add a new netdev
> backend
> > that is intended to initialize vhost-net with vhost-user backend.
> >
> > Example usage:
> >
> > qemu -m 512 \
> >      -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
> >      -numa node,memdev=mem \
> >      -chardev socket,id=chr0,path=/path/to/socket \
> >      -netdev type=vhost-user,id=net0,chardev=chr0 \
> >      -device virtio-net-pci,netdev=net0
> >
> > On non-MSIX guests the vhost feature can be forced using a special
> option:
> >
> > ...
> >      -netdev type=vhost-user,id=net0,chardev=chr0,vhostforce
> > ...
> >
> > In order to use ioeventfds, kvm should be enabled.
> >
> > The work is made on top of the NUMA patch series v3.2
> > http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg02706.html
> >
> > This code can be pulled from git@github.com:virtualopensystems/qemu.git
> vhost-user-v10
> > A simple functional test is available in tests/vhost-user-test.c
> >
> > A reference vhost-user slave for testing is also available from
> git@github.com:virtualopensystems/vapp.git
>
> I put patches 1-14 and 17 on my tree, branch vhost:
>     git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git vhost
>

Thanks for that!


> please rebase 15,16 and 18 on top, addressing comments that were sent
>
for 15 and 16 - I'll fix the comments. For 18, I don't see any, but it is a
test and it depends on NUMA patches. So we'll have to fix it eventually.


> since, and post just these, then I'll be able to merge the
> feature.
>
> Thanks!
>
>
> > Changes from v9:
> >  - Rebased on the NUMA memdev patchseries and reworked to use memdev
>
> memdev is now merged though not the numa bits.
> will still work right?
>
>
The feature depends on memory sharing. We're currently using this line to
enable it:

     -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
      -numa node,memdev=mem \

This does not seem to work in your tree. Please advice what equivalent
should be used?

regards,
Nikolay Nikolaev


>  >  - Removed -mem-path refactoring
> >  - Removed all reconnection code
> >  - Fixed 100% CPU usage in the G_IO_HUP handler after disconnect
> >  - Reworked vhost feature bits handling so vhost-user has better control
> in the negotiation
> >
> > Changes from v8:
> >  - Removed prealloc property from the -mem-path refactoring
> >  - Added and use new function - kvm_eventfds_enabled
> >  - Add virtio_queue_get_avail_idx used in vhost_virtqueue_stop to
> >    get a sane value in case of VHOST_GET_VRING_BASE failure
> >  - vhost user uses kvm_eventfds_enabled to check whether the ioeventfd
> >    capability of KVM is available
> >  - Added flag VHOST_USER_VRING_NOFD_MASK to be set when KICK, CALL or
> ERR file
> >    descriptor is invalid or ioeventfd is not available
> >
> > Changes from v7:
> >  - Slave reconnection when using chardev in server mode
> >  - qtest vhost-user-test added
> >  - New qemu_chr_fe_get_msgfds for reading multiple fds from the chardev
> >  - Mandatory features in vhost_dev, used on reconnect to verify for
> conflicts
> >  - Add vhostforce parameter to -netdev vhost-user (for non-MSIX guests)
> >  - Extend libqemustub.a to support qemu-char.c
> >
> > Changes from v6:
> >  - Remove the 'unlink' property of '-mem-path'
> >  - Extend qemu-char: blocking read, send fds, monitor for connection
> close
> >  - Vhost-user uses chardev as a backend
> >  - Poll and reconnect removed (no VHOST_USER_ECHO).
> >  - Disconnect is deteced by the chardev (G_IO_HUP event)
> >  - vhost-backend.c split to vhost-user.c
> >
> > Changes from v5:
> >  - Split -mem-path unlink option to a separate patch
> >  - Fds are passed only in the ancillary data
> >  - Stricter message size checks on receive/send
> >  - Netdev vhost-user now includes path and poll_time options
> >  - The connection probing interval is configurable
> >
> > Changes from v4:
> >  - Use error_report for errors
> >  - VhostUserMsg has new field `size` indicating the following payload
> length.
> >    Field `flags` now has version and reply bits. The structure is packed.
> >  - Send data is of variable length (`size` field in message)
> >  - Receive in 2 steps, header and payload
> >  - Add new message type VHOST_USER_ECHO, to check connection status
> >
> > Changes from v3:
> >  - Convert -mem-path to QemuOpts with prealloc, share and unlink
> properties
> >  - Set 1 sec timeout when read/write to the unix domain socket
> >  - Fix file descriptor leak
> >
> > Changes from v2:
> >  - Reconnect when the backend disappears
> >
> > Changes from v1:
> >  - Implementation of vhost-user netdev backend
> >  - Code improvements
> >
> >
> > ---
> >
> > Nikolay Nikolaev (18):
> >       Add kvm_eventfds_enabled function
> >       Add chardev API qemu_chr_fe_read_all
> >       Add chardev API qemu_chr_fe_set_msgfds
> >       Add chardev API qemu_chr_fe_get_msgfds
> >       Add G_IO_HUP handler for socket chardev
> >       vhost: add vhost_get_features and vhost_ack_features
> >       vhost_net should call the poll callback only when it is set
> >       Refactor virtio-net to use generic get_vhost_net
> >       vhost_net_init will use VhostNetOptions to get all its arguments
> >       Add vhost_ops to vhost_dev struct and replace all relevant ioctls
> >       Add vhost-backend and VhostBackendType
> >       Add vhost-user as a vhost backend.
> >       vhost-net: vhost-user feature bits support
> >       Add new vhost-user netdev backend
> >       Add the vhost-user netdev backend to the command line
> >       Add vhost-user protocol documentation
> >       libqemustub: add stubs to be able to use qemu-char.c
> >       Add qtest for vhost-user
> >
> >
> >  docs/specs/vhost-user.txt         |  261 ++++++++++++++++++++++++++++
> >  hmp-commands.hx                   |    4
> >  hw/net/vhost_net.c                |  228 +++++++++++++++++--------
> >  hw/net/virtio-net.c               |   29 +--
> >  hw/scsi/vhost-scsi.c              |   45 +++--
> >  hw/virtio/Makefile.objs           |    2
> >  hw/virtio/vhost-backend.c         |   71 ++++++++
> >  hw/virtio/vhost-user.c            |  342
> +++++++++++++++++++++++++++++++++++++
> >  hw/virtio/vhost.c                 |   82 ++++++---
> >  include/hw/virtio/vhost-backend.h |   38 ++++
> >  include/hw/virtio/vhost.h         |   13 +
> >  include/net/vhost-user.h          |   17 ++
> >  include/net/vhost_net.h           |   11 +
> >  include/sysemu/char.h             |   44 +++++
> >  include/sysemu/kvm.h              |   11 +
> >  kvm-all.c                         |    4
> >  kvm-stub.c                        |    1
> >  net/Makefile.objs                 |    2
> >  net/clients.h                     |    3
> >  net/hub.c                         |    1
> >  net/net.c                         |   25 ++-
> >  net/tap.c                         |   18 ++
> >  net/vhost-user.c                  |  265 +++++++++++++++++++++++++++++
> >  qapi-schema.json                  |   19 ++
> >  qemu-char.c                       |  277 +++++++++++++++++++++++++++---
> >  qemu-options.hx                   |   18 ++
> >  stubs/Makefile.objs               |    8 +
> >  stubs/bdrv-commit-all.c           |    7 +
> >  stubs/chr-msmouse.c               |    7 +
> >  stubs/get-next-serial.c           |    3
> >  stubs/is-daemonized.c             |    7 +
> >  stubs/machine-init-done.c         |    6 +
> >  stubs/monitor-init.c              |    6 +
> >  stubs/notify-event.c              |    6 +
> >  stubs/vc-init.c                   |    7 +
> >  tests/Makefile                    |    4
> >  tests/vhost-user-test.c           |  312
> ++++++++++++++++++++++++++++++++++
> >  37 files changed, 2011 insertions(+), 193 deletions(-)
> >  create mode 100644 docs/specs/vhost-user.txt
> >  create mode 100644 hw/virtio/vhost-backend.c
> >  create mode 100644 hw/virtio/vhost-user.c
> >  create mode 100644 include/hw/virtio/vhost-backend.h
> >  create mode 100644 include/net/vhost-user.h
> >  create mode 100644 net/vhost-user.c
> >  create mode 100644 stubs/bdrv-commit-all.c
> >  create mode 100644 stubs/chr-msmouse.c
> >  create mode 100644 stubs/get-next-serial.c
> >  create mode 100644 stubs/is-daemonized.c
> >  create mode 100644 stubs/machine-init-done.c
> >  create mode 100644 stubs/monitor-init.c
> >  create mode 100644 stubs/notify-event.c
> >  create mode 100644 stubs/vc-init.c
> >  create mode 100644 tests/vhost-user-test.c
> >
> > --
> > Signature
>
> --
> You received this message because you are subscribed to the Google Groups
> "Snabb Switch development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to snabb-devel+unsubscribe@googlegroups.com.
> To post to this group, send an email to snabb-devel@googlegroups.com.
> Visit this group at http://groups.google.com/group/snabb-devel.
>

[-- Attachment #2: Type: text/html, Size: 13936 bytes --]

Re: [Qemu-devel] [PATCH v10 18/18] Add qtest for vhost-user

[Kevin Wolf]

Am 27.05.2014 um 14:07 hat Nikolay Nikolaev geschrieben:
> This test creates a 'server' chardev to listen for vhost-user messages.
> Once VHOST_USER_SET_MEM_TABLE is received it mmaps each received region,
> and read 1k bytes from it. The read data is compared to data from readl.
> 
> The test requires hugetlbfs to be already mounted and writable. The mount
> point defaults to '/hugetlbfs' and can be specified via the environment
> variable QTEST_HUGETLBFS_PATH.
> 
> The rom pc-bios/pxe-virtio.rom is used to instantiate a virtio pcicontroller.
> 
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>

This breaks the test case build on RHEL 6 because G_TIME_SPAN_SECOND
apparently doesn't exist before glib 2.26.

Kevin

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 18/18] Add qtest for vhost-user

[Nikolay Nikolaev]

On Wed, Jul 9, 2014 at 5:24 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 27.05.2014 um 14:07 hat Nikolay Nikolaev geschrieben:
>> This test creates a 'server' chardev to listen for vhost-user messages.
>> Once VHOST_USER_SET_MEM_TABLE is received it mmaps each received region,
>> and read 1k bytes from it. The read data is compared to data from readl.
>>
>> The test requires hugetlbfs to be already mounted and writable. The mount
>> point defaults to '/hugetlbfs' and can be specified via the environment
>> variable QTEST_HUGETLBFS_PATH.
>>
>> The rom pc-bios/pxe-virtio.rom is used to instantiate a virtio pcicontroller.
>>
>> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
>> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
>
> This breaks the test case build on RHEL 6 because G_TIME_SPAN_SECOND
> apparently doesn't exist before glib 2.26.
Thanks Kevin. I have sent a patch, but have no RHEL6 to test it. Can
you please verify it?

>
> Kevin
>
> --
> You received this message because you are subscribed to the Google Groups "Snabb Switch development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to snabb-devel+unsubscribe@googlegroups.com.
> To post to this group, send an email to snabb-devel@googlegroups.com.
> Visit this group at http://groups.google.com/group/snabb-devel.

Re: [Qemu-devel] [PATCH v10 18/18] Add qtest for vhost-user

[Michael S. Tsirkin]

On Wed, Jul 09, 2014 at 04:24:03PM +0200, Kevin Wolf wrote:
> Am 27.05.2014 um 14:07 hat Nikolay Nikolaev geschrieben:
> > This test creates a 'server' chardev to listen for vhost-user messages.
> > Once VHOST_USER_SET_MEM_TABLE is received it mmaps each received region,
> > and read 1k bytes from it. The read data is compared to data from readl.
> > 
> > The test requires hugetlbfs to be already mounted and writable. The mount
> > point defaults to '/hugetlbfs' and can be specified via the environment
> > variable QTEST_HUGETLBFS_PATH.
> > 
> > The rom pc-bios/pxe-virtio.rom is used to instantiate a virtio pcicontroller.
> > 
> > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> 
> This breaks the test case build on RHEL 6 because G_TIME_SPAN_SECOND
> apparently doesn't exist before glib 2.26.
> 
> Kevin

Nikolay can you fix pls asap?
I'm fine with detecting this at configure time and disabling
the test if that's what it takes ...

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 18/18] Add qtest for vhost-user

[Nikolay Nikolaev]

On Fri, Jul 11, 2014 at 9:35 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Jul 09, 2014 at 04:24:03PM +0200, Kevin Wolf wrote:
>> Am 27.05.2014 um 14:07 hat Nikolay Nikolaev geschrieben:
>> > This test creates a 'server' chardev to listen for vhost-user messages.
>> > Once VHOST_USER_SET_MEM_TABLE is received it mmaps each received region,
>> > and read 1k bytes from it. The read data is compared to data from readl.
>> >
>> > The test requires hugetlbfs to be already mounted and writable. The mount
>> > point defaults to '/hugetlbfs' and can be specified via the environment
>> > variable QTEST_HUGETLBFS_PATH.
>> >
>> > The rom pc-bios/pxe-virtio.rom is used to instantiate a virtio pcicontroller.
>> >
>> > Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
>> > Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
>>
>> This breaks the test case build on RHEL 6 because G_TIME_SPAN_SECOND
>> apparently doesn't exist before glib 2.26.
>>
>> Kevin
>
> Nikolay can you fix pls asap?
> I'm fine with detecting this at configure time and disabling
> the test if that's what it takes ...

the fix is already in the main git tree:
http://git.qemu.org/?p=qemu.git;a=commit;h=0a58991a5f7efd6eb8a66643f4fd894b9c6874b7

>
> --
> You received this message because you are subscribed to the Google Groups "Snabb Switch development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to snabb-devel+unsubscribe@googlegroups.com.
> To post to this group, send an email to snabb-devel@googlegroups.com.
> Visit this group at http://groups.google.com/group/snabb-devel.