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

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

Posted by Jason Wang 3 years, 1 month ago

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

Posted by Philippe Mathieu-Daudé 3 years, 1 month ago

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

Posted by Stefano Garzarella 3 years, 1 month ago

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

Posted by Jason Wang 3 years, 1 month ago

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