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

[Nikolay Nikolaev]

Hello,


On Tue, Jun 10, 2014 at 1:50 PM, Michael S. Tsirkin <address@hidden> wrote:
>
> On Tue, Jun 10, 2014 at 01:34:13PM +0300, Michael S. Tsirkin wrote:
> > On Tue, Jun 10, 2014 at 01:02:16PM +0300, Nikolay Nikolaev 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>
> > > ---
> > >
> > Applied, thanks!
> > In the future pls just number them 0-18.
> > Also in the future, pls include the changelog here after ---
>
> Please check out and test the vhost branch, if everything is
> in order let me know and I'll merge this upstream.
>
This patchseries requires the NUMA patches (I see v4 is underway). It
is not working from your tree.
I wrote yesterday in reply to your question - memdev is not enough. We
want to enable memory sharing on HUGETLBFS and this is not available
on this branch. In effect vhost-user is compiled but it can not be
used. Probably you were misled by mentioning 'memdev' in the
changelog, while it shoudl have been NUMA.


>
> > >  hw/net/vhost_net.c |    4 ++
> > >  net/hub.c          |    1
> > >  net/net.c          |    3 +
> > >  net/vhost-user.c   |  115 
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  qapi-schema.json   |   19 ++++++++-
> > >  qemu-options.hx    |   18 ++++++++
> > >  7 files changed, 158 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > > index 2e462c0..bc9c032 100644
> > > --- a/hmp-commands.hx
> > > +++ b/hmp-commands.hx
> > > @@ -1206,7 +1206,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,
> > >      },
> > > @@ -1234,7 +1234,7 @@ ETEXI
> > >      {
> > >          .name       = "netdev_add",
> > >          .args_type  = "netdev:O",
> > > -        .params     = 
> > > "[user|tap|socket|vde|bridge|hubport|netmap],id=str[,prop=value][,...]",
> > > +        .params     = 
> > > "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
> > >          .help       = "add host network device",
> > >          .mhandler.cmd = hmp_netdev_add,
> > >          .command_completion = netdev_add_completion,
> > > 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 0ff2e40..c91b67b 100644
> > > --- a/net/net.c
> > > +++ b/net/net.c
> > > @@ -786,6 +786,7 @@ static int (* const 
> > > net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
> > >          [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
> > >  #endif
> > >          [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:
> > > @@ -907,6 +909,7 @@ static int net_host_check_device(const char *device)
> > >  #ifdef CONFIG_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..32b78fb 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,108 @@ 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 = true;
> > > +    } else if (strcmp(name, "path") == 0) {
> > > +        props->is_unix = true;
> > > +    } else if (strcmp(name, "server") == 0) {
> > > +        props->is_server = true;
> > > +    } else {
> > > +        error_report("vhost-user does not support a chardev"
> > > +                     " with the following option:\n %s = %s",
> > > +                     name, value);
> > > +        props->has_unsupported = true;
> > > +        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", opts->chardev);
> > > +        return NULL;
> > > +    }
> > > +
> > > +    /* 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",
> > > +                     opts->chardev);
> > > +        return NULL;
> > > +    }
> > > +
> > > +    if (props.has_unsupported) {
> > > +        error_report("chardev \"%s\" has an unsupported option",
> > > +                opts->chardev);
> > > +        return NULL;
> > > +    }
> > > +
> > > +    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)
> > > +                        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");
> > > +        return -1;
> > > +    }
> > > +
> > > +    /* verify net frontend */
> > > +    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> > > +                          (char *)name, true) == -1) {
> > > +        return -1;
> > > +    }
> > > +
> > > +    /* vhost-force for non-MSIX */
> > > +    if (vhost_user_opts->has_vhost_force) {
> > > +        vhostforce = vhost_user_opts->vhost_force;
> > > +    } 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 7bc33ea..f062ce9 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -3267,6 +3267,22 @@
> > >      '*devname':    'str' } }
> > >
> > >  ##
> > > +# @NetdevVhostUserOptions
> > > +#
> > > +# Vhost-user network backend
> > > +#
> > > +# @chardev: name of a unix socket chardev
> > > +#
> > > +# @vhost-force: #optional vhost on for non-MSIX virtio guests (default: 
> > > false).
> > > +#
> > > +# Since 2.1
> > > +##
> > > +{ 'type': 'NetdevVhostUserOptions',
> > > +  'data': {
> > > +    'chardev':        'str',
> > > +    '*vhost-force':    'bool' } }
> > > +
> > > +##
> > >  # @NetClientOptions
> > >  #
> > >  # A discriminated record of network device traits.
> > > @@ -3284,7 +3300,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 5a4eff9..1ad2528 100644
> > > --- a/qemu-options.hx
> > > +++ b/qemu-options.hx
> > > @@ -1460,6 +1460,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
> > > @@ -1791,6 +1792,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 \
Here we give and example and rely on NUMA,memdev, memory-file and share=on.

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

Once again, I'm sorry if you were misled by seeing 'memdev' in the
changelog, it's a confusion on our side.
Please advice how to proceed.

regards,
Nikolay Nikolaev