From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Marchand Subject: Re: [PATCH v3 2/2] virtio/vdev: add a new vdev named eth_cvio Date: Fri, 22 Apr 2016 09:36:27 +0200 Message-ID: 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> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 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 To: "Tan, Jianfeng" Return-path: Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by dpdk.org (Postfix) with ESMTP id 6F6112E83 for ; Fri, 22 Apr 2016 09:36:47 +0200 (CEST) Received: by mail-wm0-f52.google.com with SMTP id u206so12925621wme.1 for ; Fri, 22 Apr 2016 00:36:47 -0700 (PDT) In-Reply-To: <5719B370.8070107@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" 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 -- David Marchand