[dpdk-dev,v2,1/2] virtio: fix queue size and number of descriptors

  The virtual queue ring size and the number of slots actually usable
are separate parameters. In the most common environment (QEMU)
the virtual queue ring size is 256, but some environments the
ring maybe much larger.

The ring size comes from the host and the driver must use the
actual size passed.

The number of descriptors can be either zero to use the whole
available ring, or some value smaller. This is used to limit
the number of mbufs allocated for the receive ring. If more
descriptors are requested than available the size is silently
truncated.

Note: the ring size (from host) must be a power of two, but
the number of descriptors used can be any size from 1 to the
size of the virtual ring.

Reported-by: Ouyang Changchun <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/virtio/virtio_ethdev.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)
  

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, July 21, 2015 2:41 AM
> To: Ouyang, Changchun
> Cc: dev@dpdk.org; Stephen Hemminger
> Subject: [PATCH v2 1/2] virtio: fix queue size and number of descriptors
> 
> The virtual queue ring size and the number of slots actually usable are
> separate parameters. In the most common environment (QEMU) the virtual
> queue ring size is 256, but some environments the ring maybe much larger.
> 
> The ring size comes from the host and the driver must use the actual size
> passed.
> 
> The number of descriptors can be either zero to use the whole available ring,
> or some value smaller. This is used to limit the number of mbufs allocated for
> the receive ring. If more descriptors are requested than available the size is
> silently truncated.
> 
> Note: the ring size (from host) must be a power of two, but the number of
> descriptors used can be any size from 1 to the size of the virtual ring.
> 
> Reported-by: Ouyang Changchun <changchun.ouyang@intel.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Basically ok for this change, so
Acked-by: Changchun Ouyang <changchun.ouyang@intel.com>

> ---
>  drivers/net/virtio/virtio_ethdev.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index 9ca9bb2..d460d89 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -276,8 +276,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
>  	 */
>  	vq_size = VIRTIO_READ_REG_2(hw, VIRTIO_PCI_QUEUE_NUM);
>  	PMD_INIT_LOG(DEBUG, "vq_size: %d nb_desc:%d", vq_size,
> nb_desc);
> -	if (nb_desc == 0)
> -		nb_desc = vq_size;
>  	if (vq_size == 0) {
>  		PMD_INIT_LOG(ERR, "%s: virtqueue does not exist",
> __func__);
>  		return -EINVAL;
> @@ -288,16 +286,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev
> *dev,
>  		return -EINVAL;
>  	}
> 
> -	if (nb_desc < vq_size) {
> -		if (!rte_is_power_of_2(nb_desc)) {
> -			PMD_INIT_LOG(ERR,
> -				     "nb_desc(%u) size is not powerof 2",
> -				     nb_desc);
> -			return -EINVAL;
> -		}
> -		vq_size = nb_desc;
> -	}
> -
>  	if (queue_type == VTNET_RQ) {
>  		snprintf(vq_name, sizeof(vq_name), "port%d_rvq%d",
>  			dev->data->port_id, queue_idx);
> @@ -325,7 +313,10 @@ int virtio_dev_queue_setup(struct rte_eth_dev
> *dev,
>  	vq->queue_id = queue_idx;
>  	vq->vq_queue_index = vtpci_queue_idx;
>  	vq->vq_nentries = vq_size;
> -	vq->vq_free_cnt = vq_size;
> +
> +	if (nb_desc == 0 || nb_desc > vq_size)
> +		nb_desc = vq_size;
> +	vq->vq_free_cnt = nb_desc;
> 
>  	/*
>  	 * Reserve a memzone for vring elements
> --
> 2.1.4
  

2015-07-20 11:40, Stephen Hemminger:
> The number of descriptors can be either zero to use the whole
> available ring, or some value smaller. This is used to limit
> the number of mbufs allocated for the receive ring. If more
> descriptors are requested than available the size is silently
> truncated.
[...]
> +	if (nb_desc == 0 || nb_desc > vq_size)
> +		nb_desc = vq_size;

This behaviour should be common to every drivers and explained in the API doxygen:
http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h?id=v2.1.0-rc1#n1843