From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 169F01B067 for ; Tue, 26 Jun 2018 10:22:26 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jun 2018 01:22:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,274,1526367600"; d="scan'208";a="235642951" Received: from debian.sh.intel.com (HELO debian) ([10.67.104.228]) by orsmga005.jf.intel.com with ESMTP; 26 Jun 2018 01:22:23 -0700 Date: Tue, 26 Jun 2018 16:22:26 +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: <20180626082226.GA15665@debian> References: <1526648465-62579-1-git-send-email-dariuszx.stojaczyk@intel.com> <20180607151227.23660-1-darek.stojaczyk@gmail.com> <20180625110146.GA18211@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 08:22:27 -0000 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. Best regards, Tiwei Bie > Regards, > D. > > > > > Is my above understanding correct? Thanks! > > > > Best regards, > > Tiwei Bie > >