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

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> ... - &lt;bridge name="virbr0" stp="on" delay="5"/&gt; + &lt;bridge name="virbr0" stp="on" delay="5" macTableManager="libvirt"/&gt; &lt;domain name="example.com"/&gt; &lt;forward mode="nat" dev="eth0"/&gt; ...</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>&lt;forward&gt;</code> mode of + "nat" or "route" (or an isolated network with - no <code>&lt;forward&gt;</code> element). + no <code>&lt;forward&gt;</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

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

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

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