11:47 a.m.
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
11:47 a.m.
New subject: [libvirt] [PATCH 2/2] BSD: implement bridge add/remove port and set STP
---
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
3:48 a.m.
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
12:02 p.m.
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
12:37 p.m.
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)
{
12:44 p.m.
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
4206
days inactive
4208
days old
5 comments
4 participants
participants (4)
-
Jiri Denemark -
John Ferlan -
Ján Tomko -
Roman Bogorodskiy