Re: [dpdk-dev] [DPDK] net/virtio: fix check scatter on all Rx queues

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

Hi Zhihong,

On 8/4/21 10:31 AM, zhihongx.peng@intel.com wrote:
> From: Zhihong Peng <zhihongx.peng@intel.com>
> 
> This patch fixes the wrong way to obtain virtqueue.
> The end of virtqueue cannot be judged based on whether
> the array is NULL.

My understanding is that it is causing issue because it is confusing the
control queue with a Rx queue? I so, please be more specific on the
issue it is fixing in the commit message.

> Fixes: 4e8169eb0d2d (net/virtio: fix Rx scatter offload)
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index e58085a2c9..f2d19dc9d6 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -873,8 +873,8 @@ virtio_check_scatter_on_all_rx_queues(struct rte_eth_dev *dev,
>  	if (hw->vqs == NULL)
>  		return true;
>  
> -	for (qidx = 0; (vq = hw->vqs[2 * qidx + VTNET_SQ_RQ_QUEUE_IDX]) != NULL;
> -	     qidx++) {
> +	for (qidx = 0; qidx < hw->max_queue_pairs; qidx++) {
> +		vq = hw->vqs[2 * qidx + VTNET_SQ_RQ_QUEUE_IDX];

I agree with the change, but I would add a check to ensure vq is not
NULL to be safe wrt to NULL pointer dereferencing.

>  		rxvq = &vq->rxq;
>  		if (rxvq->mpool == NULL)
>  			continue;
> 

Thanks,
Maxime