DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: "Ouyang, Changchun" <changchun.ouyang@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] vchost: Notify application of ownership change
Date: Mon, 10 Aug 2015 10:07:36 +0200	[thread overview]
Message-ID: <55C85BC8.3050309@siemens.com> (raw)
In-Reply-To: <F52918179C57134FAEC9EA62FA2F962511C8B51B@shsmsx102.ccr.corp.intel.com>

On 2015-08-10 03:20, Ouyang, Changchun wrote:
>> -----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).
> 

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

  reply	other threads:[~2015-08-10  8:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-07 17:20 Jan Kiszka
2015-08-08  0:25 ` Ouyang, Changchun
2015-08-08  6:43   ` Jan Kiszka
2015-08-10  1:20     ` Ouyang, Changchun
2015-08-10  8:07       ` Jan Kiszka [this message]
2015-08-12  3:34 ` Xie, Huawei
2015-08-12  5:35   ` Jan Kiszka
2015-10-24 17:16   ` Thomas Monjalon
2015-10-26  5:54     ` Tetsuya Mukawa
2015-10-26  6:30       ` Yuanhan Liu
2015-10-26  8:33         ` Tetsuya Mukawa
2016-03-17 14:42 ` Thomas Monjalon
2016-03-17 14:53   ` Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55C85BC8.3050309@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=changchun.ouyang@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).