From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tetsuya Mukawa Subject: Re: [PATCH v5 1/3] vhost: Add callback and private data for vhost PMD Date: Fri, 18 Dec 2015 12:15:42 +0900 Message-ID: <56737A5E.1030803@igel.co.jp> References: <1447392031-24970-3-git-send-email-mukawa@igel.co.jp> <1448355603-21275-1-git-send-email-mukawa@igel.co.jp> <1448355603-21275-2-git-send-email-mukawa@igel.co.jp> <20151217114223.GC29571@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, ann.zhuangyanying@huawei.com To: Yuanhan Liu , "Xie, Huawei" Return-path: Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com [209.85.220.46]) by dpdk.org (Postfix) with ESMTP id 80DDF8D91 for ; Fri, 18 Dec 2015 04:15:46 +0100 (CET) Received: by mail-pa0-f46.google.com with SMTP id ur14so52705475pab.0 for ; Thu, 17 Dec 2015 19:15:46 -0800 (PST) In-Reply-To: <20151217114223.GC29571@yliu-dev.sh.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2015/12/17 20:42, Yuanhan Liu wrote: > On Tue, Nov 24, 2015 at 06:00:01PM +0900, Tetsuya Mukawa wrote: >> The vhost PMD will be a wrapper of vhost library, but some of vhost >> library APIs cannot be mapped to ethdev library APIs. >> Becasue of this, in some cases, we still need to use vhost library APIs >> for a port created by the vhost PMD. >> >> Currently, when virtio device is created and destroyed, vhost library >> will call one of callback handlers. The vhost PMD need to use this >> pair of callback handlers to know which virtio devices are connected >> actually. >> Because we can register only one pair of callbacks to vhost library, if >> the PMD use it, DPDK applications cannot have a way to know the events. >> >> This may break legacy DPDK applications that uses vhost library. To prevent >> it, this patch adds one more pair of callbacks to vhost library especially >> for the vhost PMD. >> With the patch, legacy applications can use the vhost PMD even if they need >> additional specific handling for virtio device creation and destruction. >> >> For example, legacy application can call >> rte_vhost_enable_guest_notification() in callbacks to change setting. > TBH, I never liked it since the beginning. Introducing two callbacks > for one event is a bit messy, and therefore error prone. I agree with you. > I have been thinking this occasionally last few weeks, and have came > up something that we may introduce another layer callback based on > the vhost pmd itself, by a new API: > > rte_eth_vhost_register_callback(). > > And we then call those new callback inside the vhost pmd new_device() > and vhost pmd destroy_device() implementations. > > And we could have same callbacks like vhost have, but I'm thinking > that new_device() and destroy_device() doesn't sound like a good name > to a PMD driver. Maybe a name like "link_state_changed" is better? > > What do you think of that? Yes, "link_state_changed" will be good. BTW, I thought it was ok that an DPDK app that used vhost PMD called vhost library APIs directly. But probably you may feel strangeness about it. Is this correct? If so, how about implementing legacy status interrupt mechanism to vhost PMD? For example, an DPDK app can register callback handler like "examples/link_status_interrupt". Also, if the app doesn't call vhost library APIs directly, rte_eth_vhost_portid2vdev() will be needless, because the app doesn't need to handle virtio device structure anymore. > > > On the other hand, I'm still thinking is that really necessary to let > the application be able to call vhost functions like rte_vhost_enable_guest_notification() > with the vhost PMD driver? Basic concept of my patch is that vhost PMD will provides the features that vhost library provides. How about removing rte_vhost_enable_guest_notification() from "vhost library"? (I also not sure what are use cases) If we can do this, vhost PMD also doesn't need to take care of it. Or if rte_vhost_enable_guest_notification() will be removed in the future, vhost PMD is able to ignore it. Please let me correct up my thinking about your questions. - Change concept of patch not to call vhost library APIs directly. These should be wrapped by ethdev APIs. - Remove rte_eth_vhost_portid2vdev(), because of above concept changing. - Implement legacy status changed interrupt to vhost PMD instead of using own callback mechanism. - Check if we can remove rte_vhost_enable_guest_notification() from vhost library. Hi Xie, Do you know the use cases of rte_vhost_enable_guest_notification()? Thanks, Tetsuya