[PATCH V2] virtio-net: calculating proper msix vectors on init

[PATCH V2] virtio-net: calculating proper msix vectors on init

[Jason Wang]

Currently, the default msix vectors for virtio-net-pci is 3 which is
obvious not suitable for multiqueue guest, so we depends on the user
or management tools to pass a correct vectors parameter. In fact, we
can simplifying this by calculating the number of vectors on realize.

Consider we have N queues, the number of vectors needed is 2*N + 2
(#queue pairs + plus one config interrupt and control vq). We didn't
check whether or not host support control vq because it was added
unconditionally by qemu to avoid breaking legacy guests such as Minix.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes since v1:
- Fix typo in the commit log
- Explain the magic number during vectors calculation
---
 hw/core/machine.c          |  1 +
 hw/virtio/virtio-net-pci.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 4386f57b5c..979133f8b7 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,7 @@
 GlobalProperty hw_compat_5_2[] = {
     { "ICH9-LPC", "smm-compat", "on"},
     { "PIIX4_PM", "smm-compat", "on"},
+    { "virtio-net-pci", "vectors", "3"},
 };
 const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
 
diff --git a/hw/virtio/virtio-net-pci.c b/hw/virtio/virtio-net-pci.c
index 292d13d278..aa0b3caecb 100644
--- a/hw/virtio/virtio-net-pci.c
+++ b/hw/virtio/virtio-net-pci.c
@@ -41,7 +41,8 @@ struct VirtIONetPCI {
 static Property virtio_net_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
-    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+                       DEV_NVECTORS_UNSPECIFIED),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -50,6 +51,13 @@ static void virtio_net_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     DeviceState *qdev = DEVICE(vpci_dev);
     VirtIONetPCI *dev = VIRTIO_NET_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
+    VirtIONet *net = VIRTIO_NET(vdev);
+
+    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+        vpci_dev->nvectors = 2 * MAX(net->nic_conf.peers.queues, 1)
+            + 1 /* Config interrupt */
+            + 1 /* Control vq */;
+    }
 
     virtio_net_set_netclient_name(&dev->vdev, qdev->id,
                                   object_get_typename(OBJECT(qdev)));
-- 
2.24.3 (Apple Git-128)

Re: [PATCH V2] virtio-net: calculating proper msix vectors on init

[Philippe Mathieu-Daudé]

On 3/9/21 5:23 AM, Jason Wang wrote:
> Currently, the default msix vectors for virtio-net-pci is 3 which is
> obvious not suitable for multiqueue guest, so we depends on the user
> or management tools to pass a correct vectors parameter. In fact, we
> can simplifying this by calculating the number of vectors on realize.
> 
> Consider we have N queues, the number of vectors needed is 2*N + 2
> (#queue pairs + plus one config interrupt and control vq). We didn't
> check whether or not host support control vq because it was added
> unconditionally by qemu to avoid breaking legacy guests such as Minix.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes since v1:
> - Fix typo in the commit log
> - Explain the magic number during vectors calculation
> ---
>  hw/core/machine.c          |  1 +
>  hw/virtio/virtio-net-pci.c | 10 +++++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)

(the patch had Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH V2] virtio-net: calculating proper msix vectors on init

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

On Tue, Mar 09, 2021 at 12:23:14PM +0800, Jason Wang wrote:
> Currently, the default msix vectors for virtio-net-pci is 3 which is
> obvious not suitable for multiqueue guest, so we depends on the user
> or management tools to pass a correct vectors parameter. In fact, we
> can simplifying this by calculating the number of vectors on realize.
> 
> Consider we have N queues, the number of vectors needed is 2*N + 2
> (#queue pairs + plus one config interrupt and control vq). We didn't
> check whether or not host support control vq because it was added
> unconditionally by qemu to avoid breaking legacy guests such as Minix.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes since v1:
> - Fix typo in the commit log
> - Explain the magic number during vectors calculation
> ---
>  hw/core/machine.c          |  1 +
>  hw/virtio/virtio-net-pci.c | 10 +++++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)

Nice!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V2] virtio-net: calculating proper msix vectors on init

[Stefano Garzarella]

On Tue, Mar 09, 2021 at 12:23:14PM +0800, Jason Wang wrote:
>Currently, the default msix vectors for virtio-net-pci is 3 which is
>obvious not suitable for multiqueue guest, so we depends on the user
>or management tools to pass a correct vectors parameter. In fact, we
>can simplifying this by calculating the number of vectors on realize.
>
>Consider we have N queues, the number of vectors needed is 2*N + 2
>(#queue pairs + plus one config interrupt and control vq). We didn't
>check whether or not host support control vq because it was added
>unconditionally by qemu to avoid breaking legacy guests such as Minix.
>
>Signed-off-by: Jason Wang <jasowang@redhat.com>
>---
>Changes since v1:
>- Fix typo in the commit log
>- Explain the magic number during vectors calculation
>---
> hw/core/machine.c          |  1 +
> hw/virtio/virtio-net-pci.c | 10 +++++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [PATCH V2] virtio-net: calculating proper msix vectors on init

[Jason Wang]

On 2021/3/9 12:23 下午, Jason Wang wrote:
> Currently, the default msix vectors for virtio-net-pci is 3 which is
> obvious not suitable for multiqueue guest, so we depends on the user
> or management tools to pass a correct vectors parameter. In fact, we
> can simplifying this by calculating the number of vectors on realize.
>
> Consider we have N queues, the number of vectors needed is 2*N + 2
> (#queue pairs + plus one config interrupt and control vq). We didn't
> check whether or not host support control vq because it was added
> unconditionally by qemu to avoid breaking legacy guests such as Minix.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>


Applied.

Thanks


> ---
> Changes since v1:
> - Fix typo in the commit log
> - Explain the magic number during vectors calculation
> ---
>   hw/core/machine.c          |  1 +
>   hw/virtio/virtio-net-pci.c | 10 +++++++++-
>   2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 4386f57b5c..979133f8b7 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -39,6 +39,7 @@
>   GlobalProperty hw_compat_5_2[] = {
>       { "ICH9-LPC", "smm-compat", "on"},
>       { "PIIX4_PM", "smm-compat", "on"},
> +    { "virtio-net-pci", "vectors", "3"},
>   };
>   const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
>   
> diff --git a/hw/virtio/virtio-net-pci.c b/hw/virtio/virtio-net-pci.c
> index 292d13d278..aa0b3caecb 100644
> --- a/hw/virtio/virtio-net-pci.c
> +++ b/hw/virtio/virtio-net-pci.c
> @@ -41,7 +41,8 @@ struct VirtIONetPCI {
>   static Property virtio_net_properties[] = {
>       DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
>                       VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> -    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
> +    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> +                       DEV_NVECTORS_UNSPECIFIED),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> @@ -50,6 +51,13 @@ static void virtio_net_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>       DeviceState *qdev = DEVICE(vpci_dev);
>       VirtIONetPCI *dev = VIRTIO_NET_PCI(vpci_dev);
>       DeviceState *vdev = DEVICE(&dev->vdev);
> +    VirtIONet *net = VIRTIO_NET(vdev);
> +
> +    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> +        vpci_dev->nvectors = 2 * MAX(net->nic_conf.peers.queues, 1)
> +            + 1 /* Config interrupt */
> +            + 1 /* Control vq */;
> +    }
>   
>       virtio_net_set_netclient_name(&dev->vdev, qdev->id,
>                                     object_get_typename(OBJECT(qdev)));