Re: [RFC v1 2/4] vhost-vdpa: introduce vhost-vdpa net client

[Laurent Vivier <lvivier@redhat.com>]

On 20/04/2020 11:32, Cindy Lu wrote:
> This patch set introduces a new net client type: vhost-vdpa.
> vhost-vdpa net client will set up a vDPA device which is svhostdevpecified
> by a "vhostdev" parameter.
> 
> Author: Tiwei Bie
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  include/net/vhost-vdpa.h |  18 ++++
>  include/net/vhost_net.h  |   1 +
>  net/Makefile.objs        |   2 +-
>  net/clients.h            |   2 +
>  net/net.c                |   1 +
>  net/vhost-vdpa.c         | 211 +++++++++++++++++++++++++++++++++++++++
>  qapi/net.json            |  21 +++-
>  7 files changed, 253 insertions(+), 3 deletions(-)
>  create mode 100644 include/net/vhost-vdpa.h
>  create mode 100644 net/vhost-vdpa.c
> 
> diff --git a/include/net/vhost-vdpa.h b/include/net/vhost-vdpa.h
> new file mode 100644
> index 0000000000..9ddd538dad
> --- /dev/null
> +++ b/include/net/vhost-vdpa.h
> @@ -0,0 +1,18 @@
> +/*
> + * vhost-vdpa.h
> + *
> + * Copyright(c) 2017 Intel Corporation. All rights reserved.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef VHOST_VDPA_H
> +#define VHOST_VDPA_H
> +
> +struct vhost_net;
> +struct vhost_net *vhost_vdpa_get_vhost_net(NetClientState *nc);
> +uint64_t vhost_vdpa_get_acked_features(NetClientState *nc);
> +
> +#endif /* VHOST_VDPA_H */
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index 77e47398c4..6f3a624cf7 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -39,5 +39,6 @@ int vhost_set_vring_enable(NetClientState * nc, int enable);
>  uint64_t vhost_net_get_acked_features(VHostNetState *net);
>  
>  int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
> +int vhost_set_state(NetClientState *nc, int state);
>  
>  #endif
> diff --git a/net/Makefile.objs b/net/Makefile.objs
> index c5d076d19c..da459cfc19 100644
> --- a/net/Makefile.objs
> +++ b/net/Makefile.objs
> @@ -26,7 +26,7 @@ tap-obj-$(CONFIG_SOLARIS) = tap-solaris.o
>  tap-obj-y ?= tap-stub.o
>  common-obj-$(CONFIG_POSIX) += tap.o $(tap-obj-y)
>  common-obj-$(CONFIG_WIN32) += tap-win32.o
> -
> +common-obj-$(CONFIG_VHOST_KERNEL) += vhost-vdpa.o

should it be CONFIG_VHOST_NET_USER as you use net_init_vhost_vdpa()
below inside a "#ifdef CONFIG_VHOST_NET_USER"?

Why don't you define a CONFIG_VHOST_VDPA?

>  vde.o-libs = $(VDE_LIBS)
>  
>  common-obj-$(CONFIG_CAN_BUS) += can/
> diff --git a/net/clients.h b/net/clients.h
> index a6ef267e19..92f9b59aed 100644
> --- a/net/clients.h
> +++ b/net/clients.h
> @@ -61,4 +61,6 @@ int net_init_netmap(const Netdev *netdev, const char *name,
>  int net_init_vhost_user(const Netdev *netdev, const char *name,
>                          NetClientState *peer, Error **errp);
>  
> +int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
> +                        NetClientState *peer, Error **errp);
>  #endif /* QEMU_NET_CLIENTS_H */
> diff --git a/net/net.c b/net/net.c
> index ac5080dda1..2beb95388a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -964,6 +964,7 @@ static int (* const net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
>          [NET_CLIENT_DRIVER_HUBPORT]   = net_init_hubport,
>  #ifdef CONFIG_VHOST_NET_USER          ^^^^^^^^^^^^^^^^^^^^^
                   here

>          [NET_CLIENT_DRIVER_VHOST_USER] = net_init_vhost_user,
> +        [NET_CLIENT_DRIVER_VHOST_VDPA] = net_init_vhost_vdpa,
>  #endif
>  #ifdef CONFIG_L2TPV3
>          [NET_CLIENT_DRIVER_L2TPV3]    = net_init_l2tpv3,
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> new file mode 100644
> index 0000000000..5daeba0b76
> --- /dev/null
> +++ b/net/vhost-vdpa.c
...
> +static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
> +{
> +    const char *name = opaque;
> +    const char *driver, *netdev;
> +
> +    driver = qemu_opt_get(opts, "driver");
> +    netdev = qemu_opt_get(opts, "netdev");
> +
> +    if (!driver || !netdev) {
> +        return 0;
> +    }
> +
> +    if (strcmp(netdev, name) == 0 &&
> +        !g_str_has_prefix(driver, "virtio-net-")) {
> +        error_setg(errp, "vhost-vdpa requires frontend driver virtio-net-*");
> +        return -1;
> +    }
>

So perhaps you can build the file only if CONFIG_VIRTIO_NET is set?

Thanks,
Laurent