[libvirt] [PATCH 1/2] Modify generic ethernet interface so it will work when sVirt is enabled with qemu

Add a generic utility library for working with the TUN driver (/dev/net/tun). --- diff --git a/src/Makefile.am b/src/Makefile.am index 738ee91..ddd1b77 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -88,7 +88,8 @@ UTIL_SOURCES = \ util/xml.c util/xml.h \ util/virterror.c util/virterror_internal.h \ util/virkeycode.c util/virkeycode.h \ - util/virkeymaps.h + util/virkeymaps.h \ + util/tunctl.c util/tunctl.h EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py @@ -1182,6 +1183,8 @@ if WITH_NETWORK USED_SYM_FILES += libvirt_network.syms endif +USED_SYM_FILES += libvirt_tunctl.syms + EXTRA_DIST += \ libvirt_public.syms \ libvirt_private.syms \ diff --git a/src/libvirt_tunctl.syms b/src/libvirt_tunctl.syms new file mode 100644 index 0000000..d1e00bb --- /dev/null +++ b/src/libvirt_tunctl.syms @@ -0,0 +1,5 @@ +#tunctl.h +createTap; +delTap; +tapSetInterfaceUp; +tapSetInterfaceMac; diff --git a/src/util/tunctl.c b/src/util/tunctl.c new file mode 100644 index 0000000..e758e6d --- /dev/null +++ b/src/util/tunctl.c @@ -0,0 +1,295 @@ +/* + * Copyright (C) 2007, 2009, 2011 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + */ + +# include <config.h> +# include "tunctl.h" +# include "virfile.h" + +# include <stdlib.h> +# include <stdio.h> +# include <string.h> +# include <unistd.h> +# include <fcntl.h> +# include <errno.h> +# include <arpa/inet.h> +# include <sys/types.h> +# include <sys/socket.h> +# include <sys/ioctl.h> +# include <paths.h> +# include <sys/wait.h> + +# include <linux/param.h> /* HZ */ +# include <linux/sockios.h> /* SIOCBRADDBR etc. */ +# include <linux/if_bridge.h> /* SYSFS_BRIDGE_ATTR */ +# include <linux/if_tun.h> /* IFF_TUN, IFF_NO_PI */ +# include <net/if_arp.h> /* ARPHRD_ETHER */ + +# include "internal.h" +# include "command.h" +# include "memory.h" +# include "util.h" +# include "logging.h" +# include "network.h" + +/** + * tapProbeVnetHdr: + * @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. + */ +# ifdef IFF_VNET_HDR +static int tapProbeVnetHdr(int tapfd) +{ +# if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF) + unsigned int features; + struct ifreq dummy; + + if (ioctl(tapfd, TUNGETFEATURES, &features) != 0) { + VIR_INFO("Not enabling IFF_VNET_HDR; " + "TUNGETFEATURES ioctl() not implemented"); + return 0; + } + + if (!(features & IFF_VNET_HDR)) { + VIR_INFO("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_INFO("Not enabling IFF_VNET_HDR; " + "TUNGETIFF ioctl() not implemented"); + return 0; + } + + VIR_INFO("Enabling IFF_VNET_HDR"); + + return 1; +# else + (void) tapfd; + VIR_INFO("Not enabling IFF_VNET_HDR; disabled at build time"); + return 0; +# endif +} +# endif + +/** + * createTap: + * @ifname: the interface name + * @vnet_hr: whether to try enabling IFF_VNET_HDR + * @tapfd: file descriptor return value for the new tap device + * + * Creates a tap interface. + * If the @tapfd parameter is supplied, the open tap device file + * descriptor will be returned, otherwise the TAP device will be made + * persistent and closed. The caller must use delTap to remove + * a persistent TAP devices when it is no longer needed. + * + * Returns 0 in case of success or an errno code in case of failure. + */ + +int createTap(char **ifname, + int vnet_hdr, + int *tapfd) +{ + + int fd; + struct ifreq ifr; + + if (!ifname) + return EINVAL; + + if ((fd = open("/dev/net/tun", O_RDWR)) < 0) + return errno; + + memset(&ifr, 0, sizeof(ifr)); + + ifr.ifr_flags = IFF_TAP|IFF_NO_PI; + +# ifdef IFF_VNET_HDR + if (vnet_hdr && tapProbeVnetHdr(fd)) + ifr.ifr_flags |= IFF_VNET_HDR; +# else + (void) vnet_hdr; +# endif + + if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { + errno = EINVAL; + goto error; + } + + if (ioctl(fd, TUNSETIFF, &ifr) < 0) + goto error; + + if (!tapfd && + (errno = ioctl(fd, TUNSETPERSIST, 1))) + goto error; + VIR_FREE(*ifname); + if (!(*ifname = strdup(ifr.ifr_name))) + goto error; + if(tapfd) + *tapfd = fd; + else + VIR_FORCE_CLOSE(fd); + return 0; + + error: + VIR_FORCE_CLOSE(fd); + + return errno; +} + +/** + * delTap: + * @ifname the interface name + * + * Deletes the tap interface. + * + * Returns 0 in case of success or an errno code in case of failure. + */ + +int delTap(const char *ifname) { + struct ifreq try; + int fd; + + if (!ifname) + return EINVAL; + + if ((fd = open("/dev/net/tun", O_RDWR)) < 0) + return errno; + + memset(&try, 0, sizeof(struct ifreq)); + try.ifr_flags = IFF_TAP|IFF_NO_PI; + + if (virStrcpyStatic(try.ifr_name, ifname) == NULL) { + errno = EINVAL; + goto error; + } + + if (ioctl(fd, TUNSETIFF, &try) == 0) { + if ((errno = ioctl(fd, TUNSETPERSIST, 0))) + goto error; + } + + error: + VIR_FORCE_CLOSE(fd); + + return errno; +} + +/** + * tapSetInterfaceUp: + * @ifname: the interface name + * @up: 1 for up, 0 for down + * + * Function to control if an interface is activated (up, 1) or not (down, 0) + * + * Returns 0 in case of success or an errno code in case of failure. + */ + +int tapSetInterfaceUp(const char *ifname, + int up) +{ + struct ifreq ifr; + int ifflags; + int ctrl_fd; + + if (!ifname) + return EINVAL; + + ctrl_fd = socket(AF_INET, SOCK_STREAM, 0); + if (ctrl_fd < 0) + return errno; + + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; + + if (ioctl(ctrl_fd, SIOCGIFFLAGS, &ifr) < 0) + return errno; + + ifflags = up ? (ifr.ifr_flags | IFF_UP) : (ifr.ifr_flags & ~IFF_UP); + + if (ifr.ifr_flags != ifflags) { + ifr.ifr_flags = ifflags; + + if (ioctl(ctrl_fd, SIOCSIFFLAGS, &ifr) < 0) + return errno; + } + VIR_FORCE_CLOSE(ctrl_fd); + return 0; +} + +/** + * tapSetInterfaceMac: + * @ifname: interface name to set MTU for + * @macaddr: MAC address (VIR_MAC_BUFLEN in size) + * + * This function sets the @macaddr for a given interface @ifname. This + * gets rid of the kernel's automatically assigned random MAC. + * + * Returns 0 in case of success or an errno code in case of failure. + */ +int tapSetInterfaceMac(const char *ifname, + const unsigned char *macaddr) +{ + struct ifreq ifr; + int ctrl_fd; + int err; + + if (!ifname) + return EINVAL; + + ctrl_fd = socket(AF_INET, SOCK_STREAM, 0); + if (ctrl_fd < 0) + return errno; + + memset(&ifr, 0, sizeof(struct ifreq)); + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; + /* To fill ifr.ifr_hdaddr.sa_family field */ + if (ioctl(ctrl_fd, SIOCGIFHWADDR, &ifr) != 0) + return errno; + + memcpy(ifr.ifr_hwaddr.sa_data, macaddr, VIR_MAC_BUFLEN); + + err = ioctl(ctrl_fd, SIOCSIFHWADDR, &ifr) == 0 ? 0 : errno; + + VIR_FORCE_CLOSE(ctrl_fd); + + return err; +} diff --git a/src/util/tunctl.h b/src/util/tunctl.h new file mode 100644 index 0000000..73dd4aa --- /dev/null +++ b/src/util/tunctl.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2007 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + */ + +#ifndef __QEMUD_TAP_H__ +# define __QEMUD_TAP_H__ + +# include <config.h> +# include <net/if.h> +# include <netinet/in.h> +# include "network.h" + +int createTap (char **ifname, + int vnet_hdr, + int *tapfd); +int delTap (const char *ifname); +int tapSetInterfaceUp (const char *ifname, + int up); +int tapSetInterfaceMac (const char *ifname, + const unsigned char *macaddr); + +#endif /* __QEMUD_TAP_H__ */

On 09/20/2011 05:17 PM, Tyler Coumbes wrote:
Add a generic utility library for working with the TUN driver (/dev/net/tun).
Thanks for posting these patches. Next time, can you convince your mailer to send the series as a single thread (that is, make both 1/2 and 2/2 have In-Reply-To set to point to 0/2)? 'git send-email' makes this relatively easy. Also, sending patches with a diffstat makes it easier to see at a glance how much the patch involves.
---
diff --git a/src/Makefile.am b/src/Makefile.am index 738ee91..ddd1b77 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -88,7 +88,8 @@ UTIL_SOURCES = \ util/xml.c util/xml.h \ util/virterror.c util/virterror_internal.h \ util/virkeycode.c util/virkeycode.h \ - util/virkeymaps.h + util/virkeymaps.h \ + util/tunctl.c util/tunctl.h
Being a new file, it would probably be better to go with a name starting with 'vir', as in virtunctl (that is, this file represents libvirt's wrappers around TUN, and not a generic TUN manipulation library).
EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py @@ -1182,6 +1183,8 @@ if WITH_NETWORK USED_SYM_FILES += libvirt_network.syms endif
+USED_SYM_FILES += libvirt_tunctl.syms
If [vir]tunctl is compiled unconditionally, then we don't need a new .syms file - just stick the new symbols in libvirt_private.syms. On the other hand, since TUN tends to be a Linux-specific concept, I think you need to compile this code conditionally, at which point maybe the existing libvirt_linux.syms is a better fit.
+ EXTRA_DIST += \ libvirt_public.syms \ libvirt_private.syms \
And if all else fails and you really do need to create a new .syms file, then it needs to be listed in EXTRA_DIST.
diff --git a/src/libvirt_tunctl.syms b/src/libvirt_tunctl.syms new file mode 100644 index 0000000..d1e00bb --- /dev/null +++ b/src/libvirt_tunctl.syms @@ -0,0 +1,5 @@ +#tunctl.h +createTap; +delTap; +tapSetInterfaceUp; +tapSetInterfaceMac;
This naming isn't namespace clean. It might be better to go with: virTunCreateTap virTunDeleteTap virTunSetInterfaceUp virTunSetInterfaceMac Oh, and if all of the functions are named tap instead of tun, then maybe naming the file virtap instead of virtun would make more sense. Which is it, tap or tun?
diff --git a/src/util/tunctl.c b/src/util/tunctl.c new file mode 100644 index 0000000..e758e6d --- /dev/null +++ b/src/util/tunctl.c @@ -0,0 +1,295 @@ +/* + * Copyright (C) 2007, 2009, 2011 Red Hat, Inc.
It's okay to list earlier years if your file was copying substantial layout from another file with those years, but if the file really is from scratch, it may be okay to list just 2011. What file were you copying from,
+ * + * Authors: + */
Feel free to list yourself, if you'd like. This line looks awkward with no author listing.
+ +# include<config.h> +# include "tunctl.h" +# include "virfile.h" + +# include<stdlib.h> +# include<stdio.h> +# include<string.h> +# include<unistd.h> +# include<fcntl.h> +# include<errno.h> +# include<arpa/inet.h> +# include<sys/types.h> +# include<sys/socket.h> +# include<sys/ioctl.h> +# include<paths.h> +# include<sys/wait.h> + +# include<linux/param.h> /* HZ */
Yep, definitely Linux-specific, so the Makefile.am will need to be written so that this file is only conditionally compiled - basically, your new file should be in the same conditionals as the existing src/util/bridge.c.
+# ifdef IFF_VNET_HDR +static int tapProbeVnetHdr(int tapfd) +{ +# if defined(IFF_VNET_HDR)&& defined(TUNGETFEATURES)&& defined(TUNGETIFF)
This looks very similar to brProbeVnetHdr in src/util/bridge.c. If you are going to refactor code into a new file for larger reuse later, then it may be better to do the full code motion in a single commit, rather than to add the new function now and delete the original code later. That's not a hard-and-fast rule, but whichever way you go, it would be nice to mention in the commit message that you are adding the new code here in order to refactor common code out of bridge.c later. It took me a while to realize that you were reusing code rather than writing it from scratch. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Tyler Coumbes