From c1df2c14b590b3d68b707aa4f3a570f95a6bc548 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 2 Nov 2011 13:05:27 +0000 Subject: [PATCH] Remove usage of brctl command line tool Convert the virNetDevBridgeSetSTP and virNetDevBridgeSetSTPDelay to use ioctls instead of spawning brctl. Implement the virNetDevBridgeGetSTP and virNetDevBridgeGetSTPDelay methods which were declared in the header but never existed * src/util/bridge.c: Convert to use bridge ioctls instead of brctl --- configure.ac | 2 - libvirt.spec.in | 2 - src/util/bridge.c | 203 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 186 insertions(+), 21 deletions(-) diff --git a/configure.ac b/configure.ac index e3c28fa0fe..40f6654334 100644 --- a/configure.ac +++ b/configure.ac @@ -197,8 +197,6 @@ AC_DEFINE_UNQUOTED([DNSMASQ],["$DNSMASQ"], [Location or name of the dnsmasq program]) AC_DEFINE_UNQUOTED([RADVD],["$RADVD"], [Location or name of the radvd program]) -AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"], - [Location or name of the brctl program (see bridge-utils)]) AC_DEFINE_UNQUOTED([TC],["$TC"], [Location or name of the tc profram (see iproute2)]) if test -n "$UDEVADM"; then diff --git a/libvirt.spec.in b/libvirt.spec.in index c74b0ea4a1..10280f02b6 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -251,7 +251,6 @@ Requires: %{name}-client = %{version}-%{release} # Used by many of the drivers, so turn it on whenever the # daemon is present %if %{with_libvirtd} -Requires: bridge-utils # for modprobe of pci devices Requires: module-init-tools # for /sbin/ip & /sbin/tc @@ -380,7 +379,6 @@ BuildRequires: radvd %if %{with_nwfilter} BuildRequires: ebtables %endif -BuildRequires: bridge-utils BuildRequires: module-init-tools %if %{with_sasl} BuildRequires: cyrus-sasl-devel diff --git a/src/util/bridge.c b/src/util/bridge.c index 4eafc3a14c..37c717bff1 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -52,6 +52,7 @@ # include "logging.h" # include "network.h" # include "virterror_internal.h" +# include "intprops.h" # define JIFFIES_TO_MS(j) (((j)*1000)/HZ) # define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000) @@ -99,6 +100,112 @@ static int virNetDevSetupControl(const char *ifname, return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM); } +# define SYSFS_NET_DIR "/sys/class/net" +/* + * Bridge parameters can be set via sysfs on newish kernels, + * or by ioctl on older kernels. Perhaps we could just use + * ioctl for every kernel, but its not clear what the long + * term lifespan of the ioctl interface is... + */ +static int virNetDevBridgeSet(const char *brname, + const char *paramname, /* sysfs param name */ + unsigned long value, /* new value */ + int fd, /* control socket */ + struct ifreq *ifr) /* pre-filled bridge name */ +{ + char *path = NULL; + int ret = -1; + + if (virAsprintf(&path, "%s/%s/bridge/%s", SYSFS_NET_DIR, brname, paramname) < 0) { + virReportOOMError(); + return -1; + } + + if (virFileExists(path)) { + char valuestr[INT_BUFSIZE_BOUND(value)]; + snprintf(valuestr, sizeof(valuestr), "%lu", value); + if (virFileWriteStr(path, valuestr, 0) < 0) { + virReportSystemError(errno, + _("Unable to set bridge %s %s"), brname, paramname); + goto cleanup; + } + } else { + unsigned long paramid; + if (STREQ(paramname, "stp_state")) { + paramid = BRCTL_SET_BRIDGE_STP_STATE; + } else if (STREQ(paramname, "forward_delay")) { + paramid = BRCTL_SET_BRIDGE_FORWARD_DELAY; + } else { + virReportSystemError(EINVAL, + _("Unable to set bridge %s %s"), brname, paramname); + goto cleanup; + } + unsigned long args[] = { paramid, value, 0, 0 }; + ifr->ifr_data = (char*)&args; + if (ioctl(fd, SIOCDEVPRIVATE, ifr) < 0) { + virReportSystemError(errno, + _("Unable to set bridge %s %s"), brname, paramname); + goto cleanup; + } + } + + ret = 0; +cleanup: + VIR_FREE(path); + return ret; +} + + +static int virNetDevBridgeGet(const char *brname, + const char *paramname, /* sysfs param name */ + unsigned long *value, /* current value */ + int fd, /* control socket */ + struct ifreq *ifr) /* pre-filled bridge name */ +{ + char *path = NULL; + int ret = -1; + + if (virAsprintf(&path, "%s/%s/bridge/%s", SYSFS_NET_DIR, brname, paramname) < 0) { + virReportOOMError(); + return -1; + } + + if (virFileExists(path)) { + char *valuestr; + if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long), &valuestr) < 0) + goto cleanup; + + if (virStrToLong_ul(valuestr, NULL, 10, value) < 0) { + virReportSystemError(EINVAL, + _("Unable to get bridge %s %s"), brname, paramname); + } + } else { + struct __bridge_info info; + unsigned long args[] = { BRCTL_GET_BRIDGE_INFO, (unsigned long)&info, 0, 0 }; + ifr->ifr_data = (char*)&args; + if (ioctl(fd, SIOCDEVPRIVATE, ifr) < 0) { + virReportSystemError(errno, + _("Unable to get bridge %s %s"), brname, paramname); + goto cleanup; + } + + if (STREQ(paramname, "stp_state")) { + *value = info.stp_enabled; + } else if (STREQ(paramname, "forward_delay")) { + *value = info.forward_delay; + } else { + virReportSystemError(EINVAL, + _("Unable to get bridge %s %s"), brname, paramname); + goto cleanup; + } + } + + ret = 0; +cleanup: + VIR_FREE(path); + return ret; +} + /** * virNetDevBridgeCreate: @@ -817,22 +924,54 @@ cleanup: int virNetDevBridgeSetSTPDelay(const char *brname, int delay) { - virCommandPtr cmd; + int fd = -1; int ret = -1; + struct ifreq ifr; - cmd = virCommandNew(BRCTL); - virCommandAddArgList(cmd, "setfd", brname, NULL); - virCommandAddArgFormat(cmd, "%d", delay); - - if (virCommandRun(cmd, NULL) < 0) + if ((fd = virNetDevSetupControl(brname, &ifr)) < 0) goto cleanup; - ret = 0; + ret = virNetDevBridgeSet(brname, "stp_state", MS_TO_JIFFIES(delay), + fd, &ifr); + cleanup: - virCommandFree(cmd); + VIR_FORCE_CLOSE(fd); return ret; } + +/** + * virNetDevBridgeGetSTPDelay: + * @brname: the bridge device name + * @delayms: the forward delay in milliseconds + * + * Retrives the forward delay for the bridge device @brname + * storing it in @delayms. The forward delay is only meaningful + * if STP is enabled + * + * Returns 0 on success, -1 on error+ + */ +int virNetDevBridgeGetSTPDelay(const char *brname, + int *delayms) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + unsigned long i; + + if ((fd = virNetDevSetupControl(brname, &ifr)) < 0) + goto cleanup; + + ret = virNetDevBridgeGet(brname, "stp_state", &i, + fd, &ifr); + *delayms = JIFFIES_TO_MS(i); + +cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} + + /** * virNetDevBridgeSetSTP: * @brname: the bridge name @@ -846,23 +985,53 @@ cleanup: int virNetDevBridgeSetSTP(const char *brname, bool enable) { - virCommandPtr cmd; + int fd = -1; int ret = -1; + struct ifreq ifr; - cmd = virCommandNew(BRCTL); - virCommandAddArgList(cmd, "stp", brname, - enable ? "on" : "off", - NULL); - - if (virCommandRun(cmd, NULL) < 0) + if ((fd = virNetDevSetupControl(brname, &ifr)) < 0) goto cleanup; - ret = 0; + ret = virNetDevBridgeSet(brname, "stp_state", enable ? 1 : 0, + fd, &ifr); + cleanup: - virCommandFree(cmd); + VIR_FORCE_CLOSE(fd); return ret; } + +/** + * virNetDevBridgeGetSTP: + * @brname: the bridge device name + * @enabled: returns the STP state + * + * Determine the state of the spanning tree protocol on + * the device @brname, returning the state in @enabled + * + * Returns 0 on success, -1 on error + */ +int virNetDevBridgeGetSTP(const char *brname, + bool *enabled) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + unsigned long i; + + if ((fd = virNetDevSetupControl(brname, &ifr)) < 0) + goto cleanup; + + ret = virNetDevBridgeGet(brname, "stp_state", &i, + fd, &ifr); + *enabled = i ? true : false; + +cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} + + /** * brCreateTap: * @ifname: the interface name