From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 6995C370 for ; Thu, 8 Dec 2016 08:20:51 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP; 07 Dec 2016 23:20:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,318,1477983600"; d="scan'208";a="200323956" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga004.fm.intel.com with ESMTP; 07 Dec 2016 23:20:49 -0800 Date: Thu, 8 Dec 2016 15:21:32 +0800 From: Yuanhan Liu To: Jianfeng Tan Cc: dev@dpdk.org, ferruh.yigit@intel.com, cunming.liang@intel.com Message-ID: <20161208072132.GB30698@yliu-dev.sh.intel.com> References: <1480689075-66977-1-git-send-email-jianfeng.tan@intel.com> <1480689075-66977-2-git-send-email-jianfeng.tan@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1480689075-66977-2-git-send-email-jianfeng.tan@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH 1/3] net/virtio_user: add vhost layer 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: Thu, 08 Dec 2016 07:20:51 -0000 On Fri, Dec 02, 2016 at 02:31:13PM +0000, Jianfeng Tan wrote: > To support vhost kernel as the backend of net_virtio_user in comming > patches, we abstract a vhost layer to hide all vhost_user functions. > - Move vhost_user specific structs and macros into vhost_user.c, > and only keep common definitions in vhost.h; Do that in another patch. > - Add a struct vhost_internal, and an array to store vhost_user and > vhost_kernel backends; in multiqueue case, vhost_user has only > one vhost FD, but vhost_kernel would have multiple FDs (equal to > # of queues), so we turn to use an id to index the backend info; It's okay to add a struct for that, but what's the point of referencing it by an id? Instead, you could directly allocate one and return it. This would also remove the hardcoded limit: 8 devices at most. > - Add a struct vhost_ops depending on different type of backends. Firstly, the "control" method is not well named, IMO. Something like "send_request" is better. And there is another way to achieve that: instead of introducing one callback (control, or send_request) for all requests, you could define one callback for each request. QEMU switched to that way few releases ago. In that way, you don't have to do the request translation at least. Last, please make one patch for each of 3 items. It helps review. --yliu