[dpdk-dev] discussion: creating a new class for vdpa driversxiao.w.wang at intel.com

[Andrew Rybchenko]

On 12/9/19 1:41 PM, Ori Kam wrote:
> Hi
> 
>> -----Original Message-----
>> From: dev <dev-bounces at dpdk.org> On Behalf Of Andrew Rybchenko
>> Sent: Monday, December 9, 2019 9:44 AM
>> To: Matan Azrad <matan at mellanox.com>; Liang, Cunming
>> <cunming.liang at intel.com>; Bie, Tiwei <tiwei.bie at intel.com>
>> Cc: Wang, Xiao W <xiao.w.wang at intel.com>; Thomas Monjalon
>> <thomas at monjalon.net>; maxime.coquelin at redhat.com; Wang, Zhihong
>> <zhihong.wang at intel.com>; Yigit, Ferruh <ferruh.yigit at intel.com>; Shahaf
>> Shuler <shahafs at mellanox.com>; Ori Kam <orika at mellanox.com>;
>> dev at dpdk.org; Slava Ovsiienko <viacheslavo at mellanox.com>; Asaf Penso
>> <asafp at mellanox.com>; Olga Shern <olgas at mellanox.com>
>> Subject: Re: [dpdk-dev] discussion: creating a new class for vdpa
>> driversxiao.w.wang at intel.com
>>
>> On 12/8/19 10:06 AM, Matan Azrad wrote:
>>> From: Andrew Rybchenko
>>>> On 12/6/19 8:32 AM, Liang, Cunming wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Bie, Tiwei <tiwei.bie at intel.com>
>>>>>> Sent: Friday, December 6, 2019 12:28 PM
>>>>>> To: Matan Azrad <matan at mellanox.com>
>>>>>> Cc: Wang, Xiao W <xiao.w.wang at intel.com>; Thomas Monjalon
>>>>>> <thomas at monjalon.net>; maxime.coquelin at redhat.com; Wang,
>>>> Zhihong
>>>>>> <zhihong.wang at intel.com>; Yigit, Ferruh <ferruh.yigit at intel.com>;
>>>>>> Shahaf Shuler <shahafs at mellanox.com>; Ori Kam
>>>> <orika at mellanox.com>;
>>>>>> dev at dpdk.org; Slava Ovsiienko <viacheslavo at mellanox.com>; Asaf
>>>> Penso
>>>>>> <asafp at mellanox.com>; Olga Shern <olgas at mellanox.com>; Liang,
>>>> Cunming
>>>>>> <cunming.liang at intel.com>
>>>>>> Subject: Re: discussion: creating a new class for vdpa
>>>>>> driversxiao.w.wang at intel.com
>>>>>>
>>>>>> On Thu, Dec 05, 2019 at 01:26:36PM +0000, Matan Azrad wrote:
>>>>>>> Hi all
>>>>>>>
>>>>>>> As described in RFC “[RFC] net: new vdpa PMD for Mellanox devices”,
>>>>>>> a new vdpa drivers is going to be added for Mellanox devices –
>>>>>>> mlx5_vdpa
>>>>>>>
>>>>>>> The only vdpa driver now is the IFC driver that is located in net
>> directory.
>>>>>>>
>>>>>>> The IFC driver and the new mlx5_vdpa driver provide the vdpa ops
>> and
>>>>>>> not the eth_dev ops.
>>>>>>>
>>>>>>> All the others drivers in net provide the eth-dev ops.
>>>>>>>
>>>>>>> I suggest to create a new class for vdpa drivers, to move IFC to
>>>>>>> this class and to add the mlx5_vdpa to this class too.
>>>>>>>
>>>>>>> Later, all the new drivers that implements the vdpa ops will be
>>>>>>> added to the vdpa class.
>>>>>>
>>>>>> +1. Sounds like a good idea to me.
>>>>> +1
>>>>
>>>> vDPA drivers are vendor-specific and expected to talk to vendor NIC. I.e.
>>>> there are significant chances to share code with network drivers (e.g.
>> base
>>>> driver). Should base driver be moved to drivers/common in this case or is
>> it
>>>> still allows to have vdpa driver in drivers/net together with ethdev driver?
>>>
>>> Yes, I think this should be the method, shared code should be moved to
>> the drivers/common directory.
>>> I think there is a precedence with shared code in common which shares a
>> vendor specific code between crypto and net.
>>
>> I see motivation behind driver/vdpa. However, vdpa and net
>> drivers tightly related and I would prefer to avoid extra
>> complexity here. Right now simplicity over-weights for me.
>> No strong opinion on the topic, but it definitely requires
>> better and more clear motivation why a new class should be
>> introduced and existing drivers restructured.
>>
> 
> Why do you think there is extra complexity?

Even grep becomes a bit more complicated J

> I think from design correctness it is more correct to create a dedicated class for the following reasons:
> 1. All of the classes implements a different set of ops. For example the cryptodev has a defined set of ops, same goes for the compress driver and the ethdev driver. Each ones of them 
> has different ops. Going by this definition since VDPA has a different set of ops, it makes sense that it will be in a different class.
> 
> 2. even that both drivers (eth/vdpa) handle traffic from the nic most of the code is different (the difference is also dependent on the manufacture)
> So even the shared code base is not large and can be shared using the common directory. For example ifc doesn't have any shared code.
> 
> What do you think?

The true reason is: if the difference in ops implemented
is a key difference which should enforce location in
different directories. Or underlying device class is a key.
Roughly:
 - net driver is a control+data path
 - vdpa driver is a control path only
My fear is that control path will grow more and more
(Rx mode, RSS, filters and so on) and more DPDK specific
(not base driver) code could be shared and it is a bit
easier to share if vdpa driver lives nearby net driver
since both implement net driver control path but in
different terms (ops).

Again, I have no strong opinion and just want to
foresee the future at least just a bit and make sure
that the decision is motivated with all concerns
discussed and resolved.

net/ifc and net/virtio are existing stakeholders.

>>> Actually, this is my plan to share mlx5 vdpa code with mlx5 net code by the
>> drivers/common dir (see RFC).
>>
>> I see.