From mboxrd@z Thu Jan  1 00:00:00 1970
From: Panu Matilainen <pmatilai@redhat.com>
Subject: Re: [RFC PATCH v2] vhost: Add VHOST PMD
Date: Mon, 19 Oct 2015 16:26:21 +0300
Message-ID: <5624EF7D.9090908@redhat.com>
References: <1440993326-21205-1-git-send-email-mukawa@igel.co.jp>
 <1440993326-21205-2-git-send-email-mukawa@igel.co.jp>
 <20151016125254.GA9980@bricha3-MOBL3> <56244C84.4090309@igel.co.jp>
 <74F120C019F4A64C9B78E802F6AD4CC24F7A881E@IRSMSX106.ger.corp.intel.com>
 <20151019094532.GB11324@bricha3-MOBL3> <5624CAF2.10201@igel.co.jp>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: "dev@dpdk.org" <dev@dpdk.org>,
 "ann.zhuangyanying@huawei.com" <ann.zhuangyanying@huawei.com>
To: Tetsuya Mukawa <mukawa@igel.co.jp>,
 Bruce Richardson <bruce.richardson@intel.com>,
 "Loftus, Ciara" <ciara.loftus@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by dpdk.org (Postfix) with ESMTP id EF2EC5961
 for <dev@dpdk.org>; Mon, 19 Oct 2015 15:26:24 +0200 (CEST)
In-Reply-To: <5624CAF2.10201@igel.co.jp>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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@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 -