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