Re: [PATCH v3 3/4] vhost: keep a reference to virtqueue index

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 7/25/22 22:32, David Marchand wrote:
> Having a back reference to the index of the vq in the dev->virtqueue[]
> array makes it possible to unify the internal API, with only passing dev
> and vq.
> It also allows displaying the vq index in log messages.
> 
> Remove virtqueue index checks where unneeded (like in static helpers
> called from a loop on all available virtqueue).
> Move virtqueue index validity checks the sooner possible.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v2:
> - rebased on top of cleanup that avoids some use-after-free issues in
>    case of allocation failures or numa realloactions,
> 
> Changes since v1:
> - fix vq init (that's what happens when only recompiling the vhost
>    library and not relinking testpmd...),
> 
> ---
>   lib/vhost/iotlb.c      |  5 +--
>   lib/vhost/iotlb.h      |  2 +-
>   lib/vhost/vhost.c      | 72 +++++++++++++----------------------
>   lib/vhost/vhost.h      |  3 ++
>   lib/vhost/vhost_user.c | 46 +++++++++++-----------
>   lib/vhost/virtio_net.c | 86 +++++++++++++++++++-----------------------
>   6 files changed, 91 insertions(+), 123 deletions(-)
> 

...

> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 35fa4670fd..467dfb203f 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c

...

> @@ -2275,12 +2267,11 @@ rte_vhost_clear_queue(int vid, uint16_t queue_id, struct rte_mbuf **pkts,
>   	}
>   
>   	if ((queue_id & 1) == 0)
> -		n_pkts_cpl = vhost_poll_enqueue_completed(dev, queue_id,
> -				pkts, count, dma_id, vchan_id);
> -	else {
> +		n_pkts_cpl = vhost_poll_enqueue_completed(dev, vq, pkts, count,
> +			dma_id, vchan_id);
> +	else
>   		n_pkts_cpl = async_poll_dequeue_completed(dev, vq, pkts, count,
> -					dma_id, vchan_id, dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS);
> -	}
> +			dma_id, vchan_id, dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS);

One of the two functions should be renamed for consistency, but that's
not the point of this series.

For this patch:

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime