[dpdk-dev] [PATCH] net/virtio: turn SW RxQ size to that of split vec. virtqueue

[dpdk-dev] [PATCH] net/virtio: turn SW RxQ size to that of split vec. virtqueue

[Andrew Rybchenko]

From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>

Descriptors number may be set less than queue size for split queue
vectorized Rx path. Pointers to mbufs for received packets are
obtained from SW ring, that is initially filled with them in the end
of queue setup in virtio_dev_rx_queue_setup_finish(). The begin of the
SW ring filled up to the size of descriptors number. At queue size
offset from the begin of the SW ring pointers to some fake mbuf are also
set for wrapping purpose. So the ring may contains the hole of invalid
pointers from descriptors number offset to queue size offset, and split
vectorized Rx routines could write to the invalid addresses since they
use the ring up to the queue size. Fix this by setting descriptors
number to queue size on Rx queue setup.

Fixes: fc3d66212fed ("virtio: add vector Rx")
Cc: stable@dpdk.org

Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/net/virtio/virtio_rxtx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 18f03c9fc9..97ed69596a 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -706,8 +706,14 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	}
 	vq->vq_free_thresh = rx_free_thresh;
 
-	if (nb_desc > vq->vq_nentries)
+	/*
+	 * For split ring vectorized path descriptors number must be
+	 * equal to the ring size.
+	 */
+	if (nb_desc > vq->vq_nentries ||
+	    (!virtio_with_packed_queue(hw) && hw->use_vec_rx)) {
 		nb_desc = vq->vq_nentries;
+	}
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
 
 	rxvq = &vq->rxq;
-- 
2.30.2

Re: [dpdk-dev] [PATCH] net/virtio: turn SW RxQ size to that of split vec. virtqueue

[Maxime Coquelin]

On 8/20/21 2:47 PM, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> 
> Descriptors number may be set less than queue size for split queue
> vectorized Rx path. Pointers to mbufs for received packets are
> obtained from SW ring, that is initially filled with them in the end
> of queue setup in virtio_dev_rx_queue_setup_finish(). The begin of the
> SW ring filled up to the size of descriptors number. At queue size
> offset from the begin of the SW ring pointers to some fake mbuf are also
> set for wrapping purpose. So the ring may contains the hole of invalid
> pointers from descriptors number offset to queue size offset, and split
> vectorized Rx routines could write to the invalid addresses since they
> use the ring up to the queue size. Fix this by setting descriptors
> number to queue size on Rx queue setup.
> 
> Fixes: fc3d66212fed ("virtio: add vector Rx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>  drivers/net/virtio/virtio_rxtx.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 18f03c9fc9..97ed69596a 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -706,8 +706,14 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  	}
>  	vq->vq_free_thresh = rx_free_thresh;
>  
> -	if (nb_desc > vq->vq_nentries)
> +	/*
> +	 * For split ring vectorized path descriptors number must be
> +	 * equal to the ring size.
> +	 */
> +	if (nb_desc > vq->vq_nentries ||
> +	    (!virtio_with_packed_queue(hw) && hw->use_vec_rx)) {
>  		nb_desc = vq->vq_nentries;
> +	}
>  	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
>  
>  	rxvq = &vq->rxq;
> 

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

Thanks,
Maxime

Re: [dpdk-dev] [PATCH] net/virtio: turn SW RxQ size to that of split vec. virtqueue

[Maxime Coquelin]

On 8/20/21 2:47 PM, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> 
> Descriptors number may be set less than queue size for split queue
> vectorized Rx path. Pointers to mbufs for received packets are
> obtained from SW ring, that is initially filled with them in the end
> of queue setup in virtio_dev_rx_queue_setup_finish(). The begin of the
> SW ring filled up to the size of descriptors number. At queue size
> offset from the begin of the SW ring pointers to some fake mbuf are also
> set for wrapping purpose. So the ring may contains the hole of invalid
> pointers from descriptors number offset to queue size offset, and split
> vectorized Rx routines could write to the invalid addresses since they
> use the ring up to the queue size. Fix this by setting descriptors
> number to queue size on Rx queue setup.
> 
> Fixes: fc3d66212fed ("virtio: add vector Rx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>  drivers/net/virtio/virtio_rxtx.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime