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

[Jason Wang <jasowang@redhat.com>]

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