From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 6E2F61B767 for ; Wed, 31 Jan 2018 11:03:15 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B5F8DC056827; Wed, 31 Jan 2018 10:03:14 +0000 (UTC) Received: from [10.36.112.31] (ovpn-112-31.ams2.redhat.com [10.36.112.31]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 161815EDF5; Wed, 31 Jan 2018 10:03:00 +0000 (UTC) To: Stefan Hajnoczi , dev@dpdk.org Cc: Yuanhan Liu , wei.w.wang@intel.com, mst@redhat.com, zhiyong.yang@intel.com, jasowang@redhat.com References: <20180119134444.24927-1-stefanha@redhat.com> From: Maxime Coquelin Message-ID: <1c34e10d-e59b-cae8-0bfe-fa511116ef47@redhat.com> Date: Wed, 31 Jan 2018 11:02:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180119134444.24927-1-stefanha@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 31 Jan 2018 10:03:14 +0000 (UTC) Subject: Re: [dpdk-dev] [RFC 00/24] vhost: add virtio-vhost-user transport X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Jan 2018 10:03:15 -0000 Hi Stefan, I went through the series, and I really like how it fits with existing vhost-user over af_unix socket. I will post the change I suggested to fix the regression met with SPDK and multiqueue. Thanks, Maxime On 01/19/2018 02:44 PM, Stefan Hajnoczi wrote: > This patch series implements the virtio-vhost-user device, which tunnels > vhost-user protocol messages over virtio. This lets guests act as vhost device > backends for other guests. > > The virtio-vhost-user device is the result of discussion about Wei Wang and > Zhiyong Yang's vhost-pci device. This patch series demonstrates that vhost > device backends, such as the vhost vdev driver, can work over both AF_UNIX and > virtio-vhost-user without significant modifications. This allows a lot of code > to be shared between traditional AF_UNIX vhost-user and virtio-vhost-user. The > vhost-pci patches duplicated the vhost net device backend and didn't reuse > librte_vhost: > > http://dpdk.org/ml/archives/dev/2017-November/082615.html > > User-visible changes > -------------------- > The vhost vdev can now be used when DPDK runs inside a guest with a > virtio-vhost-user PCI device: > > --vdev net_vhost0,iface="0000:00:04.0",virtio-transport=1 > > The vhost-scsi example has also been extended to support virtio-vhost-user: > > ./vhost-scsi ... -- --virtio-vhost-user-pci "0000:00:04.0" > > For more information (including instructions for running the code), see > https://wiki.qemu.org/Features/VirtioVhostUser > > Virtio device design > -------------------- > The virtio-vhost-user device is a new virtio device type. It acts as a > vhost-user transport and is an alternative for the traditional AF_UNIX > transport. > > You can find the virtio-vhost-user VIRTIO device specification here: > https://stefanha.github.io/virtio/vhost-user-slave.html#x1-2830007 > > librte_vhost API changes > ------------------------ > This patch series extends librte_vhost so that it now accepts: > > rte_vhost_driver_register("0000:00:04.0", > RTE_VHOST_USER_VIRTIO_TRANSPORT); > > All other librte_vhost API usage remains unchanged, except that the file > descriptors exposed in some structs will be -1 since there is no > file descriptor passing involved. > > This makes it extremely easy to support virtio-vhost-user in existing vhost > device backends! I have extended the vhost vdev driver and the vhost-scsi > example application in this patch series. > > Patch series overview > --------------------- > This series is based on commit 814339ba7eea13d132508af2cccec2f73568e2d0 from > dpdk-next-virtio/master. You can also get my git branch here: > > https://github.com/stefanha/dpdk/tree/virtio-vhost-user > > The initial patches refactor librte_vhost so that AF_UNIX-specific code is moved > to a new trans_af_unix.c file. This also introduces a struct > vhost_transport_ops interface that all transports will implement: > > adf412c3c vhost: move vring_call() into trans_af_unix.c > 04079a077 vhost: move AF_UNIX code from socket.c to trans_af_unix.c > 885642091 vhost: allocate per-socket transport state > 9c48377df vhost: move socket_fd and un sockaddr into trans_af_unix.c > c646e6292 vhost: move start_server/client() calls to trans_af_unix.c > db07ef7a8 vhost: move vhost_user_connection to trans_af_unix.c > d10b80163 vhost: move vhost_user_reconnect_init() into trans_af_unix.c > 1258bcd68 vhost: move vhost_user.fdset to trans_af_unix.c > bc7f6d7ab vhost: pass vhost_transport_ops through vhost_new_device() > b82187a29 vhost: embed struct virtio_net inside struct vhost_user_connection > abbd544f2 vhost: extract vhost_user.c socket I/O into transport > 0658d711b vhost: move slave_req_fd field to AF_UNIX transport > e2ecf78ed vhost: move mmap/munmap to AF_UNIX transport > > I was about to add the virtio-vhost-user PCI driver when I realized that > lib/librte_vhost/ cannot have a dependency on rte_bus_pci.h (it lives in > drivers/). The solution I chose is to move all of librte_vhost to > drivers/librte_vhost/, but I'm open to suggestions if this is undesirable. > > 8615c2140 vhost: move librte_vhost to drivers/ > > Since virtio-vhost-user is a virtio device it's necessary to perform a sequence > of device initialization steps and set up virtqueues. I didn't see a virtio > API in DPDK and didn't have time to create one myself. I copied the virtio > code from drivers/net/virtio/ as a quick hack, but really there needs to be a > librte_virtio that is shared. > > c26937c66 vhost: add virtio pci framework > > Next the virtio-vhost-user transport is added along with the > RTE_VHOST_USER_VIRTIO_TRANSPORT flag: > > 7fad5adc4 vhost: remember a vhost_virtqueue's queue index > d26922892 vhost: add virtio-vhost-user transport > f562a70ab vhost: add RTE_VHOST_USER_VIRTIO_TRANSPORT flag > > Then I extended the vhost vdev and vhost-scsi example application to support > virtio-vhost-user: > > 47434f2a2 net/vhost: add virtio-vhost-user support > 22ca05d1b examples/vhost_scsi: add --socket-file argument > db3b391dc examples/vhost_scsi: add virtio-vhost-user support > > It was also necessary to tweak dpdk-devbind.py to support virtio-vhost-user: > > 4aee6f653 usertools: add virtio-vhost-user devices to dpdk-devbind.py > > Finally, vhost-scsi seems broken to me so two workarounds were needed so it can > be tested again: > > b9d17bfaf WORKAROUND revert virtio-net mq vring deletion > cadb25e7d WORKAROUND examples/vhost_scsi: avoid broken EVENT_IDX > > Stefan Hajnoczi (24): > vhost: move vring_call() into trans_af_unix.c > vhost: move AF_UNIX code from socket.c to trans_af_unix.c > vhost: allocate per-socket transport state > vhost: move socket_fd and un sockaddr into trans_af_unix.c > vhost: move start_server/client() calls to trans_af_unix.c > vhost: move vhost_user_connection to trans_af_unix.c > vhost: move vhost_user_reconnect_init() into trans_af_unix.c > vhost: move vhost_user.fdset to trans_af_unix.c > vhost: pass vhost_transport_ops through vhost_new_device() > vhost: embed struct virtio_net inside struct vhost_user_connection > vhost: extract vhost_user.c socket I/O into transport > vhost: move slave_req_fd field to AF_UNIX transport > vhost: move mmap/munmap to AF_UNIX transport > vhost: move librte_vhost to drivers/ > vhost: add virtio pci framework > vhost: remember a vhost_virtqueue's queue index > vhost: add virtio-vhost-user transport > vhost: add RTE_VHOST_USER_VIRTIO_TRANSPORT flag > net/vhost: add virtio-vhost-user support > examples/vhost_scsi: add --socket-file argument > examples/vhost_scsi: add virtio-vhost-user support > usertools: add virtio-vhost-user devices to dpdk-devbind.py > WORKAROUND revert virtio-net mq vring deletion > WORKAROUND examples/vhost_scsi: avoid broken EVENT_IDX > > drivers/Makefile | 2 + > {lib => drivers}/librte_vhost/Makefile | 5 +- > lib/Makefile | 3 - > {lib => drivers}/librte_vhost/fd_man.h | 0 > {lib => drivers}/librte_vhost/iotlb.h | 0 > {lib => drivers}/librte_vhost/rte_vhost.h | 1 + > {lib => drivers}/librte_vhost/vhost.h | 193 +++- > {lib => drivers}/librte_vhost/vhost_user.h | 9 +- > drivers/librte_vhost/virtio_pci.h | 267 +++++ > drivers/librte_vhost/virtio_vhost_user.h | 18 + > drivers/librte_vhost/virtqueue.h | 181 ++++ > {lib => drivers}/librte_vhost/fd_man.c | 0 > {lib => drivers}/librte_vhost/iotlb.c | 0 > drivers/librte_vhost/socket.c | 282 ++++++ > drivers/librte_vhost/trans_af_unix.c | 795 +++++++++++++++ > drivers/librte_vhost/trans_virtio_vhost_user.c | 1050 ++++++++++++++++++++ > {lib => drivers}/librte_vhost/vhost.c | 18 +- > {lib => drivers}/librte_vhost/vhost_user.c | 200 +--- > {lib => drivers}/librte_vhost/virtio_net.c | 0 > drivers/librte_vhost/virtio_pci.c | 504 ++++++++++ > drivers/net/vhost/rte_eth_vhost.c | 13 + > examples/vhost_scsi/vhost_scsi.c | 104 +- > lib/librte_vhost/socket.c | 828 --------------- > .../librte_vhost/rte_vhost_version.map | 0 > usertools/dpdk-devbind.py | 8 + > 25 files changed, 3455 insertions(+), 1026 deletions(-) > rename {lib => drivers}/librte_vhost/Makefile (86%) > rename {lib => drivers}/librte_vhost/fd_man.h (100%) > rename {lib => drivers}/librte_vhost/iotlb.h (100%) > rename {lib => drivers}/librte_vhost/rte_vhost.h (99%) > rename {lib => drivers}/librte_vhost/vhost.h (68%) > rename {lib => drivers}/librte_vhost/vhost_user.h (93%) > create mode 100644 drivers/librte_vhost/virtio_pci.h > create mode 100644 drivers/librte_vhost/virtio_vhost_user.h > create mode 100644 drivers/librte_vhost/virtqueue.h > rename {lib => drivers}/librte_vhost/fd_man.c (100%) > rename {lib => drivers}/librte_vhost/iotlb.c (100%) > create mode 100644 drivers/librte_vhost/socket.c > create mode 100644 drivers/librte_vhost/trans_af_unix.c > create mode 100644 drivers/librte_vhost/trans_virtio_vhost_user.c > rename {lib => drivers}/librte_vhost/vhost.c (97%) > rename {lib => drivers}/librte_vhost/vhost_user.c (89%) > rename {lib => drivers}/librte_vhost/virtio_net.c (100%) > create mode 100644 drivers/librte_vhost/virtio_pci.c > delete mode 100644 lib/librte_vhost/socket.c > rename {lib => drivers}/librte_vhost/rte_vhost_version.map (100%) >