Re: [PATCH v3 2/2] virtio/vdev: add a new vdev named eth_cvio

[David Marchand <david.marchand@6wind.com>]

Hello,

On Fri, Apr 22, 2016 at 7:15 AM, Tan, Jianfeng <jianfeng.tan@intel.com> 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