[dpdk-dev] [PATCH v5 1/3] vhost: Add callback and private data for vhost PMD

[Tetsuya Mukawa]

On 2015/12/18 12:15, Tetsuya Mukawa wrote:
> On 2015/12/17 20:42, Yuanhan Liu wrote:
>> On Tue, Nov 24, 2015 at 06:00:01PM +0900, Tetsuya Mukawa wrote:
>>> The vhost PMD will be a wrapper of vhost library, but some of vhost
>>> library APIs cannot be mapped to ethdev library APIs.
>>> Becasue of this, in some cases, we still need to use vhost library APIs
>>> for a port created by the vhost PMD.
>>>
>>> Currently, when virtio device is created and destroyed, vhost library
>>> will call one of callback handlers. The vhost PMD need to use this
>>> pair of callback handlers to know which virtio devices are connected
>>> actually.
>>> Because we can register only one pair of callbacks to vhost library, if
>>> the PMD use it, DPDK applications cannot have a way to know the events.
>>>
>>> This may break legacy DPDK applications that uses vhost library. To prevent
>>> it, this patch adds one more pair of callbacks to vhost library especially
>>> for the vhost PMD.
>>> With the patch, legacy applications can use the vhost PMD even if they need
>>> additional specific handling for virtio device creation and destruction.
>>>
>>> For example, legacy application can call
>>> rte_vhost_enable_guest_notification() in callbacks to change setting.
>> TBH, I never liked it since the beginning. Introducing two callbacks
>> for one event is a bit messy, and therefore error prone.
> I agree with you.
>
>> I have been thinking this occasionally last few weeks, and have came
>> up something that we may introduce another layer callback based on
>> the vhost pmd itself, by a new API:
>>
>> 	rte_eth_vhost_register_callback().
>>
>> And we then call those new callback inside the vhost pmd new_device()
>> and vhost pmd destroy_device() implementations.
>>
>> And we could have same callbacks like vhost have, but I'm thinking
>> that new_device() and destroy_device() doesn't sound like a good name
>> to a PMD driver. Maybe a name like "link_state_changed" is better?
>>
>> What do you think of that?
> Yes,  "link_state_changed" will be good.
>
> BTW, I thought it was ok that an DPDK app that used vhost PMD called
> vhost library APIs directly.
> But probably you may feel strangeness about it. Is this correct?
>
> If so, how about implementing legacy status interrupt mechanism to vhost
> PMD?
> For example, an DPDK app can register callback handler like
> "examples/link_status_interrupt".

Addition:
In this case, we don't need something special pmd_priv field in vhost
library.
vhost PMD will register callbacks.
And in this callbacks, legacy interrupt mechanism will be kicked.
Then user can receive interrupt from ethdev.

Tetsuya

> Also, if the app doesn't call vhost library APIs directly,
> rte_eth_vhost_portid2vdev() will be needless, because the app doesn't
> need to handle virtio device structure anymore.
>
>>
>> On the other hand, I'm still thinking is that really necessary to let
>> the application be able to call vhost functions like rte_vhost_enable_guest_notification()
>> with the vhost PMD driver?
> Basic concept of my patch is that vhost PMD will provides the features
> that vhost library provides.
>
> How about removing rte_vhost_enable_guest_notification() from "vhost
> library"?
> (I also not sure what are use cases)
> If we can do this, vhost PMD also doesn't need to take care of it.
> Or if rte_vhost_enable_guest_notification() will be removed in the
> future, vhost PMD is able to ignore it.
>
>
> Please let me correct up my thinking about your questions.
>  - Change concept of patch not to call vhost library APIs directly.
> These should be wrapped by ethdev APIs.
>  - Remove rte_eth_vhost_portid2vdev(), because of above concept changing.
>  - Implement legacy status changed interrupt to vhost PMD instead of
> using own callback mechanism.
>  - Check if we can remove rte_vhost_enable_guest_notification() from
> vhost library.
>
>
> Hi Xie,
>
> Do you know the use cases of rte_vhost_enable_guest_notification()?
>
> Thanks,
> Tetsuya