From: "Tan, Jianfeng" <jianfeng.tan@intel.com>
To: "Xie, Huawei" <huawei.xie@intel.com>,
Yuanhan Liu <yuanhan.liu@linux.intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH] virtio: fix segfault when transmit pkts
Date: Mon, 25 Apr 2016 09:58:23 +0800 [thread overview]
Message-ID: <571D79BF.1090604@intel.com> (raw)
In-Reply-To: <C37D651A908B024F974696C65296B57B4C713546@SHSMSX101.ccr.corp.intel.com>
Hi Yuanhan & Huawei,
On 4/22/2016 10:23 PM, Xie, Huawei wrote:
> On 4/22/2016 6:43 AM, Yuanhan Liu wrote:
>> On Thu, Apr 21, 2016 at 12:36:10PM +0000, Jianfeng Tan wrote:
>>> Issue: when using virtio nic to transmit pkts, it causes segment fault.
>> Jianfeng,
>>
>> It's good to describe the issues, steps to reproduce it and how to fix
>> it. But you don't have to tell it like filling a form. Instead, you
>> could try to describe in plain English.
>>
>>> How to reproduce:
>>> a. start testpmd with vhost.
>>> $testpmd -c 0x3 -n 4 --socket-mem 1024,0 --no-pci \
>>> --vdev 'eth_vhost0,iface=/tmp/sock0,queues=1' -- -i --nb-cores=1
>> I personally would suggest to add some indentations (and some whitespace
>> lines), like
>>
>> a. start testpmd with vhost.
>> $ testpmd -c 0x3 -n 4 ... \
>> --vdev '...' ...
>>
>> b. something else ...
>> some more explanations.
>>
>> And, do not quote a command line like "$testpmd ...", it's like a var
>> in base in this way. Instead, add a space after "$ ", like what I did
>> above.
>>
>>> b. start a qemu with a virtio nic connected with the vhost-user port.
>> This should be enough. I didn't find any special options below,
>> therefore, above words is enough. However, if it's some specific
>> option introduces a bug, you could claim it aloud then.
>>
>>> $qemu -smp cores=2,sockets=1 -cpu host -enable-kvm vm-0.img -vnc :5 -m 4G \
>>> -object memory-backend-file,id=mem,size=4096M,mem-path=<path>,share=on \
>>> -numa node,memdev=mem -mem-prealloc \
>>> -chardev socket,id=char1,path=$sock_vhost \
>>> -netdev type=vhost-user,id=net1,chardev=char1 \
>>> -device virtio-net-pci,netdev=net1,mac=00:01:02:03:04:05
>>> c. enable testpmd on the host.
>>> testpmd> set fwd io
>>> testpmd> start
>>> d. start testpmd in VM.
>>> $testpmd -c 0x3 -n 4 -m 1024 -- -i --disable-hw-vlan-filter --txqflags=0xf01
> With txqflags=0xf01, virtio PMD will use virtio_xmit_pkts_simple, in
> which case the enqueue_xmit willn't called, so typo?
Since mrg_rxbuf is by default enabled, so virtio PMD will not use
virtio_xmit_pkts_simple.
>
>>> testpmd> set fwd txonly
>>> testpmd> start
>>>
>>> How to fix: this bug is because inside virtqueue_enqueue_xmit(), the flag of
>>> desc has been updated inside the do {} while (); and after the loop, all descs
>>> could have run out, so idx is VQ_RING_DESC_CHAIN_END (32768), use this idx to
>>> reference the start_dp array will lead to segment fault.
>> You missed a fix line here.
> Yes, please include the commit that introduces this bug.
Thanks to both of you for all the advice above. I'll send a v2.
>
>>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>>> ---
>>> drivers/net/virtio/virtio_rxtx.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>>> index ef21d8e..432aeab 100644
>>> --- a/drivers/net/virtio/virtio_rxtx.c
>>> +++ b/drivers/net/virtio/virtio_rxtx.c
>>> @@ -271,8 +271,6 @@ virtqueue_enqueue_xmit(struct virtqueue *txvq, struct rte_mbuf *cookie,
>>> idx = start_dp[idx].next;
>>> } while ((cookie = cookie->next) != NULL);
>>>
>>> - start_dp[idx].flags &= ~VRING_DESC_F_NEXT;
>> This looks a good fix to me. I'm just wondering why we never met it
>> before, and on which specific case do we meet it? Talking about that,
>> seems like "set fwd txonly" is with high suspicious.
> You will not meet this if you have more free descriptors than needed, so
> this depends on the relative speed of virtio xmit and vhost dequeue.
> Also if indirect feature is negotiated, it willn't trigger.
> However, without indirect, seems as long as we start testpmd with txonly
> in guest `first`, we could definitely trigger this. Jianfeng, could you
> confirm this and emphasize the order in the commit message?
Yes, exactly. And will add the scenario description into commit message.
Thanks,
Jianfeng
>
>> --yliu
>>
>
next prev parent reply other threads:[~2016-04-25 1:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-21 12:36 Jianfeng Tan
2016-04-21 22:44 ` Yuanhan Liu
2016-04-22 14:23 ` Xie, Huawei
2016-04-25 1:58 ` Tan, Jianfeng [this message]
2016-04-25 2:37 ` [dpdk-dev] [PATCH v2] " Jianfeng Tan
2016-04-25 7:33 ` Xie, Huawei
2016-04-26 3:43 ` Yuanhan Liu
2016-04-26 3:47 ` Tan, Jianfeng
2016-04-26 8:43 ` Thomas Monjalon
2016-04-26 16:54 ` Yuanhan Liu
2016-04-26 4:48 ` [dpdk-dev] [PATCH] " Stephen Hemminger
2016-04-26 5:08 ` Tan, Jianfeng
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=571D79BF.1090604@intel.com \
--to=jianfeng.tan@intel.com \
--cc=dev@dpdk.org \
--cc=huawei.xie@intel.com \
--cc=stephen@networkplumber.org \
--cc=yuanhan.liu@linux.intel.com \
/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).