From mboxrd@z Thu Jan  1 00:00:00 1970
From: Rich Lane <rich.lane@bigswitch.com>
Subject: Re: [PATCH 1/5] vhost: refactor rte_vhost_dequeue_burst
Date: Fri, 11 Dec 2015 22:55:48 -0800
Message-ID: <CAGSMBPNtc3Wy-L-HrbdA0gwxABQSxy87jwYKUvbzap8JmO3jCg@mail.gmail.com>
References: <1449122773-25510-1-git-send-email-yuanhan.liu@linux.intel.com>
 <1449122773-25510-2-git-send-email-yuanhan.liu@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Cc: dev@dpdk.org, Victor Kaplansky <vkaplans@redhat.com>,
 "Michael S. Tsirkin" <mst@redhat.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-vk0-f51.google.com (mail-vk0-f51.google.com
 [209.85.213.51]) by dpdk.org (Postfix) with ESMTP id 544758E5A
 for <dev@dpdk.org>; Sat, 12 Dec 2015 07:55:49 +0100 (CET)
Received: by vkha189 with SMTP id a189so128353096vkh.2
 for <dev@dpdk.org>; Fri, 11 Dec 2015 22:55:48 -0800 (PST)
In-Reply-To: <1449122773-25510-2-git-send-email-yuanhan.liu@linux.intel.com>
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>

On Wed, Dec 2, 2015 at 10:06 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com>
wrote:
>
> +static inline struct rte_mbuf *
> +copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
> +                 uint16_t desc_idx, struct rte_mempool *mbuf_pool)
> +{
>
...

> +
> +       desc = &vq->desc[desc_idx];
> +       desc_addr = gpa_to_vva(dev, desc->addr);
> +       rte_prefetch0((void *)(uintptr_t)desc_addr);
> +
> +       /* Discard virtio header */
> +       desc_avail  = desc->len - vq->vhost_hlen;


If desc->len is zero (happens all the time when restarting DPDK apps in the
guest) then desc_avail will be huge.


> +       desc_offset = vq->vhost_hlen;
> +
> +       mbuf_avail  = 0;
> +       mbuf_offset = 0;
> +       while (desc_avail || (desc->flags & VRING_DESC_F_NEXT) != 0) {

+               /* This desc reachs to its end, get the next one */
> +               if (desc_avail == 0) {
> +                       desc = &vq->desc[desc->next];
>

Need to check desc->next against vq->size.

There should be a limit on the number of descriptors in a chain to prevent
an infinite loop.

 uint16_t
>  rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
>         struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t
> count)
>  {
> ...
>         avail_idx =  *((volatile uint16_t *)&vq->avail->idx);
> -
> -       /* If there are no available buffers then return. */
> -       if (vq->last_used_idx == avail_idx)
> +       free_entries = avail_idx - vq->last_used_idx;
> +       if (free_entries == 0)
>                 return 0;
>

A common problem is that avail_idx goes backwards when the guest zeroes its
virtqueues. This function could check for free_entries > vq->size and reset
vq->last_used_idx.

+       LOG_DEBUG(VHOST_DATA, "(%"PRIu64") about to dequene %u buffers\n",
> +                       dev->device_fh, count);
>

Typo at "dequene".