| Message ID | 20100111172217.GJ11936@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 01/11/2010 11:22 AM, Michael S. Tsirkin wrote: > Looks like order got mixed up: vhost_net header > is added by a follow-up patch. Will be fixed > in the next revision. > > Signed-off-by: Michael S. Tsirkin<mst@redhat.com> > --- > net.c | 8 ++++++++ > net/tap.c | 29 +++++++++++++++++++++++++++++ > qemu-options.hx | 4 +++- > 3 files changed, 40 insertions(+), 1 deletions(-) > > diff --git a/net.c b/net.c > index 6ef93e6..b942d03 100644 > --- a/net.c > +++ b/net.c > @@ -976,6 +976,14 @@ static struct { > .name = "vnet_hdr", > .type = QEMU_OPT_BOOL, > .help = "enable the IFF_VNET_HDR flag on the tap interface" > + }, { > + .name = "vhost", > + .type = QEMU_OPT_BOOL, > + .help = "enable vhost-net network accelerator", > + }, { > + .name = "vhostfd", > + .type = QEMU_OPT_STRING, > + .help = "file descriptor of an already opened vhost net device", > }, > Semantically, I think making vhost it's own backend makes more sense from a user perspective. I don't think it's a significant code change. Regards, Anthony Liguori
On 01/11/2010 11:22 AM, Michael S. Tsirkin wrote: > Looks like order got mixed up: vhost_net header > is added by a follow-up patch. Will be fixed > in the next revision. > > Signed-off-by: Michael S. Tsirkin<mst@redhat.com> > --- > net.c | 8 ++++++++ > net/tap.c | 29 +++++++++++++++++++++++++++++ > qemu-options.hx | 4 +++- > 3 files changed, 40 insertions(+), 1 deletions(-) > > diff --git a/net.c b/net.c > index 6ef93e6..b942d03 100644 > --- a/net.c > +++ b/net.c > @@ -976,6 +976,14 @@ static struct { > .name = "vnet_hdr", > .type = QEMU_OPT_BOOL, > .help = "enable the IFF_VNET_HDR flag on the tap interface" > + }, { > + .name = "vhost", > + .type = QEMU_OPT_BOOL, > + .help = "enable vhost-net network accelerator", > + }, { > + .name = "vhostfd", > + .type = QEMU_OPT_STRING, > + .help = "file descriptor of an already opened vhost net device", > }, > #endif /* _WIN32 */ > { /* end of list */ } > diff --git a/net/tap.c b/net/tap.c > index 7e9ca79..d9f2e41 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -41,6 +41,8 @@ > > #include "net/tap-linux.h" > > +#include "hw/vhost_net.h" > + > /* Maximum GSO packet size (64k) plus plenty of room for > * the ethernet and virtio_net headers > */ > @@ -57,6 +59,7 @@ typedef struct TAPState { > unsigned int has_vnet_hdr : 1; > unsigned int using_vnet_hdr : 1; > unsigned int has_ufo: 1; > + struct vhost_net *vhost_net; > } TAPState; > > static int launch_script(const char *setup_script, const char *ifname, int fd); > @@ -252,6 +255,10 @@ static void tap_cleanup(VLANClientState *nc) > { > TAPState *s = DO_UPCAST(TAPState, nc, nc); > > + if (s->vhost_net) { > + vhost_net_cleanup(s->vhost_net); > + } > + > qemu_purge_queued_packets(nc); > > if (s->down_script[0]) > @@ -307,6 +314,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan, > s->has_ufo = tap_probe_has_ufo(s->fd); > tap_set_offload(&s->nc, 0, 0, 0, 0, 0); > tap_read_poll(s, 1); > + s->vhost_net = NULL; > return s; > } > > @@ -456,6 +464,27 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan > } > } > > + if (qemu_opt_get_bool(opts, "vhost", 0)) { > + int vhostfd, r; > + if (qemu_opt_get(opts, "vhostfd")) { > + r = net_handle_fd_param(mon, qemu_opt_get(opts, "vhostfd")); > + if (r == -1) { > + return -1; > + } > + vhostfd = r; > + } else { > + vhostfd = -1; > + } > + s->vhost_net = vhost_net_init(&s->nc, vhostfd); > Hrm, I don't see a patch that introduces this function? Am I missing something obvious? Regards, Anthony Liguori
On Tue, Jan 12, 2010 at 04:39:52PM -0600, Anthony Liguori wrote: > On 01/11/2010 11:22 AM, Michael S. Tsirkin wrote: >> Looks like order got mixed up: vhost_net header >> is added by a follow-up patch. Will be fixed >> in the next revision. >> >> Signed-off-by: Michael S. Tsirkin<mst@redhat.com> >> --- >> net.c | 8 ++++++++ >> net/tap.c | 29 +++++++++++++++++++++++++++++ >> qemu-options.hx | 4 +++- >> 3 files changed, 40 insertions(+), 1 deletions(-) >> >> diff --git a/net.c b/net.c >> index 6ef93e6..b942d03 100644 >> --- a/net.c >> +++ b/net.c >> @@ -976,6 +976,14 @@ static struct { >> .name = "vnet_hdr", >> .type = QEMU_OPT_BOOL, >> .help = "enable the IFF_VNET_HDR flag on the tap interface" >> + }, { >> + .name = "vhost", >> + .type = QEMU_OPT_BOOL, >> + .help = "enable vhost-net network accelerator", >> + }, { >> + .name = "vhostfd", >> + .type = QEMU_OPT_STRING, >> + .help = "file descriptor of an already opened vhost net device", >> }, >> > > Semantically, I think making vhost it's own backend makes more sense > from a user perspective. It doesn't. Users mostly do not care that vhost is used: they just get fast virtio and that's all. Users do care about e.g. tap because they need setup scripts, understand bridging etc. Do you propose -net tap be replaced with -net vhost? This means vhost will need to get tap flags if it is attached to tap and raw flags if attached to raw, etc. A separate backend that only works with virtio frontend is also ugly. OTOH an option that has effect only with virtio frontend is pretty usual, vnet_hdr is one such example. > > I don't think it's a significant code change. > > Regards, > > Anthony Liguori
diff --git a/net.c b/net.c index 6ef93e6..b942d03 100644 --- a/net.c +++ b/net.c @@ -976,6 +976,14 @@ static struct { .name = "vnet_hdr", .type = QEMU_OPT_BOOL, .help = "enable the IFF_VNET_HDR flag on the tap interface" + }, { + .name = "vhost", + .type = QEMU_OPT_BOOL, + .help = "enable vhost-net network accelerator", + }, { + .name = "vhostfd", + .type = QEMU_OPT_STRING, + .help = "file descriptor of an already opened vhost net device", }, #endif /* _WIN32 */ { /* end of list */ } diff --git a/net/tap.c b/net/tap.c index 7e9ca79..d9f2e41 100644 --- a/net/tap.c +++ b/net/tap.c @@ -41,6 +41,8 @@ #include "net/tap-linux.h" +#include "hw/vhost_net.h" + /* Maximum GSO packet size (64k) plus plenty of room for * the ethernet and virtio_net headers */ @@ -57,6 +59,7 @@ typedef struct TAPState { unsigned int has_vnet_hdr : 1; unsigned int using_vnet_hdr : 1; unsigned int has_ufo: 1; + struct vhost_net *vhost_net; } TAPState; static int launch_script(const char *setup_script, const char *ifname, int fd); @@ -252,6 +255,10 @@ static void tap_cleanup(VLANClientState *nc) { TAPState *s = DO_UPCAST(TAPState, nc, nc); + if (s->vhost_net) { + vhost_net_cleanup(s->vhost_net); + } + qemu_purge_queued_packets(nc); if (s->down_script[0]) @@ -307,6 +314,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan, s->has_ufo = tap_probe_has_ufo(s->fd); tap_set_offload(&s->nc, 0, 0, 0, 0, 0); tap_read_poll(s, 1); + s->vhost_net = NULL; return s; } @@ -456,6 +464,27 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan } } + if (qemu_opt_get_bool(opts, "vhost", 0)) { + int vhostfd, r; + if (qemu_opt_get(opts, "vhostfd")) { + r = net_handle_fd_param(mon, qemu_opt_get(opts, "vhostfd")); + if (r == -1) { + return -1; + } + vhostfd = r; + } else { + vhostfd = -1; + } + s->vhost_net = vhost_net_init(&s->nc, vhostfd); + if (!s->vhost_net) { + qemu_error("vhost-net requested but could not be initialized\n"); + return -1; + } + } else if (qemu_opt_get(opts, "vhostfd")) { + qemu_error("vhostfd= is not valid without vhost\n"); + return -1; + } + if (vlan) { vlan->nb_host_devs++; } diff --git a/qemu-options.hx b/qemu-options.hx index ecd50eb..ef185dd 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -813,7 +813,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net, "-net tap[,vlan=n][,name=str],ifname=name\n" " connect the host TAP network interface to VLAN 'n'\n" #else - "-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off]\n" + "-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h]\n" " connect the host TAP network interface to VLAN 'n' and use the\n" " network scripts 'file' (default=%s)\n" " and 'dfile' (default=%s);\n" @@ -823,6 +823,8 @@ DEF("net", HAS_ARG, QEMU_OPTION_net, " default of 'sndbuf=1048576' can be disabled using 'sndbuf=0'\n" " use vnet_hdr=off to avoid enabling the IFF_VNET_HDR tap flag; use\n" " vnet_hdr=on to make the lack of IFF_VNET_HDR support an error condition\n" + " use vhost=on to enable experimental in kernel accelerator\n" + " use 'vhostfd=h' to connect to an already opened vhost net device\n" #endif "-net socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connect=host:port]\n" " connect the vlan 'n' to another VLAN using a socket connection\n"
Looks like order got mixed up: vhost_net header is added by a follow-up patch. Will be fixed in the next revision. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- net.c | 8 ++++++++ net/tap.c | 29 +++++++++++++++++++++++++++++ qemu-options.hx | 4 +++- 3 files changed, 40 insertions(+), 1 deletions(-)