From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f50.google.com (mail-vk0-f50.google.com [209.85.213.50]) by dpdk.org (Postfix) with ESMTP id 2A6A737B0 for ; Wed, 1 Jun 2016 00:06:33 +0200 (CEST) Received: by mail-vk0-f50.google.com with SMTP id a6so1641891vkg.3 for ; Tue, 31 May 2016 15:06:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bigswitch-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=+m6I8wKNz1crsrPpt3hsT1Y18cul/ZFpg6NFrejh3vE=; b=iaPOsdnDhtussVe6QLARC0u5VtTUAxlOctD6uDAeqjxefUY4hm9iOwx1iJWFhD4Mc6 15aV1Pr0mieSMMDdeA0s9YoaYGtLqaV/y9IF/zMPNU8bYiROp2DW08SbRzuDyKHDJecX 0xt5EbOhxFOhsZKFRkz3MYTBSeBdS08BdrR/PcS7tx8maQxDZBTLXIJHu18qvXnFxDD6 3rrBZNIaMOv49OiKkhiydZm6tuqokn+NSSiqWux4FVM6aC1dgt1RMS3tIAt0O0vicLi2 qYu/DhFKXpSMcbwCd9TaMQ7GNNWYcwbp07KzQB4YjqQ+1eENcRsxltehA/8cl6pkMlIT EtUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=+m6I8wKNz1crsrPpt3hsT1Y18cul/ZFpg6NFrejh3vE=; b=XxUdHwaCsI0ttoGoqssKrqW/xcS5EWBKlC+ew65rosBjwY/+S+dNYT9zzpaP3t9FwU UOjXiDnymS6Y4kr48h0thAEun08KooBvCG3WlfmLtfteJfMpoL7IKyT+UELgc+rm0NpM VzQ4m/idmuc4Zy4+UB22K8v2vz97FUwXIDMjFnV9hCraxCp0xeyGkNhiatquWHVAZ9wf Uvvt9oJogUN+4F4WTfnWWRYialJXw9dGHNmltZRJjneWUEMKdpyX3u5UhCFJSVbFh+cQ v6475XLD7m677NuuaVFhHSiaAzHPKGAJL/VUGzi0t4uunh2gLSj+TX1pImWRoadGsnx/ wPZA== X-Gm-Message-State: ALyK8tL7RmkaNdAZb3rCNzUPOWioDIBhdS2DKOvCqQkc2ZUaPw5OW+g02EgNulAMBnJJo0Ngqux1ODyNNEvxGZs9 MIME-Version: 1.0 X-Received: by 10.31.206.133 with SMTP id e127mr196850vkg.138.1464732392636; Tue, 31 May 2016 15:06:32 -0700 (PDT) Received: by 10.31.190.14 with HTTP; Tue, 31 May 2016 15:06:32 -0700 (PDT) In-Reply-To: <1463748604-27251-1-git-send-email-i.maximets@samsung.com> References: <1463748604-27251-1-git-send-email-i.maximets@samsung.com> Date: Tue, 31 May 2016 15:06:32 -0700 Message-ID: From: Rich Lane To: Ilya Maximets Cc: dev@dpdk.org, Huawei Xie , Yuanhan Liu , Dyasly Sergey , Heetae Ahn , Jianfeng Tan Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] vhost: fix segfault on bad descriptor address. 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: Tue, 31 May 2016 22:06:33 -0000 On Fri, May 20, 2016 at 5:50 AM, Ilya Maximets wrote: > In current implementation guest application can reinitialize vrings > by executing start after stop. In the same time host application > can still poll virtqueue while device stopped in guest and it will > crash with segmentation fault while vring reinitialization because > of dereferencing of bad descriptor addresses. > I see a performance regression with this patch at large packet sizes (> 768 bytes). rte_vhost_enqueue_burst is consuming 10% more cycles. Strangely, there's actually a ~1% performance improvement at small packet sizes. The regression happens with GCC 4.8.4 and 5.3.0, but not 6.1.1. AFAICT this is just the compiler generating bad code. One difference is that it's storing the offset on the stack instead of in a register. A workaround is to move the !desc_addr check outside the unlikely macros. --- a/lib/librte_vhost/vhost_rxtx.c > +++ b/lib/librte_vhost/vhost_rxtx.c > @@ -147,10 +147,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct > vhost_virtqueue *vq, > struct virtio_net_hdr_mrg_rxbuf virtio_hdr = {{0, 0, 0, 0, 0, 0}, > 0}; > > desc = &vq->desc[desc_idx]; > - if (unlikely(desc->len < vq->vhost_hlen)) > + desc_addr = gpa_to_vva(dev, desc->addr); > + if (unlikely(desc->len < vq->vhost_hlen || !desc_addr)) > Workaround: change to "if (unlikely(desc->len < vq->vhost_hlen) || !desc_addr)". return -1; > - desc_addr = gpa_to_vva(dev, desc->addr); > rte_prefetch0((void *)(uintptr_t)desc_addr); > > virtio_enqueue_offload(m, &virtio_hdr.hdr); > @@ -184,6 +184,9 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct > vhost_virtqueue *vq, > > desc = &vq->desc[desc->next]; > desc_addr = gpa_to_vva(dev, desc->addr); > + if (unlikely(!desc_addr)) > Workaround: change to "if (!desc_addr)". > + return -1; > + > desc_offset = 0; > desc_avail = desc->len; > } >