qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 15/18] Add the vhost-user


From: Nikolay Nikolaev
Subject: Re: [Qemu-devel] [snabb-devel] Re: [PATCH v10 15/18] Add the vhost-user netdev backend to the command line
Date: Mon, 9 Jun 2014 16:28:23 +0300

Hello,


On Thu, Jun 5, 2014 at 5:37 PM, Luiz Capitulino <address@hidden> wrote:
On Tue, 27 May 2014 15:06:43 +0300
Nikolay Nikolaev <address@hidden> wrote:

> The supplied chardev id will be inspected for supported options. Only
> a socket backend, with a set path (i.e. a Unix socket) and optionally
> the server parameter set, will be allowed. Other options (nowait, telnet)
> will make the chardev unusable and the netdev will not be initialised.
>
> Additional checks for validity:
>   - requires `-numa node,memdev=..`
>   - requires `-device virtio-net-*`
>
> The `vhostforce` option is used to force vhost-net when we deal with
> non-MSIX guests.
>
> Signed-off-by: Antonios Motakis <address@hidden>
> Signed-off-by: Nikolay Nikolaev <address@hidden>

I gave a quick review and apart from some minor comments below it seems
good to me, but I think it would be good to have Eric's review too:

Acked-by: Luiz Capitulino <address@hidden>
Thanks!

> ---
>  hmp-commands.hx    |    4 +-
>  hw/net/vhost_net.c |    4 ++
>  net/hub.c          |    1
>  net/net.c          |   25 ++++++-----
>  net/vhost-user.c   |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json   |   19 ++++++++-
>  qemu-options.hx    |   18 ++++++++
>  7 files changed, 169 insertions(+), 16 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8971f1b..ef3782c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1205,7 +1205,7 @@ ETEXI
>      {
>          .name       = "host_net_add",
>          .args_type  = "device:s,opts:s?",
> -        .params     = "tap|user|socket|vde|netmap|dump [options]",
> +        .params     = "tap|user|socket|vde|netmap|vhost-user|dump [options]",
>          .help       = "add host VLAN client",
>          .mhandler.cmd = net_host_device_add,
>      },
> @@ -1233,7 +1233,7 @@ ETEXI
>      {
>          .name       = "netdev_add",
>          .args_type  = "netdev:O",
> -        .params     = "[user|tap|socket|hubport|netmap],id=str[,prop=value][,...]",
> +        .params     = "[user|tap|socket|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
>          .help       = "add host network device",
>          .mhandler.cmd = hmp_netdev_add,
>      },
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 5f06736..7ac7c21 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -15,6 +15,7 @@
>
>  #include "net/net.h"
>  #include "net/tap.h"
> +#include "net/vhost-user.h"
>
>  #include "hw/virtio/virtio-net.h"
>  #include "net/vhost_net.h"
> @@ -360,6 +361,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>      case NET_CLIENT_OPTIONS_KIND_TAP:
>          vhost_net = tap_get_vhost_net(nc);
>          break;
> +    case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> +        vhost_net = vhost_user_get_vhost_net(nc);
> +        break;
>      default:
>          break;
>      }
> diff --git a/net/hub.c b/net/hub.c
> index 33a99c9..7e0f2d6 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -322,6 +322,7 @@ void net_hub_check_clients(void)
>              case NET_CLIENT_OPTIONS_KIND_TAP:
>              case NET_CLIENT_OPTIONS_KIND_SOCKET:
>              case NET_CLIENT_OPTIONS_KIND_VDE:
> +            case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>                  has_host_dev = 1;
>                  break;
>              default:
> diff --git a/net/net.c b/net/net.c
> index 9db4dba..907f679 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -769,23 +769,24 @@ static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
>      const NetClientOptions *opts,
>      const char *name,
>      NetClientState *peer) = {
> -        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
> +        [NET_CLIENT_OPTIONS_KIND_NIC]           = net_init_nic,
>  #ifdef CONFIG_SLIRP
> -        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
> +        [NET_CLIENT_OPTIONS_KIND_USER]          = net_init_slirp,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
> -        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
> +        [NET_CLIENT_OPTIONS_KIND_TAP]           = net_init_tap,
> +        [NET_CLIENT_OPTIONS_KIND_SOCKET]        = net_init_socket,
>  #ifdef CONFIG_VDE
> -        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
> +        [NET_CLIENT_OPTIONS_KIND_VDE]           = net_init_vde,
>  #endif
>  #ifdef CONFIG_NETMAP
> -        [NET_CLIENT_OPTIONS_KIND_NETMAP]    = net_init_netmap,
> +        [NET_CLIENT_OPTIONS_KIND_NETMAP]        = net_init_netmap,
>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
> +        [NET_CLIENT_OPTIONS_KIND_DUMP]          = net_init_dump,
>  #ifdef CONFIG_NET_BRIDGE
> -        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> +        [NET_CLIENT_OPTIONS_KIND_BRIDGE]        = net_init_bridge,

These changes are unrelated.
OK. Removing them.

>  #endif
> -        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
> +        [NET_CLIENT_OPTIONS_KIND_HUBPORT]       = net_init_hubport,
> +        [NET_CLIENT_OPTIONS_KIND_VHOST_USER]    = net_init_vhost_user,
>  };
>
>
> @@ -819,6 +820,7 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          case NET_CLIENT_OPTIONS_KIND_BRIDGE:
>  #endif
>          case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> +        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
>              break;
>
>          default:
> @@ -902,11 +904,12 @@ static int net_host_check_device(const char *device)
>                                         , "bridge"
>  #endif
>  #ifdef CONFIG_SLIRP
> -                                       ,"user"
> +                                       , "user"
>  #endif
>  #ifdef CONFIG_VDE
> -                                       ,"vde"
> +                                       , "vde"
>  #endif
> +                                       , "vhost-user"
>      };
>      for (i = 0; i < ARRAY_SIZE(valid_param_list); i++) {
>          if (!strncmp(valid_param_list[i], device,
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 4bdd19d..69a5eb4 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -12,6 +12,7 @@
>  #include "net/vhost_net.h"
>  #include "net/vhost-user.h"
>  #include "sysemu/char.h"
> +#include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>
>  typedef struct VhostUserState {
> @@ -21,9 +22,17 @@ typedef struct VhostUserState {
>      VHostNetState *vhost_net;
>  } VhostUserState;
>
> +typedef struct VhostUserChardevProps {
> +    bool is_socket;
> +    bool is_unix;
> +    bool is_server;
> +    bool has_unsupported;
> +} VhostUserChardevProps;
> +
>  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
>  {
>      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> +    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
>      return s->vhost_net;
>  }
>
> @@ -82,7 +91,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
>  }
>
>  static NetClientInfo net_vhost_user_info = {
> -        .type = 0,
> +        .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
>          .size = sizeof(VhostUserState),
>          .cleanup = vhost_user_cleanup,
>          .has_vnet_hdr = vhost_user_has_vnet_hdr,
> @@ -148,8 +157,109 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
>      return 0;
>  }
>
> +static int net_vhost_chardev_opts(const char *name, const char *value,
> +        void *opaque)
> +{
> +    VhostUserChardevProps *props = opaque;
> +
> +    if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) {
> +        props->is_socket = 1;
> +    } else if (strcmp(name, "path") == 0) {
> +        props->is_unix = 1;
> +    } else if (strcmp(name, "server") == 0) {
> +        props->is_server = 1;
> +    } else {
> +        error_report("vhost-user does not support a chardev"
> +                     " with the following option:\n %s = %s",
> +                     name, value);
> +        props->has_unsupported = 1;
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static CharDriverState *net_vhost_parse_chardev(
> +        const NetdevVhostUserOptions *opts)
> +{
> +    CharDriverState *chr = qemu_chr_find(opts->chardev);
> +    VhostUserChardevProps props;
> +
> +    if (chr == NULL) {
> +        error_report("chardev \"%s\" not found\n", opts->chardev);
> +        return 0;
> +    }
> +
> +    /* inspect chardev opts */
> +    memset(&props, 0, sizeof(props));
> +    qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, false);
> +
> +    if (!props.is_socket || !props.is_unix) {
> +        error_report("chardev \"%s\" is not a unix socket\n",
> +                     opts->chardev);
> +        return 0;
> +    }
> +
> +    if (props.has_unsupported) {
> +        error_report("chardev \"%s\" has an unsupported option\n",
> +                opts->chardev);
> +        return 0;
> +    }
> +
> +    qemu_chr_fe_claim_no_fail(chr);
> +
> +    return chr;
> +}
> +
> +static int net_vhost_check_net(QemuOpts *opts, void *opaque)
> +{
> +    const char *name = opaque;
> +    const char *driver, *netdev;
> +    const char virtio_name[] = "virtio-net-";
> +
> +    driver = qemu_opt_get(opts, "driver");
> +    netdev = qemu_opt_get(opts, "netdev");
> +
> +    if (!driver || !netdev) {
> +        return 0;
> +    }
> +
> +    if ((strcmp(netdev, name) == 0)
> +            && (strncmp(driver, virtio_name, strlen(virtio_name)) != 0)) {
> +        error_report("vhost-user requires frontend driver virtio-net-*");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  int net_init_vhost_user(const NetClientOptions *opts, const char *name,
>                     NetClientState *peer)
>  {
> -    return net_vhost_user_init(peer, "vhost_user", 0, 0, 0);
> +    const NetdevVhostUserOptions *vhost_user_opts;
> +    CharDriverState *chr;
> +    bool vhostforce;
> +
> +    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
> +    vhost_user_opts = opts->vhost_user;
> +
> +    chr = net_vhost_parse_chardev(vhost_user_opts);
> +    if (!chr) {
> +        error_report("No suitable chardev found\n");
> +        return -1;
> +    }
> +
> +    /* verify net frontend */
> +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> +                          (void *)name, true) == -1) {
> +        return -1;
> +    }
> +
> +    /* vhostforce for non-MSIX */
> +    if (vhost_user_opts->has_vhostforce) {
> +        vhostforce = vhost_user_opts->vhostforce;
> +    } else {
> +        vhostforce = false;
> +    }
> +
> +    return net_vhost_user_init(peer, "vhost_user", name, chr, vhostforce);
>  }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 1f28177..f458dd8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3264,6 +3264,22 @@
>      '*devname':    'str' } }
>
>  ##
> +# @NetdevVhostUserOptions
> +#
> +# Vhost-user network backend
> +#
> +# @path: control socket path
> +#
> +# @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
> +#
> +# Since 2.1
> +##
> +{ 'type': 'NetdevVhostUserOptions',
> +  'data': {
> +    'chardev':        'str',

chardev or path?
Right, it's chardev.

> +    '*vhostforce':    'bool' } }
> +
> +##
>  # @NetClientOptions
>  #
>  # A discriminated record of network device traits.
> @@ -3281,7 +3297,8 @@
>      'dump':     'NetdevDumpOptions',
>      'bridge':   'NetdevBridgeOptions',
>      'hubport':  'NetdevHubPortOptions',
> -    'netmap':   'NetdevNetmapOptions' } }
> +    'netmap':   'NetdevNetmapOptions',
> +    'vhost-user': 'NetdevVhostUserOptions' } }
>
>  ##
>  # @NetLegacy
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 7f4ab83..2514264 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1459,6 +1459,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>  #ifdef CONFIG_NETMAP
>      "netmap|"
>  #endif
> +    "vhost-user|"
>      "socket|"
>      "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
>  STEXI
> @@ -1790,6 +1791,23 @@ The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
>  netdev. address@hidden and @code{-device} with parameter @option{vlan} create the
>  required hub automatically.
>
> address@hidden -netdev vhost-user,address@hidden,vhostforce=on|off]
> +
> +Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
> +be a unix domain socket backed one. The vhost-user uses a specifically defined
> +protocol to pass vhost ioctl replacement messages to an application on the other
> +end of the socket. On non-MSIX guests, the feature can be forced with
> address@hidden
> +
> +Example:
> address@hidden
> +qemu -m 512 -object memory-file,id=mem,size=512M,mem-path=/hugetlbfs,share=on \
> +     -numa node,memdev=mem \
> +     -chardev socket,path=/path/to/socket \
> +     -netdev type=vhost-user,id=net0,chardev=chr0 \
> +     -device virtio-net-pci,netdev=net0
> address@hidden example
> +
> address@hidden -net dump[,address@hidden,address@hidden,address@hidden
>  Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu-vlan0.pcap} by default).
>  At most @var{len} bytes (64k by default) per packet are stored. The file format is
>
>

--
You received this message because you are subscribed to the Google Groups "Snabb Switch development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to address@hidden.
To post to this group, send an email to address@hidden.
Visit this group at http://groups.google.com/group/snabb-devel.

regards,
Nikolay Nikolaev

reply via email to

[Prev in Thread] Current Thread [Next in Thread]