[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD

[Richardson, Bruce]

> -----Original Message-----
> From: Panu Matilainen [mailto:pmatilai at redhat.com]
> Sent: Monday, October 19, 2015 2:26 PM
> To: Tetsuya Mukawa <mukawa at igel.co.jp>; Richardson, Bruce
> <bruce.richardson at intel.com>; Loftus, Ciara <ciara.loftus at intel.com>
> Cc: dev at dpdk.org; ann.zhuangyanying at huawei.com
> Subject: Re: [dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
> 
> On 10/19/2015 01:50 PM, Tetsuya Mukawa wrote:
> > On 2015/10/19 18:45, Bruce Richardson wrote:
> >> On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote:
> >>>> On 2015/10/16 21:52, Bruce Richardson wrote:
> >>>>> On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote:
> >>>>>> The patch introduces a new PMD. This PMD is implemented as thin
> >>>> wrapper
> >>>>>> of librte_vhost. It means librte_vhost is also needed to compile
> the PMD.
> >>>>>> The PMD can have 'iface' parameter like below to specify a path
> >>>>>> to
> >>>> connect
> >>>>>> to a virtio-net device.
> >>>>>>
> >>>>>> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i
> >>>>>>
> >>>>>> To connect above testpmd, here is qemu command example.
> >>>>>>
> >>>>>> $ qemu-system-x86_64 \
> >>>>>>          <snip>
> >>>>>>          -chardev socket,id=chr0,path=/tmp/sock0 \
> >>>>>>          -netdev vhost-user,id=net0,chardev=chr0,vhostforce \
> >>>>>>          -device virtio-net-pci,netdev=net0
> >>>>>>
> >>>>>> Signed-off-by: Tetsuya Mukawa <mukawa at igel.co.jp>
> >>>>> With this PMD in place, is there any need to keep the existing
> >>>>> vhost library around as a separate entity? Can the existing
> >>>>> library be
> >>>> subsumed/converted into
> >>>>> a standard PMD?
> >>>>>
> >>>>> /Bruce
> >>>> Hi Bruce,
> >>>>
> >>>> I concern about whether the PMD has all features of librte_vhost,
> >>>> because librte_vhost provides more features and freedom than ethdev
> >>>> API provides.
> >>>> In some cases, user needs to choose limited implementation without
> >>>> librte_vhost.
> >>>> I am going to eliminate such cases while implementing the PMD.
> >>>> But I don't have strong belief that we can remove librte_vhost now.
> >>>>
> >>>> So how about keeping current separation in next DPDK?
> >>>> I guess people will try to replace librte_vhost to vhost PMD,
> >>>> because apparently using ethdev APIs will be useful in many cases.
> >>>> And we will get feedbacks like "vhost PMD needs to support like this
> usage".
> >>>> (Or we will not have feedbacks, but it's also OK.) Then, we will be
> >>>> able to merge librte_vhost and vhost PMD.
> >>> I agree with the above. One the concerns I had when reviewing the
> patch was that the PMD removes some freedom that is available with the
> library. Eg. Ability to implement the new_device and destroy_device
> callbacks. If using the PMD you are constrained to the implementations of
> these in the PMD driver, but if using librte_vhost, you can implement your
> own with whatever functionality you like - a good example of this can be
> seen in the vhost sample app.
> >>> On the other hand, the PMD is useful in that it removes a lot of
> complexity for the user and may work for some more general use cases. So I
> would be in favour of having both options available too.
> >>>
> >>> Ciara
> >>>
> >> Thanks.
> >> However, just because the libraries are merged does not mean that you
> >> need be limited by PMD functionality. Many PMDs provide additional
> >> library-specific functions over and above their PMD capabilities. The
> >> bonded PMD is a good example here, as it has a whole set of extra
> >> functions to create and manipulate bonded devices - things that are
> >> obviously not part of the general ethdev API. Other vPMDs similarly
> include functions to allow them to be created on the fly too.
> >>
> >> regards,
> >> /Bruce
> >
> > Hi Bruce,
> >
> > I appreciate for showing a good example. I haven't noticed the PMD.
> > I will check the bonding PMD, and try to remove librte_vhost without
> > losing freedom and features of the library.
> 
> Hi,
> 
> Just a gentle reminder - if you consider removing (even if by just
> replacing/renaming) an entire library, it needs to happen the ABI
> deprecation process.
> 
> It seems obvious enough. But for all the ABI policing here, somehow we all
> failed to notice the two compatibility breaking rename-elephants in the
> room during 2.1 development:
> - libintel_dpdk was renamed to libdpdk
> - librte_pmd_virtio_uio was renamed to librte_pmd_virtio
> 
> Of course these cases are easy to work around with symlinks, and are
> unrelated to the matter at hand. Just wanting to make sure such things
> dont happen again.
> 
> 	- Panu -

Still doesn't hurt to remind us, Panu! Thanks. :-)