[dpdk-dev,v4,1/2] vhost: Add callback and private data for vhost PMD

Message ID 1447392031-24970-2-git-send-email-mukawa@igel.co.jp (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Tetsuya Mukawa Nov. 13, 2015, 5:20 a.m. UTC
  These variables are needed to be able to manage one of virtio devices
using both vhost library APIs and vhost PMD.
For example, if vhost PMD uses current callback handler and private data
provided by vhost library, A DPDK application that links vhost library
cannot use some of vhost library APIs. To avoid it, callback and private
data for vhost PMD are needed.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_vhost/rte_vhost_version.map        |  6 +++
 lib/librte_vhost/rte_virtio_net.h             |  3 ++
 lib/librte_vhost/vhost_user/virtio-net-user.c | 13 +++---
 lib/librte_vhost/virtio-net.c                 | 60 +++++++++++++++++++++++++--
 lib/librte_vhost/virtio-net.h                 |  4 +-
 5 files changed, 73 insertions(+), 13 deletions(-)
  

Comments

Yuanhan Liu Nov. 17, 2015, 1:29 p.m. UTC | #1
On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
> These variables are needed to be able to manage one of virtio devices
> using both vhost library APIs and vhost PMD.
> For example, if vhost PMD uses current callback handler and private data
> provided by vhost library, A DPDK application that links vhost library
> cannot use some of vhost library APIs.

Can you be more specific about this?

	--yliu
  
Tetsuya Mukawa Nov. 19, 2015, 2:03 a.m. UTC | #2
On 2015/11/17 22:29, Yuanhan Liu wrote:
> On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
>> These variables are needed to be able to manage one of virtio devices
>> using both vhost library APIs and vhost PMD.
>> For example, if vhost PMD uses current callback handler and private data
>> provided by vhost library, A DPDK application that links vhost library
>> cannot use some of vhost library APIs.
> Can you be more specific about this?
>
> 	--yliu

How about like below?

commit log:
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.

Tetsuya
  
Yuanhan Liu Nov. 19, 2015, 2:18 a.m. UTC | #3
On Thu, Nov 19, 2015 at 11:03:50AM +0900, Tetsuya Mukawa wrote:
> On 2015/11/17 22:29, Yuanhan Liu wrote:
> > On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
> >> These variables are needed to be able to manage one of virtio devices
> >> using both vhost library APIs and vhost PMD.
> >> For example, if vhost PMD uses current callback handler and private data
> >> provided by vhost library, A DPDK application that links vhost library
> >> cannot use some of vhost library APIs.
> > Can you be more specific about this?
> >
> > 	--yliu
> 
> How about like below?
> 
> commit log:
> 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.

Will (and why) the two co-exist at same time?

	--yliu

> 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.
> 
> Tetsuya
  
Tetsuya Mukawa Nov. 19, 2015, 3:13 a.m. UTC | #4
On 2015/11/19 11:18, Yuanhan Liu wrote:
> On Thu, Nov 19, 2015 at 11:03:50AM +0900, Tetsuya Mukawa wrote:
>> On 2015/11/17 22:29, Yuanhan Liu wrote:
>>> On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
>>>> These variables are needed to be able to manage one of virtio devices
>>>> using both vhost library APIs and vhost PMD.
>>>> For example, if vhost PMD uses current callback handler and private data
>>>> provided by vhost library, A DPDK application that links vhost library
>>>> cannot use some of vhost library APIs.
>>> Can you be more specific about this?
>>>
>>> 	--yliu
>> How about like below?
>>
>> commit log:
>> 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.
> Will (and why) the two co-exist at same time?

Yes it is. Sure, I will describe below in commit log.

Because we cannot map some of vhost library APIs to ethdev APIs, in some
cases, we still
need to use vhost library APIs for a port created by the vhost PMD. One
of example is
rte_vhost_enable_guest_notification().

Thanks,
Tetsuya


>
> 	--yliu
>
>> 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.
>>
>> Tetsuya
  
Yuanhan Liu Nov. 19, 2015, 3:33 a.m. UTC | #5
On Thu, Nov 19, 2015 at 12:13:38PM +0900, Tetsuya Mukawa wrote:
> On 2015/11/19 11:18, Yuanhan Liu wrote:
> > On Thu, Nov 19, 2015 at 11:03:50AM +0900, Tetsuya Mukawa wrote:
> >> On 2015/11/17 22:29, Yuanhan Liu wrote:
> >>> On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
> >>>> These variables are needed to be able to manage one of virtio devices
> >>>> using both vhost library APIs and vhost PMD.
> >>>> For example, if vhost PMD uses current callback handler and private data
> >>>> provided by vhost library, A DPDK application that links vhost library
> >>>> cannot use some of vhost library APIs.
> >>> Can you be more specific about this?
> >>>
> >>> 	--yliu
> >> How about like below?
> >>
> >> commit log:
> >> 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.
> > Will (and why) the two co-exist at same time?
> 
> Yes it is. Sure, I will describe below in commit log.
> 
> Because we cannot map some of vhost library APIs to ethdev APIs, in some
> cases, we still
> need to use vhost library APIs for a port created by the vhost PMD. One
> of example is
> rte_vhost_enable_guest_notification().

I don't get it why it has something to do with a standalone PMD callback.
And if you don't call rte_vhost_enable_guest_notification() inside vhost
PMD, where else can you call that? I mean, you can't start vhost-pmd
and vhost-swithc in the mean time, right?

And, pmd callback and the old notify callback will not exist at same
time in one case, right? If so, why is that needed?

BTW, if it's a MUST, would you provide a specific example?


	--yliu
> 
> Thanks,
> Tetsuya
> 
> 
> >
> > 	--yliu
> >
> >> 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.
> >>
> >> Tetsuya
  
Tetsuya Mukawa Nov. 19, 2015, 5:14 a.m. UTC | #6
On 2015/11/19 12:33, Yuanhan Liu wrote:
> On Thu, Nov 19, 2015 at 12:13:38PM +0900, Tetsuya Mukawa wrote:
>> On 2015/11/19 11:18, Yuanhan Liu wrote:
>>> On Thu, Nov 19, 2015 at 11:03:50AM +0900, Tetsuya Mukawa wrote:
>>>> On 2015/11/17 22:29, Yuanhan Liu wrote:
>>>>> On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
>>>>>> These variables are needed to be able to manage one of virtio devices
>>>>>> using both vhost library APIs and vhost PMD.
>>>>>> For example, if vhost PMD uses current callback handler and private data
>>>>>> provided by vhost library, A DPDK application that links vhost library
>>>>>> cannot use some of vhost library APIs.
>>>>> Can you be more specific about this?
>>>>>
>>>>> 	--yliu
>>>> How about like below?
>>>>
>>>> commit log:
>>>> 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.
>>> Will (and why) the two co-exist at same time?
>> Yes it is. Sure, I will describe below in commit log.
>>
>> Because we cannot map some of vhost library APIs to ethdev APIs, in some
>> cases, we still
>> need to use vhost library APIs for a port created by the vhost PMD. One
>> of example is
>> rte_vhost_enable_guest_notification().
> I don't get it why it has something to do with a standalone PMD callback.
> And if you don't call rte_vhost_enable_guest_notification() inside vhost
> PMD, where else can you call that? I mean, you can't start vhost-pmd
> and vhost-swithc in the mean time, right?

No it's not true, even after connecting to virtio-net device, you can
change the flag.
It's just a hint for virtio-net driver, and it will be used while queuing.
(We may be able to change the flag, even while sending or receiving packets)

>
> And, pmd callback and the old notify callback will not exist at same
> time in one case, right? If so, why is that needed?
>
> BTW, if it's a MUST, would you provide a specific example?

Actually, this patch is not a MUST.

But still the users need callback handlers to know when virtio-net
device is connected or disconnected.
This is because the user can call rte_vhost_enable_guest_notification()
only while connection is established.

Probably we can use link status changed callback of the PMD for this
purpose.
(The vhost PMD will notice DPDK application using link status callback)

But I am not sure whether we need to implement link status changed
callback for this purpose.
While processing this callback handler, the users will only calls vhost
library APIs that ethdev API cannot map, or store some variables related
with vhost library.
If so, this callback handler itself is specific for using vhost library.
And it may be ok that callback itself is implemented as one of vhost
library APIs.

Tetsuya

>
> 	--yliu
>> Thanks,
>> Tetsuya
>>
>>
>>> 	--yliu
>>>
>>>> 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.
>>>>
>>>> Tetsuya
  
Yuanhan Liu Nov. 19, 2015, 5:45 a.m. UTC | #7
On Thu, Nov 19, 2015 at 02:14:13PM +0900, Tetsuya Mukawa wrote:
> On 2015/11/19 12:33, Yuanhan Liu wrote:
> > On Thu, Nov 19, 2015 at 12:13:38PM +0900, Tetsuya Mukawa wrote:
> >> On 2015/11/19 11:18, Yuanhan Liu wrote:
> >>> On Thu, Nov 19, 2015 at 11:03:50AM +0900, Tetsuya Mukawa wrote:
> >>>> On 2015/11/17 22:29, Yuanhan Liu wrote:
> >>>>> On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
> >>>>>> These variables are needed to be able to manage one of virtio devices
> >>>>>> using both vhost library APIs and vhost PMD.
> >>>>>> For example, if vhost PMD uses current callback handler and private data
> >>>>>> provided by vhost library, A DPDK application that links vhost library
> >>>>>> cannot use some of vhost library APIs.
> >>>>> Can you be more specific about this?
> >>>>>
> >>>>> 	--yliu
> >>>> How about like below?
> >>>>
> >>>> commit log:
> >>>> 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.
> >>> Will (and why) the two co-exist at same time?
> >> Yes it is. Sure, I will describe below in commit log.
> >>
> >> Because we cannot map some of vhost library APIs to ethdev APIs, in some
> >> cases, we still
> >> need to use vhost library APIs for a port created by the vhost PMD. One
> >> of example is
> >> rte_vhost_enable_guest_notification().
> > I don't get it why it has something to do with a standalone PMD callback.
> > And if you don't call rte_vhost_enable_guest_notification() inside vhost
> > PMD, where else can you call that? I mean, you can't start vhost-pmd
> > and vhost-swithc in the mean time, right?
> 
> No it's not true, even after connecting to virtio-net device, you can
> change the flag.
> It's just a hint for virtio-net driver, and it will be used while queuing.
> (We may be able to change the flag, even while sending or receiving packets)
> 
> >
> > And, pmd callback and the old notify callback will not exist at same
> > time in one case, right? If so, why is that needed?
> >
> > BTW, if it's a MUST, would you provide a specific example?
> 
> Actually, this patch is not a MUST.
> 
> But still the users need callback handlers to know when virtio-net
> device is connected or disconnected.
> This is because the user can call rte_vhost_enable_guest_notification()
> only while connection is established.

What does "the user" mean? Is there a second user of vhost lib besides
vhost PMD, that he has to interact with those connected devices? If so,
how?

	--yliu
> 
> Probably we can use link status changed callback of the PMD for this
> purpose.
> (The vhost PMD will notice DPDK application using link status callback)
> 
> But I am not sure whether we need to implement link status changed
> callback for this purpose.
> While processing this callback handler, the users will only calls vhost
> library APIs that ethdev API cannot map, or store some variables related
> with vhost library.
> If so, this callback handler itself is specific for using vhost library.
> And it may be ok that callback itself is implemented as one of vhost
> library APIs.
> 
> Tetsuya
> 
> >
> > 	--yliu
> >> Thanks,
> >> Tetsuya
> >>
> >>
> >>> 	--yliu
> >>>
> >>>> 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.
> >>>>
> >>>> Tetsuya
  
Tetsuya Mukawa Nov. 19, 2015, 5:58 a.m. UTC | #8
On 2015/11/19 14:45, Yuanhan Liu wrote:
> On Thu, Nov 19, 2015 at 02:14:13PM +0900, Tetsuya Mukawa wrote:
>> On 2015/11/19 12:33, Yuanhan Liu wrote:
>>> On Thu, Nov 19, 2015 at 12:13:38PM +0900, Tetsuya Mukawa wrote:
>>>> On 2015/11/19 11:18, Yuanhan Liu wrote:
>>>>> On Thu, Nov 19, 2015 at 11:03:50AM +0900, Tetsuya Mukawa wrote:
>>>>>> On 2015/11/17 22:29, Yuanhan Liu wrote:
>>>>>>> On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
>>>>>>>> These variables are needed to be able to manage one of virtio devices
>>>>>>>> using both vhost library APIs and vhost PMD.
>>>>>>>> For example, if vhost PMD uses current callback handler and private data
>>>>>>>> provided by vhost library, A DPDK application that links vhost library
>>>>>>>> cannot use some of vhost library APIs.
>>>>>>> Can you be more specific about this?
>>>>>>>
>>>>>>> 	--yliu
>>>>>> How about like below?
>>>>>>
>>>>>> commit log:
>>>>>> 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.
>>>>> Will (and why) the two co-exist at same time?
>>>> Yes it is. Sure, I will describe below in commit log.
>>>>
>>>> Because we cannot map some of vhost library APIs to ethdev APIs, in some
>>>> cases, we still
>>>> need to use vhost library APIs for a port created by the vhost PMD. One
>>>> of example is
>>>> rte_vhost_enable_guest_notification().
>>> I don't get it why it has something to do with a standalone PMD callback.
>>> And if you don't call rte_vhost_enable_guest_notification() inside vhost
>>> PMD, where else can you call that? I mean, you can't start vhost-pmd
>>> and vhost-swithc in the mean time, right?
>> No it's not true, even after connecting to virtio-net device, you can
>> change the flag.
>> It's just a hint for virtio-net driver, and it will be used while queuing.
>> (We may be able to change the flag, even while sending or receiving packets)
>>
>>> And, pmd callback and the old notify callback will not exist at same
>>> time in one case, right? If so, why is that needed?
>>>
>>> BTW, if it's a MUST, would you provide a specific example?
>> Actually, this patch is not a MUST.
>>
>> But still the users need callback handlers to know when virtio-net
>> device is connected or disconnected.
>> This is because the user can call rte_vhost_enable_guest_notification()
>> only while connection is established.
> What does "the user" mean? Is there a second user of vhost lib besides
> vhost PMD, that he has to interact with those connected devices? If so,
> how?

Sorry, my English is wrong.
Not a second user.

For example, If DPDK application has a port created by vhost PMD, then
needs to call rte_vhost_enable_guest_notification() to the port.
DPDK application needs to know when virtio-net device is connected or
disconnected, because the function is only valid while connecting.
But without callback handler, DPDK application cannot know it.

This is what I wanted to explain.

Thanks,
Tetsuya

> 	--yliu
>> Probably we can use link status changed callback of the PMD for this
>> purpose.
>> (The vhost PMD will notice DPDK application using link status callback)
>>
>> But I am not sure whether we need to implement link status changed
>> callback for this purpose.
>> While processing this callback handler, the users will only calls vhost
>> library APIs that ethdev API cannot map, or store some variables related
>> with vhost library.
>> If so, this callback handler itself is specific for using vhost library.
>> And it may be ok that callback itself is implemented as one of vhost
>> library APIs.
>>
>> Tetsuya
>>
>>> 	--yliu
>>>> Thanks,
>>>> Tetsuya
>>>>
>>>>
>>>>> 	--yliu
>>>>>
>>>>>> 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.
>>>>>>
>>>>>> Tetsuya
  
Yuanhan Liu Nov. 19, 2015, 6:31 a.m. UTC | #9
On Thu, Nov 19, 2015 at 02:58:56PM +0900, Tetsuya Mukawa wrote:
> On 2015/11/19 14:45, Yuanhan Liu wrote:
> > On Thu, Nov 19, 2015 at 02:14:13PM +0900, Tetsuya Mukawa wrote:
> >> On 2015/11/19 12:33, Yuanhan Liu wrote:
> >>> On Thu, Nov 19, 2015 at 12:13:38PM +0900, Tetsuya Mukawa wrote:
> >>>> On 2015/11/19 11:18, Yuanhan Liu wrote:
> >>>>> On Thu, Nov 19, 2015 at 11:03:50AM +0900, Tetsuya Mukawa wrote:
> >>>>>> On 2015/11/17 22:29, Yuanhan Liu wrote:
> >>>>>>> On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
> >>>>>>>> These variables are needed to be able to manage one of virtio devices
> >>>>>>>> using both vhost library APIs and vhost PMD.
> >>>>>>>> For example, if vhost PMD uses current callback handler and private data
> >>>>>>>> provided by vhost library, A DPDK application that links vhost library
> >>>>>>>> cannot use some of vhost library APIs.
> >>>>>>> Can you be more specific about this?
> >>>>>>>
> >>>>>>> 	--yliu
> >>>>>> How about like below?
> >>>>>>
> >>>>>> commit log:
> >>>>>> 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.
> >>>>> Will (and why) the two co-exist at same time?
> >>>> Yes it is. Sure, I will describe below in commit log.
> >>>>
> >>>> Because we cannot map some of vhost library APIs to ethdev APIs, in some
> >>>> cases, we still
> >>>> need to use vhost library APIs for a port created by the vhost PMD. One
> >>>> of example is
> >>>> rte_vhost_enable_guest_notification().
> >>> I don't get it why it has something to do with a standalone PMD callback.
> >>> And if you don't call rte_vhost_enable_guest_notification() inside vhost
> >>> PMD, where else can you call that? I mean, you can't start vhost-pmd
> >>> and vhost-swithc in the mean time, right?
> >> No it's not true, even after connecting to virtio-net device, you can
> >> change the flag.
> >> It's just a hint for virtio-net driver, and it will be used while queuing.
> >> (We may be able to change the flag, even while sending or receiving packets)
> >>
> >>> And, pmd callback and the old notify callback will not exist at same
> >>> time in one case, right? If so, why is that needed?
> >>>
> >>> BTW, if it's a MUST, would you provide a specific example?
> >> Actually, this patch is not a MUST.
> >>
> >> But still the users need callback handlers to know when virtio-net
> >> device is connected or disconnected.
> >> This is because the user can call rte_vhost_enable_guest_notification()
> >> only while connection is established.
> > What does "the user" mean? Is there a second user of vhost lib besides
> > vhost PMD, that he has to interact with those connected devices? If so,
> > how?
> 
> Sorry, my English is wrong.
> Not a second user.
> 
> For example, If DPDK application has a port created by vhost PMD, then
> needs to call rte_vhost_enable_guest_notification() to the port.

So, you are mixing the usage of vhost PMD and vhost lib in a DPDK
application? Say,

        DPDK application
               start_vhost_pmd
                      rte_vhost_driver_pmd_callback_register
               rte_vhost_driver_callback_register

I know little about PMD, and not quite sure it's a good combo.

Huawei, comments?

	--yliu

> DPDK application needs to know when virtio-net device is connected or
> disconnected, because the function is only valid while connecting.
> But without callback handler, DPDK application cannot know it.
> 
> This is what I wanted to explain.
> 
> Thanks,
> Tetsuya
> 
> > 	--yliu
> >> Probably we can use link status changed callback of the PMD for this
> >> purpose.
> >> (The vhost PMD will notice DPDK application using link status callback)
> >>
> >> But I am not sure whether we need to implement link status changed
> >> callback for this purpose.
> >> While processing this callback handler, the users will only calls vhost
> >> library APIs that ethdev API cannot map, or store some variables related
> >> with vhost library.
> >> If so, this callback handler itself is specific for using vhost library.
> >> And it may be ok that callback itself is implemented as one of vhost
> >> library APIs.
> >>
> >> Tetsuya
> >>
> >>> 	--yliu
> >>>> Thanks,
> >>>> Tetsuya
> >>>>
> >>>>
> >>>>> 	--yliu
> >>>>>
> >>>>>> 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.
> >>>>>>
> >>>>>> Tetsuya
  
Tetsuya Mukawa Nov. 19, 2015, 6:37 a.m. UTC | #10
On 2015/11/19 15:31, Yuanhan Liu wrote:
> On Thu, Nov 19, 2015 at 02:58:56PM +0900, Tetsuya Mukawa wrote:
>> On 2015/11/19 14:45, Yuanhan Liu wrote:
>>> On Thu, Nov 19, 2015 at 02:14:13PM +0900, Tetsuya Mukawa wrote:
>>>> On 2015/11/19 12:33, Yuanhan Liu wrote:
>>>>> On Thu, Nov 19, 2015 at 12:13:38PM +0900, Tetsuya Mukawa wrote:
>>>>>> On 2015/11/19 11:18, Yuanhan Liu wrote:
>>>>>>> On Thu, Nov 19, 2015 at 11:03:50AM +0900, Tetsuya Mukawa wrote:
>>>>>>>> On 2015/11/17 22:29, Yuanhan Liu wrote:
>>>>>>>>> On Fri, Nov 13, 2015 at 02:20:30PM +0900, Tetsuya Mukawa wrote:
>>>>>>>>>> These variables are needed to be able to manage one of virtio devices
>>>>>>>>>> using both vhost library APIs and vhost PMD.
>>>>>>>>>> For example, if vhost PMD uses current callback handler and private data
>>>>>>>>>> provided by vhost library, A DPDK application that links vhost library
>>>>>>>>>> cannot use some of vhost library APIs.
>>>>>>>>> Can you be more specific about this?
>>>>>>>>>
>>>>>>>>> 	--yliu
>>>>>>>> How about like below?
>>>>>>>>
>>>>>>>> commit log:
>>>>>>>> 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.
>>>>>>> Will (and why) the two co-exist at same time?
>>>>>> Yes it is. Sure, I will describe below in commit log.
>>>>>>
>>>>>> Because we cannot map some of vhost library APIs to ethdev APIs, in some
>>>>>> cases, we still
>>>>>> need to use vhost library APIs for a port created by the vhost PMD. One
>>>>>> of example is
>>>>>> rte_vhost_enable_guest_notification().
>>>>> I don't get it why it has something to do with a standalone PMD callback.
>>>>> And if you don't call rte_vhost_enable_guest_notification() inside vhost
>>>>> PMD, where else can you call that? I mean, you can't start vhost-pmd
>>>>> and vhost-swithc in the mean time, right?
>>>> No it's not true, even after connecting to virtio-net device, you can
>>>> change the flag.
>>>> It's just a hint for virtio-net driver, and it will be used while queuing.
>>>> (We may be able to change the flag, even while sending or receiving packets)
>>>>
>>>>> And, pmd callback and the old notify callback will not exist at same
>>>>> time in one case, right? If so, why is that needed?
>>>>>
>>>>> BTW, if it's a MUST, would you provide a specific example?
>>>> Actually, this patch is not a MUST.
>>>>
>>>> But still the users need callback handlers to know when virtio-net
>>>> device is connected or disconnected.
>>>> This is because the user can call rte_vhost_enable_guest_notification()
>>>> only while connection is established.
>>> What does "the user" mean? Is there a second user of vhost lib besides
>>> vhost PMD, that he has to interact with those connected devices? If so,
>>> how?
>> Sorry, my English is wrong.
>> Not a second user.
>>
>> For example, If DPDK application has a port created by vhost PMD, then
>> needs to call rte_vhost_enable_guest_notification() to the port.
> So, you are mixing the usage of vhost PMD and vhost lib in a DPDK
> application? Say,

Yes, that is my intention.
Using ethdev(PMD) APIs and some library specific APIs to a same port is
used in bonding PMD also.

Thanks,
Tetsuya

>         DPDK application
>                start_vhost_pmd
>                       rte_vhost_driver_pmd_callback_register
>                rte_vhost_driver_callback_register
>
> I know little about PMD, and not quite sure it's a good combo.
>
> Huawei, comments?
>
> 	--yliu
>
>> DPDK application needs to know when virtio-net device is connected or
>> disconnected, because the function is only valid while connecting.
>> But without callback handler, DPDK application cannot know it.
>>
>> This is what I wanted to explain.
>>
>> Thanks,
>> Tetsuya
>>
>>> 	--yliu
>>>> Probably we can use link status changed callback of the PMD for this
>>>> purpose.
>>>> (The vhost PMD will notice DPDK application using link status callback)
>>>>
>>>> But I am not sure whether we need to implement link status changed
>>>> callback for this purpose.
>>>> While processing this callback handler, the users will only calls vhost
>>>> library APIs that ethdev API cannot map, or store some variables related
>>>> with vhost library.
>>>> If so, this callback handler itself is specific for using vhost library.
>>>> And it may be ok that callback itself is implemented as one of vhost
>>>> library APIs.
>>>>
>>>> Tetsuya
>>>>
>>>>> 	--yliu
>>>>>> Thanks,
>>>>>> Tetsuya
>>>>>>
>>>>>>
>>>>>>> 	--yliu
>>>>>>>
>>>>>>>> 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.
>>>>>>>>
>>>>>>>> Tetsuya
  

Patch

diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 3d8709e..00a9ce5 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -20,3 +20,9 @@  DPDK_2.1 {
 	rte_vhost_driver_unregister;
 
 } DPDK_2.0;
+
+DPDK_2.2 {
+	global:
+
+	rte_vhost_driver_pmd_callback_register;
+} DPDK_2.1;
diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 5687452..3ef6e58 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -128,6 +128,7 @@  struct virtio_net {
 	char			ifname[IF_NAME_SZ];	/**< Name of the tap device or socket path. */
 	uint32_t		virt_qp_nb;	/**< number of queue pair we have allocated */
 	void			*priv;		/**< private context */
+	void			*pmd_priv;	/**< private context for vhost PMD */
 	struct vhost_virtqueue	*virtqueue[VHOST_MAX_QUEUE_PAIRS * 2];	/**< Contains all virtqueue information. */
 } __rte_cache_aligned;
 
@@ -224,6 +225,8 @@  int rte_vhost_driver_unregister(const char *dev_name);
 
 /* Register callbacks. */
 int rte_vhost_driver_callback_register(struct virtio_net_device_ops const * const);
+/* Register callbacks for vhost PMD (Only for internal). */
+int rte_vhost_driver_pmd_callback_register(struct virtio_net_device_ops const * const);
 /* Start vhost driver session blocking loop. */
 int rte_vhost_driver_session_start(void);
 
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c b/lib/librte_vhost/vhost_user/virtio-net-user.c
index d07452a..d8ae2fc 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -111,7 +111,7 @@  user_set_mem_table(struct vhost_device_ctx ctx, struct VhostUserMsg *pmsg)
 
 	/* Remove from the data plane. */
 	if (dev->flags & VIRTIO_DEV_RUNNING)
-		notify_ops->destroy_device(dev);
+		notify_destroy_device(dev);
 
 	if (dev->mem) {
 		free_mem_region(dev);
@@ -272,7 +272,7 @@  user_set_vring_kick(struct vhost_device_ctx ctx, struct VhostUserMsg *pmsg)
 
 	if (virtio_is_ready(dev) &&
 		!(dev->flags & VIRTIO_DEV_RUNNING))
-			notify_ops->new_device(dev);
+			notify_new_device(dev);
 }
 
 /*
@@ -288,7 +288,7 @@  user_get_vring_base(struct vhost_device_ctx ctx,
 		return -1;
 	/* We have to stop the queue (virtio) if it is running. */
 	if (dev->flags & VIRTIO_DEV_RUNNING)
-		notify_ops->destroy_device(dev);
+		notify_destroy_device(dev);
 
 	/* Here we are safe to get the last used index */
 	ops->get_vring_base(ctx, state->index, state);
@@ -324,10 +324,7 @@  user_set_vring_enable(struct vhost_device_ctx ctx,
 		"set queue enable: %d to qp idx: %d\n",
 		enable, state->index);
 
-	if (notify_ops->vring_state_changed) {
-		notify_ops->vring_state_changed(dev, base_idx / VIRTIO_QNUM,
-						enable);
-	}
+	notify_vring_state_changed(dev, base_idx / VIRTIO_QNUM, enable);
 
 	dev->virtqueue[base_idx + VIRTIO_RXQ]->enabled = enable;
 	dev->virtqueue[base_idx + VIRTIO_TXQ]->enabled = enable;
@@ -341,7 +338,7 @@  user_destroy_device(struct vhost_device_ctx ctx)
 	struct virtio_net *dev = get_device(ctx);
 
 	if (dev && (dev->flags & VIRTIO_DEV_RUNNING))
-		notify_ops->destroy_device(dev);
+		notify_destroy_device(dev);
 
 	if (dev && dev->mem) {
 		free_mem_region(dev);
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index cc917da..886c104 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -65,6 +65,8 @@  struct virtio_net_config_ll {
 
 /* device ops to add/remove device to/from data core. */
 struct virtio_net_device_ops const *notify_ops;
+/* device ops for vhost PMD to add/remove device to/from data core. */
+struct virtio_net_device_ops const *pmd_notify_ops;
 /* root address of the linked list of managed virtio devices */
 static struct virtio_net_config_ll *ll_root;
 
@@ -81,6 +83,45 @@  static struct virtio_net_config_ll *ll_root;
 static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
 
 
+int
+notify_new_device(struct virtio_net *dev)
+{
+	if ((pmd_notify_ops != NULL) && (pmd_notify_ops->new_device != NULL)) {
+		int ret = pmd_notify_ops->new_device(dev);
+
+		if (ret != 0)
+			return ret;
+	}
+	if ((notify_ops != NULL) && (notify_ops->new_device != NULL))
+		return notify_ops->new_device(dev);
+
+	return 0;
+}
+
+void
+notify_destroy_device(volatile struct virtio_net *dev)
+{
+	if ((pmd_notify_ops != NULL) && (pmd_notify_ops->destroy_device != NULL))
+		pmd_notify_ops->destroy_device(dev);
+	if ((notify_ops != NULL) && (notify_ops->destroy_device != NULL))
+		notify_ops->destroy_device(dev);
+}
+
+int
+notify_vring_state_changed(struct virtio_net *dev, uint16_t queue_id, int enable)
+{
+	if ((pmd_notify_ops != NULL) && (pmd_notify_ops->vring_state_changed != NULL)) {
+		int ret = pmd_notify_ops->vring_state_changed(dev, queue_id, enable);
+
+		if (ret != 0)
+			return ret;
+	}
+	if ((notify_ops != NULL) && (notify_ops->vring_state_changed != NULL))
+		return notify_ops->vring_state_changed(dev, queue_id, enable);
+
+	return 0;
+}
+
 /*
  * Converts QEMU virtual address to Vhost virtual address. This function is
  * used to convert the ring addresses to our address space.
@@ -374,7 +415,7 @@  destroy_device(struct vhost_device_ctx ctx)
 			 * the function to remove it from the data core.
 			 */
 			if ((ll_dev_cur->dev.flags & VIRTIO_DEV_RUNNING))
-				notify_ops->destroy_device(&(ll_dev_cur->dev));
+				notify_destroy_device(&(ll_dev_cur->dev));
 			ll_dev_cur = rm_config_ll_entry(ll_dev_cur,
 					ll_dev_last);
 		} else {
@@ -432,7 +473,7 @@  reset_owner(struct vhost_device_ctx ctx)
 		return -1;
 
 	if (dev->flags & VIRTIO_DEV_RUNNING)
-		notify_ops->destroy_device(dev);
+		notify_destroy_device(dev);
 
 	cleanup_device(dev);
 	reset_device(dev);
@@ -790,12 +831,12 @@  set_backend(struct vhost_device_ctx ctx, struct vhost_vring_file *file)
 	if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
 		if (((int)dev->virtqueue[VIRTIO_TXQ]->backend != VIRTIO_DEV_STOPPED) &&
 			((int)dev->virtqueue[VIRTIO_RXQ]->backend != VIRTIO_DEV_STOPPED)) {
-			return notify_ops->new_device(dev);
+			return notify_new_device(dev);
 		}
 	/* Otherwise we remove it. */
 	} else
 		if (file->fd == VIRTIO_DEV_STOPPED)
-			notify_ops->destroy_device(dev);
+			notify_destroy_device(dev);
 	return 0;
 }
 
@@ -879,3 +920,14 @@  rte_vhost_driver_callback_register(struct virtio_net_device_ops const * const op
 
 	return 0;
 }
+
+/*
+ * Register ops so that we can add/remove device to data core.
+ */
+int
+rte_vhost_driver_pmd_callback_register(struct virtio_net_device_ops const * const ops)
+{
+	pmd_notify_ops = ops;
+
+	return 0;
+}
diff --git a/lib/librte_vhost/virtio-net.h b/lib/librte_vhost/virtio-net.h
index 75fb57e..0816e71 100644
--- a/lib/librte_vhost/virtio-net.h
+++ b/lib/librte_vhost/virtio-net.h
@@ -37,7 +37,9 @@ 
 #include "vhost-net.h"
 #include "rte_virtio_net.h"
 
-struct virtio_net_device_ops const *notify_ops;
 struct virtio_net *get_device(struct vhost_device_ctx ctx);
 
+int notify_new_device(struct virtio_net *dev);
+void notify_destroy_device(volatile struct virtio_net *dev);
+int notify_vring_state_changed(struct virtio_net *dev, uint16_t queue_id, int enable);
 #endif