util: centralize tap device MAC address 1st byte "0xFE" modification

When a tap device for a domain is created and attached to a bridge,
the first byte of the tap device MAC address is set to 0xFE, while the
rest is set to match the MAC address that will be presented to the
guest as its network device MAC address. Setting this high value in
the tap's MAC address discourages the bridge from using the tap
device's MAC address as the bridge's own MAC address (Linux bridges
always take on the lowest numbered MAC address of all attached devices
as their own).

In one case within libvirt, a tap device is created and attached to
the bridge with the intent that its MAC address be taken on by the
bridge as its own (this is used to assure that the bridge has a fixed
MAC address to prevent network outages created by the bridge MAC
address "flapping" as guests are started and stopped). In this case,
the first byte of the mac address is *not* altered to 0xFE.

In the current code, callers to virNetDevTapCreateInBridgePort each
make the MAC address modification themselves before calling, which
leads to code duplication, and also prevents lower level functions
from knowing the real MAC address being used by the guest. The problem
here is that openvswitch bridges must be informed about this MAC
address, or they will be unable to pass traffic to/from the guest.

This patch centralizes the location of the MAC address "0xFE fixup"
into virNetDevTapCreateInBridgePort(), meaning 1) callers of this
function no longer need the extra strange bit of code, and 2)
bitNetDevTapCreateBridgeInPort itself now is called with the guest's
unaltered MAC address, and can pass it on, unmodified, to
virNetDevOpenvswitchAddPort.

There is no other behavioral change created by this patch.

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8575d3e756c3b0411e495852f6c4a1b3fb3934cc..3e1e0313c98f59a6f39fcfd8964df40b0fa9b184 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1766,7 +1766,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
         }
         if (virNetDevTapCreateInBridgePort(network->def->bridge,
                                            &macTapIfName, network->def->mac,
-                                           0, false, NULL, NULL) < 0) {
+                                           false, 0, false, NULL, NULL) < 0) {
             VIR_FREE(macTapIfName);
             goto err0;
         }

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 170b01087eaae50ed1f9022a6d0ab15bd712ace6..a6536ab2eab90e23ddb22385d92ee4a0fcfd86ad 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -180,7 +180,6 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
     int tapfd = -1;
     int vnet_hdr = 0;
     bool template_ifname = false;
-    unsigned char tapmac[VIR_MAC_BUFLEN];
     int actualType = virDomainNetGetActualType(net);
 
     if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
@@ -244,9 +243,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
         net->model && STREQ(net->model, "virtio"))
         vnet_hdr = 1;
 
-    memcpy(tapmac, net->mac, VIR_MAC_BUFLEN);
-    tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */
-    err = virNetDevTapCreateInBridgePort(brname, &net->ifname, tapmac,
+    err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, true,
                                          vnet_hdr, true, &tapfd,
                                          virDomainNetGetActualVirtPortProfile(net));
     virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0);

diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index dbbbfdafef5a93a569c21f46f9e21066bb77668c..c7b29a0c501e39c13591ea127dfd228df1ef872d 100644 (file)
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -127,7 +127,6 @@ umlConnectTapDevice(virConnectPtr conn,
                     const char *bridge)
 {
     bool template_ifname = false;
-    unsigned char tapmac[VIR_MAC_BUFLEN];
 
     if (!net->ifname ||
         STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) ||
@@ -139,9 +138,7 @@ umlConnectTapDevice(virConnectPtr conn,
         template_ifname = true;
     }
 
-    memcpy(tapmac, net->mac, VIR_MAC_BUFLEN);
-    tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */
-    if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, tapmac,
+    if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, true,
                                        0, true, NULL,
                                        virDomainNetGetActualVirtPortProfile(net)) < 0) {
         if (template_ifname)

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 0fce08deb61c7c725c0efaa7cc2acc99db70a89b..868ba57b08202d5e60f6b2313a7b75764dff801c 100644 (file)
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -22,6 +22,7 @@
 
 #include <config.h>
 
+#include "virmacaddr.h"
 #include "virnetdevtap.h"
 #include "virnetdev.h"
 #include "virnetdevbridge.h"
@@ -248,6 +249,7 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
  * @brname: the bridge name
  * @ifname: the interface name (or name template)
  * @macaddr: desired MAC address (VIR_MAC_BUFLEN long)
+ * @discourage: whether bridge should be discouraged from using macaddr
  * @vnet_hdr: whether to try enabling IFF_VNET_HDR
  * @tapfd: file descriptor return value for the new tap device
  * @virtPortProfile: bridge/port specific configuration
@@ -265,11 +267,14 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
 int virNetDevTapCreateInBridgePort(const char *brname,
                                    char **ifname,
                                    const unsigned char *macaddr,
+                                   bool discourage,
                                    int vnet_hdr,
                                    bool up,
                                    int *tapfd,
                                    virNetDevVPortProfilePtr virtPortProfile)
 {
+    unsigned char tapmac[VIR_MAC_BUFLEN];
+
     if (virNetDevTapCreate(ifname, vnet_hdr, tapfd) < 0)
         return -1;
 
@@ -279,7 +284,11 @@ int virNetDevTapCreateInBridgePort(const char *brname,
      * seeing the kernel allocate random MAC for the TAP
      * device before we set our static MAC.
      */
-    if (virNetDevSetMAC(*ifname, macaddr) < 0)
+    memcpy(tapmac, macaddr, VIR_MAC_BUFLEN);
+    if (discourage)
+        tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */
+
+    if (virNetDevSetMAC(*ifname, tapmac) < 0)
         goto error;
 
     /* We need to set the interface MTU before adding it

diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index 918f3dce0ac83609deac8630babd6b0f0e23f1db..fc50e22e402ddd355730ce7782b4d107623923b6 100644 (file)
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -37,6 +37,7 @@ int virNetDevTapDelete(const char *ifname)
 int virNetDevTapCreateInBridgePort(const char *brname,
                                    char **ifname,
                                    const unsigned char *macaddr,
+                                   bool discourage,
                                    int vnet_hdr,
                                    bool up,
                                    int *tapfd,