[libvirt] [PATCH 1/2] BSD: implement virNetDevBridgeCreate() and virNetDevBridgeDelete()
Implementation uses SIOCIFCREATE2 and SIOCIFDESTROY ioctls. Also, drop static virNetDevSetupControl() as we have public one avialable now. --- src/util/virnetdevbridge.c | 91 ++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 130829c..df606d2 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -23,6 +23,7 @@ #include <config.h> #include "virnetdevbridge.h" +#include "virnetdev.h" #include "virerror.h" #include "virutil.h" #include "virfile.h" @@ -48,49 +49,6 @@ #if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) -static int virNetDevSetupControlFull(const char *ifname, - struct ifreq *ifr, - int domain, - int type) -{ - int fd; - - if (ifname && ifr) { - memset(ifr, 0, sizeof(*ifr)); - - if (virStrcpyStatic(ifr->ifr_name, ifname) == NULL) { - virReportSystemError(ERANGE, - _("Network interface name '%s' is too long"), - ifname); - return -1; - } - } - - if ((fd = socket(domain, type, 0)) < 0) { - virReportSystemError(errno, "%s", - _("Cannot open network interface control socket")); - return -1; - } - - if (virSetInherit(fd, false) < 0) { - virReportSystemError(errno, "%s", - _("Cannot set close-on-exec flag for socket")); - VIR_FORCE_CLOSE(fd); - return -1; - } - - return fd; -} - - -static int virNetDevSetupControl(const char *ifname, - struct ifreq *ifr) -{ - return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM); -} -#endif - -#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) # define SYSFS_NET_DIR "/sys/class/net" /* * Bridge parameters can be set via sysfs on newish kernels, @@ -233,6 +191,31 @@ cleanup: VIR_FORCE_CLOSE(fd); return ret; } +#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFCREATE2) +int virNetDevBridgeCreate(const char *brname) +{ + int s; + struct ifreq ifr; + int ret = - 1; + + if ((s = virNetDevSetupControl("bridge", &ifr)) < 0) + return -1; + + if (ioctl(s, SIOCIFCREATE2, &ifr) < 0) { + virReportSystemError(errno, "%s", + _("Unable to create bridge device")); + goto cleanup; + } + + if (virNetDevSetName(ifr.ifr_name, brname) == -1) { + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FORCE_CLOSE(s); + return ret; +} #else int virNetDevBridgeCreate(const char *brname) { @@ -271,6 +254,28 @@ cleanup: VIR_FORCE_CLOSE(fd); return ret; } +#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFDESTROY) +int virNetDevBridgeDelete(const char *brname) +{ + int s; + struct ifreq ifr; + int ret = -1; + + if ((s = virNetDevSetupControl(brname, &ifr)) < 0) + return -1; + + if (ioctl(s, SIOCIFDESTROY, &ifr) < 0) { + virReportSystemError(errno, + _("Unable to remove bridge %s"), + brname); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FORCE_CLOSE(s); + return ret; +} #else int virNetDevBridgeDelete(const char *brname ATTRIBUTE_UNUSED) { -- 1.8.2.3
--- configure.ac | 11 ++++ src/util/virnetdevbridge.c | 134 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 59a3d6d..ef246a6 100644 --- a/configure.ac +++ b/configure.ac @@ -2399,6 +2399,17 @@ AC_CHECK_DECLS([link_addr], #include <net/if_dl.h> ]) +# Check for BSD approach for bridge management +AC_CHECK_DECLS([BRDGSFD, BRDGADD, BRDGDEL], + [AC_DEFINE([HAVE_BSD_BRIDGE_MGMT], + [1], + [whether BSD style bridge management is available])], + [], + [#include <net/if.h> + #include <net/ethernet.h> + #include <net/if_bridgevar.h> + ]) + # Detect when running under the clang static analyzer's scan-build driver # or Coverity-prevent's cov-build. Define STATIC_ANALYSIS accordingly. AC_CACHE_CHECK([whether this build is done by a static analysis tool], diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index df606d2..ffcb4a4 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -45,9 +45,52 @@ # define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000) #endif +#if defined(HAVE_BSD_BRIDGE_MGMT) +# include <net/ethernet.h> +# include <net/if_bridgevar.h> +#endif + #define VIR_FROM_THIS VIR_FROM_NONE +#if defined(HAVE_BSD_BRIDGE_MGMT) +static int virNetDevBridgeCmd(const char *brname, + u_long op, + void *arg, + size_t argsize) +{ + int s; + int ret = -1; + struct ifdrv ifd; + + memset(&ifd, 0, sizeof(ifd)); + + if ((s = socket(AF_LOCAL, SOCK_DGRAM, 0)) < 0) { + virReportSystemError(errno, "%s", + _("Cannot open network interface control socket")); + return -1; + } + + if (virStrcpyStatic(ifd.ifd_name, brname) == NULL) { + virReportSystemError(ERANGE, + _("Network interface name '%s' is too long"), + brname); + goto cleanup; + } + + ifd.ifd_cmd = op; + ifd.ifd_len = argsize; + ifd.ifd_data = arg; + + ret = ioctl(s, SIOCSDRVSPEC, &ifd); + +cleanup: + VIR_FORCE_CLOSE(s); + + return ret; +} +#endif + #if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) # define SYSFS_NET_DIR "/sys/class/net" /* @@ -322,6 +365,28 @@ cleanup: VIR_FORCE_CLOSE(fd); return ret; } +#elif defined(HAVE_BSD_BRIDGE_MGMT) +int virNetDevBridgeAddPort(const char *brname, + const char *ifname) +{ + struct ifbreq req; + + memset(&req, 0, sizeof(req)); + if (virStrcpyStatic(req.ifbr_ifsname, ifname) == NULL) { + virReportSystemError(ERANGE, + _("Network interface name '%s' is too long"), + ifname); + return -1; + } + + if (virNetDevBridgeCmd(brname, BRDGADD, &req, sizeof(req)) < 0) { + virReportSystemError(errno, + _("Unable to add bridge %s port %s"), brname, ifname); + return -1; + } + + return 0; +} #else int virNetDevBridgeAddPort(const char *brname, const char *ifname) @@ -370,6 +435,28 @@ cleanup: VIR_FORCE_CLOSE(fd); return ret; } +#elif defined(HAVE_BSD_BRIDGE_MGMT) +int virNetDevBridgeRemovePort(const char *brname, + const char *ifname) +{ + struct ifbreq req; + + memset(&req, 0, sizeof(req)); + if (virStrcpyStatic(req.ifbr_ifsname, ifname) == NULL) { + virReportSystemError(ERANGE, + _("Network interface name '%s' is too long"), + ifname); + return -1; + } + + if (virNetDevBridgeCmd(brname, BRDGDEL, &req, sizeof(req)) < 0) { + virReportSystemError(errno, + _("Unable to remove bridge %s port %s"), brname, ifname); + return -1; + } + + return 0; +} #else int virNetDevBridgeRemovePort(const char *brname, const char *ifname) @@ -501,7 +588,50 @@ cleanup: VIR_FORCE_CLOSE(fd); return ret; } -#else /* !__linux__ */ +#elif defined(HAVE_BSD_BRIDGE_MGMT) +int virNetDevBridgeSetSTPDelay(const char *brname, + int delay) +{ + struct ifbrparam param; + + /* FreeBSD doesn't allow setting STP delay < 4 */ + delay = delay < 4 ? 4 : delay; + param.ifbrp_fwddelay = ((u_long)delay) & 0xff; + + if (virNetDevBridgeCmd(brname, BRDGSFD, ¶m, sizeof(param)) < 0) { + virReportSystemError(errno, + _("Unable to set STP delay on %s"), brname); + return -1; + } + + return 0; +} +int virNetDevBridgeGetSTPDelay(const char *brname, + int *delay ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Unable to get STP delay on %s on this platform"), + brname); + return -1; +} + +int virNetDevBridgeSetSTP(const char *brname ATTRIBUTE_UNUSED, + bool enable ATTRIBUTE_UNUSED) + +{ + /* FreeBSD doesn't allow to set STP per bridge, + * only per-device in bridge */ + return 0; +} +int virNetDevBridgeGetSTP(const char *brname, + bool *enable ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Unable to get STP on %s on this platform"), + brname); + return -1; +} +#else int virNetDevBridgeSetSTPDelay(const char *brname, int delay ATTRIBUTE_UNUSED) { @@ -536,4 +666,4 @@ int virNetDevBridgeGetSTP(const char *brname, brname); return -1; } -#endif /* __linux__ */ +#endif -- 1.8.2.3
On Wed, Jun 19, 2013 at 20:47:30 +0400, Roman Bogorodskiy wrote:
Implementation uses SIOCIFCREATE2 and SIOCIFDESTROY ioctls. Also, drop static virNetDevSetupControl() as we have public one avialable now. --- src/util/virnetdevbridge.c | 91 ++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 43 deletions(-)
Looks good and it still compiles on linux :-) ACK to both patches and pushed, thanks. Jirka
Jiri Denemark wrote:
On Wed, Jun 19, 2013 at 20:47:30 +0400, Roman Bogorodskiy wrote:
Implementation uses SIOCIFCREATE2 and SIOCIFDESTROY ioctls. Also, drop static virNetDevSetupControl() as we have public one avialable now. --- src/util/virnetdevbridge.c | 91 ++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 43 deletions(-)
Looks good and it still compiles on linux :-)
ACK to both patches and pushed, thanks.
Unfortunately I've just noticed that I made a sad mistake. :( virNetDevBridgeSetSTPDelay expects delay in milliseconds but I have implemented in expecting seconds. I'll send a fix tomorrow, sorry about that. Roman Bogorodskiy
On 06/19/2013 12:47 PM, Roman Bogorodskiy wrote:
Implementation uses SIOCIFCREATE2 and SIOCIFDESTROY ioctls. Also, drop static virNetDevSetupControl() as we have public one avialable now.
Dropping the static/local virNetDevSetupControlFull() and virNetDevSetupControl() seems to have triggered another side effect with my daily Coverity build which uses "./autogen.sh --system lv_cv_static_analysis=yes" prior to building. I'm not sure how else to trigger it or all The definition of the function is as follows: int virNetDevSetupControl(const char *ifname, virIfreq *ifr) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); However, there's two callers in virnetdevbridge.c that call it with "NULL, NULL": #if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR) int virNetDevBridgeCreate(const char *brname) ... if ((fd = virNetDevSetupControl(NULL, NULL)) < 0) return -1; ... and #if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR) int virNetDevBridgeDelete(const char *brname) ... if ((fd = virNetDevSetupControl(NULL, NULL)) < 0) return -1; ... That's because internal.h has: # ifndef ATTRIBUTE_NONNULL # if __GNUC_PREREQ (3, 3) # if STATIC_ANALYSIS # define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m))) # else # define ATTRIBUTE_NONNULL(m) __attribute__(()) # endif # else # define ATTRIBUTE_NONNULL(m) # endif # endif John
--- src/util/virnetdevbridge.c | 91 ++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 43 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 130829c..df606d2 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -23,6 +23,7 @@ #include <config.h>
#include "virnetdevbridge.h" +#include "virnetdev.h" #include "virerror.h" #include "virutil.h" #include "virfile.h" @@ -48,49 +49,6 @@
#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) -static int virNetDevSetupControlFull(const char *ifname, - struct ifreq *ifr, - int domain, - int type) -{ - int fd; - - if (ifname && ifr) { - memset(ifr, 0, sizeof(*ifr)); - - if (virStrcpyStatic(ifr->ifr_name, ifname) == NULL) { - virReportSystemError(ERANGE, - _("Network interface name '%s' is too long"), - ifname); - return -1; - } - } - - if ((fd = socket(domain, type, 0)) < 0) { - virReportSystemError(errno, "%s", - _("Cannot open network interface control socket")); - return -1; - } - - if (virSetInherit(fd, false) < 0) { - virReportSystemError(errno, "%s", - _("Cannot set close-on-exec flag for socket")); - VIR_FORCE_CLOSE(fd); - return -1; - } - - return fd; -} - - -static int virNetDevSetupControl(const char *ifname, - struct ifreq *ifr) -{ - return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM); -} -#endif - -#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) # define SYSFS_NET_DIR "/sys/class/net" /* * Bridge parameters can be set via sysfs on newish kernels, @@ -233,6 +191,31 @@ cleanup: VIR_FORCE_CLOSE(fd); return ret; } +#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFCREATE2) +int virNetDevBridgeCreate(const char *brname) +{ + int s; + struct ifreq ifr; + int ret = - 1; + + if ((s = virNetDevSetupControl("bridge", &ifr)) < 0) + return -1; + + if (ioctl(s, SIOCIFCREATE2, &ifr) < 0) { + virReportSystemError(errno, "%s", + _("Unable to create bridge device")); + goto cleanup; + } + + if (virNetDevSetName(ifr.ifr_name, brname) == -1) { + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FORCE_CLOSE(s); + return ret; +} #else int virNetDevBridgeCreate(const char *brname) { @@ -271,6 +254,28 @@ cleanup: VIR_FORCE_CLOSE(fd); return ret; } +#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFDESTROY) +int virNetDevBridgeDelete(const char *brname) +{ + int s; + struct ifreq ifr; + int ret = -1; + + if ((s = virNetDevSetupControl(brname, &ifr)) < 0) + return -1; + + if (ioctl(s, SIOCIFDESTROY, &ifr) < 0) { + virReportSystemError(errno, + _("Unable to remove bridge %s"), + brname); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FORCE_CLOSE(s); + return ret; +} #else int virNetDevBridgeDelete(const char *brname ATTRIBUTE_UNUSED) {
On 06/21/2013 07:37 PM, John Ferlan wrote:
On 06/19/2013 12:47 PM, Roman Bogorodskiy wrote:
Implementation uses SIOCIFCREATE2 and SIOCIFDESTROY ioctls. Also, drop static virNetDevSetupControl() as we have public one avialable now.
Dropping the static/local virNetDevSetupControlFull() and virNetDevSetupControl() seems to have triggered another side effect with my daily Coverity build which uses "./autogen.sh --system lv_cv_static_analysis=yes" prior to building.
I'm not sure how else to trigger it or all
The definition of the function is as follows:
int virNetDevSetupControl(const char *ifname, virIfreq *ifr) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
Thanks for pointing that out, I'll update my fix: https://www.redhat.com/archives/libvir-list/2013-June/msg00916.html Jan
participants (4)
-
Jiri Denemark -
John Ferlan -
Ján Tomko -
Roman Bogorodskiy