midnight
The idea behind these patches is the following:
1) most virtual machines only have a single MAC address behind each
interface, and that MAC address is known by libvirt.
2) If we (i.e. libvirt) manually add an entry to the bridge's
forwarding database (fdb) for the MAC address associated with a port
on the bridge, we can turn off learning and unicast_flooding for that
port.
3) kernels starting with 3.15 (and actually working correctly starting
in kernel 3.17) will notice that all of a bridge's ports have flood
and learning turned off, and in that case will turn off promiscuous
mode on all ports. If all but one of the ports have flood/learning
turned off, then promiscuous will be turned off on that port (and left
on for all the other ports)
4) When (4) can be done, there is a measurable performance
advantage. It can also *kind of* help security, as it will prevent a
guest from doing anything useful if it changes its MAC address (but
won't prevent the guest from *sending* packets with a spoofed MAC
address).
NB: These only work with a fixed MAC address, and no vlan tags set in
the guest. Support for both of those will be coming.
This series is the same as V2, which was previously ACK (pending final determination of
attribute name):
https://www.redhat.com/archives/libvir-list/2014-December/msg00173.html
but with the name of the attribute changed - in V2 it was:
fdb="learnWithFlooding|managed"
and it is now:
macTableManager="kernel|libvirt"
which more accurately reflects what is being controlled with the attribute.
Laine Stump (9):
util: new functions for setting bridge and bridge port attributes
util: functions to manage bridge fdb (forwarding database)
conf: new network bridge device attribute macTableManager
network: save bridge name in ActualNetDef when actualType==network too
network: store network macTableManager setting in NetDef actual object
network: setup bridge devices for macTableManager='libvirt'
qemu: setup tap devices for macTableManager='libvirt'
qemu: always use virDomainNetGetActualBridgeName to get interface's
bridge
lxc: always use virDomainNetGetActualBridgeName to get interface's
bridge
docs/formatnetwork.html.in | 50 ++-
docs/schemas/network.rng | 9 +
src/conf/domain_conf.c | 130 ++++---
src/conf/domain_conf.h | 2 +
src/conf/network_conf.c | 51 ++-
src/conf/network_conf.h | 11 +
src/libvirt_private.syms | 11 +
src/lxc/lxc_driver.c | 26 +-
src/lxc/lxc_process.c | 26 +-
src/network/bridge_driver.c | 78 +++++
src/qemu/qemu_command.c | 53 +--
src/qemu/qemu_hotplug.c | 54 +--
src/util/virnetdevbridge.c | 382 ++++++++++++++++++++-
src/util/virnetdevbridge.h | 44 ++-
tests/networkxml2xmlin/host-bridge-no-flood.xml | 6 +
.../nat-network-explicit-flood.xml | 21 ++
tests/networkxml2xmlout/host-bridge-no-flood.xml | 6 +
.../nat-network-explicit-flood.xml | 23 ++
tests/networkxml2xmltest.c | 2 +
19 files changed, 796 insertions(+), 189 deletions(-)
create mode 100644 tests/networkxml2xmlin/host-bridge-no-flood.xml
create mode 100644 tests/networkxml2xmlin/nat-network-explicit-flood.xml
create mode 100644 tests/networkxml2xmlout/host-bridge-no-flood.xml
create mode 100644 tests/networkxml2xmlout/nat-network-explicit-flood.xml
--
1.9.3
midnight
New subject: [libvirt] [PATCHv3 1/9] util: new functions for setting bridge and bridge port attributes
These functions all set/get items in the sysfs for a bridge device.
---
No change since V1.
src/libvirt_private.syms | 6 ++
src/util/virnetdevbridge.c | 235 ++++++++++++++++++++++++++++++++++++++++++++-
src/util/virnetdevbridge.h | 28 +++++-
3 files changed, 266 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1853a9c..7cb57a9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1663,9 +1663,15 @@ virNetDevBridgeCreate;
virNetDevBridgeDelete;
virNetDevBridgeGetSTP;
virNetDevBridgeGetSTPDelay;
+virNetDevBridgeGetVlanFiltering;
+virNetDevBridgePortGetLearning;
+virNetDevBridgePortGetUnicastFlood;
+virNetDevBridgePortSetLearning;
+virNetDevBridgePortSetUnicastFlood;
virNetDevBridgeRemovePort;
virNetDevBridgeSetSTP;
virNetDevBridgeSetSTPDelay;
+virNetDevBridgeSetVlanFiltering;
# util/virnetdevmacvlan.h
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index 15434de..c6a3e8e 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007-2013 Red Hat, Inc.
+ * Copyright (C) 2007-2014 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
@@ -219,6 +219,170 @@ static int virNetDevBridgeGet(const char *brname,
}
#endif /* __linux__ */
+#if defined(__linux__)
+static int
+virNetDevBridgePortSet(const char *brname,
+ const char *ifname,
+ const char *paramname,
+ unsigned long value)
+{
+ char *path = NULL;
+ char valuestr[INT_BUFSIZE_BOUND(value)];
+ int ret = -1;
+
+ snprintf(valuestr, sizeof(valuestr), "%lu", value);
+
+ if (virAsprintf(&path, "%s/%s/brif/%s/%s",
+ SYSFS_NET_DIR, brname, ifname, paramname) < 0)
+ return -1;
+
+ if (!virFileExists(path))
+ errno = EINVAL;
+ else
+ ret = virFileWriteStr(path, valuestr, 0);
+
+ if (ret < 0) {
+ virReportSystemError(errno,
+ _("Unable to set bridge %s port %s %s to %s"),
+ brname, ifname, paramname, valuestr);
+ }
+
+ VIR_FREE(path);
+ return ret;
+}
+
+
+static int
+virNetDevBridgePortGet(const char *brname,
+ const char *ifname,
+ const char *paramname,
+ unsigned long *value)
+{
+ char *path = NULL;
+ char *valuestr = NULL;
+ int ret = -1;
+
+ if (virAsprintf(&path, "%s/%s/brif/%s/%s",
+ SYSFS_NET_DIR, brname, ifname, paramname) < 0)
+ return -1;
+
+ 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 port %s %s"),
+ brname, ifname, paramname);
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(path);
+ VIR_FREE(valuestr);
+ return ret;
+}
+
+
+int
+virNetDevBridgePortGetLearning(const char *brname,
+ const char *ifname,
+ bool *enable)
+{
+ int ret = -1;
+ unsigned long value;
+
+ if (virNetDevBridgePortGet(brname, ifname, "learning", &value) < 0)
+ goto cleanup;
+
+ *enable = !!value;
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+
+int
+virNetDevBridgePortSetLearning(const char *brname,
+ const char *ifname,
+ bool enable)
+{
+ return virNetDevBridgePortSet(brname, ifname, "learning", enable ? 1 : 0);
+}
+
+
+int
+virNetDevBridgePortGetUnicastFlood(const char *brname,
+ const char *ifname,
+ bool *enable)
+{
+ int ret = -1;
+ unsigned long value;
+
+ if (virNetDevBridgePortGet(brname, ifname, "unicast_flood", &value)
< 0)
+ goto cleanup;
+
+ *enable = !!value;
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+
+int
+virNetDevBridgePortSetUnicastFlood(const char *brname,
+ const char *ifname,
+ bool enable)
+{
+ return virNetDevBridgePortSet(brname, ifname, "unicast_flood", enable ? 1 :
0);
+}
+
+
+#else
+int
+virNetDevBridgePortGetLearning(const char *brname ATTRIBUTE_UNUSED,
+ const char *ifname ATTRIBUTE_UNUSED,
+ bool *enable ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Unable to get bridge port learning on this
platform"));
+ return -1;
+}
+
+
+int
+virNetDevBridgePortSetLearning(const char *brname ATTRIBUTE_UNUSED,
+ const char *ifname ATTRIBUTE_UNUSED,
+ bool enable)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Unable to set bridge port learning on this
platform"));
+ return -1;
+}
+
+
+int
+virNetDevBridgePortGetUnicastFlood(const char *brname ATTRIBUTE_UNUSED,
+ const char *ifname ATTRIBUTE_UNUSED,
+ bool *enable ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Unable to get bridge port unicast_flood on this
platform"));
+ return -1;
+}
+
+
+int
+virNetDevBridgePortSetUnicastFlood(const char *brname ATTRIBUTE_UNUSED,
+ const char *ifname ATTRIBUTE_UNUSED,
+ bool enable ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Unable to set bridge port unicast_flood on this
platform"));
+ return -1;
+}
+#endif
+
/**
* virNetDevBridgeCreate:
@@ -517,7 +681,7 @@ int virNetDevBridgeSetSTPDelay(const char *brname,
* @brname: the bridge device name
* @delayms: the forward delay in milliseconds
*
- * Retrives the forward delay for the bridge device @brname
+ * Retrieves the forward delay for the bridge device @brname
* storing it in @delayms. The forward delay is only meaningful
* if STP is enabled
*
@@ -682,3 +846,70 @@ int virNetDevBridgeGetSTP(const char *brname,
return -1;
}
#endif
+
+#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__)
+/**
+ * virNetDevBridgeGetVlanFiltering:
+ * @brname: the bridge device name
+ * @enable: true or false
+ *
+ * Retrieves the vlan_filtering setting for the bridge device @brname
+ * storing it in @enable.
+ *
+ * Returns 0 on success, -1 on error
+ */
+int
+virNetDevBridgeGetVlanFiltering(const char *brname,
+ bool *enable)
+{
+ int ret = -1;
+ unsigned long value;
+
+ if (virNetDevBridgeGet(brname, "vlan_filtering", &value, -1, NULL) <
0)
+ goto cleanup;
+
+ *enable = !!value;
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+
+/**
+ * virNetDevBridgeSetVlanFiltering:
+ * @brname: the bridge name
+ * @enable: true or false
+ *
+ * Set the bridge vlan_filtering mode
+ *
+ * Returns 0 in case of success or -1 on failure
+ */
+
+int
+virNetDevBridgeSetVlanFiltering(const char *brname,
+ bool enable)
+{
+ return virNetDevBridgeSet(brname, "vlan_filtering", enable ? 1 : 0, -1,
NULL);
+}
+
+
+#else
+int
+virNetDevBridgeGetVlanFiltering(const char *brname ATTRIBUTE_UNUSED,
+ bool *enable ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Unable to get bridge vlan_filtering on this
platform"));
+ return -1;
+}
+
+
+int
+virNetDevBridgeSetVlanFiltering(const char *brname ATTRIBUTE_UNUSED,
+ bool enable ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Unable to set bridge vlan_filtering on this
platform"));
+ return -1;
+}
+#endif
diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h
index 13776d2..8188a2f 100644
--- a/src/util/virnetdevbridge.h
+++ b/src/util/virnetdevbridge.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007-2011 Red Hat, Inc.
+ * Copyright (C) 2007-2012, 2014 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
@@ -51,4 +51,30 @@ int virNetDevBridgeGetSTP(const char *brname,
bool *enable)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virNetDevBridgeSetVlanFiltering(const char *brname,
+ bool enable)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+int virNetDevBridgeGetVlanFiltering(const char *brname,
+ bool *enable)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+
+int virNetDevBridgePortGetLearning(const char *brname,
+ const char *ifname,
+ bool *enable)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_RETURN_CHECK;
+int virNetDevBridgePortSetLearning(const char *brname,
+ const char *ifname,
+ bool enable)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virNetDevBridgePortGetUnicastFlood(const char *brname,
+ const char *ifname,
+ bool *enable)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_RETURN_CHECK;
+int virNetDevBridgePortSetUnicastFlood(const char *brname,
+ const char *ifname,
+ bool enable)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+
#endif /* __VIR_NETDEV_BRIDGE_H__ */
--
1.9.3
midnight
New subject: [libvirt] [PATCHv3 2/9] util: functions to manage bridge fdb (forwarding database)
These two functions use netlink RTM_NEWNEIGH and RTM_DELNEIGH messages
to add and delete entries from a bridge's fdb. The bridge itself is
not referenced in the arguments to the functions, only the name of the
device that is attached to the bridge (since a device can only be
attached to one bridge at a time, and must be attached for this
function to make sense, the kernel easily infers which bridge's fdb is
being modified by looking at the device name/index).
---
No change from V2.
src/libvirt_private.syms | 2 +
src/util/virnetdevbridge.c | 147 +++++++++++++++++++++++++++++++++++++++++++++
src/util/virnetdevbridge.h | 16 +++++
3 files changed, 165 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7cb57a9..3e445fc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1661,6 +1661,8 @@ virNetDevBandwidthUpdateRate;
virNetDevBridgeAddPort;
virNetDevBridgeCreate;
virNetDevBridgeDelete;
+virNetDevBridgeFDBAdd;
+virNetDevBridgeFDBDel;
virNetDevBridgeGetSTP;
virNetDevBridgeGetSTPDelay;
virNetDevBridgeGetVlanFiltering;
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index c6a3e8e..78c4a25 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -37,6 +37,9 @@
#include <netinet/in.h>
#ifdef __linux__
+# if defined(HAVE_LIBNL)
+# include "virnetlink.h"
+# endif
# include <linux/sockios.h>
# include <linux/param.h> /* HZ */
# if NETINET_LINUX_WORKAROUND
@@ -913,3 +916,147 @@ virNetDevBridgeSetVlanFiltering(const char *brname
ATTRIBUTE_UNUSED,
return -1;
}
#endif
+
+
+#if defined(__linux__) && defined(HAVE_LIBNL)
+/* virNetDevBridgeFDBAddDel:
+ * @mac: the MAC address being added to the table
+ * @ifname: name of the port (interface) of the bridge that wants this MAC
+ * @flags: any of virNetDevBridgeFDBFlags ORed together.
+ * @isAdd: true if adding the entry, fals if deleting
+ *
+ * Use netlink RTM_NEWNEIGH and RTM_DELNEIGH messages to add and
+ * delete entries from a bridge's fdb. The bridge itself is not
+ * referenced in the arguments to the function, only the name of the
+ * device that is attached to the bridge (since a device can only be
+ * attached to one bridge at a time, and must be attached for this
+ * function to make sense, the kernel easily infers which bridge's fdb
+ * is being modified by looking at the device name/index).
+ *
+ * Attempting to add an existing entry, or delete a non-existing entry
+ * *is* an error.
+ *
+ * returns 0 on success, -1 on failure.
+ */
+static int
+virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname,
+ unsigned int flags, bool isAdd)
+{
+ int ret = -1;
+ struct nlmsghdr *resp = NULL;
+ struct nlmsgerr *err;
+ unsigned int recvbuflen;
+ struct nl_msg *nl_msg;
+ struct ndmsg ndm = { .ndm_family = PF_BRIDGE, .ndm_state = NUD_NOARP };
+
+ if (virNetDevGetIndex(ifname, &ndm.ndm_ifindex) < 0)
+ return -1;
+
+ if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_ROUTER)
+ ndm.ndm_flags |= NTF_ROUTER;
+ if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_SELF)
+ ndm.ndm_flags |= NTF_SELF;
+ if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_MASTER)
+ ndm.ndm_flags |= NTF_MASTER;
+ /* default self (same as iproute2's bridge command */
+ if (!(ndm.ndm_flags & (NTF_MASTER | NTF_SELF)))
+ ndm.ndm_flags |= NTF_SELF;
+
+ if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_PERMANENT)
+ ndm.ndm_state |= NUD_PERMANENT;
+ if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_TEMP)
+ ndm.ndm_state |= NUD_REACHABLE;
+ /* default permanent, same as iproute2's bridge command */
+ if (!(ndm.ndm_state & (NUD_PERMANENT | NUD_REACHABLE)))
+ ndm.ndm_state |= NUD_PERMANENT;
+
+ nl_msg = nlmsg_alloc_simple(isAdd ? RTM_NEWNEIGH : RTM_DELNEIGH,
+ NLM_F_REQUEST |
+ (isAdd ? (NLM_F_CREATE | NLM_F_EXCL) : 0));
+ if (!nl_msg) {
+ virReportOOMError();
+ return -1;
+ }
+
+ if (nlmsg_append(nl_msg, &ndm, sizeof(ndm), NLMSG_ALIGNTO) < 0)
+ goto buffer_too_small;
+ if (nla_put(nl_msg, NDA_LLADDR, VIR_MAC_BUFLEN, mac) < 0)
+ goto buffer_too_small;
+
+ /* NB: this message can also accept a Destination IP, a port, a
+ * vlan tag, and a via (see iproute2/bridge/fdb.c:fdb_modify()),
+ * but those aren't required for our application
+ */
+
+ 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 adding fdb entry for %s"), ifname);
+ goto cleanup;
+ }
+ break;
+ case NLMSG_DONE:
+ break;
+
+ default:
+ goto malformed_resp;
+ }
+
+ ret = 0;
+ cleanup:
+ nlmsg_free(nl_msg);
+ VIR_FREE(resp);
+ return ret;
+
+ 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;
+}
+
+
+#else
+static int
+virNetDevBridgeFDBAddDel(const virMacAddr *mac ATTRIBUTE_UNUSED,
+ const char *ifname ATTRIBUTE_UNUSED,
+ unsigned int fdbFlags ATTRIBUTE_UNUSED,
+ bool isAdd ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Unable to add/delete fdb entries on this
platform"));
+ return -1;
+}
+
+
+#endif
+
+int
+virNetDevBridgeFDBAdd(const virMacAddr *mac, const char *ifname,
+ unsigned int flags)
+{
+ return virNetDevBridgeFDBAddDel(mac, ifname, flags, true);
+}
+
+
+int
+virNetDevBridgeFDBDel(const virMacAddr *mac, const char *ifname,
+ unsigned int flags)
+{
+ return virNetDevBridgeFDBAddDel(mac, ifname, flags, false);
+}
diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h
index 8188a2f..141f231 100644
--- a/src/util/virnetdevbridge.h
+++ b/src/util/virnetdevbridge.h
@@ -24,6 +24,7 @@
# define __VIR_NETDEV_BRIDGE_H__
# include "internal.h"
+# include "virmacaddr.h"
int virNetDevBridgeCreate(const char *brname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
@@ -77,4 +78,19 @@ int virNetDevBridgePortSetUnicastFlood(const char *brname,
bool enable)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+typedef enum {
+ VIR_NETDEVBRIDGE_FDB_FLAG_ROUTER = (1 << 0),
+ VIR_NETDEVBRIDGE_FDB_FLAG_SELF = (1 << 1),
+ VIR_NETDEVBRIDGE_FDB_FLAG_MASTER = (1 << 2),
+
+ VIR_NETDEVBRIDGE_FDB_FLAG_PERMANENT = (1 << 3),
+ VIR_NETDEVBRIDGE_FDB_FLAG_TEMP = (1 << 4),
+} virNetDevBridgeFDBFlags;
+
+int virNetDevBridgeFDBAdd(const virMacAddr *mac, const char *ifname,
+ unsigned int flags)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virNetDevBridgeFDBDel(const virMacAddr *mac, const char *ifname,
+ unsigned int flags)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
#endif /* __VIR_NETDEV_BRIDGE_H__ */
--
1.9.3
midnight
New subject: [libvirt] [PATCHv3 3/9] conf: new network bridge device attribute macTableManager
The macTableManager attribute of a network's bridge subelement tells
libvirt how the bridge's MAC address table (used to determine the
egress port for packets) is managed. In the default mode, "kernel",
management is left to the kernel, which usually determines entries in
part by turning on promiscuous mode on all ports of the bridge,
flooding packets to all ports when the correct destination is unknown,
and adding/removing entries to the fdb as it sees incoming traffic
from particular MAC addresses. In "libvirt" mode, libvirt turns off
learning and flooding on all the bridge ports connected to guest
domain interfaces, and adds/removes entries according to the MAC
addresses in the domain interface configurations. A side effect of
turning off learning and unicast_flood on the ports of a bridge is
that (with Linux kernel 3.17 and newer), the kernel can automatically
turn off promiscuous mode on one or more of the bridge's ports
(usually only the one interface that is used to connect the bridge to
the physical network). The result is better performance (because
packets aren't being flooded to all ports, and can be dropped earlier
when they are of no interest) and slightly better security (a guest
can still send out packets with a spoofed source MAC address, but will
only receive traffic intended for the guest interface's configured MAC
address).
The attribute looks like this in the configuration:
<network>
<name>test</name>
<bridge name='br0' macTableManager='libvirt'/>
...
This patch only adds the config knob, documentation, and test
cases. The functionality behind this knob is added in later patches.
---
Change from V2 - changed attribute name from "fdb" to
"macTableManager", and values from "learnWithFlooding" and
"managed" to "kernel" and "libvirt".
docs/formatnetwork.html.in | 50 ++++++++++++++++++---
docs/schemas/network.rng | 9 ++++
src/conf/network_conf.c | 51 +++++++++++++++++-----
src/conf/network_conf.h | 11 +++++
src/libvirt_private.syms | 2 +
tests/networkxml2xmlin/host-bridge-no-flood.xml | 6 +++
.../nat-network-explicit-flood.xml | 21 +++++++++
tests/networkxml2xmlout/host-bridge-no-flood.xml | 6 +++
.../nat-network-explicit-flood.xml | 23 ++++++++++
tests/networkxml2xmltest.c | 2 +
10 files changed, 164 insertions(+), 17 deletions(-)
create mode 100644 tests/networkxml2xmlin/host-bridge-no-flood.xml
create mode 100644 tests/networkxml2xmlin/nat-network-explicit-flood.xml
create mode 100644 tests/networkxml2xmlout/host-bridge-no-flood.xml
create mode 100644 tests/networkxml2xmlout/nat-network-explicit-flood.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 7bcf316..ca0e207 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -81,7 +81,7 @@
<pre>
...
- <bridge name="virbr0" stp="on"
delay="5"/>
+ <bridge name="virbr0" stp="on" delay="5"
macTableManager="libvirt"/>
<domain name="example.com"/>
<forward mode="nat" dev="eth0"/>
...</pre>
@@ -92,18 +92,56 @@
defines the name of a bridge device which will be used to construct
the virtual network. The virtual machines will be connected to this
bridge device allowing them to talk to each other. The bridge device
- may also be connected to the LAN. It is recommended that bridge
- device names started with the prefix <code>vir</code>, but the name
- <code>virbr0</code> is reserved for the "default" virtual
- network. This element should always be provided when defining
+ may also be connected to the LAN. When defining
a new network with a <code><forward></code> mode of
+
"nat" or "route" (or an isolated network with
- no <code><forward></code> element).
+ no <code><forward></code> element), libvirt will
+ automatically generate a unique name for the bridge device if
+ none is given, and this name will be permanently stored in the
+ network configuration so that that the same name will be used
+ every time the network is started. For these types of networks
+ (nat, routed, and isolated), a bridge name beginning with the
+ prefix "virbr" is recommended (and that is what is
+ auto-generated), but not enforced.
Attribute <code>stp</code> specifies if Spanning Tree Protocol
is 'on' or 'off' (default is
'on'). Attribute <code>delay</code> sets the bridge's
forward
delay value in seconds (default is 0).
<span class="since">Since 0.3.0</span>
+
+ <p>
+ The <code>macTableManager</code> attribute of the bridge
+ element is used to tell libvirt how the bridge's MAC address
+ table (used to determine the correct egress port for packets
+ based on destination MAC address) will be managed. In the
+ default <code>kernel</code> setting, the kernel
+ automatically adds and removes entries, typically using
+ learning, flooding, and promiscuous mode on the bridge's
+ ports in order to determine the proper egress port for
+ packets. When <code>macTableManager</code> is set
+ to <code>libvirt</code>, libvirt disables kernel management
+ of the MAC table (in the case of the Linux host bridge, this
+ means enabling vlan_filtering on the bridge, and disabling
+ learning and unicast_filter for all bridge ports), and
+ explicitly adds/removes entries to the table according to
+ the MAC addresses in the domain interface configurations.
+ Allowing libvirt to manage the MAC table can improve
+ performance - with a Linux host bridge, for example, turning
+ off learning and unicast_flood on ports has its own
+ performance advantage, and can also lead to an additional
+ boost by permitting the kernel to automatically turn off
+ promiscuous mode on some ports of the bridge (in particular,
+ the port attaching the bridge to the physical
+ network). However, it can also cause some networking setups
+ to stop working (e.g. vlan tagging, multicast,
+ guest-initiated changes to MAC address) and is not supported
+ by older kernels.
+ <span class="since">Since 1.2.11, requires kernel 3.17 or
+ newer</span>
+ </p>
+
+
</dd>
<dt><code>domain</code></dt>
<dd>
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 4546f80..9a7d156 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -65,6 +65,15 @@
</attribute>
</optional>
+ <optional>
+ <attribute name="macTableManager">
+ <choice>
+ <value>kernel</value>
+ <value>libvirt</value>
+ </choice>
+ </attribute>
+ </optional>
+
</element>
</optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 97719ed..0d09def 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -55,6 +55,10 @@ VIR_ENUM_IMPL(virNetworkForward,
VIR_NETWORK_FORWARD_LAST,
"none", "nat", "route", "bridge",
"private", "vepa", "passthrough", "hostdev")
+VIR_ENUM_IMPL(virNetworkBridgeMACTableManager,
+ VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LAST,
+ "default", "kernel", "libvirt")
+
VIR_ENUM_DECL(virNetworkForwardHostdevDevice)
VIR_ENUM_IMPL(virNetworkForwardHostdevDevice,
VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST,
@@ -2108,6 +2112,18 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
VIR_FREE(tmp);
+ tmp = virXPathString("string(./bridge[1]/@macTableManager)", ctxt);
+ if (tmp) {
+ if ((def->macTableManager
+ = virNetworkBridgeMACTableManagerTypeFromString(tmp)) <= 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid macTableManager setting '%s' "
+ "in network '%s'"), tmp, def->name);
+ goto error;
+ }
+ VIR_FREE(tmp);
+ }
+
tmp = virXPathString("string(./mac[1]/@address)", ctxt);
if (tmp) {
if (virMacAddrParse(tmp, &def->mac) < 0) {
@@ -2290,6 +2306,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->name);
goto error;
}
+ if (def->macTableManager) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("bridge macTableManager setting not allowed "
+ "in %s mode (network '%s')"),
+ virNetworkForwardTypeToString(def->forward.type),
+ def->name);
+ goto error;
+ }
/* fall through to next case */
case VIR_NETWORK_FORWARD_BRIDGE:
if (def->delay || stp) {
@@ -2783,22 +2807,27 @@ virNetworkDefFormatBuf(virBufferPtr buf,
virBufferAddLit(buf, "</forward>\n");
}
+
if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
- def->forward.type == VIR_NETWORK_FORWARD_NAT ||
- def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
+ def->forward.type == VIR_NETWORK_FORWARD_NAT ||
+ def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
+ def->bridge || def->macTableManager) {
virBufferAddLit(buf, "<bridge");
- if (def->bridge)
- virBufferEscapeString(buf, " name='%s'", def->bridge);
- virBufferAsprintf(buf, " stp='%s' delay='%ld'/>\n",
- def->stp ? "on" : "off",
- def->delay);
- } else if (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE &&
- def->bridge) {
- virBufferEscapeString(buf, "<bridge name='%s'/>\n",
def->bridge);
+ virBufferEscapeString(buf, " name='%s'", def->bridge);
+ if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
+ def->forward.type == VIR_NETWORK_FORWARD_NAT ||
+ def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
+ virBufferAsprintf(buf, " stp='%s' delay='%ld'",
+ def->stp ? "on" : "off",
def->delay);
+ }
+ if (def->macTableManager) {
+ virBufferAsprintf(buf, " macTableManager='%s'",
+
virNetworkBridgeMACTableManagerTypeToString(def->macTableManager));
+ }
+ virBufferAddLit(buf, "/>\n");
}
-
if (def->mac_specified) {
char macaddr[VIR_MAC_STRING_BUFLEN];
virMacAddrFormat(&def->mac, macaddr);
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 660cd2d..8110028 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -54,6 +54,16 @@ typedef enum {
} virNetworkForwardType;
typedef enum {
+ VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_DEFAULT = 0,
+ VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_KERNEL,
+ VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT,
+
+ VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LAST,
+} virNetworkBridgeMACTableManagerType;
+
+VIR_ENUM_DECL(virNetworkBridgeMACTableManager)
+
+typedef enum {
VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NONE = 0,
VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI,
VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV,
@@ -231,6 +241,7 @@ struct _virNetworkDef {
int connections; /* # of guest interfaces connected to this network */
char *bridge; /* Name of bridge device */
+ int macTableManager; /* enum virNetworkBridgeMACTableManager */
char *domain;
unsigned long delay; /* Bridge forward delay (ms) */
bool stp; /* Spanning tree protocol */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3e445fc..645d516 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -529,6 +529,8 @@ virNetDevVPortTypeToString;
# conf/network_conf.h
virNetworkAssignDef;
+virNetworkBridgeMACTableManagerTypeFromString;
+virNetworkBridgeMACTableManagerTypeToString;
virNetworkConfigChangeSetup;
virNetworkConfigFile;
virNetworkDefCopy;
diff --git a/tests/networkxml2xmlin/host-bridge-no-flood.xml
b/tests/networkxml2xmlin/host-bridge-no-flood.xml
new file mode 100644
index 0000000..562e697
--- /dev/null
+++ b/tests/networkxml2xmlin/host-bridge-no-flood.xml
@@ -0,0 +1,6 @@
+<network>
+ <name>host-bridge-net</name>
+ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8e</uuid>
+ <forward mode="bridge"/>
+ <bridge name="br0" macTableManager="libvirt"/>
+</network>
diff --git a/tests/networkxml2xmlin/nat-network-explicit-flood.xml
b/tests/networkxml2xmlin/nat-network-explicit-flood.xml
new file mode 100644
index 0000000..9738b81
--- /dev/null
+++ b/tests/networkxml2xmlin/nat-network-explicit-flood.xml
@@ -0,0 +1,21 @@
+<network>
+ <name>default</name>
+ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+ <bridge name="virbr0" macTableManager="kernel"/>
+ <forward mode="nat" dev="eth1"/>
+ <ip address="192.168.122.1" netmask="255.255.255.0">
+ <dhcp>
+ <range start="192.168.122.2" end="192.168.122.254"/>
+ <host mac="00:16:3e:77:e2:ed" name="a.example.com"
ip="192.168.122.10"/>
+ <host mac="00:16:3e:3e:a9:1a" name="b.example.com"
ip="192.168.122.11"/>
+ </dhcp>
+ </ip>
+ <ip family="ipv4" address="192.168.123.1"
netmask="255.255.255.0">
+ </ip>
+ <ip family="ipv6" address="2001:db8:ac10:fe01::1"
prefix="64">
+ </ip>
+ <ip family="ipv6" address="2001:db8:ac10:fd01::1"
prefix="64">
+ </ip>
+ <ip family="ipv4" address="10.24.10.1">
+ </ip>
+</network>
diff --git a/tests/networkxml2xmlout/host-bridge-no-flood.xml
b/tests/networkxml2xmlout/host-bridge-no-flood.xml
new file mode 100644
index 0000000..bdbbf3b
--- /dev/null
+++ b/tests/networkxml2xmlout/host-bridge-no-flood.xml
@@ -0,0 +1,6 @@
+<network>
+ <name>host-bridge-net</name>
+ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8e</uuid>
+ <forward mode='bridge'/>
+ <bridge name='br0' macTableManager='libvirt'/>
+</network>
diff --git a/tests/networkxml2xmlout/nat-network-explicit-flood.xml
b/tests/networkxml2xmlout/nat-network-explicit-flood.xml
new file mode 100644
index 0000000..305c3b7
--- /dev/null
+++ b/tests/networkxml2xmlout/nat-network-explicit-flood.xml
@@ -0,0 +1,23 @@
+<network>
+ <name>default</name>
+ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+ <forward dev='eth1' mode='nat'>
+ <interface dev='eth1'/>
+ </forward>
+ <bridge name='virbr0' stp='on' delay='0'
macTableManager='kernel'/>
+ <ip address='192.168.122.1' netmask='255.255.255.0'>
+ <dhcp>
+ <range start='192.168.122.2' end='192.168.122.254'/>
+ <host mac='00:16:3e:77:e2:ed' name='a.example.com'
ip='192.168.122.10'/>
+ <host mac='00:16:3e:3e:a9:1a' name='b.example.com'
ip='192.168.122.11'/>
+ </dhcp>
+ </ip>
+ <ip family='ipv4' address='192.168.123.1'
netmask='255.255.255.0'>
+ </ip>
+ <ip family='ipv6' address='2001:db8:ac10:fe01::1'
prefix='64'>
+ </ip>
+ <ip family='ipv6' address='2001:db8:ac10:fd01::1'
prefix='64'>
+ </ip>
+ <ip family='ipv4' address='10.24.10.1'>
+ </ip>
+</network>
diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c
index 65ac591..34a5211 100644
--- a/tests/networkxml2xmltest.c
+++ b/tests/networkxml2xmltest.c
@@ -120,6 +120,8 @@ mymain(void)
DO_TEST("hostdev");
DO_TEST_FULL("hostdev-pf", VIR_NETWORK_XML_INACTIVE);
DO_TEST("passthrough-address-crash");
+ DO_TEST("nat-network-explicit-flood");
+ DO_TEST("host-bridge-no-flood");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
1.9.3
12:33 a.m.
New subject: [libvirt] [PATCHv3 3/9] conf: new network bridge device attribute macTableManager
On Mon, Dec 08, 2014 at 11:00:03AM -0500, Laine Stump wrote:
The macTableManager attribute of a network's bridge subelement
tells
libvirt how the bridge's MAC address table (used to determine the
egress port for packets) is managed. In the default mode, "kernel",
management is left to the kernel, which usually determines entries in
part by turning on promiscuous mode on all ports of the bridge,
flooding packets to all ports when the correct destination is unknown,
and adding/removing entries to the fdb as it sees incoming traffic
from particular MAC addresses. In "libvirt" mode, libvirt turns off
learning and flooding on all the bridge ports connected to guest
domain interfaces, and adds/removes entries according to the MAC
addresses in the domain interface configurations. A side effect of
turning off learning and unicast_flood on the ports of a bridge is
that (with Linux kernel 3.17 and newer), the kernel can automatically
turn off promiscuous mode on one or more of the bridge's ports
(usually only the one interface that is used to connect the bridge to
the physical network). The result is better performance (because
packets aren't being flooded to all ports, and can be dropped earlier
when they are of no interest) and slightly better security (a guest
can still send out packets with a spoofed source MAC address, but will
only receive traffic intended for the guest interface's configured MAC
address).
The attribute looks like this in the configuration:
<network>
<name>test</name>
<bridge name='br0' macTableManager='libvirt'/>
...
This patch only adds the config knob, documentation, and test
cases. The functionality behind this knob is added in later patches.
ACK, design looks good now.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
midnight
New subject: [libvirt] [PATCHv3 4/9] network: save bridge name in ActualNetDef when actualType==network too
When the actualType of a virDomainNetDef is "network", it means that
we are connecting to a libvirt-managed network (routed, natted, or
isolated) which does use a bridge device (created by libvirt). In the
past we have required drivers such as qemu to call the public API to
retrieve the bridge name in this case (even though it is available in
the NetDef's ActualNetDef if the actualType is "bridge" (i.e., an
externally-created bridge that isn't managed by libvirt). There is no
real reason for this difference, and as a matter of fact it
complicates things for qemu. Also, there is another bridge-related
attribute (macTableManager) that will need to be available in both
cases, so this makes things consistent.
In order to avoid problems when restarting libvirtd after an update
from an older version that *doesn't* store the network's bridgename in
the ActualNetDef, we also need to put it in place during
networkNotifyActualDevice() (this function is run for each interface
of each domain whenever libvirtd is restarted).
Along with making the bridge name available in the internal object, it
is also now reported in the <source> element of the <interface> state
XML (or the <actual> subelement in the internally-stored format).
The one oddity about this change is that usually there is a separate
union for every different "type" in a higher level object (e.g. in the
case of a virDomainNetDef there are separate "network" and "bridge"
members of the union that pivots on the type), but in this case
network and bridge types both have exactly the same attributes, so the
"bridge" member is used for both type==network and type==bridge.
---
No change from V2.
src/conf/domain_conf.c | 102 +++++++++++++++++++++++---------------------
src/network/bridge_driver.c | 20 +++++++++
2 files changed, 73 insertions(+), 49 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d81c37..f45969f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1354,6 +1354,7 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
VIR_FREE(def->data.bridge.brname);
break;
case VIR_DOMAIN_NET_TYPE_DIRECT:
@@ -7106,9 +7107,12 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
goto error;
}
VIR_FREE(class_id);
- } else if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+ }
+ if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ actual->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
char *brname = virXPathString("string(./source/@bridge)", ctxt);
- if (!brname) {
+
+ if (!brname && actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing <source> element with bridge name in
"
"interface's <actual> element"));
@@ -17126,60 +17130,59 @@ virDomainHostdevDefFormatCaps(virBufferPtr buf,
static int
virDomainActualNetDefContentsFormat(virBufferPtr buf,
virDomainNetDefPtr def,
- const char *typeStr,
bool inSubelement,
unsigned int flags)
{
- const char *mode;
-
- switch (virDomainNetGetActualType(def)) {
- case VIR_DOMAIN_NET_TYPE_BRIDGE:
- virBufferEscapeString(buf, "<source bridge='%s'/>\n",
- virDomainNetGetActualBridgeName(def));
- break;
-
- case VIR_DOMAIN_NET_TYPE_DIRECT:
- virBufferAddLit(buf, "<source");
- virBufferEscapeString(buf, " dev='%s'",
- virDomainNetGetActualDirectDev(def));
+ int actualType = virDomainNetGetActualType(def);
- mode = virNetDevMacVLanModeTypeToString(virDomainNetGetActualDirectMode(def));
- if (!mode) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected source mode %d"),
- virDomainNetGetActualDirectMode(def));
- return -1;
- }
- virBufferAsprintf(buf, " mode='%s'/>\n", mode);
- break;
-
- case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
if (virDomainHostdevDefFormatSubsys(buf, virDomainNetGetActualHostdev(def),
flags, true) < 0) {
return -1;
}
- break;
-
- case VIR_DOMAIN_NET_TYPE_NETWORK:
- if (!inSubelement) {
- /* the <source> element isn't included in <actual>
- * (i.e. when we're putting our output into a subelement
- * rather than inline) because the main element has the
- * same info already. If we're outputting inline, though,
- * we *do* need to output <source>, because the caller
- * won't have done it.
+ } else {
+ virBufferAddLit(buf, "<source");
+ if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK && !inSubelement) {
+ /* When we're putting our output into the <actual>
+ * subelement rather than the main <interface>, the
+ * network name isn't included in the <source> because the
+ * main interface element's <source> has the same info
+ * already. If we've been called to output directly into
+ * the main element's <source> though (the case here -
+ * "!inSubElement"), we *do* need to output the network
+ * name, because the caller won't have done it).
*/
- virBufferEscapeString(buf, "<source
network='%s'/>\n",
- def->data.network.name);
+ virBufferEscapeString(buf, " network='%s'",
def->data.network.name);
}
- if (def->data.network.actual &&
def->data.network.actual->class_id)
- virBufferAsprintf(buf, "<class id='%u'/>\n",
- def->data.network.actual->class_id);
- break;
- default:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected actual net type %s"), typeStr);
- return -1;
+ if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ /* actualType == NETWORK includes the name of the bridge
+ * that is used by the network, whether we are
+ * "inSubElement" or not.
+ */
+ virBufferEscapeString(buf, " bridge='%s'",
+ virDomainNetGetActualBridgeName(def));
+ } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ const char *mode;
+
+ virBufferEscapeString(buf, " dev='%s'",
+ virDomainNetGetActualDirectDev(def));
+ mode =
virNetDevMacVLanModeTypeToString(virDomainNetGetActualDirectMode(def));
+ if (!mode) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected source mode %d"),
+ virDomainNetGetActualDirectMode(def));
+ return -1;
+ }
+ virBufferAsprintf(buf, " mode='%s'", mode);
+ }
+
+ virBufferAddLit(buf, "/>\n");
+ }
+ if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT &&
+ def->data.network.actual && def->data.network.actual->class_id)
{
+ virBufferAsprintf(buf, "<class id='%u'/>\n",
+ def->data.network.actual->class_id);
}
if (virNetDevVlanFormat(virDomainNetGetActualVlan(def), buf) < 0)
@@ -17227,7 +17230,7 @@ virDomainActualNetDefFormat(virBufferPtr buf,
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
- if (virDomainActualNetDefContentsFormat(buf, def, typeStr, true, flags) < 0)
+ if (virDomainActualNetDefContentsFormat(buf, def, true, flags) < 0)
return -1;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</actual>\n");
@@ -17398,7 +17401,7 @@ virDomainNetDefFormat(virBufferPtr buf,
* the standard place... (this is for public reporting of
* interface status)
*/
- if (virDomainActualNetDefContentsFormat(buf, def, typeStr, false, flags) < 0)
+ if (virDomainActualNetDefContentsFormat(buf, def, false, flags) < 0)
return -1;
} else {
/* ...but if we've asked for the inactive XML (rather than
@@ -20751,7 +20754,8 @@ virDomainNetGetActualBridgeName(virDomainNetDefPtr iface)
return iface->data.bridge.brname;
if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
iface->data.network.actual &&
- iface->data.network.actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE)
+ (iface->data.network.actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ iface->data.network.actual->type == VIR_DOMAIN_NET_TYPE_NETWORK))
return iface->data.network.actual->data.bridge.brname;
return NULL;
}
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 1459f04..a41510b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3795,6 +3795,15 @@ networkAllocateActualDevice(virDomainDefPtr dom,
*/
iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK;
+ /* we also store the bridge device
+ * in iface->data.network.actual->data.bridge for later use
+ * after the domain's tap device is created (to attach to the
+ * bridge and set flood/learning mode on the tap device)
+ */
+ if (VIR_STRDUP(iface->data.network.actual->data.bridge.brname,
+ netdef->bridge) < 0)
+ goto error;
+
if (networkPlugBandwidth(network, iface) < 0)
goto error;
@@ -4131,6 +4140,17 @@ networkNotifyActualDevice(virDomainDefPtr dom,
}
netdef = network->def;
+ /* if we're restarting libvirtd after an upgrade from a version
+ * that didn't save bridge name in actualNetDef for
+ * actualType==network, we need to copy it in so that it will be
+ * available in all cases
+ */
+ if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ !iface->data.network.actual->data.bridge.brname &&
+ (VIR_STRDUP(iface->data.network.actual->data.bridge.brname,
+ netdef->bridge) < 0))
+ goto error;
+
if (!iface->data.network.actual ||
(actualType != VIR_DOMAIN_NET_TYPE_DIRECT &&
actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV)) {
--
1.9.3
midnight
New subject: [libvirt] [PATCHv3 5/9] network: store network macTableManager setting in NetDef actual object
At the time that the network driver allocates a connection to a
network, the tap device that will be used hasn't yet been created -
that will be done later by qemu (or lxc or whoever) - but if the
network has macTableManager='libvirt', then when we do get around to
creating the tap device, we will need to add an entry for it to the
network bridge's fdb (forwarding database) *and* turn off learning and
unicast_flood for that tap device in the bridge's sysfs settings. This
means that qemu needs to know both the bridge name as well as the
setting of macTableManager, so we either need to create a new API to
retrieve that info, or just pass it back in the ActualNetDef that is
created during networkAllocateActualDevice. We choose the latter
method, since it's already done for the bridge device, and it has the
side effect of making the information available in domain status.
(NB: in the future, I think that the tap device should actually be
created by networkAllocateActualDevice(), as that will solve several
other problems, but that is a battle for another day, and this
information will still be useful outside the network driver)
---
Change from V2: attribute name.
src/conf/domain_conf.c | 30 ++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/libvirt_private.syms | 1 +
src/network/bridge_driver.c | 6 +++++-
4 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f45969f..843cdec 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -51,6 +51,7 @@
#include "netdev_bandwidth_conf.h"
#include "netdev_vlan_conf.h"
#include "device_conf.h"
+#include "network_conf.h"
#include "virtpm.h"
#include "virstring.h"
@@ -7003,6 +7004,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
char *mode = NULL;
char *addrtype = NULL;
char *trustGuestRxFilters = NULL;
+ char *macTableManager = NULL;
if (VIR_ALLOC(actual) < 0)
return -1;
@@ -7119,6 +7121,16 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
goto error;
}
actual->data.bridge.brname = brname;
+ macTableManager = virXPathString("string(./source/@macTableManager)",
ctxt);
+ if (macTableManager &&
+ (actual->data.bridge.macTableManager
+ = virNetworkBridgeMACTableManagerTypeFromString(macTableManager)) <= 0)
{
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid macTableManager setting '%s' "
+ "in domain interface's <actual>
element"),
+ macTableManager);
+ goto error;
+ }
}
bandwidth_node = virXPathNode("./bandwidth", ctxt);
@@ -7139,6 +7151,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
VIR_FREE(mode);
VIR_FREE(addrtype);
VIR_FREE(trustGuestRxFilters);
+ VIR_FREE(macTableManager);
virDomainActualNetDefFree(actual);
ctxt->node = save_ctxt;
@@ -17156,12 +17169,18 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
}
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ int macTableManager = virDomainNetGetActualBridgeMACTableManager(def);
+
/* actualType == NETWORK includes the name of the bridge
* that is used by the network, whether we are
* "inSubElement" or not.
*/
virBufferEscapeString(buf, " bridge='%s'",
virDomainNetGetActualBridgeName(def));
+ if (macTableManager) {
+ virBufferAsprintf(buf, " macTableManager='%s'",
+
virNetworkBridgeMACTableManagerTypeToString(macTableManager));
+ }
} else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
const char *mode;
@@ -20760,6 +20779,17 @@ virDomainNetGetActualBridgeName(virDomainNetDefPtr iface)
return NULL;
}
+int
+virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface)
+{
+ if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ iface->data.network.actual &&
+ (iface->data.network.actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ iface->data.network.actual->type == VIR_DOMAIN_NET_TYPE_NETWORK))
+ return iface->data.network.actual->data.bridge.macTableManager;
+ return 0;
+}
+
const char *
virDomainNetGetActualDirectDev(virDomainNetDefPtr iface)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 439f3c0..e10b3c5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -888,6 +888,7 @@ struct _virDomainActualNetDef {
union {
struct {
char *brname;
+ int macTableManager; /* enum virNetworkBridgeMACTableManagerType */
} bridge;
struct {
char *linkdev;
@@ -2540,6 +2541,7 @@ int virDomainGraphicsListenSetNetwork(virDomainGraphicsDefPtr def,
int virDomainNetGetActualType(virDomainNetDefPtr iface);
const char *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface);
+int virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface);
const char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface);
int virDomainNetGetActualDirectMode(virDomainNetDefPtr iface);
virDomainHostdevDefPtr virDomainNetGetActualHostdev(virDomainNetDefPtr iface);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 645d516..6df2784 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -328,6 +328,7 @@ virDomainNetFind;
virDomainNetFindIdx;
virDomainNetGenerateMAC;
virDomainNetGetActualBandwidth;
+virDomainNetGetActualBridgeMACTableManager;
virDomainNetGetActualBridgeName;
virDomainNetGetActualDirectDev;
virDomainNetGetActualDirectMode;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a41510b..93d36ab 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3795,7 +3795,7 @@ networkAllocateActualDevice(virDomainDefPtr dom,
*/
iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK;
- /* we also store the bridge device
+ /* we also store the bridge device and macTableManager settings
* in iface->data.network.actual->data.bridge for later use
* after the domain's tap device is created (to attach to the
* bridge and set flood/learning mode on the tap device)
@@ -3803,6 +3803,8 @@ networkAllocateActualDevice(virDomainDefPtr dom,
if (VIR_STRDUP(iface->data.network.actual->data.bridge.brname,
netdef->bridge) < 0)
goto error;
+ iface->data.network.actual->data.bridge.macTableManager
+ = netdef->macTableManager;
if (networkPlugBandwidth(network, iface) < 0)
goto error;
@@ -3818,6 +3820,8 @@ networkAllocateActualDevice(virDomainDefPtr dom,
if (VIR_STRDUP(iface->data.network.actual->data.bridge.brname,
netdef->bridge) < 0)
goto error;
+ iface->data.network.actual->data.bridge.macTableManager
+ = netdef->macTableManager;
/* merge virtualports from interface, network, and portgroup to
* arrive at actual virtualport to use
--
1.9.3
midnight
New subject: [libvirt] [PATCHv3 6/9] network: setup bridge devices for macTableManager='libvirt'
When the bridge device for a network has macTableManager='libvirt' the
intent is that all kernel management of the bridge's MAC table
(Forwarding Database, or fdb, in the case of a Linux Host Bridge) be
disabled, with libvirt handling updates to the table instead. The
setup required for the bridge itself is:
1) set the "vlan_filtering" property of the bridge device to 1.
2) If the bridge has a "Dummy" tap device used to set a fixed MAC
address on the bridge (which is always the case for a bridge created
by libvirt, and never the case for a bridge created by the host system
network config), turn off learning and unicast_flood on this tap (this
is needed even though this tap is never IFF_UP, because the kernel
ignores the IFF_UP flag of devices when using their settings to
automatically decide whether or not to turn off promiscuous mode for
any attached device).
(1) is done both for libvirt-created/managed bridges, and for bridges
that are created by the host system config, while (2) is done only for
bridges created by libvirt (i.e. for forward modes of nat, routed, and
isolated bridges)
There is no attempt to turn vlan_filtering off when destroying the
network because in the case of a libvirt-created bridge, the bridge is
about to be destroyed anyway, and in the case of a system bridge, if
the other devices attached to the bridge could operate properly before
destroying libvirt's network object, they will continue to operate
properly (this is similar to the way that libvirt will enable
ip_forwarding whenever a routed/natted network is started, but will
never attempt to disable it if they are stopped).
---
Change from V2: attribute name.
src/network/bridge_driver.c | 54 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 93d36ab..29222d0 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1906,6 +1906,29 @@ networkAddAddrToBridge(virNetworkObjPtr network,
return 0;
}
+
+static int
+networkStartHandleMACTableManagerMode(virNetworkObjPtr network,
+ const char *macTapIfName)
+{
+ const char *brname = network->def->bridge;
+
+ if (brname &&
+ network->def->macTableManager
+ == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
+ if (virNetDevBridgeSetVlanFiltering(brname, true) < 0)
+ return -1;
+ if (macTapIfName) {
+ if (virNetDevBridgePortSetLearning(brname, macTapIfName, false) < 0)
+ return -1;
+ if (virNetDevBridgePortSetUnicastFlood(brname, macTapIfName, false) < 0)
+ return -1;
+ }
+ }
+ return 0;
+}
+
+
/* add an IP (static) route to a bridge */
static int
networkAddRouteToBridge(virNetworkObjPtr network,
@@ -2032,6 +2055,9 @@ networkStartNetworkVirtual(virNetworkObjPtr network)
goto err2;
}
+ if (networkStartHandleMACTableManagerMode(network, macTapIfName) < 0)
+ goto err2;
+
/* Bring up the bridge interface */
if (virNetDevSetOnline(network->def->bridge, 1) < 0)
goto err2;
@@ -2176,6 +2202,27 @@ static int networkShutdownNetworkVirtual(virNetworkObjPtr network)
}
+static int
+networkStartNetworkBridge(virNetworkObjPtr network)
+{
+ /* put anything here that needs to be done each time a network of
+ * type BRIDGE, is started. On failure, undo anything you've done,
+ * and return -1. On success return 0.
+ */
+ return networkStartHandleMACTableManagerMode(network, NULL);
+}
+
+static int
+networkShutdownNetworkBridge(virNetworkObjPtr network ATTRIBUTE_UNUSED)
+{
+ /* put anything here that needs to be done each time a network of
+ * type BRIDGE is shutdown. On failure, undo anything you've done,
+ * and return -1. On success return 0.
+ */
+ return 0;
+}
+
+
/* networkCreateInterfacePool:
* @netdef: the original NetDef from the network
*
@@ -2339,6 +2386,10 @@ networkStartNetwork(virNetworkObjPtr network)
break;
case VIR_NETWORK_FORWARD_BRIDGE:
+ if (networkStartNetworkBridge(network) < 0)
+ goto cleanup;
+ break;
+
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
@@ -2405,6 +2456,9 @@ static int networkShutdownNetwork(virNetworkObjPtr network)
break;
case VIR_NETWORK_FORWARD_BRIDGE:
+ ret = networkShutdownNetworkBridge(network);
+ break;
+
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
--
1.9.3
midnight
New subject: [libvirt] [PATCHv3 7/9] qemu: setup tap devices for macTableManager='libvirt'
When libvirt is managing the MAC table of a Linux host bridge, it must
turn off learning and unicast_flood for each tap device attached to
that bridge, then add a Forwarding Database (fdb) entry for the tap
device using the MAC address from the domain interface config.
Once we have disabled learning and flooding, any packet that has a
destination MAC address not present in the fdb will be dropped by the
bridge. This, along with the opportunistic disabling of promiscuous
mode[*], can result in enhanced network performance. and a potential
slight security improvement.
[*] If there is only one device on the bridge with learning/unicast_flood
enabled, then that device will automatically have promiscuous mode
disabled. If there are *no* devices with learning/unicast_flood
enabled (e.g. for a libvirt "route", "nat", or isolated network that
has no physical device attached), then all non-tap devices will have
promiscuous mode disabled (tap devices always have promiscuous mode
enabled, which may be a bug in the kernel, but in practice has 0
effect).
None of this has any effect for kernels prior to 3.15 (upstream kernel
commit 2796d0c648c940b4796f84384fbcfb0a2399db84 "bridge: Automatically
manage port promiscuous mode"). Even after that, until kernel 3.17
(upstream commit 5be5a2df40f005ea7fb7e280e87bbbcfcf1c2fc0 "bridge: Add
filtering support for default_pvid") traffic will not be properly
forwarded without manually adding vlan table entries. Unfortunately,
although the presence of the first patch is signalled by existence of
the "learning" and "unicast_flood" options in sysfs, there is no
reliable way to query whether or not the system's kernel has the
second of those patches installed, the only thing that can be done is
to try the setting and see if traffic continues to pass.
---
Change from V2: attribute name and commit log explanation.
src/qemu/qemu_command.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1831323..11ed58b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -35,6 +35,7 @@
#include "virerror.h"
#include "virfile.h"
#include "virnetdev.h"
+#include "virnetdevbridge.h"
#include "virstring.h"
#include "virtime.h"
#include "viruuid.h"
@@ -344,6 +345,24 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
virDomainAuditNetDevice(def, net, tunpath, false);
goto cleanup;
}
+ if (virDomainNetGetActualBridgeMACTableManager(net)
+ == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
+ /* libvirt is managing the FDB of the bridge this device
+ * is attaching to, so we need to turn off learning and
+ * unicast_flood on the device to prevent the kernel from
+ * adding any FDB entries for it, then add an fdb entry
+ * outselves, using the MAC address from the interface
+ * config.
+ */
+ if (virNetDevBridgePortSetLearning(brname, net->ifname, false) < 0)
+ goto cleanup;
+ if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false) <
0)
+ goto cleanup;
+ if (virNetDevBridgeFDBAdd(&net->mac, net->ifname,
+ VIR_NETDEVBRIDGE_FDB_FLAG_MASTER |
+ VIR_NETDEVBRIDGE_FDB_FLAG_TEMP) < 0)
+ goto cleanup;
+ }
} else {
if (qemuCreateInBridgePortWithHelper(cfg, brname,
&net->ifname,
--
1.9.3
midnight
New subject: [libvirt] [PATCHv3 8/9] qemu: always use virDomainNetGetActualBridgeName to get interface's bridge
qemuNetworkIfaceConnect() used to have a special case for
actualType='network' (a network with forward mode of route, nat, or
isolated) to call the libvirt public API to retrieve the bridge being
used by a network. That is no longer necessary - since all network
types that use a bridge and tap device now get the bridge name stored
in the ActualNetDef, we can just always use
virDomainNetGetActualBridgeName() instead.
(an audit of the two callers to qemuNetworkIfaceConnect() confirms
that it is never called for any other type of network, so the dead
code in the else statement (logging an internal error if it is called
for any other type of network) is eliminated in the process.)
---
No change from V2.
src/qemu/qemu_command.c | 34 +++++++++----------------------
src/qemu/qemu_hotplug.c | 54 +++++++------------------------------------------
2 files changed, 16 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 11ed58b..48bdf4e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -277,6 +277,11 @@ static int qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr
cfg,
return *tapfd < 0 ? -1 : 0;
}
+/* qemuNetworkIfaceConnect - *only* called if actualType is
+ * VIR_DOMAIN_NET_TYPE_NETWORK or VIR_DOMAIN_NET_TYPE_BRIDGE (i.e. if
+ * the connection is made with a tap device connecting to a bridge
+ * device)
+ */
int
qemuNetworkIfaceConnect(virDomainDefPtr def,
virConnectPtr conn,
@@ -286,39 +291,19 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
int *tapfd,
int *tapfdSize)
{
- char *brname = NULL;
+ const char *brname;
int ret = -1;
unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
bool template_ifname = false;
- int actualType = virDomainNetGetActualType(net);
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *tunpath = "/dev/net/tun";
if (net->backend.tap)
tunpath = net->backend.tap;
- if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
- bool fail = false;
- virNetworkPtr network = virNetworkLookupByName(conn,
- net->data.network.name);
- if (!network)
- return ret;
-
- if (!(brname = virNetworkGetBridgeName(network)))
- fail = true;
-
- virObjectUnref(network);
- if (fail)
- return ret;
-
- } else if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0)
- return ret;
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Network type %d is not supported"),
- virDomainNetGetActualType(net));
- return ret;
+ if (!(brname = virDomainNetGetActualBridgeName(net))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge
name"));
+ goto cleanup;
}
if (!net->ifname ||
@@ -400,7 +385,6 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
if (template_ifname)
VIR_FREE(net->ifname);
}
- VIR_FREE(brname);
virObjectUnref(cfg);
return ret;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9467d7d..0d97b57 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1746,58 +1746,20 @@ static virDomainNetDefPtr *qemuDomainFindNet(virDomainObjPtr vm,
return NULL;
}
-static char *
-qemuDomainNetGetBridgeName(virConnectPtr conn, virDomainNetDefPtr net)
-{
- char *brname = NULL;
- int actualType = virDomainNetGetActualType(net);
-
- if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- const char *tmpbr = virDomainNetGetActualBridgeName(net);
- if (!tmpbr) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("interface is missing bridge name"));
- goto cleanup;
- }
- /* we need a copy, not just a pointer to the original */
- if (VIR_STRDUP(brname, tmpbr) < 0)
- goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
- virNetworkPtr network;
-
- if (!(network = virNetworkLookupByName(conn, net->data.network.name))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Couldn't find network '%s'"),
- net->data.network.name);
- goto cleanup;
- }
- brname = virNetworkGetBridgeName(network);
-
- virObjectUnref(network);
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Interface type %d has no bridge name"),
- virDomainNetGetActualType(net));
- }
-
- cleanup:
- return brname;
-}
static int
-qemuDomainChangeNetBridge(virConnectPtr conn,
- virDomainObjPtr vm,
+qemuDomainChangeNetBridge(virDomainObjPtr vm,
virDomainNetDefPtr olddev,
virDomainNetDefPtr newdev)
{
int ret = -1;
- char *oldbridge = NULL, *newbridge = NULL;
+ const char *oldbridge = virDomainNetGetActualBridgeName(olddev);
+ const char *newbridge = virDomainNetGetActualBridgeName(newdev);
- if (!(oldbridge = qemuDomainNetGetBridgeName(conn, olddev)))
- goto cleanup;
-
- if (!(newbridge = qemuDomainNetGetBridgeName(conn, newdev)))
+ if (!oldbridge || !newbridge) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge
name"));
goto cleanup;
+ }
VIR_DEBUG("Change bridge for interface %s: %s -> %s",
olddev->ifname, oldbridge, newbridge);
@@ -1836,8 +1798,6 @@ qemuDomainChangeNetBridge(virConnectPtr conn,
/* caller will replace entire olddev with newdev in domain nets list */
ret = 0;
cleanup:
- VIR_FREE(oldbridge);
- VIR_FREE(newbridge);
return ret;
}
@@ -2231,7 +2191,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
}
if (needBridgeChange) {
- if (qemuDomainChangeNetBridge(dom->conn, vm, olddev, newdev) < 0)
+ if (qemuDomainChangeNetBridge(vm, olddev, newdev) < 0)
goto cleanup;
/* we successfully switched to the new bridge, and we've
* determined that the rest of newdev is equivalent to olddev,
--
1.9.3
midnight
New subject: [libvirt] [PATCHv3 9/9] lxc: always use virDomainNetGetActualBridgeName to get interface's bridge
lxcProcessSetupInterfaces() used to have a special case for
actualType='network' (a network with forward mode of route, nat, or
isolated) to call the libvirt public API to retrieve the bridge being
used by a network. That is no longer necessary - since all network
types that use a bridge and tap device now get the bridge name stored
in the ActualNetDef, we can just always use
virDomainNetGetActualBridgeName() instead.
---
Change from V2: change attribute name and fix commit log to reference
lxc rather than qemu function.
src/lxc/lxc_driver.c | 26 ++------------------------
src/lxc/lxc_process.c | 26 +-------------------------
2 files changed, 3 insertions(+), 49 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 97caee3..51c664f 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4161,7 +4161,8 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
actualType = virDomainNetGetActualType(net);
switch (actualType) {
- case VIR_DOMAIN_NET_TYPE_BRIDGE: {
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ case VIR_DOMAIN_NET_TYPE_NETWORK: {
const char *brname = virDomainNetGetActualBridgeName(net);
if (!brname) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -4174,29 +4175,6 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
brname)))
goto cleanup;
} break;
- case VIR_DOMAIN_NET_TYPE_NETWORK: {
- virNetworkPtr network;
- char *brname = NULL;
- bool fail = false;
-
- if (!(network = virNetworkLookupByName(conn, net->data.network.name)))
- goto cleanup;
- if (!(brname = virNetworkGetBridgeName(network)))
- fail = true;
-
- virObjectUnref(network);
- if (fail)
- goto cleanup;
-
- if (!(veth = virLXCProcessSetupInterfaceBridged(conn,
- vm->def,
- net,
- brname))) {
- VIR_FREE(brname);
- goto cleanup;
- }
- VIR_FREE(brname);
- } break;
case VIR_DOMAIN_NET_TYPE_DIRECT: {
if (!(veth = virLXCProcessSetupInterfaceDirect(conn,
vm->def,
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index de574a9..632fc17 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -382,31 +382,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
type = virDomainNetGetActualType(net);
switch (type) {
- case VIR_DOMAIN_NET_TYPE_NETWORK: {
- virNetworkPtr network;
- char *brname = NULL;
- bool fail = false;
-
- if (!(network = virNetworkLookupByName(conn,
- net->data.network.name)))
- goto cleanup;
- if (!(brname = virNetworkGetBridgeName(network)))
- fail = true;
-
- virObjectUnref(network);
- if (fail)
- goto cleanup;
-
- if (!(veth = virLXCProcessSetupInterfaceBridged(conn,
- def,
- net,
- brname))) {
- VIR_FREE(brname);
- goto cleanup;
- }
- VIR_FREE(brname);
- break;
- }
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_BRIDGE: {
const char *brname = virDomainNetGetActualBridgeName(net);
if (!brname) {
--
1.9.3
3602
days inactive
3602
days old
11 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Laine Stump