Re: [dpdk-dev] [PATCH] vchost: Notify application of ownership change

["Ouyang, Changchun" <changchun.ouyang@intel.com>]

> -----Original Message-----
> From: Jan Kiszka [mailto:jan.kiszka@siemens.com]
> Sent: Saturday, August 8, 2015 2:43 PM
> To: Ouyang, Changchun; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] vchost: Notify application of ownership
> change
> 
> On 2015-08-08 02:25, Ouyang, Changchun wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jan Kiszka
> >> Sent: Saturday, August 8, 2015 1:21 AM
> >> To: dev@dpdk.org
> >> 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.
> 
> >
> >>
> >> 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.
> 
> > 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).

> 
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>
> >> 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(+)
> >>
> >> diff --git a/lib/librte_vhost/virtio-net.c
> >> b/lib/librte_vhost/virtio-net.c index
> >> b520ec5..3c5b5b2 100644
> >> --- a/lib/librte_vhost/virtio-net.c
> >> +++ b/lib/librte_vhost/virtio-net.c
> >> @@ -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.
> 
> 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 

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