From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id E6D6E2BE3 for ; Fri, 22 Apr 2016 16:23:10 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP; 22 Apr 2016 07:23:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,517,1455004800"; d="scan'208";a="790201040" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga003.jf.intel.com with ESMTP; 22 Apr 2016 07:23:09 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 22 Apr 2016 07:23:09 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.136]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.155]) with mapi id 14.03.0248.002; Fri, 22 Apr 2016 22:23:07 +0800 From: "Xie, Huawei" To: Yuanhan Liu , "Tan, Jianfeng" CC: "dev@dpdk.org" , Stephen Hemminger Thread-Topic: [PATCH] virtio: fix segfault when transmit pkts Thread-Index: AdGcon0Ih32GtMAVQiGSU+WPIC/Jgw== Date: Fri, 22 Apr 2016 14:23:07 +0000 Message-ID: References: <1461242170-146337-1-git-send-email-jianfeng.tan@intel.com> <20160421224444.GC7603@yliu-dev.sh.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] virtio: fix segfault when transmit pkts X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Apr 2016 14:23:11 -0000 On 4/22/2016 6:43 AM, Yuanhan Liu wrote:=0A= > On Thu, Apr 21, 2016 at 12:36:10PM +0000, Jianfeng Tan wrote:=0A= >> Issue: when using virtio nic to transmit pkts, it causes segment fault.= =0A= > Jianfeng,=0A= >=0A= > It's good to describe the issues, steps to reproduce it and how to fix=0A= > it. But you don't have to tell it like filling a form. Instead, you=0A= > could try to describe in plain English.=0A= >=0A= >> How to reproduce:=0A= >> a. start testpmd with vhost.=0A= >> $testpmd -c 0x3 -n 4 --socket-mem 1024,0 --no-pci \=0A= >> --vdev 'eth_vhost0,iface=3D/tmp/sock0,queues=3D1' -- -i --nb-cores=3D1= =0A= > I personally would suggest to add some indentations (and some whitespace= =0A= > lines), like=0A= >=0A= > a. start testpmd with vhost.=0A= > $ testpmd -c 0x3 -n 4 ... \=0A= > --vdev '...' ...=0A= >=0A= > b. something else ...=0A= > some more explanations.=0A= >=0A= > And, do not quote a command line like "$testpmd ...", it's like a var=0A= > in base in this way. Instead, add a space after "$ ", like what I did=0A= > above.=0A= >=0A= >> b. start a qemu with a virtio nic connected with the vhost-user port.=0A= > This should be enough. I didn't find any special options below,=0A= > therefore, above words is enough. However, if it's some specific=0A= > option introduces a bug, you could claim it aloud then.=0A= >=0A= >> $qemu -smp cores=3D2,sockets=3D1 -cpu host -enable-kvm vm-0.img -vnc :5 = -m 4G \=0A= >> -object memory-backend-file,id=3Dmem,size=3D4096M,mem-path=3D,sh= are=3Don \=0A= >> -numa node,memdev=3Dmem -mem-prealloc \=0A= >> -chardev socket,id=3Dchar1,path=3D$sock_vhost \=0A= >> -netdev type=3Dvhost-user,id=3Dnet1,chardev=3Dchar1 \=0A= >> -device virtio-net-pci,netdev=3Dnet1,mac=3D00:01:02:03:04:05=0A= >> c. enable testpmd on the host.=0A= >> testpmd> set fwd io=0A= >> testpmd> start=0A= >> d. start testpmd in VM.=0A= >> $testpmd -c 0x3 -n 4 -m 1024 -- -i --disable-hw-vlan-filter --txqflags= =3D0xf01=0A= =0A= With txqflags=3D0xf01, virtio PMD will use virtio_xmit_pkts_simple, in=0A= which case the enqueue_xmit willn't called, so typo?=0A= =0A= >> testpmd> set fwd txonly=0A= >> testpmd> start=0A= >>=0A= >> How to fix: this bug is because inside virtqueue_enqueue_xmit(), the fla= g of=0A= >> desc has been updated inside the do {} while (); and after the loop, all= descs=0A= >> could have run out, so idx is VQ_RING_DESC_CHAIN_END (32768), use this i= dx to=0A= >> reference the start_dp array will lead to segment fault.=0A= > You missed a fix line here.=0A= =0A= Yes, please include the commit that introduces this bug.=0A= =0A= >=0A= >> Signed-off-by: Jianfeng Tan =0A= >> ---=0A= >> drivers/net/virtio/virtio_rxtx.c | 2 --=0A= >> 1 file changed, 2 deletions(-)=0A= >>=0A= >> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virti= o_rxtx.c=0A= >> index ef21d8e..432aeab 100644=0A= >> --- a/drivers/net/virtio/virtio_rxtx.c=0A= >> +++ b/drivers/net/virtio/virtio_rxtx.c=0A= >> @@ -271,8 +271,6 @@ virtqueue_enqueue_xmit(struct virtqueue *txvq, struc= t rte_mbuf *cookie,=0A= >> idx =3D start_dp[idx].next;=0A= >> } while ((cookie =3D cookie->next) !=3D NULL);=0A= >> =0A= >> - start_dp[idx].flags &=3D ~VRING_DESC_F_NEXT;=0A= > This looks a good fix to me. I'm just wondering why we never met it=0A= > before, and on which specific case do we meet it? Talking about that,=0A= > seems like "set fwd txonly" is with high suspicious.=0A= =0A= You will not meet this if you have more free descriptors than needed, so=0A= this depends on the relative speed of virtio xmit and vhost dequeue.=0A= Also if indirect feature is negotiated, it willn't trigger.=0A= However, without indirect, seems as long as we start testpmd with txonly=0A= in guest `first`, we could definitely trigger this. Jianfeng, could you=0A= confirm this and emphasize the order in the commit message?=0A= =0A= >=0A= > --yliu=0A= >=0A= =0A=