Re: [PATCH 18/18] vhost-vdpa: multiqueue support

[Jason Wang <jasowang@redhat.com>]

在 2021/7/1 下午2:51, Eugenio Perez Martin 写道:
> On Mon, Jun 21, 2021 at 6:18 AM Jason Wang <jasowang@redhat.com> wrote:
>> This patch implements the multiqueue support for vhost-vdpa. This is
>> done simply by reading the number of queue pairs from the config space
>> and initialize the datapath and control path net client.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>   hw/net/virtio-net.c |  3 +-
>>   net/vhost-vdpa.c    | 98 ++++++++++++++++++++++++++++++++++++++++-----
>>   2 files changed, 91 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 5074b521cf..2c2ed98c0b 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -3370,7 +3370,8 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>>
>>       n->max_ncs = MAX(n->nic_conf.peers.queues, 1);
>>
>> -    /* Figure out the datapath queue pairs since the bakcend could
>> +    /*
>> +     * Figure out the datapath queue pairs since the bakcend could
> If we are going to modify the comment we could s/bakcend/backend/.


Will fix.


>
>>        * provide control queue via peers as well.
>>        */
>>       if (n->nic_conf.peers.queues) {
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index cc11b2ec40..048344b4bc 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -18,6 +18,7 @@
>>   #include "qemu/error-report.h"
>>   #include "qemu/option.h"
>>   #include "qapi/error.h"
>> +#include <linux/vhost.h>
>>   #include <sys/ioctl.h>
>>   #include <err.h>
>>   #include "standard-headers/linux/virtio_net.h"
>> @@ -52,6 +53,8 @@ const int vdpa_feature_bits[] = {
>>       VIRTIO_NET_F_HOST_UFO,
>>       VIRTIO_NET_F_MRG_RXBUF,
>>       VIRTIO_NET_F_MTU,
>> +    VIRTIO_NET_F_MQ,
>> +    VIRTIO_NET_F_CTRL_VQ,
>
> Hi!
>
> I'm not sure if it's qemu the one that must control it, but I cannot
> use vdpa_sim of linux 5.13 (i.e., with no control vq patches) with
> this series applied:
>
> [    3.967421] virtio_net virtio0: device advertises feature
> VIRTIO_NET_F_CTRL_RX but not VIRTIO_NET_F_CTRL_VQ
> [    3.968613] virtio_net: probe of virtio0 failed with error -22


Interesting, looks like a bug somewhere.

We never advertise CTRL_RX in the case of simulator.


>
> Did you mention it somewhere else and I've missed it? or is it
> actually a bug in the device? In this second case, I think we should
> still workaround it in qemu, because old vdpasim_net with no
> VIRTIO_NET_F_CTRL_VQ still works ok without this patch.


Should be a bug, will have a look.

Thanks


>
> Thanks!
>
>>       VIRTIO_F_IOMMU_PLATFORM,
>>       VIRTIO_F_RING_PACKED,
>>       VIRTIO_NET_F_RSS,
>> @@ -82,7 +85,8 @@ static int vhost_vdpa_net_check_device_id(struct vhost_net *net)
>>       return ret;
>>   }
>>
>> -static int vhost_vdpa_add(NetClientState *ncs, void *be)
>> +static int vhost_vdpa_add(NetClientState *ncs, void *be, int qp_index,
>> +                          int nvqs)
>>   {
>>       VhostNetOptions options;
>>       struct vhost_net *net = NULL;
>> @@ -95,7 +99,7 @@ static int vhost_vdpa_add(NetClientState *ncs, void *be)
>>       options.net_backend = ncs;
>>       options.opaque      = be;
>>       options.busyloop_timeout = 0;
>> -    options.nvqs = 2;
>> +    options.nvqs = nvqs;
>>
>>       net = vhost_net_init(&options);
>>       if (!net) {
>> @@ -159,18 +163,28 @@ static NetClientInfo net_vhost_vdpa_info = {
>>   static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
>>                                              const char *device,
>>                                              const char *name,
>> -                                           int vdpa_device_fd)
>> +                                           int vdpa_device_fd,
>> +                                           int qp_index,
>> +                                           int nvqs,
>> +                                           bool is_datapath)
>>   {
>>       NetClientState *nc = NULL;
>>       VhostVDPAState *s;
>>       int ret = 0;
>>       assert(name);
>> -    nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device, name);
>> +    if (is_datapath) {
>> +        nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
>> +                                 name);
>> +    } else {
>> +        nc = qemu_new_net_control_client(&net_vhost_vdpa_info, peer,
>> +                                         device, name);
>> +    }
>>       snprintf(nc->info_str, sizeof(nc->info_str), TYPE_VHOST_VDPA);
>>       s = DO_UPCAST(VhostVDPAState, nc, nc);
>>
>>       s->vhost_vdpa.device_fd = vdpa_device_fd;
>> -    ret = vhost_vdpa_add(nc, (void *)&s->vhost_vdpa);
>> +    s->vhost_vdpa.index = qp_index;
>> +    ret = vhost_vdpa_add(nc, (void *)&s->vhost_vdpa, qp_index, nvqs);
>>       if (ret) {
>>           qemu_del_net_client(nc);
>>           return NULL;
>> @@ -196,12 +210,52 @@ static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
>>       return 0;
>>   }
>>
>> +static int vhost_vdpa_get_max_qps(int fd, int *has_cvq, Error **errp)
>> +{
>> +    unsigned long config_size = offsetof(struct vhost_vdpa_config, buf);
>> +    struct vhost_vdpa_config *config;
>> +    __virtio16 *max_qps;
>> +    uint64_t features;
>> +    int ret;
>> +
>> +    ret = ioctl(fd, VHOST_GET_FEATURES, &features);
>> +    if (ret) {
>> +        error_setg(errp, "Fail to query features from vhost-vDPA device");
>> +        return ret;
>> +    }
>> +
>> +    if (features & (1 << VIRTIO_NET_F_CTRL_VQ)) {
>> +        *has_cvq = 1;
>> +    } else {
>> +        *has_cvq = 0;
>> +    }
>> +
>> +    if (features & (1 << VIRTIO_NET_F_MQ)) {
>> +        config = g_malloc0(config_size + sizeof(*max_qps));
>> +        config->off = offsetof(struct virtio_net_config, max_virtqueue_pairs);
>> +        config->len = sizeof(*max_qps);
>> +
>> +        ret = ioctl(fd, VHOST_VDPA_GET_CONFIG, config);
>> +        if (ret) {
>> +            error_setg(errp, "Fail to get config from vhost-vDPA device");
>> +            return -ret;
>> +        }
>> +
>> +        max_qps = (__virtio16 *)&config->buf;
>> +
>> +        return lduw_le_p(max_qps);
>> +    }
>> +
>> +    return 1;
>> +}
>> +
>>   int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
>>                           NetClientState *peer, Error **errp)
>>   {
>>       const NetdevVhostVDPAOptions *opts;
>>       int vdpa_device_fd;
>> -    NetClientState *nc;
>> +    NetClientState **ncs, *nc;
>> +    int qps, i, has_cvq = 0;
>>
>>       assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA);
>>       opts = &netdev->u.vhost_vdpa;
>> @@ -216,11 +270,37 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
>>           return -errno;
>>       }
>>
>> -    nc = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, vdpa_device_fd);
>> -    if (!nc) {
>> +    qps = vhost_vdpa_get_max_qps(vdpa_device_fd, &has_cvq, errp);
>> +    if (qps < 0) {
>>           qemu_close(vdpa_device_fd);
>> -        return -1;
>> +        return qps;
>> +    }
>> +
>> +    ncs = g_malloc0(sizeof(*ncs) * qps);
>> +
>> +    for (i = 0; i < qps; i++) {
>> +        ncs[i] = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name,
>> +                                     vdpa_device_fd, i, 2, true);
>> +        if (!ncs[i])
>> +            goto err;
>>       }
>>
>> +    if (has_cvq) {
>> +        nc = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name,
>> +                                 vdpa_device_fd, i, 1, false);
>> +        if (!nc)
>> +            goto err;
>> +    }
>> +
>> +    g_free(ncs);
>>       return 0;
>> +
>> +err:
>> +    if (i) {
>> +        qemu_del_net_client(ncs[0]);
>> +    }
>> +    qemu_close(vdpa_device_fd);
>> +    g_free(ncs);
>> +
>> +    return -1;
>>   }
>> --
>> 2.25.1
>>