[libvirt] [PATCH] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds
IFF_VNET_HDR is a tun/tap flag that allows you to send and receive large (i.e. GSO) packets and packets with partial checksums. Setting the flag means that every packet is proceeded by the same header which virtio uses to communicate GSO/csum metadata. By enabling this flag on the tap fds we create, we greatly increase the achievable throughput with virtio_net. However, we need to be careful to only set the flag when a) QEMU has support for this ABI and b) the value of the flag is queryable using the TUNGETIFF ioctl. It's nearly five months since kvm-74 - the first KVM release with this feature - was released. Up until now, we've not added libvirt support because there is no clean way to detect support for this in QEMU at runtime. A brief attempt to add a "info capabilities" monitor command to QEMU floundered. Perfect is the enemy of good enough. Probing the KVM version will suffice for now. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- src/bridge.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/bridge.h | 1 + src/qemu_conf.c | 33 +++++++++++++++++++------- src/qemu_conf.h | 1 + 4 files changed, 93 insertions(+), 9 deletions(-) diff --git a/src/bridge.c b/src/bridge.c index 30afa6d..9c4ca74 100644 --- a/src/bridge.c +++ b/src/bridge.c @@ -47,6 +47,7 @@ #include "internal.h" #include "memory.h" #include "util.h" +#include "logging.h" #define MAX_BRIDGE_ID 256 @@ -403,10 +404,67 @@ static int brSetInterfaceMtu(brControl *ctl, } /** + * brProbeVnetHdr: + * @tapfd: a tun/tap file descriptor + * + * Check whether it is safe to enable the IFF_VNET_HDR flag on the + * tap interface. + * + * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow + * guests to pass larger (GSO) packets, with partial checksums, to + * the host. This greatly increases the achievable throughput. + * + * It is only useful to enable this when we're setting up a virtio + * interface. And it is only *safe* to enable it when we know for + * sure that a) qemu has support for IFF_VNET_HDR and b) the running + * kernel implements the TUNGETIFF ioctl(), which qemu needs to query + * the supplied tapfd. + * + * Returns 0 in case of success or an errno code in case of failure. + */ +static int +brProbeVnetHdr(int tapfd) +{ +#if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF) + unsigned int features; + struct ifreq dummy; + + if (ioctl(tapfd, TUNGETFEATURES, &features) != 0) { + VIR_INFO0(_("Not enabling IFF_VNET_HDR; " + "TUNGETFEATURES ioctl() not implemented")); + return 0; + } + + if (!(features & IFF_VNET_HDR)) { + VIR_INFO0(_("Not enabling IFF_VNET_HDR; " + "TUNGETFEATURES ioctl() reports no IFF_VNET_HDR")); + return 0; + } + + /* The kernel will always return -1 at this point. + * If TUNGETIFF is not implemented then errno == EBADFD. + */ + if (ioctl(tapfd, TUNGETIFF, &dummy) != -1 || errno != EBADFD) { + VIR_INFO0(_("Not enabling IFF_VNET_HDR; " + "TUNGETIFF ioctl() not implemented")); + return 0; + } + + VIR_INFO0(_("Enabling IFF_VNET_HDR")); + + return 1; +#else + VIR_INFO0(_("Not enabling IFF_VNET_HDR; disabled at build time")); + return 0; +#endif +} + +/** * brAddTap: * @ctl: bridge control pointer * @bridge: the bridge name * @ifname: the interface name (or name template) + * @vnet_hdr: whether to try enabling IFF_VNET_HDR * @tapfd: file descriptor return value for the new tap device * * This function creates a new tap device on a bridge. @ifname can be either @@ -420,6 +478,7 @@ int brAddTap(brControl *ctl, const char *bridge, char **ifname, + int vnet_hdr, int *tapfd) { int id, subst, fd; @@ -435,6 +494,9 @@ brAddTap(brControl *ctl, if ((fd = open("/dev/net/tun", O_RDWR)) < 0) return errno; + if (vnet_hdr) + vnet_hdr = brProbeVnetHdr(fd); + do { struct ifreq try; int len; @@ -443,6 +505,11 @@ brAddTap(brControl *ctl, try.ifr_flags = IFF_TAP|IFF_NO_PI; +#ifdef IFF_VNET_HDR + if (vnet_hdr) + try.ifr_flags |= IFF_VNET_HDR; +#endif + if (subst) { len = snprintf(try.ifr_name, BR_IFNAME_MAXLEN, *ifname, id); if (len >= BR_IFNAME_MAXLEN) { diff --git a/src/bridge.h b/src/bridge.h index 2172b92..2491123 100644 --- a/src/bridge.h +++ b/src/bridge.h @@ -63,6 +63,7 @@ int brDeleteInterface (brControl *ctl, int brAddTap (brControl *ctl, const char *bridge, char **ifname, + int vnet_hdr, int *tapfd); int brSetInterfaceUp (brControl *ctl, diff --git a/src/qemu_conf.c b/src/qemu_conf.c index ee5cf62..bd5d414 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -353,7 +353,7 @@ int qemudExtractVersionInfo(const char *qemu, int newstdout = -1; int ret = -1, status; unsigned int major, minor, micro; - unsigned int version; + unsigned int version, kvm_version; unsigned int flags = 0; if (retflags) @@ -371,10 +371,13 @@ int qemudExtractVersionInfo(const char *qemu, if (len < 0) goto cleanup2; - if (sscanf(help, "QEMU PC emulator version %u.%u.%u", - &major, &minor, µ) != 3) { + if (sscanf(help, "QEMU PC emulator version %u.%u.%u (kvm-%u)", + &major, &minor, µ, &kvm_version) != 4) + kvm_version = 0; + + if (!kvm_version && sscanf(help, "QEMU PC emulator version %u.%u.%u", + &major, &minor, µ) != 3) goto cleanup2; - } version = (major * 1000 * 1000) + (minor * 1000) + micro; @@ -394,6 +397,8 @@ int qemudExtractVersionInfo(const char *qemu, flags |= QEMUD_CMD_FLAG_DRIVE_BOOT; if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; + if (kvm_version >= 74) + flags |= QEMUD_CMD_FLAG_VNET_HDR; if (retversion) *retversion = version; @@ -404,6 +409,8 @@ int qemudExtractVersionInfo(const char *qemu, qemudDebug("Version %d %d %d Cooked version: %d, with flags ? %d", major, minor, micro, version, flags); + if (kvm_version) + qemudDebug("KVM version %d detected", kvm_version); cleanup2: VIR_FREE(help); @@ -467,7 +474,8 @@ qemudNetworkIfaceConnect(virConnectPtr conn, int **tapfds, int *ntapfds, virDomainNetDefPtr net, - int vlan) + int vlan, + int vnet_hdr) { char *brname; char tapfdstr[4+3+32+7]; @@ -517,7 +525,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, } if ((err = brAddTap(driver->brctl, brname, - &net->ifname, &tapfd))) { + &net->ifname, vnet_hdr, &tapfd))) { if (errno == ENOTSUP) { /* In this particular case, give a better diagnostic. */ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -1029,9 +1037,16 @@ int qemudBuildCommandLine(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: { - char *tap = qemudNetworkIfaceConnect(conn, driver, - tapfds, ntapfds, - net, vlan); + char *tap; + int vnet_hdr = 0; + + if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR && + net->model && STREQ(net->model, "virtio")) + vnet_hdr = 1; + + tap = qemudNetworkIfaceConnect(conn, driver, + tapfds, ntapfds, + net, vlan, vnet_hdr); if (tap == NULL) goto error; ADD_ARG(tap); diff --git a/src/qemu_conf.h b/src/qemu_conf.h index db2491c..65b08ac 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -48,6 +48,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_NAME = (1 << 5), QEMUD_CMD_FLAG_UUID = (1 << 6), QEMUD_CMD_FLAG_DOMID = (1 << 7), /* Xenner only */ + QEMUD_CMD_FLAG_VNET_HDR = (1 << 8), }; /* Main driver state */ -- 1.6.0.6
On Mon, Jan 26, 2009 at 10:39:25AM +0000, Mark McLoughlin wrote:
IFF_VNET_HDR is a tun/tap flag that allows you to send and receive large (i.e. GSO) packets and packets with partial checksums. Setting the flag means that every packet is proceeded by the same header which virtio uses to communicate GSO/csum metadata.
Translating this for people not familiar with the intricacies of recent Linux networking changes ... GSO = generic segmentation offload. In a baremetal Linux install, the network driver can pass the job of splitting large packets over to the network card. In virtualized environments, the "network card" is, for example, a virtio backend running in the host. Because the network bridge runs entirely inside the host kernel, there are no physical limitations on packet size as there would be if it was real ethernet, so we can use this mechanism to pass over-sized packets to the host. Another advantage is that you don't need to compute checksums over the packets which are sent this way. "VNET_HDR" as far as I can gather refers to the special header that virtio_net prepends to such over-sized packets. I'm not quite clear if userspace has to add this header, but if so presumably that is done inside qemu userspace(?). Libvirt sets the flag on the socket, passes the socket by number to qemu, and qemu needs to be able to query whether the flag was set. So the patch concerns itself with making sure that all the relevant bits of this are supported. Correct me if I'm wrong here ...
By enabling this flag on the tap fds we create, we greatly increase the achievable throughput with virtio_net.
However, we need to be careful to only set the flag when a) QEMU has support for this ABI and b) the value of the flag is queryable using the TUNGETIFF ioctl.
It's nearly five months since kvm-74 - the first KVM release with this feature - was released. Up until now, we've not added libvirt support because there is no clean way to detect support for this in QEMU at runtime. A brief attempt to add a "info capabilities" monitor command to QEMU floundered. Perfect is the enemy of good enough. Probing the KVM version will suffice for now.
Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- src/bridge.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/bridge.h | 1 + src/qemu_conf.c | 33 +++++++++++++++++++------- src/qemu_conf.h | 1 + 4 files changed, 93 insertions(+), 9 deletions(-)
diff --git a/src/bridge.c b/src/bridge.c index 30afa6d..9c4ca74 100644 --- a/src/bridge.c +++ b/src/bridge.c @@ -47,6 +47,7 @@ #include "internal.h" #include "memory.h" #include "util.h" +#include "logging.h"
#define MAX_BRIDGE_ID 256
@@ -403,10 +404,67 @@ static int brSetInterfaceMtu(brControl *ctl, }
/** + * brProbeVnetHdr: + * @tapfd: a tun/tap file descriptor + * + * Check whether it is safe to enable the IFF_VNET_HDR flag on the + * tap interface. + * + * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow + * guests to pass larger (GSO) packets, with partial checksums, to + * the host. This greatly increases the achievable throughput. + * + * It is only useful to enable this when we're setting up a virtio + * interface. And it is only *safe* to enable it when we know for + * sure that a) qemu has support for IFF_VNET_HDR and b) the running + * kernel implements the TUNGETIFF ioctl(), which qemu needs to query + * the supplied tapfd. + * + * Returns 0 in case of success or an errno code in case of failure. + */ +static int +brProbeVnetHdr(int tapfd) +{ +#if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF) + unsigned int features; + struct ifreq dummy; + + if (ioctl(tapfd, TUNGETFEATURES, &features) != 0) { + VIR_INFO0(_("Not enabling IFF_VNET_HDR; " + "TUNGETFEATURES ioctl() not implemented")); + return 0; + } + + if (!(features & IFF_VNET_HDR)) { + VIR_INFO0(_("Not enabling IFF_VNET_HDR; " + "TUNGETFEATURES ioctl() reports no IFF_VNET_HDR")); + return 0; + } + + /* The kernel will always return -1 at this point. + * If TUNGETIFF is not implemented then errno == EBADFD. + */ + if (ioctl(tapfd, TUNGETIFF, &dummy) != -1 || errno != EBADFD) { + VIR_INFO0(_("Not enabling IFF_VNET_HDR; " + "TUNGETIFF ioctl() not implemented")); + return 0; + } + + VIR_INFO0(_("Enabling IFF_VNET_HDR")); + + return 1; +#else + VIR_INFO0(_("Not enabling IFF_VNET_HDR; disabled at build time")); + return 0; +#endif +} + +/** * brAddTap: * @ctl: bridge control pointer * @bridge: the bridge name * @ifname: the interface name (or name template) + * @vnet_hdr: whether to try enabling IFF_VNET_HDR * @tapfd: file descriptor return value for the new tap device * * This function creates a new tap device on a bridge. @ifname can be either @@ -420,6 +478,7 @@ int brAddTap(brControl *ctl, const char *bridge, char **ifname, + int vnet_hdr, int *tapfd) { int id, subst, fd; @@ -435,6 +494,9 @@ brAddTap(brControl *ctl, if ((fd = open("/dev/net/tun", O_RDWR)) < 0) return errno;
+ if (vnet_hdr) + vnet_hdr = brProbeVnetHdr(fd); + do { struct ifreq try; int len; @@ -443,6 +505,11 @@ brAddTap(brControl *ctl,
try.ifr_flags = IFF_TAP|IFF_NO_PI;
+#ifdef IFF_VNET_HDR + if (vnet_hdr) + try.ifr_flags |= IFF_VNET_HDR; +#endif + if (subst) { len = snprintf(try.ifr_name, BR_IFNAME_MAXLEN, *ifname, id); if (len >= BR_IFNAME_MAXLEN) { diff --git a/src/bridge.h b/src/bridge.h index 2172b92..2491123 100644 --- a/src/bridge.h +++ b/src/bridge.h @@ -63,6 +63,7 @@ int brDeleteInterface (brControl *ctl, int brAddTap (brControl *ctl, const char *bridge, char **ifname, + int vnet_hdr, int *tapfd);
int brSetInterfaceUp (brControl *ctl, diff --git a/src/qemu_conf.c b/src/qemu_conf.c index ee5cf62..bd5d414 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -353,7 +353,7 @@ int qemudExtractVersionInfo(const char *qemu, int newstdout = -1; int ret = -1, status; unsigned int major, minor, micro; - unsigned int version; + unsigned int version, kvm_version; unsigned int flags = 0;
if (retflags) @@ -371,10 +371,13 @@ int qemudExtractVersionInfo(const char *qemu, if (len < 0) goto cleanup2;
- if (sscanf(help, "QEMU PC emulator version %u.%u.%u", - &major, &minor, µ) != 3) { + if (sscanf(help, "QEMU PC emulator version %u.%u.%u (kvm-%u)", + &major, &minor, µ, &kvm_version) != 4) + kvm_version = 0; + + if (!kvm_version && sscanf(help, "QEMU PC emulator version %u.%u.%u", + &major, &minor, µ) != 3) goto cleanup2; - }
Looks good, and having the separate KVM version number may be useful in future too.
version = (major * 1000 * 1000) + (minor * 1000) + micro;
@@ -394,6 +397,8 @@ int qemudExtractVersionInfo(const char *qemu, flags |= QEMUD_CMD_FLAG_DRIVE_BOOT; if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; + if (kvm_version >= 74) + flags |= QEMUD_CMD_FLAG_VNET_HDR;
if (retversion) *retversion = version; @@ -404,6 +409,8 @@ int qemudExtractVersionInfo(const char *qemu,
qemudDebug("Version %d %d %d Cooked version: %d, with flags ? %d", major, minor, micro, version, flags); + if (kvm_version) + qemudDebug("KVM version %d detected", kvm_version);
cleanup2: VIR_FREE(help); @@ -467,7 +474,8 @@ qemudNetworkIfaceConnect(virConnectPtr conn, int **tapfds, int *ntapfds, virDomainNetDefPtr net, - int vlan) + int vlan, + int vnet_hdr) { char *brname; char tapfdstr[4+3+32+7]; @@ -517,7 +525,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, }
if ((err = brAddTap(driver->brctl, brname, - &net->ifname, &tapfd))) { + &net->ifname, vnet_hdr, &tapfd))) { if (errno == ENOTSUP) { /* In this particular case, give a better diagnostic. */ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -1029,9 +1037,16 @@ int qemudBuildCommandLine(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: { - char *tap = qemudNetworkIfaceConnect(conn, driver, - tapfds, ntapfds, - net, vlan); + char *tap; + int vnet_hdr = 0; + + if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR && + net->model && STREQ(net->model, "virtio")) + vnet_hdr = 1; + + tap = qemudNetworkIfaceConnect(conn, driver, + tapfds, ntapfds, + net, vlan, vnet_hdr); if (tap == NULL) goto error; ADD_ARG(tap); diff --git a/src/qemu_conf.h b/src/qemu_conf.h index db2491c..65b08ac 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -48,6 +48,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_NAME = (1 << 5), QEMUD_CMD_FLAG_UUID = (1 << 6), QEMUD_CMD_FLAG_DOMID = (1 << 7), /* Xenner only */ + QEMUD_CMD_FLAG_VNET_HDR = (1 << 8), };
/* Main driver state */
Looks OK to me, +1. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
Hi Rich, On Mon, 2009-01-26 at 11:39 +0000, Richard W.M. Jones wrote:
On Mon, Jan 26, 2009 at 10:39:25AM +0000, Mark McLoughlin wrote:
IFF_VNET_HDR is a tun/tap flag that allows you to send and receive large (i.e. GSO) packets and packets with partial checksums. Setting the flag means that every packet is proceeded by the same header which virtio uses to communicate GSO/csum metadata.
Translating this for people not familiar with the intricacies of recent Linux networking changes ...
GSO = generic segmentation offload. In a baremetal Linux install, the network driver can pass the job of splitting large packets over to the network card. In virtualized environments, the "network card" is, for example, a virtio backend running in the host. Because the network bridge runs entirely inside the host kernel, there are no physical limitations on packet size as there would be if it was real ethernet, so we can use this mechanism to pass over-sized packets to the host. Another advantage is that you don't need to compute checksums over the packets which are sent this way.
Yep, see also: http://blogs.gnome.org/markmc/2008/05/28/checksums-scatter-gather-io-and-seg...
"VNET_HDR" as far as I can gather refers to the special header that virtio_net prepends to such over-sized packets. I'm not quite clear if userspace has to add this header, but if so presumably that is done inside qemu userspace(?).
VNET_HDR refers to the tap interface. A tap interface is what qemu uses to inject ethernet frames into the kernel networking stack. Normally, you just write() and read() each raw frame with a single syscall per frame. VNET_HDR is a flag for tap interfaces to say that frames we read() and write() will have a struct virtio_net_hdr prepended so as to give the kernel/qemu information about partial checksums and GSO. We need to set this flag before we bring the interface up and add it to a bridge. That's why libvirt has to do this rather than just leaving it up to qemu.
Libvirt sets the flag on the socket, passes the socket by number to qemu, and qemu needs to be able to query whether the flag was set. So the patch concerns itself with making sure that all the relevant bits of this are supported.
Correct me if I'm wrong here ...
You're spot on, thanks for elaborating. Cheers, Mark.
On Mon, Jan 26, 2009 at 10:39:25AM +0000, Mark McLoughlin wrote:
IFF_VNET_HDR is a tun/tap flag that allows you to send and receive large (i.e. GSO) packets and packets with partial checksums. Setting the flag means that every packet is proceeded by the same header which virtio uses to communicate GSO/csum metadata.
By enabling this flag on the tap fds we create, we greatly increase the achievable throughput with virtio_net.
However, we need to be careful to only set the flag when a) QEMU has support for this ABI and b) the value of the flag is queryable using the TUNGETIFF ioctl.
It's nearly five months since kvm-74 - the first KVM release with this feature - was released. Up until now, we've not added libvirt support because there is no clean way to detect support for this in QEMU at runtime. A brief attempt to add a "info capabilities" monitor command to QEMU floundered. Perfect is the enemy of good enough. Probing the KVM version will suffice for now.
ACK to the approach taken here. I've had todo the same version based check for the migration features, since that has broken compatability and is also totally undocumented in the command line help output. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, 2009-01-27 at 10:05 +0000, Daniel P. Berrange wrote:
On Mon, Jan 26, 2009 at 10:39:25AM +0000, Mark McLoughlin wrote:
It's nearly five months since kvm-74 - the first KVM release with this feature - was released. Up until now, we've not added libvirt support because there is no clean way to detect support for this in QEMU at runtime. A brief attempt to add a "info capabilities" monitor command to QEMU floundered. Perfect is the enemy of good enough. Probing the KVM version will suffice for now.
ACK to the approach taken here. I've had todo the same version based check for the migration features, since that has broken compatability and is also totally undocumented in the command line help output.
Thanks, committed now. Cheers, Mark.
participants (3)
-
Daniel P. Berrange -
Mark McLoughlin -
Richard W.M. Jones