Message ID | 20230605142125.3881859-1-lvivier@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | virtio-net: correctly report maximum tx_queue_size value | expand |
On Mon, Jun 5, 2023 at 10:22 PM Laurent Vivier <lvivier@redhat.com> 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> 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 > >
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
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: mst@redhat.com > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > 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
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; }
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> --- hw/net/virtio-net.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)