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

[PATCH] 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 pais + 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>
---
 hw/core/machine.c          | 1 +
 hw/virtio/virtio-net-pci.c | 8 +++++++-
 2 files changed, 8 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..2894c46b66 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,11 @@ 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) + 2;
+    }
 
     virtio_net_set_netclient_name(&dev->vdev, qdev->id,
                                   object_get_typename(OBJECT(qdev)));
-- 
2.24.3 (Apple Git-128)

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

[Philippe Mathieu-Daudé]

Hi Jason,

On 3/8/21 6:30 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 pais + plus one config interrupt and control vq). We didn't

Typo "pairs".

> 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>
> ---
>  hw/core/machine.c          | 1 +
>  hw/virtio/virtio-net-pci.c | 8 +++++++-
>  2 files changed, 8 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..2894c46b66 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,11 @@ 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) + 2;

Please either document that magic '2':

           vpci_dev->nvectors = 2 * MAX(net->nic_conf.peers.queues, 1)
                                + 1 /* Config interrupt */
                                + 1 /* Control vq */;

Or add self-explicit definitions and use them in place.

> +    }
>  
>      virtio_net_set_netclient_name(&dev->vdev, qdev->id,
>                                    object_get_typename(OBJECT(qdev)));
> 

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

[Stefan Hajnoczi]

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

On Mon, Mar 08, 2021 at 01:30:59PM +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 pais + plus one config interrupt and control vq). We didn't

Maybe this can be squashed in but it's not important:
s/pais/pairs/

> 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>
> ---
>  hw/core/machine.c          | 1 +
>  hw/virtio/virtio-net-pci.c | 8 +++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)

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

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

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

[Jason Wang]

On 2021/3/8 6:06 下午, Philippe Mathieu-Daudé wrote:
> Hi Jason,
>
> On 3/8/21 6:30 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 pais + plus one config interrupt and control vq). We didn't
> Typo "pairs".
>
>> 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>
>> ---
>>   hw/core/machine.c          | 1 +
>>   hw/virtio/virtio-net-pci.c | 8 +++++++-
>>   2 files changed, 8 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..2894c46b66 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,11 @@ 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) + 2;
> Please either document that magic '2':
>
>             vpci_dev->nvectors = 2 * MAX(net->nic_conf.peers.queues, 1)
>                                  + 1 /* Config interrupt */
>                                  + 1 /* Control vq */;
>
> Or add self-explicit definitions and use them in place.


Ok, V2 is sent.

Thanks


>
>> +    }
>>   
>>       virtio_net_set_netclient_name(&dev->vdev, qdev->id,
>>                                     object_get_typename(OBJECT(qdev)));
>>
>