[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: |
Michael S. Tsirkin |
|
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:31:59 +0300 |
On Mon, Jun 09, 2014 at 04:28:23PM +0300, Nikolay Nikolaev wrote:
> 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
Pls remember to base on top of vhost branch in my tree,
this way you will not need to re-post merged patches.
If you want me to drop some merged patch from my tree,
include a revert and I'll figure it out.
--
MST