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

2023-06-07 Thread Michael Tokarev 

05.06.2023 17:21, 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: m...@redhat.com
Signed-off-by: Laurent Vivier 
---
  hw/net/virtio-net.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Is it a -stable material, or not worth fixing in -stable?
To me it looks like it should be fixed.

Thanks,

/mjt

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

2023-06-06 Thread 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: m...@redhat.com
> Signed-off-by: Laurent Vivier 

Reviewed-by: Michael S. Tsirkin 

> ---
>  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

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

2023-06-05 Thread Jason Wang 
On Mon, Jun 5, 2023 at 10:22 PM 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: m...@redhat.com
> Signed-off-by: Laurent Vivier 

Queued.

Thanks

> ---
>  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
>
>

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

2023-06-05 Thread Laurent Vivier 
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: m...@redhat.com
Signed-off-by: Laurent Vivier 
---
 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