From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f42.google.com (mail-pa0-f42.google.com [209.85.220.42]) by dpdk.org (Postfix) with ESMTP id 1D960DE0 for ; Mon, 21 Dec 2015 03:10:03 +0100 (CET) Received: by mail-pa0-f42.google.com with SMTP id wq6so90861039pac.1 for ; Sun, 20 Dec 2015 18:10:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=igel-co-jp.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-type:content-transfer-encoding; bh=ViIbRyV4Ht0chj0Pt1lcW+6ELUMspyaCkGQX7SvfGSA=; b=egav6/6b5qU++fHV3+ceCt/P/ZRXeta1jDKBVIDCkdV3F81AXbu4zeSDSND9oZNTiS /oV3l8O5TTFLDqzFISCMFfWyVgFtZnu0BxRQgeMel9pb36Y3bh1aLS5L+sQCwxv9iSND Ehpjcz8faK9SjPeRtjFrl+efiwRjhZoe50KXUXs6l0jqMMSciSd9hHseVWJLOoxi9sd7 yVIRa53A6k0Zw5bYlViTMysjIrD9IrPgCL4ZCOzxRbO/9jmqb33S0Z8lrZd9SdB3jWDe n1mir1tlI6WjFU/j2zzaar0YeSIAgtMfG5uguxCMUqAS/NeCacXe5C+SulS9YsJsFgIA XARw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=ViIbRyV4Ht0chj0Pt1lcW+6ELUMspyaCkGQX7SvfGSA=; b=eEI2gPaq0bountaXPwrUfalYZBcVfk4Y7LYdReCmPHIpT74G/Q+zHYwrcrlda+t/DH oYINCJVptb3KSnUS/sk4i04X7AwH7lUv1wgYcjc+swp0ugB4IsOiyH2/JsaHeLi/GMPS rMRh8rnMZUbaFC3G66fxfsX2mgzyKOB5zuCijlbJcBES7eI2cHk6COPRI9DRNz/hmq3r QEZmcIhVrGKjxmATKBl1PEv1zv+uD9k0qSabkj1hPx0fGfjDsovbuOH3DqGK2ohM1kQZ rhXSq/qoG7lSTZc5/lsA0He1KDcp47fCIBj395h6WIAju06CcGOl2Y4p72YEmgWkkhG9 R+KA== X-Gm-Message-State: ALoCoQmdr1jg5LCaMiTxzDcmk5TylvCxsYUFCYP4xFcoMCVZczNgDAs6HRbfBVOCCkOyjN7TGP6uRBbs1yYbW6nMUpv2PA9OYA== X-Received: by 10.66.219.194 with SMTP id pq2mr23594650pac.107.1450663802328; Sun, 20 Dec 2015 18:10:02 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by smtp.googlemail.com with ESMTPSA id tm4sm35281352pab.3.2015.12.20.18.10.00 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 20 Dec 2015 18:10:01 -0800 (PST) To: "Xie, Huawei" , Yuanhan Liu 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> <56737A5E.1030803@igel.co.jp> <20151218041536.GI29571@yliu-dev.sh.intel.com> From: Tetsuya Mukawa X-Enigmail-Draft-Status: N1110 Message-ID: <56775F78.9040603@igel.co.jp> Date: Mon, 21 Dec 2015 11:10:00 +0900 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" , "ann.zhuangyanying@huawei.com" Subject: Re: [dpdk-dev] [PATCH v5 1/3] vhost: Add callback and private data for vhost PMD X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Dec 2015 02:10:03 -0000 On 2015/12/18 19:03, Xie, Huawei wrote: > On 12/18/2015 12:15 PM, Yuanhan Liu wrote: >> On Fri, Dec 18, 2015 at 12:15:42PM +0900, Tetsuya Mukawa wrote: >>> 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? >> Unluckily, that's true :) >> >>> 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. >> I don't think that's necessary. Let's just treat it as a normal pmd >> driver, having nothing to do with vhost library. >> >>> 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. >> You could either call it in vhost-pmd (which you already have done that), >> or ignore it in vhost-pmd, but dont' remove it from vhost library. >> >>> 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. >> So, how about making it __fare__ simple as the first step, to get merged >> easily, that we don't assume the applications will call any vhost library >> functions any more, so that we don't need the callback, and we don't need >> the rte_eth_vhost_portid2vdev(), either. Again, just let it be a fare >> normal (nothing special) pmd driver. (UNLESS, there is a real must, which >> I don't see so far). >> >> Tetsuya, what do you think of that then? >> >>> Hi Xie, >>> >>> Do you know the use cases of rte_vhost_enable_guest_notification()? > If vhost runs in loop mode, it doesn't need to be notified. You have > wrapped vhost as the PMD, which is nice for OVS integration. If we > require that all PMDs could be polled by select/poll, then we could use > this API for vhost PMD, and wait on the kickfd. For physical nics, we > could wait on the fd for user space interrupt. Thanks for clarification. I will ignore the function for first release of vhost PMD. Thanks, Tetsuya >> Setting the arg to 0 avoids the guest kicking the virtqueue, which >> is good for performance, and we should keep it. >> >> --yliu >>