vchost: Notify application of ownership change

PATCH In review Jan Kiszka 6 participants 13 messages Aug 7, 2015 – Mar 17, 2016

Share & tools

Jan Kiszka

author

2015-08-07 17:20 UTC #0 lore raw

PATCH 1/1 vchost: Notify application of ownership change

On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.

Signed-off-by Jan Kiszka <[email protected]>

---

This is the surprisingly simple answer to my questions in
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.

 lib/librte_vhost/virtio-net.c | 3 +++
 1 file changed, 3 insertions(+)

lib/librte_vhost/virtio-net.c +3 -0

index b520ec5..3c5b5b2 100644
@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
+
 	cleanup_device(&ll_dev->dev);
 	init_device(&ll_dev->dev);

Signature · 1 line

2.1.4

Ouyang, Changchun

2015-08-08 00:25 UTC #1 lore raw

Quoted 7 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jan Kiszka
Sent: Saturday, August 8, 2015 1:21 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH] vchost: Notify application of ownership change

Vchost should be vhost in the title

Quoted 5 lines

On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.

What's your qemu version?
Any validation work on this patch?

Quoted 15 lines

Signed-off-by Jan Kiszka <[email protected]>
---
This is the surprisingly simple answer to my questions in
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
 lib/librte_vhost/virtio-net.c | 3 +++
 1 file changed, 3 insertions(+)
lib/librte_vhost/virtio-net.c index renamed from lib/librte_vhost/virtio-net.c +0 -0
b520ec5..3c5b5b2 100644

lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 407

Quoted 6 lines

lib/librte_vhost/virtio-net.c +3 -0

@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
+

I am not sure whether destroy_device here will affect the second time init_device(below) and new_device(after the reset) or not. Need validation.

Quoted 5 lines

lib/librte_vhost/virtio-net.c +0 -1
 	cleanup_device(&ll_dev->dev);
 	init_device(&ll_dev->dev);
 
--
2.1.4

Jan Kiszka

author

2015-08-08 06:43 UTC #2 lore raw

Quoted 9 lines · Changchun

Quoted 7 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jan Kiszka
Sent: Saturday, August 8, 2015 1:21 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH] vchost: Notify application of ownership change
Vchost should be vhost in the title

Oops. Unless I need to resend for some other reason, I guess the commit can fix this up.

Quoted 7 lines

Quoted 6 lines
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
What's your qemu version?

git head, see my other reply for details.

Quoted 1 line

Any validation work on this patch?

What do you mean with this? Test cases? Or steps to reproduce? For the latter, just fire up a recent qemu, let the guest enable the virtio device, then reboot or simply terminate qemu.

Quoted 16 lines

Quoted 16 lines
Signed-off-by Jan Kiszka <[email protected]>
---
This is the surprisingly simple answer to my questions in
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
 lib/librte_vhost/virtio-net.c | 3 +++
 1 file changed, 3 insertions(+)
lib/librte_vhost/virtio-net.c index renamed from lib/librte_vhost/virtio-net.c +0 -0
b520ec5..3c5b5b2 100644
lib/librte_vhost/virtio-net.c +0 -0
@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 

Quoted 8 lines

lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 407
Quoted 5 lines
lib/librte_vhost/virtio-net.c +3 -0
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
+
I am not sure whether destroy_device here will affect the second time init_device(below) and new_device(after the reset) or not. Need validation.

Cannot follow, what do you mean with "second time"? If the callback could invoke something that causes cleanup_device to be called as well? That's at least not the case with vhost-switch, but I'm far from being familiar with the API to asses if that is possible in general.

Jan

Quoted 6 lines

Quoted 6 lines
lib/librte_vhost/virtio-net.c +0 -1
 	cleanup_device(&ll_dev->dev);
 	init_device(&ll_dev->dev);
 
--
2.1.4

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Ouyang, Changchun

2015-08-10 01:21 UTC #3 lore raw

Quoted 32 lines

-----Original Message-----
From: Jan Kiszka [mailto:[email protected]]
Sent: Saturday, August 8, 2015 2:43 PM
To: Ouyang, Changchun; [email protected]
Subject: Re: [dpdk-dev] [PATCH] vchost: Notify application of ownership change
Quoted 10 lines · Changchun
Quoted 8 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jan Kiszka
Sent: Saturday, August 8, 2015 1:21 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH] vchost: Notify application of ownership change
Vchost should be vhost in the title
Oops. Unless I need to resend for some other reason, I guess the commit can fix this up.
Quoted 7 lines
Quoted 6 lines
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
What's your qemu version?

Quoted 7 lines

git head, see my other reply for details.
Quoted 1 line
Any validation work on this patch?
What do you mean with this? Test cases? Or steps to reproduce? For the latter, just fire up a recent qemu, let the guest enable the virtio device, then reboot or simply terminate qemu.

Here, I mean test case,
Need make sure the change works on both qemu 2.4(with the reset commit in qemu) and qemu2.2/2.3(without the commit in qemu).

Quoted 25 lines

Quoted 18 lines
Quoted 18 lines
Signed-off-by Jan Kiszka <[email protected]>
---
This is the surprisingly simple answer to my questions in
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
 lib/librte_vhost/virtio-net.c | 3 +++
 1 file changed, 3 insertions(+)
(quoted diff) +0 -0
b/lib/librte_vhost/virtio-net.c index
b520ec5..3c5b5b2 100644
lib/librte_vhost/virtio-net.c +0 -0
@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
Quoted 7 lines
lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 407
Quoted 5 lines
lib/librte_vhost/virtio-net.c +3 -0
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
+
I am not sure whether destroy_device here will affect the second time

Quoted 7 lines

init_device(below) and new_device(after the reset) or not.
Quoted 1 line
Need validation.
Cannot follow, what do you mean with "second time"? If the callback could invoke something that causes cleanup_device to be called as well? That's at least not the case with vhost-switch, but I'm far from being familiar with the API to asses if that is possible in general.

RESET is often followed by a second time virtio device creation.
If you have chance to run testpmd with virtio PMD on guest, that would be that case:
Call RESET, and then create virtio device again to make it work for packets rx/tx

Quoted 14 lines

Jan
Quoted 6 lines
Quoted 6 lines
lib/librte_vhost/virtio-net.c +0 -1
 	cleanup_device(&ll_dev->dev);
 	init_device(&ll_dev->dev);
 
--
2.1.4
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux

Jan Kiszka

author

2015-08-10 08:07 UTC #4 lore raw

Quoted 32 lines · Changchun

Quoted 32 lines
-----Original Message-----
From: Jan Kiszka [mailto:[email protected]]
Sent: Saturday, August 8, 2015 2:43 PM
To: Ouyang, Changchun; [email protected]
Subject: Re: [dpdk-dev] [PATCH] vchost: Notify application of ownership change
Quoted 10 lines · Changchun
Quoted 8 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jan Kiszka
Sent: Saturday, August 8, 2015 1:21 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH] vchost: Notify application of ownership change
Vchost should be vhost in the title
Oops. Unless I need to resend for some other reason, I guess the commit can fix this up.
Quoted 7 lines
Quoted 6 lines
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
What's your qemu version?
git head, see my other reply for details.

Quoted 9 lines

Quoted 5 lines
Quoted 1 line
Any validation work on this patch?
What do you mean with this? Test cases? Or steps to reproduce? For the latter, just fire up a recent qemu, let the guest enable the virtio device, then reboot or simply terminate qemu.
Here, I mean test case,
Need make sure the change works on both qemu 2.4(with the reset commit in qemu) and qemu2.2/2.3(without the commit in qemu).

I suspect, 2.2 and 2.3 stable have this fix queued already. If not, we should trigger this. The previous versions were subtly broken and shouldn't be used for production purposes.

Quoted 28 lines

Quoted 28 lines
Quoted 18 lines
Quoted 18 lines
Signed-off-by Jan Kiszka <[email protected]>
---
This is the surprisingly simple answer to my questions in
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
 lib/librte_vhost/virtio-net.c | 3 +++
 1 file changed, 3 insertions(+)
(quoted diff) +0 -0
b/lib/librte_vhost/virtio-net.c index
b520ec5..3c5b5b2 100644
lib/librte_vhost/virtio-net.c +0 -0
@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
Quoted 7 lines
lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 407
Quoted 5 lines
lib/librte_vhost/virtio-net.c +3 -0
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
+
I am not sure whether destroy_device here will affect the second time
init_device(below) and new_device(after the reset) or not.
Quoted 1 line
Need validation.

Quoted 8 lines

Quoted 4 lines
Cannot follow, what do you mean with "second time"? If the callback could invoke something that causes cleanup_device to be called as well? That's at least not the case with vhost-switch, but I'm far from being familiar with the API to asses if that is possible in general.
RESET is often followed by a second time virtio device creation.
If you have chance to run testpmd with virtio PMD on guest, that would be that case:
Call RESET, and then create virtio device again to make it work for packets rx/tx

OK, need to dig into this, probably not the next days, though.

Thanks,
Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Xie, Huawei

2015-08-12 03:34 UTC #5 lore raw

In reply to Jan Kiszka

Quoted 19 lines · Jan Kiszka

On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
Signed-off-by Jan Kiszka <[email protected]>
---
This is the surprisingly simple answer to my questions in
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
 lib/librte_vhost/virtio-net.c | 3 +++
 1 file changed, 3 insertions(+)
lib/librte_vhost/virtio-net.c +0 -0
index b520ec5..3c5b5b2 100644

lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 406

Quoted 5 lines

lib/librte_vhost/virtio-net.c +2 -0

@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);

To me this patch makes sense here.
Whether RESET_OWNER is really needed is another question. Whenever the vhost itself needs to process the vhost device, we need to notify the switch application to remove it from data plane.

Quoted 4 lines

lib/librte_vhost/virtio-net.c +1 -0

+
 	cleanup_device(&ll_dev->dev);
 	init_device(&ll_dev->dev);

Jan Kiszka

author

2015-08-12 05:35 UTC #6 lore raw

Quoted 21 lines · Huawei

Quoted 20 lines · Jan Kiszka
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
Signed-off-by Jan Kiszka <[email protected]>
---
This is the surprisingly simple answer to my questions in
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
 lib/librte_vhost/virtio-net.c | 3 +++
 1 file changed, 3 insertions(+)
lib/librte_vhost/virtio-net.c +0 -0
index b520ec5..3c5b5b2 100644
@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 

Quoted 7 lines

lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 406
Quoted 4 lines
lib/librte_vhost/virtio-net.c +2 -0
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
To me this patch makes sense here.
Whether RESET_OWNER is really needed is another question. Whenever the

Is there a better way for the client for telling vhost to stop delivering, ie. touching the client's memory?

Jan

Quoted 7 lines

vhost itself needs to process the vhost device, we need to notify the switch application to remove it from data plane.
Quoted 4 lines
lib/librte_vhost/virtio-net.c +1 -0
+
 	cleanup_device(&ll_dev->dev);
 	init_device(&ll_dev->dev);
 

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Thomas Monjalon

2015-10-24 17:18 UTC #7 lore raw

In reply to Xie, Huawei

2015-08-12 03:34, Xie, Huawei:

Quoted 5 lines · Jan Kiszka

Quoted 4 lines
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.

[...]

Quoted 6 lines

lib/librte_vhost/virtio-net.c +0 -0

@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
 	ll_dev = get_config_ll_entry(ctx);

Quoted 7 lines

lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 406
Quoted 2 lines
lib/librte_vhost/virtio-net.c +2 -0
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
To me this patch makes sense here.
Whether RESET_OWNER is really needed is another question. Whenever the vhost itself needs to process the vhost device, we need to notify the switch application to remove it from data plane.

Huawei,
some patches have been accepted for RESET_OWNER management. Is this patch obsolete?

Tetsuya Mukawa

2015-10-26 05:54 UTC #8 lore raw

Quoted 19 lines · Thomas Monjalon

2015-08-12 03:34, Xie, Huawei:
Quoted 5 lines · Jan Kiszka
Quoted 4 lines
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
[...]
Quoted 4 lines
Quoted 4 lines
lib/librte_vhost/virtio-net.c +0 -0
@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
Quoted 8 lines
lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 406
Quoted 4 lines
lib/librte_vhost/virtio-net.c +2 -0
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
To me this patch makes sense here.
Whether RESET_OWNER is really needed is another question. Whenever the vhost itself needs to process the vhost device, we need to notify the switch application to remove it from data plane.

Quoted 3 lines

Huawei,
some patches have been accepted for RESET_OWNER management. Is this patch obsolete?

Hi Yuanhan and Huawei,

I also have the same question. Do we have a patch for this issue?

Today, I've download Yuanhan's multiple queues patches and applied it on latest dpdk tree.
Then, tried to apply my vhost PMD patch on it.

When I check the patch, it seems I've faced this issue. Here are steps to reproduce.

1. Start vhost-user backend application.
(In my case, testpmd using vhost PMD is the application)
2. Start a VM with vhost-user.
You can see below message from the backend application.
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
VHOST_CONFIG: set queue enable: 1 to qp idx: 0
(snip)
VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
3. After booting Linux on guest, bind the virtio-net device to igb_uio. Then below messages are shown.
VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
VHOST_CONFIG: read message VHOST_USER_GET_VRING_BASE

The point is we will have VHOST_USER_RESET_OWNER before VHOST_USER_GET_VRING_BASE.
Currently, in RESET_OWNER function, all virtio-net data is initialized. As a result, we also initialize virtio-net flags.
When we get GET_VRING_BASE, we cannot call destroy callback handler because RUNNING flag has been initialized already.

I guess when we get RESET_OWNER message, I don't need to do anything.
And all finalizations should be done in GET_VRING_BASE.
(Or some finalizations might be done when next SET_MEM_TABLE is called.)

Thanks,
Tetsuya

Yuanhan Liu

2015-10-26 06:29 UTC #9 lore raw

Quoted 20 lines · Tetsuya Mukawa

Quoted 19 lines · Thomas Monjalon
2015-08-12 03:34, Xie, Huawei:
Quoted 5 lines · Jan Kiszka
Quoted 4 lines
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
[...]
Quoted 12 lines
lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 406
Quoted 8 lines
lib/librte_vhost/virtio-net.c +2 -0
@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
To me this patch makes sense here.
Whether RESET_OWNER is really needed is another question. Whenever the vhost itself needs to process the vhost device, we need to notify the switch application to remove it from data plane.

Quoted 3 lines

Quoted 3 lines
Huawei,
some patches have been accepted for RESET_OWNER management. Is this patch obsolete?

I think it's still appliable, at least so far.

Quoted 11 lines

Hi Yuanhan and Huawei,
I also have the same question. Do we have a patch for this issue?
Today, I've download Yuanhan's multiple queues patches and applied it on latest dpdk tree.
Then, tried to apply my vhost PMD patch on it.
When I check the patch, it seems I've faced this issue. Here are steps to reproduce.

Above patch should fix your issue, right? If so, we need it.

Quoted 15 lines

1. Start vhost-user backend application.
(In my case, testpmd using vhost PMD is the application)
2. Start a VM with vhost-user.
You can see below message from the backend application.
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
VHOST_CONFIG: set queue enable: 1 to qp idx: 0
(snip)
VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
3. After booting Linux on guest, bind the virtio-net device to igb_uio. Then below messages are shown.
VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
VHOST_CONFIG: read message VHOST_USER_GET_VRING_BASE

Quoted 2 lines

The point is we will have VHOST_USER_RESET_OWNER before VHOST_USER_GET_VRING_BASE.

Note that there is an ongoing work at QEMU community (from me) to handle RESET_OWNER correctly: it will be moved to somewhere else instead of before VHOST_USER_GET_VRING_BASE.

	--yliu

Quoted 11 lines

Currently, in RESET_OWNER function, all virtio-net data is initialized. As a result, we also initialize virtio-net flags.
When we get GET_VRING_BASE, we cannot call destroy callback handler because RUNNING flag has been initialized already.
I guess when we get RESET_OWNER message, I don't need to do anything.
And all finalizations should be done in GET_VRING_BASE.
(Or some finalizations might be done when next SET_MEM_TABLE is called.)
Thanks,
Tetsuya

Tetsuya Mukawa

2015-10-26 08:33 UTC #10 lore raw

Quoted 30 lines · Yuanhan Liu

Quoted 20 lines · Tetsuya Mukawa
Quoted 19 lines · Thomas Monjalon
2015-08-12 03:34, Xie, Huawei:
Quoted 5 lines · Jan Kiszka
Quoted 4 lines
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
[...]
Quoted 12 lines
lib/librte_vhost/virtio-net.c reset_owner(struct vhost_device_ctx ctx) · line 406
Quoted 8 lines
lib/librte_vhost/virtio-net.c +2 -0
@@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
 
 	ll_dev = get_config_ll_entry(ctx);
 
+	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
+		notify_ops->destroy_device(&ll_dev->dev);
To me this patch makes sense here.
Whether RESET_OWNER is really needed is another question. Whenever the vhost itself needs to process the vhost device, we need to notify the switch application to remove it from data plane.
Quoted 3 lines
Quoted 3 lines
Huawei,
some patches have been accepted for RESET_OWNER management. Is this patch obsolete?
I think it's still appliable, at least so far.
Quoted 4 lines
Hi Yuanhan and Huawei,
I also have the same question. Do we have a patch for this issue?

Quoted 7 lines

Quoted 6 lines
Today, I've download Yuanhan's multiple queues patches and applied it on latest dpdk tree.
Then, tried to apply my vhost PMD patch on it.
When I check the patch, it seems I've faced this issue. Here are steps to reproduce.
Above patch should fix your issue, right? If so, we need it.

Yes, the patch will fix the issue.

Quoted 14 lines

Quoted 14 lines
1. Start vhost-user backend application.
(In my case, testpmd using vhost PMD is the application)
2. Start a VM with vhost-user.
You can see below message from the backend application.
VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
VHOST_CONFIG: set queue enable: 1 to qp idx: 0
(snip)
VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
3. After booting Linux on guest, bind the virtio-net device to igb_uio. Then below messages are shown.
VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
VHOST_CONFIG: read message VHOST_USER_GET_VRING_BASE

Quoted 7 lines

Quoted 2 lines
The point is we will have VHOST_USER_RESET_OWNER before VHOST_USER_GET_VRING_BASE.
Note that there is an ongoing work at QEMU community (from me) to handle RESET_OWNER correctly: it will be moved to somewhere else instead of before VHOST_USER_GET_VRING_BASE.
	--yliu

Sounds great! Thanks for handling it.

Tetsuya

Thomas Monjalon

2016-03-17 14:44 UTC #11 lore raw

In reply to Jan Kiszka

2015-08-07 19:20, Jan Kiszka:

Quoted 6 lines

On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.
Signed-off-by Jan Kiszka <[email protected]>

For an unknown reason, this patch has been missed and another one replaced it in DPDK 2.2:
http://dpdk.org/browse/dpdk/commit/?id=d243ecf0

Jan Kiszka

author

2016-03-17 14:53 UTC #12 lore raw

Quoted 6 lines · Thomas Monjalon

2015-08-07 19:20, Jan Kiszka:
Quoted 5 lines
On VHOST_*_RESET_OWNER, we reinitialize the device but without telling the application. That will cause crashes when it continues to invoke vhost services on the device. Fix it by calling the destruction hook if the device is still in use.

Quoted 6 lines

Quoted 1 line
Signed-off-by Jan Kiszka <[email protected]>
For an unknown reason, this patch has been missed and another one replaced it in DPDK 2.2:
http://dpdk.org/browse/dpdk/commit/?id=d243ecf0

But the bug is fixed now - that is what matters :)

Thanks,
Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux