From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 4C4BC1B063 for ; Tue, 26 Jun 2018 10:30:10 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id AC4572203C; Tue, 26 Jun 2018 04:30:09 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Tue, 26 Jun 2018 04:30:09 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=L9MnGxk/LcGHr4oaGwja6KnNPS XDRMivBZMZuWIe03w=; b=BHMjNpw4HvriM59HMiEKnJrNOU4373iXFnJNGOcGs+ /AndHf5672SVjBmEw+w+ezGRI/Rj+uvbGJdP8jhByKB4pIptK0Qn4eSnF/sMstAS zoipIPs7btEV/soI4agWbq3s1V0yeS5HZljf5CDi5pFs+6tpjOTukFdOuPF+9w6v 4= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=L9MnGx k/LcGHr4oaGwja6KnNPSXDRMivBZMZuWIe03w=; b=RNS5rw04Axxt4TGeVRv3M7 8/Ibhp1GDocd5sU1Pw/0zkcGznGBGO8lv46SUzUPA63EvdukMqHUP69GXUBivQqo fiQHk/xigPXW3C3FYfKOiHD+4+VrweNrePE8mBdAC/uKlI1F+IuqJ+LaJEX/0F18 TqyvuEK0wuTWLHo9AeKi/bhrh8fJWrzEDpdMcsoi4ZM3o/BQPg7imjJAh54ESD4e k//6n2+vMie3Af/XPIH03Dr11n4tqkEh5NNBAnlVmC/HiMensX7cEB0P4QfAWCdj va8DNZ2JAoxMU41LZWQHLpraCFxAk0orTN+tTi+5pplCm+OLKDhJFcTTvVIDAwBg == X-ME-Proxy: X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 3B78110292; Tue, 26 Jun 2018 04:30:08 -0400 (EDT) From: Thomas Monjalon To: Tiwei Bie Cc: "Stojaczyk, DariuszX" , Dariusz Stojaczyk , "dev@dpdk.org" , Maxime Coquelin , Tetsuya Mukawa , Stefan Hajnoczi , "yliu@fridaylinux.org" , "Harris, James R" , "Kulasek, TomaszX" , "Wodkowski, PawelX" Date: Tue, 26 Jun 2018 10:30:07 +0200 Message-ID: <5169781.u0Q09RttXa@xps> In-Reply-To: <20180626082226.GA15665@debian> References: <1526648465-62579-1-git-send-email-dariuszx.stojaczyk@intel.com> <20180626082226.GA15665@debian> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [RFC v3 0/7] vhost2: new librte_vhost2 proposal 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: Tue, 26 Jun 2018 08:30:10 -0000 26/06/2018 10:22, Tiwei Bie: > On Mon, Jun 25, 2018 at 08:17:08PM +0800, Stojaczyk, DariuszX wrote: > > From: Tiwei Bie > > > > > > Hi Dariusz, > > > > Hi Tiwei, > > > > > Thank you for putting efforts in making the DPDK > > > vhost more generic! > > > > > > From my understanding, your proposal is that: > > > > > > 1) Introduce rte_vhost2 to provide the APIs which > > > allow users to implement vhost backends like > > > SCSI, net, crypto, .. > > > > That's right. > > > > > 2) Refactor the existing rte_vhost to use rte_vhost2. > > > The rte_vhost will still provide below existing > > > sets of APIs: > > > 1. The APIs which allow users to implement > > > external vhost backends (these APIs were > > > designed for SPDK previously) > > > 2. The APIs provided by the net backend > > > 3. The APIs provided by the crypto backend > > > And above APIs in rte_vhost won't be changed. > > > > That's correct. Rte_vhost would register its own rte_vhost2_tgt_ops underneath and will call existing vhost_device_ops for e.g. starting the device once all queues are started. > > Currently I have below concerns and questions: > > - The rte_vhost's problem is still there. Even though > rte_vhost2 is introduced, the net and crypto backends > in rte_vhost won't benefit from the new callbacks. > > The existing rte_vhost in DPDK not only provides the > APIs for DPDK applications to implement the external > backends. But also provides high performance net and > crypto backends implementation (maybe more in the > future). So it's important that besides the DPDK > applications which implement their external backends, > the DPDK applications which use the builtin backends > will also benefit from the new callbacks. > > So we should have a clear plan on how will the legacy > callbacks in rte_vhost be dealt with in the next step. > > Besides, the new library's name is a bit misleading. > It makes the existing rte_vhost library sound like an > obsolete library. But actually the existing rte_vhost > isn't an obsolete library. It will still provide the > net and crypto backends. So if we want to introduce > this new library, we should give it a better name. > > - It's possible to solve rte_vhost's problem you met > by refactoring the existing vhost library directly > instead of re-implementing a new vhost library from > scratch and keeping the old one's problem as is. +1 > In this way, it will solve the problem you met and > also solve the problem for rte_vhost. Why not go > this way? Something like: > > Below is the existing callbacks set in rte_vhost.h: > > /** > * Device and vring operations. > */ > struct vhost_device_ops { > ...... > }; > > It's a legacy implementation, and doesn't really > follow the DPDK API design (e.g. no rte_ prefix). > We can design and implement a new message handling > and a new set of callbacks for rte_vhost to solve > the problem you met without changing the old one. > Something like: > > struct rte_vhost_device_ops { > ...... > } > > int > vhost_user_msg_handler(struct vhost_dev *vdev, struct vhost_user_msg *msg) > { > ...... > > if (!vdev->is_using_new_device_ops) { > // Call the existing message handler > return vhost_user_msg_handler_legacy(vdev, msg); > } > > // Implement the new logic here > ...... > } > > A vhost application is allowed to register only struct > rte_vhost_device_ops or struct vhost_device_ops (which > should be deprecated in the future). The two ops cannot > be registered at the same time. > > The existing applications could use the old ops. And > if an application registers struct rte_vhost_device_ops, > the new callbacks and message handler will be used.