From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tiwei Bie <tiwei.bie@intel.com>
Subject: Re: [PATCH 2/3] net/virtio: rationalize queue flushing
Date: Thu, 18 Jan 2018 22:05:49 +0800
Message-ID: <20180118140549.tnqzaytw36grtaff@debian-xvivbkq.sh.intel.com>
References: <20180118090733.12728-1-olivier.matz@6wind.com>
 <20180118090733.12728-3-olivier.matz@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Cc: dev@dpdk.org, Yuanhan Liu <yliu@fridaylinux.org>,
 Maxime Coquelin <maxime.coquelin@redhat.com>, stable@dpdk.org
To: Olivier Matz <olivier.matz@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Content-Disposition: inline
In-Reply-To: <20180118090733.12728-3-olivier.matz@6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://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 Thu, Jan 18, 2018 at 10:07:32AM +0100, Olivier Matz wrote:
> Rationalize the function virtio_dev_free_mbufs():
> 
> - skip NULL vqs instead of crashing: this is required for the
>   next commit
> - use the same kind of loop than in virtio_free_queues()
> - also flush mbufs from the control queue (this is useless yet)
> - factorize common code between rxq, txq, cq
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 55 ++++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index c7426951c..d8b3b8ed7 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1868,47 +1868,44 @@ virtio_dev_start(struct rte_eth_dev *dev)
>  
>  static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
>  {
[...]
>  
> -		mbuf_num = 0;
> -		while ((buf = virtqueue_detach_unused(txvq->vq)) != NULL) {
> +		while ((buf = virtqueue_detach_unused(vq)) != NULL) {

Thanks for working on this! The virtqueue_detach_unused() can't
handle the vector Rx case correctly. Because vq->vq_descx[] is
initialized for vector Rx, but isn't updated by the vector Rx.
So together with the next commit, it may cause problems during
dev_stop/dev_configure/dev_start if vector Rx is used.

Thanks,
Tiwei

>  			rte_pktmbuf_free(buf);
>  			mbuf_num++;
>  		}
>  
> -		PMD_INIT_LOG(DEBUG, "free %d mbufs", mbuf_num);
>  		PMD_INIT_LOG(DEBUG,
> -			     "After freeing txq[%d] used and unused buf", i);
> -		VIRTQUEUE_DUMP(txvq->vq);
> +			"After freeing %s[%d] used and unused buf",
> +			type, i);
> +		VIRTQUEUE_DUMP(vq);
>  	}
> +
> +	PMD_INIT_LOG(DEBUG, "%d mbufs freed", mbuf_num);
>  }
>  
>  /*
> -- 
> 2.11.0
>