Message ID | 1461551865-15930-1-git-send-email-jianfeng.tan@intel.com (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Yuanhan Liu |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 3BC902B8C; Mon, 25 Apr 2016 04:37:48 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id EFDAB2904 for <dev@dpdk.org>; Mon, 25 Apr 2016 04:37:46 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP; 24 Apr 2016 19:37:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,530,1455004800"; d="scan'208";a="939359898" Received: from dpdk06.sh.intel.com ([10.239.128.225]) by orsmga001.jf.intel.com with ESMTP; 24 Apr 2016 19:37:44 -0700 From: Jianfeng Tan <jianfeng.tan@intel.com> To: dev@dpdk.org Cc: huawei.xie@intel.com, yuanhan.liu@linux.intel.com, Jianfeng Tan <jianfeng.tan@intel.com> Date: Mon, 25 Apr 2016 02:37:45 +0000 Message-Id: <1461551865-15930-1-git-send-email-jianfeng.tan@intel.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1461242170-146337-1-git-send-email-jianfeng.tan@intel.com> References: <1461242170-146337-1-git-send-email-jianfeng.tan@intel.com> Subject: [dpdk-dev] [PATCH v2] 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 <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Jianfeng Tan
April 25, 2016, 2:37 a.m. UTC
Issue: when using virtio nic to transmit pkts, it causes segment fault.
How to reproduce:
Basically, we need to construct a case with vm send packets to vhost-user,
and this issue does not happen when transmitting packets using indirect
desc. Besides, make sure all descriptors are exhausted before vhost
dequeues any packets.
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
b. start a qemu with a virtio nic connected with the vhost-user port, just
make sure mrg_rxbuf is enabled.
c. enable testpmd on the host.
testpmd> set fwd io
testpmd> start (better without start vhost-user)
d. start testpmd in VM.
$testpmd -c 0x3 -n 4 -m 1024 -- -i --disable-hw-vlan-filter --txqflags=0xf01
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 (), not necessary to update after
the loop. (And if we do that after the loop, if all descs could have run out,
idx is VQ_RING_DESC_CHAIN_END (32768), use this idx to reference the start_dp
array will lead to segment fault.)
Fixes: dd856dfcb9e ("virtio: use any layout on Tx")
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
v2: refine the commit message.
drivers/net/virtio/virtio_rxtx.c | 2 --
1 file changed, 2 deletions(-)
Comments
On 4/25/2016 10:37 AM, Tan, Jianfeng wrote: > Issue: when using virtio nic to transmit pkts, it causes segment fault. > > How to reproduce: > Basically, we need to construct a case with vm send packets to vhost-user, > and this issue does not happen when transmitting packets using indirect > desc. Besides, make sure all descriptors are exhausted before vhost > dequeues any packets. > > 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 > > b. start a qemu with a virtio nic connected with the vhost-user port, just > make sure mrg_rxbuf is enabled. > > c. enable testpmd on the host. > testpmd> set fwd io > testpmd> start (better without start vhost-user) > > d. start testpmd in VM. > $testpmd -c 0x3 -n 4 -m 1024 -- -i --disable-hw-vlan-filter --txqflags=0xf01 > 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 (), not necessary to update after > the loop. (And if we do that after the loop, if all descs could have run out, > idx is VQ_RING_DESC_CHAIN_END (32768), use this idx to reference the start_dp > array will lead to segment fault.) > > Fixes: dd856dfcb9e ("virtio: use any layout on Tx") > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> > --- > v2: refine the commit message. > > 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; > - > if (use_indirect) > idx = txvq->vq_ring.desc[head_idx].next; > Ack the code. Acked-by: Huawei Xie <huawei.xie@intel.com>
On Mon, Apr 25, 2016 at 02:37:45AM +0000, Jianfeng Tan wrote: > Issue: when using virtio nic to transmit pkts, it causes segment fault. > > How to reproduce: > Basically, we need to construct a case with vm send packets to vhost-user, > and this issue does not happen when transmitting packets using indirect > desc. Besides, make sure all descriptors are exhausted before vhost > dequeues any packets. > > 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 > > b. start a qemu with a virtio nic connected with the vhost-user port, just > make sure mrg_rxbuf is enabled. > > c. enable testpmd on the host. > testpmd> set fwd io > testpmd> start (better without start vhost-user) > > d. start testpmd in VM. > $testpmd -c 0x3 -n 4 -m 1024 -- -i --disable-hw-vlan-filter --txqflags=0xf01 > 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 (), not necessary to update after > the loop. That's not a right "because": you were stating a fact of the right way to do setup desc flags, but not the cause of this bug. > (And if we do that after the loop, if all descs could have run out, > idx is VQ_RING_DESC_CHAIN_END (32768), use this idx to reference the start_dp > array will lead to segment fault.) And that's the cause. So, you should state the cause first, then the fix (which we already have), but not in the verse order you just did. So, I'd like to reword the commit log a bit, to something like following. What do you think of it? If no objection, I could merge it soon. Thanks for the fix, BTW! --yliu --- Subject: virtio: fix segfault on Tx desc flags setup After the do-while loop, idx could be VQ_RING_DESC_CHAIN_END (32768) when it's the last vring desc buf we can get. Therefore, following expresssion could lead to a segfault error, as it tries to access beyond the desc memory boundary. start_dp[idx].flags &= ~VRING_DESC_F_NEXT; This bug could be reproduced easily with "set fwd txonly" in the guest PMD, where the dequeue on host is slower than the guest Tx, that running out of free desc buf is pretty easy. The fix is straightforward and easy, just remove it, as we have already set desc flags properly inside the do-while loop. Fixes: dd856dfcb9e ("virtio: use any layout on Tx")
Hi Yuanhan, > -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > Sent: Tuesday, April 26, 2016 11:43 AM > To: Tan, Jianfeng > Cc: dev@dpdk.org; Xie, Huawei > Subject: Re: [PATCH v2] virtio: fix segfault when transmit pkts > > On Mon, Apr 25, 2016 at 02:37:45AM +0000, Jianfeng Tan wrote: > > Issue: when using virtio nic to transmit pkts, it causes segment fault. > > > > How to reproduce: > > Basically, we need to construct a case with vm send packets to vhost-user, > > and this issue does not happen when transmitting packets using indirect > > desc. Besides, make sure all descriptors are exhausted before vhost > > dequeues any packets. > > > > 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 > > > > b. start a qemu with a virtio nic connected with the vhost-user port, just > > make sure mrg_rxbuf is enabled. > > > > c. enable testpmd on the host. > > testpmd> set fwd io > > testpmd> start (better without start vhost-user) > > > > d. start testpmd in VM. > > $testpmd -c 0x3 -n 4 -m 1024 -- -i --disable-hw-vlan-filter --txqflags=0xf01 > > 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 (), not necessary to update > after > > the loop. > > That's not a right "because": you were stating a fact of the right way > to do setup desc flags, but not the cause of this bug. > > > (And if we do that after the loop, if all descs could have run out, > > idx is VQ_RING_DESC_CHAIN_END (32768), use this idx to reference the > start_dp > > array will lead to segment fault.) > > And that's the cause. So, you should state the cause first, then the fix > (which we already have), but not in the verse order you just did. > > So, I'd like to reword the commit log a bit, to something like following. > What do you think of it? If no objection, I could merge it soon. Thanks > for the fix, BTW! > Your refinement sounds much better, thanks! Jianfeng
Talking about wording, 2016-04-25 20:43, Yuanhan Liu: > --- > Subject: virtio: fix segfault on Tx desc flags setup I think the english word "crash" is better than "segfault".
On Tue, Apr 26, 2016 at 10:43:35AM +0200, Thomas Monjalon wrote: > Talking about wording, > > 2016-04-25 20:43, Yuanhan Liu: > > --- > > Subject: virtio: fix segfault on Tx desc flags setup > > I think the english word "crash" is better than "segfault". Yes. Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> And, applied to dpdk-next-virtio, with the commit log rewording. Thanks. --yliu
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; - if (use_indirect) idx = txvq->vq_ring.desc[head_idx].next;