From c32dbec468b176d4a6019d9e642eb9adc593bf36 Mon Sep 17 00:00:00 2001 From: Sheng Yang Date: Tue, 15 Jan 2013 14:51:59 -0800 Subject: [PATCH] CLOUDSTACK-985: Revert "Using different MAC for a pair of redundant routers" The different MAC address for a pair of redundant router have issues when short time network outrage happened. When this happened: 1. BACKUP(r-2) cannot receive the broadcast from MASTER(r-1). 2. Then r-2 would announce it's MASTER after 3 seconds, and send gratuitous ARP to the gateway of public ip(usually a rack router). 3. The gateway of public ip would update it's ARP cache to associate the public ip of the network to the MAC of r-2. 4. In the meantime, r-1 still sending out VRRP broadcast(due to network issue, the broadcast never arrived at r-2), and acting as MASTER. 5. After network outrage, r-2 would receive the higher priority VRRP broadcast from MASTER again, then receded as BACKUP. 6. But the public gateway would still associate public ip with MAC of r-2, thus caused the issue. r-1 would no longer able to receive any packets from public network. And there is no way for r-1 to send gratuitous ARP again, because it's always consider itself as MASTER, no state changed, and no hook existed for receiving lower priority broadcast. So I would revert this change, and introduce another commit to ensure the newly create redundant router would share the same MAC as the first one. This reverts commit 9f257aa60b62f24193bba3f7c902e7779632e01e. --- .../VirtualNetworkApplianceManagerImpl.java | 26 ++++--------------- server/src/com/cloud/vm/dao/NicDao.java | 2 -- server/src/com/cloud/vm/dao/NicDaoImpl.java | 8 ------ 3 files changed, 5 insertions(+), 31 deletions(-) diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 492e1dc20b1..c8fd86e821c 100755 --- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -1469,31 +1469,15 @@ public class VirtualNetworkApplianceManagerImpl implements VirtualNetworkApplian offeringId = _offering.getId(); } + PublicIp sourceNatIp = null; + if (publicNetwork) { + sourceNatIp = _networkMgr.assignSourceNatIpAddressToGuestNetwork(owner, guestNetwork); + } + // 3) deploy virtual router(s) int count = routerCount - routers.size(); DeploymentPlan plan = planAndRouters.first(); for (int i = 0; i < count; i++) { - PublicIp sourceNatIp = null; - if (publicNetwork) { - int failCount = 0; - // Generate different MAC for VR - while (sourceNatIp == null) { - sourceNatIp = _networkMgr.assignSourceNatIpAddressToGuestNetwork(owner, guestNetwork); - NicVO nic = _nicDao.findByMacAddress(sourceNatIp.getMacAddress()); - // We got duplicate MAC here, so regenerate the mac - if (nic != null) { - s_logger.debug("Failed to find a different mac for redundant router. Try again. The current mac is " + sourceNatIp.getMacAddress()); - sourceNatIp = null; - failCount ++; - } - //Prevent infinite loop - if (failCount > 3) { - s_logger.error("Failed to find a different mac for redundant router! Abort operation!"); - throw new InsufficientAddressCapacityException("Failed to find a different mac for redundant router", null, offeringId); - } - } - } - List> networks = createRouterNetworks(owner, isRedundant, plan, guestNetwork, new Pair(publicNetwork, sourceNatIp)); //don't start the router as we are holding the network lock that needs to be released at the end of router allocation diff --git a/server/src/com/cloud/vm/dao/NicDao.java b/server/src/com/cloud/vm/dao/NicDao.java index af3c7b379c1..762048b65bf 100644 --- a/server/src/com/cloud/vm/dao/NicDao.java +++ b/server/src/com/cloud/vm/dao/NicDao.java @@ -58,6 +58,4 @@ public interface NicDao extends GenericDao { NicVO findByNetworkIdInstanceIdAndBroadcastUri(long networkId, long instanceId, String broadcastUri); NicVO findByIp4AddressAndNetworkIdAndInstanceId(long networkId, long instanceId, String ip4Address); - - NicVO findByMacAddress(String macAddress); } diff --git a/server/src/com/cloud/vm/dao/NicDaoImpl.java b/server/src/com/cloud/vm/dao/NicDaoImpl.java index 00da2eb96a4..3cd7fa6b488 100644 --- a/server/src/com/cloud/vm/dao/NicDaoImpl.java +++ b/server/src/com/cloud/vm/dao/NicDaoImpl.java @@ -50,7 +50,6 @@ public class NicDaoImpl extends GenericDaoBase implements NicDao { AllFieldsSearch.and("address", AllFieldsSearch.entity().getIp4Address(), Op.EQ); AllFieldsSearch.and("isDefault", AllFieldsSearch.entity().isDefaultNic(), Op.EQ); AllFieldsSearch.and("broadcastUri", AllFieldsSearch.entity().getBroadcastUri(), Op.EQ); - AllFieldsSearch.and("macAddress", AllFieldsSearch.entity().getMacAddress(), Op.EQ); AllFieldsSearch.done(); IpSearch = createSearchBuilder(String.class); @@ -200,11 +199,4 @@ public class NicDaoImpl extends GenericDaoBase implements NicDao { sc.setParameters("address", ip4Address); return findOneBy(sc); } - - @Override - public NicVO findByMacAddress(String macAddress) { - SearchCriteria sc = AllFieldsSearch.create(); - sc.setParameters("macAddress", macAddress); - return findOneBy(sc); - } }