Re: [PATCH] virtio-net: correctly report maximum tx_queue_size value

[Michael S. Tsirkin]

On Mon, Jun 05, 2023 at 04:21:25PM +0200, Laurent Vivier wrote:
> Maximum value for tx_queue_size depends on the backend type.
> 1024 for vDPA/vhost-user, 256 for all the others.
> 
> The value is returned by virtio_net_max_tx_queue_size() to set the
> parameter:
> 
>     n->net_conf.tx_queue_size = MIN(virtio_net_max_tx_queue_size(n),
>                                     n->net_conf.tx_queue_size);
> 
> But the parameter checking uses VIRTQUEUE_MAX_SIZE (1024).
> 
> So the parameter is silently ignored and ethtool reports a different
> value than the one provided by the user.
> 
>    ... -netdev tap,... -device virtio-net,tx_queue_size=1024
> 
>     # ethtool -g enp0s2
>     Ring parameters for enp0s2:
>     Pre-set maximums:
>     RX:               256
>     RX Mini:  n/a
>     RX Jumbo: n/a
>     TX:               256
>     Current hardware settings:
>     RX:               256
>     RX Mini:  n/a
>     RX Jumbo: n/a
>     TX:               256
> 
>    ... -netdev vhost-user,... -device virtio-net,tx_queue_size=2048
> 
>     Invalid tx_queue_size (= 2048), must be a power of 2 between 256 and 1024
> 
> With this patch the correct maximum value is checked and displayed.
> 
> For vDPA/vhost-user:
> 
>     Invalid tx_queue_size (= 2048), must be a power of 2 between 256 and 1024
> 
> For all the others:
> 
>     Invalid tx_queue_size (= 512), must be a power of 2 between 256 and 256
> 
> Fixes: 2eef278b9e63 ("virtio-net: fix tx queue size for !vhost-user")
> Cc: mst@redhat.com
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/net/virtio-net.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 6df6b7329d25..e9905aac83ad 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3630,12 +3630,12 @@ static void virtio_net_device_realize(DeviceState 
> *dev, Error **errp)
>      }
>  
>      if (n->net_conf.tx_queue_size < VIRTIO_NET_TX_QUEUE_MIN_SIZE ||
> -        n->net_conf.tx_queue_size > VIRTQUEUE_MAX_SIZE ||
> +        n->net_conf.tx_queue_size > virtio_net_max_tx_queue_size(n) ||
>          !is_power_of_2(n->net_conf.tx_queue_size)) {
>          error_setg(errp, "Invalid tx_queue_size (= %" PRIu16 "), "
>                     "must be a power of 2 between %d and %d",
>                     n->net_conf.tx_queue_size, VIRTIO_NET_TX_QUEUE_MIN_SIZE,
> -                   VIRTQUEUE_MAX_SIZE);
> +                   virtio_net_max_tx_queue_size(n));
>          virtio_cleanup(vdev);
>          return;
>      }
> -- 
> 2.39.2