Re: [dpdk-dev] [PATCH 0/3] add ifcvf driver

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi Xiao,

On 03/27/2018 06:40 AM, Wang, Xiao W wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Monday, March 26, 2018 9:30 PM
>> To: Wang, Xiao W <xiao.w.wang@intel.com>; dev@dpdk.org
>> Cc: Wang, Zhihong <zhihong.wang@intel.com>; yliu@fridaylinux.org; Liang,
>> Cunming <cunming.liang@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Chen,
>> Junjie J <junjie.j.chen@intel.com>; Daly, Dan <dan.daly@intel.com>
>> Subject: Re: [PATCH 0/3] add ifcvf driver
>>
>>
>>
>> On 03/26/2018 11:05 AM, Wang, Xiao W wrote:
>>> Hi Maxime,
>>>
>>>> -----Original Message-----
>>>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>>>> Sent: Sunday, March 25, 2018 5:51 PM
>>>> To: Wang, Xiao W <xiao.w.wang@intel.com>; dev@dpdk.org
>>>> Cc: Wang, Zhihong <zhihong.wang@intel.com>; yliu@fridaylinux.org; Liang,
>>>> Cunming <cunming.liang@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
>> Chen,
>>>> Junjie J <junjie.j.chen@intel.com>; Daly, Dan <dan.daly@intel.com>
>>>> Subject: Re: [PATCH 0/3] add ifcvf driver
>>>>
>>>>
>>>>
>>>> On 03/23/2018 11:27 AM, Wang, Xiao W wrote:
>>>>> Hi Maxime,
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>>>>>> Sent: Thursday, March 22, 2018 4:48 AM
>>>>>> To: Wang, Xiao W <xiao.w.wang@intel.com>; dev@dpdk.org
>>>>>> Cc: Wang, Zhihong <zhihong.wang@intel.com>; yliu@fridaylinux.org;
>> Liang,
>>>>>> Cunming <cunming.liang@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
>>>> Chen,
>>>>>> Junjie J <junjie.j.chen@intel.com>; Daly, Dan <dan.daly@intel.com>
>>>>>> Subject: Re: [PATCH 0/3] add ifcvf driver
>>>>>>
>>>>>> Hi Xiao,
>>>>>>
>>>>>> On 03/15/2018 05:49 PM, Wang, Xiao W wrote:
>>>>>>> Hi Maxime,
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>>>>>>>> Sent: Sunday, March 11, 2018 2:24 AM
>>>>>>>> To: Wang, Xiao W <xiao.w.wang@intel.com>; dev@dpdk.org
>>>>>>>> Cc: Wang, Zhihong <zhihong.wang@intel.com>; yliu@fridaylinux.org;
>>>> Liang,
>>>>>>>> Cunming <cunming.liang@intel.com>; Xu, Rosen
>> <rosen.xu@intel.com>;
>>>>>> Chen,
>>>>>>>> Junjie J <junjie.j.chen@intel.com>; Daly, Dan <dan.daly@intel.com>
>>>>>>>> Subject: Re: [PATCH 0/3] add ifcvf driver
>>>>>>>>
>>>>>>>> Hi Xiao,
>>>>>>>>
>>>>>>>> On 03/10/2018 12:08 AM, Xiao Wang wrote:
>>>>>>>>> This patch set has dependency on
>>>>>>>> http://dpdk.org/dev/patchwork/patch/35635/
>>>>>>>>> (vhost: support selective datapath);
>>>>>>>>>
>>>>>>>>> ifc VF is compatible with virtio vring operations, this driver
>> implements
>>>>>>>>> vDPA driver ops which configures ifc VF to be a vhost data path
>>>> accelerator.
>>>>>>>>>
>>>>>>>>> ifcvf driver uses vdev as a control domain to manage ifc VFs that
>> belong
>>>>>>>>> to it. It registers vDPA device ops to vhost lib to enable these VFs to
>> be
>>>>>>>>> used as vhost data path accelerator.
>>>>>>>>>
>>>>>>>>> Live migration feature is supported by ifc VF and this driver enables
>>>>>>>>> it based on vhost lib.
>>>>>>>>>
>>>>>>>>> vDPA needs to create different containers for different devices, thus
>> this
>>>>>>>>> patch set adds APIs in eal/vfio to support multiple container.
>>>>>>>> Thanks for this! That will avoind having to duplicate these functions
>>>>>>>> for every new offload driver.
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Junjie Chen (1):
>>>>>>>>>        eal/vfio: add support for multiple container
>>>>>>>>>
>>>>>>>>> Xiao Wang (2):
>>>>>>>>>        bus/pci: expose sysfs parsing API
>>>>>>>>
>>>>>>>> Still, I'm not convinced the offload device should be a virtual device.
>>>>>>>> It is a real PCI device, why not having a new device type for offload
>>>>>>>> devices, and let the device to be probed automatically by the existing
>>>>>>>> device model?
>>>>>>>
>>>>>>> IFC VFs are generated from SRIOV, with the PF driven by kernel driver.
>>>>>>> In DPDK we need to have something to represent PF, to register itself as
>>>>>>> a vDPA engine, so a virtual device is used for this purpose.
>>>>>> I went through the code, and something is not clear to me.
>>>>>>
>>>>>> Why do we need to have a representation of the PF in DPDK?
>>>>>> Why cannot we just bind at VF level?
>>>>>
>>>>> 1. With the vdev representation we could use it to talk to PF kernel driver
>> to
>>>> do flow configuration, we can implement
>>>>> flow API on the vdev in future for this purpose. Using a vdev allows
>>>> introducing this kind of control plane thing.
>>>>>
>>>>> 2. When port representor is ready, we would integrate it into ifcvf driver,
>>>> then each VF will have a
>>>>> Representor port. For now we don’t have port representor, so this patch
>> set
>>>> manages VF resource internally.
>>>>
>>>> Ok, we may need to have a vdev to represent the PF, but we need to be
>>>> able to bind at VF level anyway.
>>>
>>> Device management on VF level is feasible, according to the previous port-
>> representor patch.
>>> A tuple of (PF_addr, VF_index) can identify a certain VF, we have vport_mask
>>> and device addr to describe a PF, and we can specify a VF index to create a
>> representor port,
>>> so , the VF port creation will be on-demand at VF level.
>>>
>>> +struct port_rep_parameters {
>>> +	uint64_t vport_mask;
>>> +	struct {
>>> +		char bus[RTE_DEV_NAME_MAX_LEN];
>>> +		char device[RTE_DEV_NAME_MAX_LEN];
>>> +	} parent;
>>> +};
>>>
>>> +int
>>> +rte_representor_port_register(char *pf_addr_str,
>>> +		uint32_t vport_id, uint16_t *port_id)
>>
>> IIUC, even with this using port representor, we'll still have the
>> problem of having the VFs probed first as Virtio driver, right?
>>
>> In my opinion, the IFCVF devices in offload mode are to be managed
>> differently than having a way to represent on host side VFs assigned to
>> a VM.
>>
>> In offload case, you have a real device to deal with, else we
>> wouldn't have to bind it with VFIO.
>>
>> Maybe we could have a real device probed as proposed yesterday [0], and
>> this device gets registered to the port representor for the PF?
> 
> Adding a list of offload-device in eal is a way to skip virtio pmd probe [0].
> I think using device devargs could also achieve that: add a parameter "vdpa=1"
> to the device, virtio pmd parses the devargs and detects that the device is in vdpa mode,
> quits its probe immediately.
> Devargs could be flexible of allowing one device with multi driver case.

That's a good idea, but if we want the vDPA VF to probed in a generic
way, will it work? I think the Virtio PMD won't be probed but no other
driver will be tried, but I might be wrong.

Thanks,
Maxime
> BRs,
> Xiao
> 
>>
>> Thanks,
>> Maxime
>>> Besides, IFCVF supports live migration, vDPA exerts IFCVF device better than
>> QEMU (this patch has enabled LM feature).
>>> vDPA is the main usage model for IFCVF, and one DPDK application taking
>> control of all the VF resource
>>> management is a straightforward usage model.
>>>
>>> Best Regards,
>>> Xiao
>>>
>>>>
>>>> Else, how do you support passing two VFs of the same PF to different
>>>> DPDK applications?
>>>> Or have some VFs managed by Kernel or QEMU and some by the DPDK
>>>> application? My feeling is that current implementation is creating an
>>>> artificial constraint.
>>>>
>>>> Isn't there a possibility to have the virtual representation for the PF
>>>> to be probed separately? Or created automatically when the first VF of a
>>>> PF is probed (and later VFs attach to the PF rep when probed)?
>>>>
>>>> Doing this, we could use the generic device probing.
>>
>> [0]:
>>>> For IFCVF, to specify we want it to be probed as an offload device
>>>> instead of a virtio device, we could have a new EAL parameter to specify
>>>> for a given device if we want it to be probed as an offload device (for
>>>> example --offload=00:01.1).
>>>> Offload drivers would register by passing a flag that specifies they are
>>>> an offload driver.
>>>> This new argument would be optional and only used to force the device to
>>>> being probed in offload mode. For devices that have their own device
>>>> IDs for offload mode, then it would be automatic.
>>>>
>>>> Regards,
>>>> Maxime
>>>>> BRs,
>>>>> Xiao
>>>>>