2:43 p.m.
This patch series comes out of a comment in:
https://bugzilla.redhat.com/show_bug.cgi?id=1125755
that pointed out that sioctl(SIOCBRDELBR) won't delete a bridge
interface if it is IFF_UP, but the netlink RTM_DELLINK message doesn't
care - it will delete it anyway. In these patches we switch to using
RTM_DELLINK to avoid failures caused by a bridge interface that is
IFF_UP.
Laine Stump (4):
util: netlink function to delete any network device
util: replace body of virNetDevMacVLanDelete() with
virNetlinkDelLink()
util: use netlink to delete bridge devices
util: use netlink to create bridge devices
src/libvirt_private.syms | 1 +
src/util/virnetdevbridge.c | 90 +++++++++++++++++++++++++++++++++++++++++++--
src/util/virnetdevmacvlan.c | 67 +--------------------------------
src/util/virnetlink.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
src/util/virnetlink.h | 3 +-
5 files changed, 181 insertions(+), 69 deletions(-)
--
2.1.0
2:43 p.m.
New subject: [libvirt] [PATCH 1/4] util: netlink function to delete any network device
libvirt has always used the netlink RTM_DELLINK message to delete
macvtap/macvlan devices, but it can actually be used to delete other
types of network devices, such as bonds and bridges. This patch makes
virNetDevMacVLanDelete() available as a generic function so it can
intelligibly be called to delete these other types of interfaces.
---
src/libvirt_private.syms | 1 +
src/util/virnetlink.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virnetlink.h | 3 +-
3 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ca3520d..631edf3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1805,6 +1805,7 @@ virNetDevVPortProfileOpTypeToString;
# util/virnetlink.h
virNetlinkCommand;
+virNetlinkDelLink;
virNetlinkEventAddClient;
virNetlinkEventRemoveClient;
virNetlinkEventServiceIsRunning;
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index d52f66a..86c9c9c 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -277,6 +277,87 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
}
+/**
+ * virNetlinkDelLink:
+ *
+ * @ifname: Name of the link
+ *
+ * delete a network "link" (aka interface aka device) with the given
+ * name. This works for many different types of network devices,
+ * including macvtap and bridges.
+ *
+ * Returns 0 on success, -1 on fatal error.
+ */
+int
+virNetlinkDelLink(const char *ifname)
+{
+ int rc = -1;
+ struct nlmsghdr *resp = NULL;
+ struct nlmsgerr *err;
+ struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
+ unsigned int recvbuflen;
+ struct nl_msg *nl_msg;
+
+ nl_msg = nlmsg_alloc_simple(RTM_DELLINK,
+ NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
+ if (!nl_msg) {
+ virReportOOMError();
+ return -1;
+ }
+
+ if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
+ goto buffer_too_small;
+
+ if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
+ goto buffer_too_small;
+
+ if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0,
+ NETLINK_ROUTE, 0) < 0) {
+ goto cleanup;
+ }
+
+ if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
+ goto malformed_resp;
+
+ switch (resp->nlmsg_type) {
+ case NLMSG_ERROR:
+ err = (struct nlmsgerr *)NLMSG_DATA(resp);
+ if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+ goto malformed_resp;
+
+ if (err->error) {
+ virReportSystemError(-err->error,
+ _("error destroying network device %s"),
+ ifname);
+ goto cleanup;
+ }
+ break;
+
+ case NLMSG_DONE:
+ break;
+
+ default:
+ goto malformed_resp;
+ }
+
+ rc = 0;
+ cleanup:
+ nlmsg_free(nl_msg);
+ VIR_FREE(resp);
+ return rc;
+
+ malformed_resp:
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed netlink response message"));
+ goto cleanup;
+
+ buffer_too_small:
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("allocated netlink buffer is too small"));
+ goto cleanup;
+}
+
+
int
virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen)
{
@@ -803,6 +884,14 @@ int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED,
return -1;
}
+
+int
+virNetlinkDelLink(const char *ifname ATTRIBUTE_UNSUPPORTED)
+{
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
+ return -1;
+}
+
/**
* stopNetlinkEventServer: stop the monitor to receive netlink
* messages for libvirtd
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 1a3e06d..06c3cd0 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2013, 2015 Red Hat, Inc.
* Copyright (C) 2010-2012 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -51,6 +51,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
struct nlmsghdr **resp, unsigned int *respbuflen,
uint32_t src_pid, uint32_t dst_pid,
unsigned int protocol, unsigned int groups);
+int virNetlinkDelLink(const char *ifname);
int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen);
--
2.1.0
4:49 p.m.
New subject: [libvirt] [PATCH 1/4] util: netlink function to delete any network device
On 03/23/2015 03:43 PM, Laine Stump wrote:
libvirt has always used the netlink RTM_DELLINK message to delete
macvtap/macvlan devices, but it can actually be used to delete other
types of network devices, such as bonds and bridges. This patch makes
virNetDevMacVLanDelete() available as a generic function so it can
intelligibly be called to delete these other types of interfaces.
---
src/libvirt_private.syms | 1 +
src/util/virnetlink.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virnetlink.h | 3 +-
3 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ca3520d..631edf3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1805,6 +1805,7 @@ virNetDevVPortProfileOpTypeToString;
# util/virnetlink.h
virNetlinkCommand;
+virNetlinkDelLink;
virNetlinkEventAddClient;
virNetlinkEventRemoveClient;
virNetlinkEventServiceIsRunning;
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index d52f66a..86c9c9c 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -277,6 +277,87 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
}
+/**
+ * virNetlinkDelLink:
+ *
+ * @ifname: Name of the link
+ *
+ * delete a network "link" (aka interface aka device) with the given
+ * name. This works for many different types of network devices,
+ * including macvtap and bridges.
+ *
+ * Returns 0 on success, -1 on fatal error.
+ */
+int
+virNetlinkDelLink(const char *ifname)
+{
+ int rc = -1;
+ struct nlmsghdr *resp = NULL;
+ struct nlmsgerr *err;
+ struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
+ unsigned int recvbuflen;
+ struct nl_msg *nl_msg;
+
+ nl_msg = nlmsg_alloc_simple(RTM_DELLINK,
+ NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
+ if (!nl_msg) {
+ virReportOOMError();
+ return -1;
+ }
+
+ if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
+ goto buffer_too_small;
+
+ if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
+ goto buffer_too_small;
+
+ if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0,
+ NETLINK_ROUTE, 0) < 0) {
+ goto cleanup;
+ }
+
+ if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
+ goto malformed_resp;
+
+ switch (resp->nlmsg_type) {
+ case NLMSG_ERROR:
+ err = (struct nlmsgerr *)NLMSG_DATA(resp);
+ if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+ goto malformed_resp;
+
+ if (err->error) {
+ virReportSystemError(-err->error,
+ _("error destroying network device %s"),
+ ifname);
+ goto cleanup;
+ }
+ break;
+
+ case NLMSG_DONE:
+ break;
+
+ default:
+ goto malformed_resp;
+ }
+
+ rc = 0;
+ cleanup:
+ nlmsg_free(nl_msg);
+ VIR_FREE(resp);
+ return rc;
+
+ malformed_resp:
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed netlink response message"));
+ goto cleanup;
+
+ buffer_too_small:
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("allocated netlink buffer is too small"));
+ goto cleanup;
+}
+
+
int
virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen)
{
@@ -803,6 +884,14 @@ int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED,
return -1;
}
+
+int
+virNetlinkDelLink(const char *ifname ATTRIBUTE_UNSUPPORTED)
+{
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
+ return -1;
+}
+
/**
* stopNetlinkEventServer: stop the monitor to receive netlink
* messages for libvirtd
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 1a3e06d..06c3cd0 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2013, 2015 Red Hat, Inc.
* Copyright (C) 2010-2012 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -51,6 +51,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
struct nlmsghdr **resp, unsigned int *respbuflen,
uint32_t src_pid, uint32_t dst_pid,
unsigned int protocol, unsigned int groups);
+int virNetlinkDelLink(const char *ifname);
Like the virNetDevMacVLanDelete which this is essentially "replacing" -
add a :
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
ACK with the change
John
int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen);
2:43 p.m.
New subject: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
These two functions are identical, so no sense in having the
duplication. I resisted the temptation to replace calls to
virNetDevMacVLanDelete() with calls to virNetlinkDelLink() just in
case some mythical future platform has macvtap devices that aren't
managed with netlink (or in case we some day need to do more than just
tell the kernel to delete the device).
---
src/util/virnetdevmacvlan.c | 67 ++-------------------------------------------
1 file changed, 2 insertions(+), 65 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index ec959a9..5fd2097 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2014 Red Hat, Inc.
+ * Copyright (C) 2010-2015 Red Hat, Inc.
* Copyright (C) 2010-2012 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -220,70 +220,7 @@ virNetDevMacVLanCreate(const char *ifname,
*/
int virNetDevMacVLanDelete(const char *ifname)
{
- int rc = -1;
- struct nlmsghdr *resp = NULL;
- struct nlmsgerr *err;
- struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
- unsigned int recvbuflen;
- struct nl_msg *nl_msg;
-
- nl_msg = nlmsg_alloc_simple(RTM_DELLINK,
- NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
- if (!nl_msg) {
- virReportOOMError();
- return -1;
- }
-
- if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
- goto buffer_too_small;
-
- if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
- goto buffer_too_small;
-
- if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0,
- NETLINK_ROUTE, 0) < 0) {
- goto cleanup;
- }
-
- if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
- goto malformed_resp;
-
- switch (resp->nlmsg_type) {
- case NLMSG_ERROR:
- err = (struct nlmsgerr *)NLMSG_DATA(resp);
- if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
- goto malformed_resp;
-
- if (err->error) {
- virReportSystemError(-err->error,
- _("error destroying %s interface"),
- ifname);
- goto cleanup;
- }
- break;
-
- case NLMSG_DONE:
- break;
-
- default:
- goto malformed_resp;
- }
-
- rc = 0;
- cleanup:
- nlmsg_free(nl_msg);
- VIR_FREE(resp);
- return rc;
-
- malformed_resp:
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("malformed netlink response message"));
- goto cleanup;
-
- buffer_too_small:
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("allocated netlink buffer is too small"));
- goto cleanup;
+ return virNetlinkDelLink(ifname);
}
--
2.1.0
4:50 p.m.
New subject: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
On 03/23/2015 03:43 PM, Laine Stump wrote:
These two functions are identical, so no sense in having the
duplication. I resisted the temptation to replace calls to
virNetDevMacVLanDelete() with calls to virNetlinkDelLink() just in
case some mythical future platform has macvtap devices that aren't
managed with netlink (or in case we some day need to do more than just
tell the kernel to delete the device).
---
src/util/virnetdevmacvlan.c | 67 ++-------------------------------------------
1 file changed, 2 insertions(+), 65 deletions(-)
ACK -
John
5:25 p.m.
New subject: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
On 03/23/2015 03:43 PM, Laine Stump wrote:
These two functions are identical, so no sense in having the
duplication. I resisted the temptation to replace calls to
virNetDevMacVLanDelete() with calls to virNetlinkDelLink() just in
case some mythical future platform has macvtap devices that aren't
managed with netlink (or in case we some day need to do more than just
tell the kernel to delete the device).
---
src/util/virnetdevmacvlan.c | 67 ++-------------------------------------------
1 file changed, 2 insertions(+), 65 deletions(-)
hmm interesting... Started reading the next patch and noticed
something... Perhaps I was quick on the trigger finger for this one...
This module was compiled with "if WITH_MACVTAP", but the API being
replaced/called uses "#if defined(__linux__) && defined(HAVE_LIBNL)"
thus this module never cared about linux & HAVE_LIBNL, but now depends
on an API that does. My reading comprehension of the Makefile in order
to determine whether anything matters is "limited"...
Of course this module has libnl calls in it already so perhaps either
WITH_MACVTAP implies __linux__ and HAVE_LIBNL or perhaps that something
that needs to be adjusted. Not my specialty!
John
2:03 p.m.
New subject: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
On 03/24/2015 06:25 PM, John Ferlan wrote:
On 03/23/2015 03:43 PM, Laine Stump wrote:
> These two functions are identical, so no sense in having the
> duplication. I resisted the temptation to replace calls to
> virNetDevMacVLanDelete() with calls to virNetlinkDelLink() just in
> case some mythical future platform has macvtap devices that aren't
> managed with netlink (or in case we some day need to do more than just
> tell the kernel to delete the device).
> ---
> src/util/virnetdevmacvlan.c | 67 ++-------------------------------------------
> 1 file changed, 2 insertions(+), 65 deletions(-)
>
hmm interesting... Started reading the next patch and noticed
something... Perhaps I was quick on the trigger finger for this one...
This module was compiled with "if WITH_MACVTAP", but the API being
replaced/called uses "#if defined(__linux__) && defined(HAVE_LIBNL)"
thus this module never cared about linux & HAVE_LIBNL, but now depends
on an API that does. My reading comprehension of the Makefile in order
to determine whether anything matters is "limited"...
Of course this module has libnl calls in it already so perhaps either
WITH_MACVTAP implies __linux__ and HAVE_LIBNL or perhaps that something
that needs to be adjusted.
configure assures that HAVE_LIBNL is true if --with-macvtap is
requested, so it's not possible to have WITH_MACVTAP without HAVE_LIBNL.
As for __linux__, I'm not even sure why that got in to begin with. I
don't know that libnl (or netlink) is available on anything that isn't
Linux. Definitely macvtap isn't on anything non-Linux though, so I'd say
we're safe there as well.
Not my specialty!
but thankfully you weren't afraid to review it anyway. Thanks!
2:43 p.m.
New subject: [libvirt] [PATCH 3/4] util: use netlink to delete bridge devices
https://bugzilla.redhat.com/show_bug.cgi?id=1125755
reported that a stray bridge device was left on the system when a
libvirt network failed to start due to an illegal iptables rule caused
by bad config. Apparently the reason this was happening was that
NetworkManager was noticing immediately when the bridge device was
created and automatically setting it IFF_UP. libvirt would then try to
setup the iptables rules, get an error back, and since libvirt had
never IFF_UPed the bridge, it didn't expect that it needed to set it
~IFF_UP before deleting it during the cleanup process. But the
ioctl(SIOCBRDELBR) ioctl will fail to delete a bridge if it is IFF_UP.
Since that bug was reported, NetworkManager has gotten a bit more
polite in this respect, but just in case something similar happens in
the future, this patch switches to using the netlink RTM_DELLINK
message to delete the bridge - unlike SIOCBRDELBR, it will delete the
requested bridge no matter what the setting of IFF_UP.
---
src/util/virnetdevbridge.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index d9dcc39..2bad40a 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2015 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
@@ -457,7 +457,15 @@ int virNetDevBridgeCreate(const char *brname)
*
* Returns 0 in case of success or an errno code in case of failure.
*/
-#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR)
+#if defined(__linux__) && defined(HAVE_LIBNL)
+int virNetDevBridgeDelete(const char *brname)
+{
+ /* If netlink is available, use it, as it is successful at
+ * deleting a bridge even if it is currently IFF_UP.
+ */
+ return virNetlinkDelLink(brname);
+}
+#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR)
int virNetDevBridgeDelete(const char *brname)
{
int fd = -1;
--
2.1.0
5:26 p.m.
New subject: [libvirt] [PATCH 3/4] util: use netlink to delete bridge devices
On 03/23/2015 03:43 PM, Laine Stump wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1125755
reported that a stray bridge device was left on the system when a
libvirt network failed to start due to an illegal iptables rule caused
by bad config. Apparently the reason this was happening was that
NetworkManager was noticing immediately when the bridge device was
created and automatically setting it IFF_UP. libvirt would then try to
setup the iptables rules, get an error back, and since libvirt had
never IFF_UPed the bridge, it didn't expect that it needed to set it
~IFF_UP before deleting it during the cleanup process. But the
ioctl(SIOCBRDELBR) ioctl will fail to delete a bridge if it is IFF_UP.
Since that bug was reported, NetworkManager has gotten a bit more
polite in this respect, but just in case something similar happens in
the future, this patch switches to using the netlink RTM_DELLINK
message to delete the bridge - unlike SIOCBRDELBR, it will delete the
requested bridge no matter what the setting of IFF_UP.
---
src/util/virnetdevbridge.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
ACK -
John
2:43 p.m.
New subject: [libvirt] [PATCH 4/4] util: use netlink to create bridge devices
Just as it is possible to delete a bridge device with the netlink
RTM_DELLINK message, one can be created with the RTM_NEWLINK
message. Because of differences in the format of the message, it's not
as straightforward as with virNetlinkDelLink() to create a single
utility function that can be used to create any type of interface, so
the new netlink version of virNetDevBridgeCreate() does its own
construction of the netlink message and calls virNetlinkCommand()
itself.
This doesn't provide any extra functionality, just provides symmetry
with the previous commit.
NB: We *could* alter the API of virNetDevBridgeCreate() to take a MAC
address, and directly program that mac address into the bridge (by
adding an IFLA_ADDRESS attribute, as is done in
virNetDevMacVLanCreate()) rather than separately creating the "dummy
tap" (e.g. virbr0-nic) to maintain a fixed mac address on the bridge,
but the commit history of virnetdevbridge.c shows that the presence of
this dummy tap is essential in some older versions of the kernel
(between 2.6.39 and 3.1 or 3.2, possibly?) to proper operation of IPv6
DAD, and I don't want to take the chance of breaking something that I
don't have the time/setup to test (my RHEL6 box is at kernel
2.6.32-544, and the next lowest kernel I have is 3.17)
---
src/util/virnetdevbridge.c | 78 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 77 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index 2bad40a..6be8aa3 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -395,7 +395,83 @@ virNetDevBridgePortSetUnicastFlood(const char *brname
ATTRIBUTE_UNUSED,
*
* Returns 0 in case of success or -1 on failure
*/
-#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
+#if defined(__linux__) && defined(HAVE_LIBNL)
+int virNetDevBridgeCreate(const char *brname)
+{
+ /* use a netlink RTM_NEWLINK message to create the bridge */
+ const char *type = "bridge";
+ int rc = -1;
+ struct nlmsghdr *resp = NULL;
+ struct nlmsgerr *err;
+ struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
+ unsigned int recvbuflen;
+ struct nl_msg *nl_msg;
+ struct nlattr *linkinfo;
+
+ nl_msg = nlmsg_alloc_simple(RTM_NEWLINK,
+ NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
+ if (!nl_msg) {
+ virReportOOMError();
+ return -1;
+ }
+
+ if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
+ goto buffer_too_small;
+ if (nla_put(nl_msg, IFLA_IFNAME, strlen(brname)+1, brname) < 0)
+ goto buffer_too_small;
+ if (!(linkinfo = nla_nest_start(nl_msg, IFLA_LINKINFO)))
+ goto buffer_too_small;
+ if (nla_put(nl_msg, IFLA_INFO_KIND, strlen(type), type) < 0)
+ goto buffer_too_small;
+ nla_nest_end(nl_msg, linkinfo);
+
+ if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0,
+ NETLINK_ROUTE, 0) < 0) {
+ goto cleanup;
+ }
+
+ if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
+ goto malformed_resp;
+
+ switch (resp->nlmsg_type) {
+ case NLMSG_ERROR:
+ err = (struct nlmsgerr *)NLMSG_DATA(resp);
+ if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+ goto malformed_resp;
+
+ switch (err->error) {
+ case 0:
+ break;
+ default:
+ virReportSystemError(-err->error,
+ _("error creating bridge interface %s"),
+ brname);
+ goto cleanup;
+ }
+ break;
+
+ case NLMSG_DONE:
+ break;
+ default:
+ goto malformed_resp;
+ }
+
+ rc = 0;
+ cleanup:
+ nlmsg_free(nl_msg);
+ VIR_FREE(resp);
+ return rc;
+
+ malformed_resp:
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed netlink response message"));
+ goto cleanup;
+ buffer_too_small:
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("allocated netlink buffer is too small"));
+ goto cleanup;
+}
+#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
int virNetDevBridgeCreate(const char *brname)
{
int fd = -1;
--
2.1.0
5:39 p.m.
New subject: [libvirt] [PATCH 4/4] util: use netlink to create bridge devices
On 03/23/2015 03:43 PM, Laine Stump wrote:
Just as it is possible to delete a bridge device with the netlink
RTM_DELLINK message, one can be created with the RTM_NEWLINK
message. Because of differences in the format of the message, it's not
as straightforward as with virNetlinkDelLink() to create a single
utility function that can be used to create any type of interface, so
the new netlink version of virNetDevBridgeCreate() does its own
construction of the netlink message and calls virNetlinkCommand()
itself.
This doesn't provide any extra functionality, just provides symmetry
with the previous commit.
NB: We *could* alter the API of virNetDevBridgeCreate() to take a MAC
address, and directly program that mac address into the bridge (by
adding an IFLA_ADDRESS attribute, as is done in
virNetDevMacVLanCreate()) rather than separately creating the "dummy
tap" (e.g. virbr0-nic) to maintain a fixed mac address on the bridge,
but the commit history of virnetdevbridge.c shows that the presence of
this dummy tap is essential in some older versions of the kernel
(between 2.6.39 and 3.1 or 3.2, possibly?) to proper operation of IPv6
DAD, and I don't want to take the chance of breaking something that I
don't have the time/setup to test (my RHEL6 box is at kernel
2.6.32-544, and the next lowest kernel I have is 3.17)
---
src/util/virnetdevbridge.c | 78 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 77 insertions(+), 1 deletion(-)
What's here seems reasonable - I don't have "history" on my side
to
answer the older versions of the kernel query...
One other difference between this and virNetDevMacVLanCreate is the
error path that handles *retry if the name is already in use... Whether
that's a worthy addition here or not is your call...
ACK -
John
1:56 p.m.
New subject: [libvirt] [PATCH 4/4] util: use netlink to create bridge devices
On 03/24/2015 06:39 PM, John Ferlan wrote:
On 03/23/2015 03:43 PM, Laine Stump wrote:
> Just as it is possible to delete a bridge device with the netlink
> RTM_DELLINK message, one can be created with the RTM_NEWLINK
> message. Because of differences in the format of the message, it's not
> as straightforward as with virNetlinkDelLink() to create a single
> utility function that can be used to create any type of interface, so
> the new netlink version of virNetDevBridgeCreate() does its own
> construction of the netlink message and calls virNetlinkCommand()
> itself.
>
> This doesn't provide any extra functionality, just provides symmetry
> with the previous commit.
>
> NB: We *could* alter the API of virNetDevBridgeCreate() to take a MAC
> address, and directly program that mac address into the bridge (by
> adding an IFLA_ADDRESS attribute, as is done in
> virNetDevMacVLanCreate()) rather than separately creating the "dummy
> tap" (e.g. virbr0-nic) to maintain a fixed mac address on the bridge,
> but the commit history of virnetdevbridge.c shows that the presence of
> this dummy tap is essential in some older versions of the kernel
> (between 2.6.39 and 3.1 or 3.2, possibly?) to proper operation of IPv6
> DAD, and I don't want to take the chance of breaking something that I
> don't have the time/setup to test (my RHEL6 box is at kernel
> 2.6.32-544, and the next lowest kernel I have is 3.17)
> ---
> src/util/virnetdevbridge.c | 78 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 77 insertions(+), 1 deletion(-)
>
What's here seems reasonable - I don't have "history" on my side to
answer the older versions of the kernel query...
One other difference between this and virNetDevMacVLanCreate is the
error path that handles *retry if the name is already in use...
Hmm. Interesting idea, but it wouldn't be useful with the current way
that things work - unlike macvtap devices, which are created one per
domain-interface and whose name we don't care about, by the time we get
to the point that we want to create a bridge device (which is one for
each network), we've already committed to the name we're going to use.
Some restructuring of network startup could make that useful though.
Still, I think I'll push this as it is for now (since it meets the
current usage demands, and maintains the existing API for
virNetDevBridgeCreate()) and think about building in EEXIST handling
later (this could be especially problematic, since the same capability
would need to be put into the other two implementations of
virNetDevBridgeCreate(), and one of those is for a platform that I don't
have setup to test on).
Thanks for the reviews!
3529
days inactive
3532
days old
11 comments
2 participants
participants (2)
-
John Ferlan -
Laine Stump