On 2015/10/21 19:22, Bruce Richardson wrote: > On Wed, Oct 21, 2015 at 09:25:12AM +0300, Panu Matilainen wrote: >> On 10/21/2015 07:35 AM, Tetsuya Mukawa wrote: >>> On 2015/10/19 22:27, Richardson, Bruce wrote: >>>>> -----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. :-) >>> Hi, >>> >>> Thanks for reminder. I've checked the DPDK documentation. >>> I will submit deprecation notice to follow DPDK deprecation process. >>> (Probably we will be able to remove vhost library in DPDK-2.3 or later.) >>> >>> BTW, I will merge vhost library and PMD like below. >>> Step1. Move vhost library under vhost PMD. >>> Step2. Rename current APIs. >>> Step3. Add a function to get a pointer of "struct virtio_net device" by >>> a portno. >>> >>> Last steps allows us to be able to convert a portno to the pointer of >>> corresponding vrtio_net device. >>> And we can still use features and freedom vhost library APIs provided. >> Just wondering, is that *really* worth the price of breaking every single >> vhost library user out there? >> >> I mean, this is not about removing some bitrotten function or two which >> nobody cares about anymore but removing (by renaming) one of the more widely >> (AFAICS) used libraries and its entire API. >> >> If current APIs are kept then compatibility is largely a matter of planting >> a strategic symlink or two, but it might make the API look inconsistent. >> >> But just wondering about the benefit of this merge thing, compared to just >> adding a vhost pmd and leaving the library be. The ABI process is not there >> to make life miserable for DPDK developers, its there to help make DPDK >> nicer for *other* developers. And the first and the foremost rule is simply: >> dont break backwards compatibility. Not unless there's a damn good reason to >> doing so, and I fail to see that reason here. >> >> - Panu - >> > Good question, and I'll accept that maybe it's not worth doing. I'm not that > much of an expert on the internals and APIs of vhost library. > > However, the merge I was looking for was more from a code locality point > of view, to have all the vhost code in one directory (under drivers/net), > than spread across multiple ones. What API's need to be deprecated > or not as part of that work, is a separate question, and so in theory we could > create a combined vhost library that does not deprecate anything (though to > avoid a build-up of technical debt, we'll probably want to deprecate some > functions). > > I'll leave it up to the vhost experts do decide what's best, but for me, any > library that handles transmission and reception of packets outside of a DPDK > app should be a PMD library using ethdev rx/tx burst routines, and located > under drivers/net. (KNI is another obvious target for such a move and > conversion). > > Regards, > /Bruce >
Hi, I have submitted latest patches. I will keep vhost library until we will have agreement to merge it to vhost PMD. Regards, Testuya
