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. Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/c32dbec4 Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/c32dbec4 Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/c32dbec4 Branch: refs/heads/master Commit: c32dbec468b176d4a6019d9e642eb9adc593bf36 Parents: 47251b5 Author: Sheng Yang <[email protected]> Authored: Tue Jan 15 14:51:59 2013 -0800 Committer: Sheng Yang <[email protected]> Committed: Tue Jan 15 15:02:21 2013 -0800 ---------------------------------------------------------------------- .../router/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(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/c32dbec4/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 492e1dc..c8fd86e 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<Pair<NetworkVO, NicProfile>> networks = createRouterNetworks(owner, isRedundant, plan, guestNetwork, new Pair<Boolean, PublicIp>(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 http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/c32dbec4/server/src/com/cloud/vm/dao/NicDao.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/vm/dao/NicDao.java b/server/src/com/cloud/vm/dao/NicDao.java index af3c7b3..762048b 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, Long> { NicVO findByNetworkIdInstanceIdAndBroadcastUri(long networkId, long instanceId, String broadcastUri); NicVO findByIp4AddressAndNetworkIdAndInstanceId(long networkId, long instanceId, String ip4Address); - - NicVO findByMacAddress(String macAddress); } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/c32dbec4/server/src/com/cloud/vm/dao/NicDaoImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/vm/dao/NicDaoImpl.java b/server/src/com/cloud/vm/dao/NicDaoImpl.java index 00da2eb..3cd7fa6 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<NicVO, Long> 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<NicVO, Long> implements NicDao { sc.setParameters("address", ip4Address); return findOneBy(sc); } - - @Override - public NicVO findByMacAddress(String macAddress) { - SearchCriteria<NicVO> sc = AllFieldsSearch.create(); - sc.setParameters("macAddress", macAddress); - return findOneBy(sc); - } }
