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

[Tetsuya Mukawa]

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".

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