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 E5DF62BD4 for ; Fri, 22 Apr 2016 12:25:14 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP; 22 Apr 2016 03:25:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,516,1455004800"; d="scan'208";a="690422939" Received: from shwdeisgchi083.ccr.corp.intel.com (HELO [10.239.67.193]) ([10.239.67.193]) by FMSMGA003.fm.intel.com with ESMTP; 22 Apr 2016 03:25:11 -0700 To: David Marchand References: <1446748276-132087-1-git-send-email-jianfeng.tan@intel.com> <1461207396-42742-1-git-send-email-jianfeng.tan@intel.com> <1461207396-42742-3-git-send-email-jianfeng.tan@intel.com> <5719B370.8070107@intel.com> Cc: "dev@dpdk.org" , Huawei Xie , Rich Lane , Yuanhan Liu , "Michael S. Tsirkin" , nakajima.yoshihiro@lab.ntt.co.jp, p.fedin@samsung.com, "Qiu, Michael" , ann.zhuangyanying@huawei.com, Tetsuya Mukawa , nhorman@tuxdrver.com From: "Tan, Jianfeng" Message-ID: <5719FC06.7050205@intel.com> Date: Fri, 22 Apr 2016 18:25:10 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 2/2] virtio/vdev: add a new vdev named eth_cvio 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: Fri, 22 Apr 2016 10:25:15 -0000 Hi, On 4/22/2016 3:36 PM, David Marchand wrote: > Hello, > > On Fri, Apr 22, 2016 at 7:15 AM, Tan, Jianfeng wrote: >> On 4/21/2016 4:51 PM, David Marchand wrote: >>> virtio code relies on drv_flags (even updating it while this should be >>> per-device). >>> So first, virtio should rely on dev_flags. >> >> Mainly drv_flags's RTE_PCI_DRV_INTR_LSC, and RTE_PCI_DRV_DETACHABLE bit is >> used. I understand the issue, pointed out by you here, that if two virtio >> devices are used in a VM, one with feature VIRTIO_NET_F_STATUS, and the >> other without feature VIRTIO_NET_F_STATUS (under the case that two vhost >> backends are used). Then it leads to uncertainty of the behavior. >> >> Since the flags has been copied into dev_flags after features negotiated, I >> believe we should use dev_flags instead of drv_flags. A patch to fix this >> will be sent. > Ok. > >>> The rest needs to be astracted in some virtio ops ? >> So with that fix goes, we may make it more clear now. Do you mean this? > Well, here, we have what looks like to be two drivers (one pci and one vdev). > You tried to keep all this code together, to avoid duplicating it, > which sounds sane. > But in the end, you are trying to make this work by adding checks > where this can't. > I am not saying we should duplicate the code, but maybe having some > internal virtio ops / abstraction would do the trick and avoid those > checks. > > > The reason of those comments is that dev_type in ethdev is going to > disappear, see [1] and [2]. > Drivers are called through their own specific ethdev/crypto ops and > so, those drivers know implicitely that their are either pci or vdev > (or whatever in the future) drivers. > > > [1]: http://dpdk.org/ml/archives/dev/2016-April/037686.html > [2]: http://dpdk.org/ml/archives/dev/2016-January/031390.html Thank you for this import information. A quick check, we can remove those checks, as you said, to virtio ops. Great! Thanks, Jianfeng