qemu-devel
[Top][All Lists]
Advanced

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

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


From: Luiz Capitulino
Subject: Re: [Qemu-devel] [PATCH v10 15/18] Add the vhost-user netdev backend to the command line
Date: Thu, 5 Jun 2014 10:37:10 -0400

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>

> ---
>  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.

>  #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?

> +    '*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.  @code{-net} 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
> +
>  @item -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
> 
> 




reply via email to

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