From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f180.google.com (mail-wi0-f180.google.com [209.85.212.180]) by dpdk.org (Postfix) with ESMTP id 966EC71 for ; Wed, 1 Oct 2014 13:42:12 +0200 (CEST) Received: by mail-wi0-f180.google.com with SMTP id em10so286811wid.7 for ; Wed, 01 Oct 2014 04:48:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=YI6yrXyx3LkxI/FFNBUG5/wjRyRsBRKNNLUFw8hbfbE=; b=I7CSSy4aNG+rgE9JL1PCBljB/N7xHOlO8jpj7UDSRFNqN3Z2w7IwdD7gGiXs2fCYmz D2ZJyG1NsY6lB3EqXHzXXboIq+kNqFhdOqY1Dbt9+Yd9Uyxy9jmIalcmCzmWS0IpDnME nztmJQsfilQCzOYJXmfV4W0t8v5cgDFVTpxve49+ZQARtGuW8Tue0196+zcxjFVGwlaX rXaCOK8NSy9Q/lgHkG7sK4VHCWMZAzYeARPLJ3RHLLbiWj+IKmW2eIkPjlsLK4q3d4zo nuyzvajWlRGHxphUvNr5LI6SEC4OS8Kdb2Lo31n13WUpQCJbWLQ4HNqBjCR2kxnmNTPa WRRQ== X-Gm-Message-State: ALoCoQlwqb5SdX5U+eeUZRZ6VK/LOOWNx3LRjCpGd7KyIKExIgU1Cxy0Pfq2Ka6NJ3Lw6AAa48wv X-Received: by 10.194.103.200 with SMTP id fy8mr4189534wjb.123.1412164137491; Wed, 01 Oct 2014 04:48:57 -0700 (PDT) Received: from [10.16.0.195] (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id t1sm18317840wiy.8.2014.10.01.04.48.56 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 01 Oct 2014 04:48:57 -0700 (PDT) Message-ID: <542BEA28.3030300@6wind.com> Date: Wed, 01 Oct 2014 13:48:56 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 MIME-Version: 1.0 To: Ouyang Changchun , dev@dpdk.org References: <1409812499-15656-1-git-send-email-changchun.ouyang@intel.com> In-Reply-To: <1409812499-15656-1-git-send-email-changchun.ouyang@intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] virtio: Fix vring entry number issue 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: Wed, 01 Oct 2014 11:42:12 -0000 Hello, On 09/04/2014 08:34 AM, Ouyang Changchun wrote: > Fix one issue in virtio TX: it needs one more vring entry to hold > the virtio header when transmitting packets, it is used later to > determine whether to free more entries from used vring. > > Signed-off-by: Changchun Ouyang > Reviewed-by: Huawei Xie > Tested-by: Jingguo Fu > --- > lib/librte_pmd_virtio/virtio_rxtx.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_pmd_virtio/virtio_rxtx.c b/lib/librte_pmd_virtio/virtio_rxtx.c > index 0b10108..b1c189c 100644 > --- a/lib/librte_pmd_virtio/virtio_rxtx.c > +++ b/lib/librte_pmd_virtio/virtio_rxtx.c > @@ -699,7 +699,8 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) > num = (uint16_t)(likely(nb_used < VIRTIO_MBUF_BURST_SZ) ? nb_used : VIRTIO_MBUF_BURST_SZ); > > while (nb_tx < nb_pkts) { > - int need = tx_pkts[nb_tx]->pkt.nb_segs - txvq->vq_free_cnt; > + /* Need one more entry for virtio header. */ > + int need = tx_pkts[nb_tx]->pkt.nb_segs - txvq->vq_free_cnt + 1; > int deq_cnt = RTE_MIN(need, (int)num); > > num -= (deq_cnt > 0) ? deq_cnt : 0; > In the commit log, you talk about vring entries, but to me it seems that it is about virtio descriptors. I think it could be useful to explain what was the consequence of this issue. Is it a performance issue? In my understanding, the problem is that we won't dequeue mbufs from the "used" vring, resulting in a possible "blocking" of the queue. Is it correct? Also, I think it would help the review to explain in which conditions the problem is triggered and how the fix was tested. Last comment, I'm wondering if the next test should also be modified: if (tx_pkts[nb_tx]->nb_segs <= txvq->vq_free_cnt) { Into: if (tx_pkts[nb_tx]->nb_segs <= txvq->vq_free_cnt + 1) { (or maybe using the "need" variable) Do you agree? Regards, Olivier