net/virtio: turn SW RxQ size to that of split vec. virtqueue

Message ID 20210820124752.3522683-1-andrew.rybchenko@oktetlabs.ru (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series net/virtio: turn SW RxQ size to that of split vec. virtqueue |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning apply patch failure
ci/Intel-compilation warning apply issues

Commit Message

Andrew Rybchenko Aug. 20, 2021, 12:47 p.m. UTC
  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(-)
  

Comments

Maxime Coquelin Sept. 13, 2021, 7:46 p.m. UTC | #1
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
  
Maxime Coquelin Sept. 14, 2021, 11:26 a.m. UTC | #2
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
  

Patch

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;