From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id A3D565B2C for ; Tue, 26 Jun 2018 11:14:28 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jun 2018 02:14:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,274,1526367600"; d="scan'208";a="67335779" Received: from debian.sh.intel.com (HELO debian) ([10.67.104.228]) by fmsmga001.fm.intel.com with ESMTP; 26 Jun 2018 02:14:25 -0700 Date: Tue, 26 Jun 2018 17:14:28 +0800 From: Tiwei Bie To: "Stojaczyk, DariuszX" Cc: Dariusz Stojaczyk , "dev@dpdk.org" , Maxime Coquelin , Tetsuya Mukawa , Stefan Hajnoczi , Thomas Monjalon , "yliu@fridaylinux.org" , "Harris, James R" , "Kulasek, TomaszX" , "Wodkowski, PawelX" Message-ID: <20180626091428.GA20198@debian> References: <1526648465-62579-1-git-send-email-dariuszx.stojaczyk@intel.com> <20180607151227.23660-1-darek.stojaczyk@gmail.com> <20180625110146.GA18211@debian> <20180626082226.GA15665@debian> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.5 (2018-04-13) 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 09:14:29 -0000 On Tue, Jun 26, 2018 at 04:47:33PM +0800, Stojaczyk, DariuszX wrote: > > -----Original Message----- > > From: Bie, Tiwei > > Sent: Tuesday, June 26, 2018 10:22 AM > > To: Stojaczyk, DariuszX > > Cc: Dariusz Stojaczyk ; dev@dpdk.org; Maxime > > Coquelin ; Tetsuya Mukawa > > ; Stefan Hajnoczi ; Thomas > > Monjalon ; yliu@fridaylinux.org; Harris, James R > > ; Kulasek, TomaszX ; > > Wodkowski, PawelX > > Subject: Re: [dpdk-dev] [RFC v3 0/7] vhost2: new librte_vhost2 proposal > > > > On Mon, Jun 25, 2018 at 08:17:08PM +0800, Stojaczyk, DariuszX wrote: > > > > -----Original Message----- > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tiwei Bie > > > > Sent: Monday, June 25, 2018 1:02 PM > > > > > > > > > > > > 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. > > > > 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. > > Please notice that some features like vIOMMU are not even a part of the public rte_vhost API. Only vhost-net benefits from vIOMMU right now. Separating vhost-net from a generic vhost library (rte_vhost2) would avoid making such design mistakes in future. What's the point of having a single rte_vhost library, if some vhost-user features are only implemented for vhost-net. These APIs can be safely added at any time. And we can also ask developers to add public APIs if it matters when adding new features in the future. I don't think it's a big problem. Best regards, Tiwei Bie > > > > > Best regards, > > Tiwei Bie > > > > > > > Regards, > > > D. > > > > > > > > > > > Is my above understanding correct? Thanks! > > > > > > > > Best regards, > > > > Tiwei Bie > > > >