From c1b164d70c738b0d7de530417f49a142680fe294 Mon Sep 17 00:00:00 2001 From: Ansis Atteka Date: Thu, 16 Feb 2012 15:49:57 -0800 Subject: [PATCH] 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. --- src/network/bridge_driver.c | 2 +- src/qemu/qemu_command.c | 5 +---- src/uml/uml_conf.c | 5 +---- src/util/virnetdevtap.c | 11 ++++++++++- src/util/virnetdevtap.h | 1 + 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 8575d3e756..3e1e0313c9 100644 --- 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 170b01087e..a6536ab2ea 100644 --- 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 dbbbfdafef..c7b29a0c50 100644 --- 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 0fce08deb6..868ba57b08 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -22,6 +22,7 @@ #include +#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 918f3dce0a..fc50e22e40 100644 --- 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, -- 2.47.2