Re: [dpdk-stable] [PATCH 2/3] net/virtio: rationalize queue flushing

[Tiwei Bie <tiwei.bie@intel.com>]

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
>