From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id BF220A0471 for ; Thu, 20 Jun 2019 13:32:45 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2F5CC1D37B; Thu, 20 Jun 2019 13:32:44 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 3BFA71D378 for ; Thu, 20 Jun 2019 13:32:42 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20190620113241euoutp02d6834d35143a130af020f840dce3fa05~p5Nfp5cDt2500125001euoutp02g for ; Thu, 20 Jun 2019 11:32:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20190620113241euoutp02d6834d35143a130af020f840dce3fa05~p5Nfp5cDt2500125001euoutp02g DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1561030361; bh=48VLxZd93glWCyVP46wb6/2PGUq71NhpBKzFKD4oLDU=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=OlijliSVxq5Xum+8rLcFMUf0eAaBl6mSpENpX0zC8xjqJJ0MKr0AVsKZ99AxALrZG UySZHQ6gxQpugKOznfHBxi+13pjqgtorBTkaaGYo+8ozzzShLgXCD9S3Bnpaf6AQ03 Ca+HH0f+aaO99hVrjF4ZmvsBZGn2NGCfU2Vj99Xs= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20190620113241eucas1p1fb4442f88e33b314640ebc6f8ed0a3cc~p5NfMDJk11366013660eucas1p1s; Thu, 20 Jun 2019 11:32:41 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 9E.BD.04325.8DE6B0D5; Thu, 20 Jun 2019 12:32:41 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20190620113240eucas1p22ca4faa64a36bbb7aec38a81298ade56~p5NeRzudR1689616896eucas1p2T; Thu, 20 Jun 2019 11:32:40 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20190620113240eusmtrp2708f972a20900df2b9b056ee8695195e~p5NeRNxOY2288822888eusmtrp2L; Thu, 20 Jun 2019 11:32:40 +0000 (GMT) X-AuditID: cbfec7f5-b75ff700000010e5-83-5d0b6ed8a183 Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 94.5E.04140.8DE6B0D5; Thu, 20 Jun 2019 12:32:40 +0100 (BST) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20190620113239eusmtip19857627698ab3f5b7cf5ddfc0bb4025d~p5NdslJ680139401394eusmtip1k; Thu, 20 Jun 2019 11:32:39 +0000 (GMT) To: Nikos Dragazis , dev@dpdk.org Cc: Maxime Coquelin , Tiwei Bie , Zhihong Wang , Stefan Hajnoczi , Wei Wang , Stojaczyk Dariusz , Vangelis Koukis From: Ilya Maximets Message-ID: Date: Thu, 20 Jun 2019 14:32:39 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <1560957293-17294-1-git-send-email-ndragazis@arrikto.com> Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrOKsWRmVeSWpSXmKPExsWy7djPc7o387hjDSZOY7R4/3sNo8W7T9uZ LK60/2S3ONa5h8ViSfsVFovXk/6zWmxt+M9kcfb3I2aL33MvMFpsvjiJyYHL4+ajj4wevxYs ZfVYvOclk8f7fVfZPPq2rGIMYI3isklJzcksSy3St0vgyni1OqzggWtFz+4LbA2Mu0y7GDk4 JARMJNa1SncxcnEICaxglFg7dQMLhPOFUeLbxiYghxPI+cwosfUTP0zDhr8cEDXLGSV+vLnK DOF8ZJRYveEfWIOwgI/EgilzwGwRAQuJy+t/sYEUMQvMY5KYNnMmK0iCTUBH4tTqI4wgU3kF 7CQmfDUFCbMIqEq8/D+ZHcQWFYiQ+LJzEyOIzSsgKHFy5hOwmZwCbhJ7ts9nA7GZBcQlmr6s ZIWw5SWat84GO0hC4BS7xIrHb5lAEhICLhK/HvQyQtjCEq+Ob2GHsGUk/u+cD1VTL3G/5SUj RHMHo8T0Q/+gEvYSW16fYwc5lFlAU2L9Ln2IsKPEzi2PmSGhwidx460gxA18EpO2TYcK80p0 tAlBVKtI/D64nBnClpK4+e4z+wRGpVlIPpuF5JtZSL6ZhbB3ASPLKkbx1NLi3PTUYuO81HK9 4sTc4tK8dL3k/NxNjMDkdPrf8a87GPf9STrEKMDBqMTDe0KLK1aINbGsuDL3EKMEB7OSCO9T Ru5YId6UxMqq1KL8+KLSnNTiQ4zSHCxK4rzVDA+ihQTSE0tSs1NTC1KLYLJMHJxSDYzc/+Zm OVm5B846wiWvVi07a29kR36vRNGjiiOzU6OPaXctZROvy/N7JHTCOkD2ZVxNeeNCbu7q6oD7 Uh897kz5bsRrx/1D5U+a5myuuqA199NOPD8ra/5F6avkm92m9dWzNxiFfDUS/DLxg/j/FboW Psl/ku9ND31zKznoZlLdipPC+vVhgUosxRmJhlrMRcWJANXf2exKAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrFIsWRmVeSWpSXmKPExsVy+t/xu7o38rhjDQ73iFq8/72G0eLdp+1M Flfaf7JbHOvcw2KxpP0Ki8XrSf9ZLbY2/GeyOPv7EbPF77kXGC02X5zE5MDlcfPRR0aPXwuW snos3vOSyeP9vqtsHn1bVjEGsEbp2RTll5akKmTkF5fYKkUbWhjpGVpa6BmZWOoZGpvHWhmZ Kunb2aSk5mSWpRbp2yXoZbxaHVbwwLWiZ/cFtgbGXaZdjBwcEgImEhv+cnQxcnEICSxllPja uIC5i5ETKC4l8ePXBVYIW1jiz7UuNoii94wSx94dZAdJCAv4SCyYMocFxBYRsJC4vP4XG4jN LLCASWLKAk8QW0jAVeLfzylMIDabgI7EqdVHGEEW8wrYSUz4agoSZhFQlXj5fzLYSFGBCInZ uxrARvIKCEqcnPkEzOYUcJPYs30+1Hh1iT/zLjFD2OISTV9WskLY8hLNW2czT2AUmoWkfRaS lllIWmYhaVnAyLKKUSS1tDg3PbfYSK84Mbe4NC9dLzk/dxMjMB63Hfu5ZQdj17vgQ4wCHIxK PLwntLhihVgTy4orcw8xSnAwK4nwPmXkjhXiTUmsrEotyo8vKs1JLT7EaAr03ERmKdHkfGCq yCuJNzQ1NLewNDQ3Njc2s1AS5+0QOBgjJJCeWJKanZpakFoE08fEwSnVwNh5IyLE7vLBLdOj zTP8Py0u5Ty1UdUk7nOcja9t0WLXmMPXj2S387ntzZZPPbTysC2HsjrzaRlroal7c7uUVhzl rRDS7f2v71Sg0hPDHhJ3d/KO/d+NU/681H+z2CrzxSz9U79WHQ76nV5f+VN9ad3euhk8+o3p 4R8tuox+3Zxt5r328/mFmkosxRmJhlrMRcWJAHU2BAvdAgAA X-CMS-MailID: 20190620113240eucas1p22ca4faa64a36bbb7aec38a81298ade56 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20190620113240eucas1p22ca4faa64a36bbb7aec38a81298ade56 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20190620113240eucas1p22ca4faa64a36bbb7aec38a81298ade56 References: <1560957293-17294-1-git-send-email-ndragazis@arrikto.com> Subject: Re: [dpdk-dev] [PATCH 00/28] 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 19.06.2019 18:14, Nikos Dragazis wrote: > Hi everyone, Hi. I didn't look at the code, just a few comments inline. > > this patch series introduces the concept of the virtio-vhost-user > transport. This is actually a revised version of an earlier RFC > implementation that has been proposed by Stefan Hajnoczi [1]. Though > this is a great feature, it seems to have been stalled, so I’d like to > restart the conversation on this and hopefully get it merged with your > help. Let me give you an overview. > > The virtio-vhost-user transport is a vhost-user transport implementation > that is based on the virtio-vhost-user device. Its key difference with > the existing transport is that it allows deploying vhost-user targets > inside dedicated Storage Appliance VMs instead of host user space. In > other words, it allows having guests that act as vhost-user backends for > other guests. > > The virtio-vhost-user device implements the vhost-user control plane > (master-slave communication) as follows: > > 1. it parses the vhost-user messages from the vhost-user unix domain > socket and forwards them to the slave guest through virtqueues > > 2. it maps the vhost memory regions in QEMU’s process address space and > exposes them to the slave guest as a RAM-backed PCI MMIO region > > 3. it hooks up doorbells to the callfds. The slave guest can use these > doorbells to interrupt the master guest driver > > The device code has not yet been merged into upstream QEMU, but this is > definitely the end goal. The current state is that we are awaiting for > the approval of the virtio spec. > > I have Cced Darek from the SPDK community who has helped me a lot by > reviewing this series. Note that any device type could be implemented > over this new transport. So, adding the virtio-vhost-user transport in > DPDK would allow using it from SPDK as well. > > Getting into the code internals, this patch series makes the following > changes: > > 1. introduce a generic interface for the transport-specific operations. > Each of the two available transports, the pre-existing AF_UNIX > transport and the virtio-vhost-user transport, is going to implement > this interface. The AF_UNIX-specific code has been extracted from the > core vhost-user code and is now part of the AF_UNIX transport > implementation in trans_af_unix.c. > > 2. introduce the virtio-vhost-user transport. The virtio-vhost-user > transport requires a driver for the virtio-vhost-user devices. The > driver along with the transport implementation have been packed into > a separate library in `drivers/virtio_vhost_user/`. The necessary > virtio-pci code has been copied from `drivers/net/virtio/`. Some > additional changes have been made so that the driver can utilize the > additional resources of the virtio-vhost-user device. > > 3. update librte_vhost public API to enable choosing transport for each > new vhost device. Extend the vhost net driver and vhost-scsi example > application to export this new API to the end user. > > The primary changes I did to Stefan’s RFC implementation are the > following: > > 1. moved postcopy live migration code into trans_af_unix.c. Postcopy > live migration relies on the userfault fd mechanism, which cannot be > supported by virtio-vhost-user. > > 2. moved setup of the log memory region into trans_af_unix.c. Setting up > the log memory region involves mapping/unmapping guest memory. This > is an AF_UNIX transport-specific operation. Logging dirty pages is the main concept of live migration support. Does it mean that the live migration is not supported for virtio-vhost-user at all? > > 3. introduced a vhost transport operation for > process_slave_message_reply() > > 4. moved the virtio-vhost-user transport/driver into a separate library > in `drivers/virtio_vhost_user/`. This required making vhost.h and > vhost_user.h part of librte_vhost public API and exporting some > private symbols via the version script. This looks better to me that > just moving the entire librte_vhost into `drivers/`. I am not sure if > this is the most appropriate solution. I am looking forward to your > suggestions on this. Moving the virtio-vhost-user code to a separate driver looks strange for me. What is the purpose? Exporting a lot of vhost internal structures will lead to a frequent API/ABI breakages and will slow down accepting changes to releases in general. It looks inconsistent to have 'trans_af_unix.c' in 'lib/librte_vhost/' and 'trans_virtio_vhost_user.c' in 'drivers/virtio_vhost_user/' because these files should be similar in provided functionality, hence, should be located in similar places. > > 5. made use of the virtio PCI capabilities for the additional device > resources (doorbells, shared memory). This required changes in > virtio_pci.c and trans_virtio_vhost_user.c. > > 6. [minor] changed some commit headlines to comply with > check-git-log.sh. > > Please, have a look and let me know about your thoughts. Any > reviews/pointers/suggestions are welcome. > > Best regards, > Nikos > > [1] http://mails.dpdk.org/archives/dev/2018-January/088155.html > > > Nikos Dragazis (23): > vhost: introduce vhost transport operations structure > vhost: move socket management code > vhost: move socket fd and un sockaddr > vhost: move vhost-user connection > vhost: move vhost-user reconnection > vhost: move vhost-user fdset > vhost: propagate vhost transport operations > vhost: use a single structure for the device state > vhost: extract socket I/O into transport > vhost: move slave request fd and lock > vhost: move mmap/munmap > vhost: move setup of the log memory region > vhost: remove main fd parameter from msg handlers > vhost: move postcopy live migration code > vhost: support registering additional vhost-user transports > drivers/virtio_vhost_user: add virtio PCI framework > drivers: add virtio-vhost-user transport > drivers/virtio_vhost_user: use additional device resources > vhost: add flag for choosing vhost-user transport > net/vhost: add virtio-vhost-user support > mk: link apps with virtio-vhost-user driver > config: add option for the virtio-vhost-user transport > usertools: add virtio-vhost-user devices to dpdk-devbind.py > > Stefan Hajnoczi (5): > vhost: allocate per-socket transport state > vhost: move start server/client calls > vhost: add index field in vhost virtqueues > examples/vhost_scsi: add --socket-file argument > examples/vhost_scsi: add virtio-vhost-user support > > config/common_base | 6 + > config/common_linux | 1 + > drivers/Makefile | 5 + > drivers/net/vhost/rte_eth_vhost.c | 13 + > drivers/virtio_vhost_user/Makefile | 27 + > .../rte_virtio_vhost_user_version.map | 4 + > .../virtio_vhost_user/trans_virtio_vhost_user.c | 1077 +++++++++++++++++++ > drivers/virtio_vhost_user/virtio_pci.c | 520 ++++++++++ > drivers/virtio_vhost_user/virtio_pci.h | 289 ++++++ > drivers/virtio_vhost_user/virtio_vhost_user.h | 18 + > drivers/virtio_vhost_user/virtqueue.h | 181 ++++ > examples/vhost_scsi/vhost_scsi.c | 103 +- > lib/librte_vhost/Makefile | 4 +- > lib/librte_vhost/rte_vhost.h | 1 + > lib/librte_vhost/rte_vhost_version.map | 11 + > lib/librte_vhost/socket.c | 685 +----------- > lib/librte_vhost/trans_af_unix.c | 1094 ++++++++++++++++++++ > lib/librte_vhost/vhost.c | 22 +- > lib/librte_vhost/vhost.h | 298 +++++- > lib/librte_vhost/vhost_user.c | 474 ++------- > lib/librte_vhost/vhost_user.h | 10 +- > mk/rte.app.mk | 6 + > usertools/dpdk-devbind.py | 7 + > 23 files changed, 3764 insertions(+), 1092 deletions(-) > create mode 100644 drivers/virtio_vhost_user/Makefile > create mode 100644 drivers/virtio_vhost_user/rte_virtio_vhost_user_version.map > create mode 100644 drivers/virtio_vhost_user/trans_virtio_vhost_user.c > create mode 100644 drivers/virtio_vhost_user/virtio_pci.c > create mode 100644 drivers/virtio_vhost_user/virtio_pci.h > create mode 100644 drivers/virtio_vhost_user/virtio_vhost_user.h > create mode 100644 drivers/virtio_vhost_user/virtqueue.h > create mode 100644 lib/librte_vhost/trans_af_unix.c >