On 2015-08-10 03:20, Ouyang, Changchun wrote: >> -----Original Message----- >> From: Jan Kiszka [mailto:jan.kiszka at siemens.com] >> Sent: Saturday, August 8, 2015 2:43 PM >> To: Ouyang, Changchun; dev at 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 at dpdk.org] On Behalf Of Jan Kiszka >>>> Sent: Saturday, August 8, 2015 1:21 AM >>>> To: dev at 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). >
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. >> >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka at 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 OK, need to dig into this, probably not the next days, though. Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux