guarantee MAC uniqueness (#7634)

Co-authored-by: Bryan Lima <42067040+BryanMLima@users.noreply.github.com>

api/src/main/java/com/cloud/network/NetworkModel.java

@@ -90,7 +90,7 @@ public interface NetworkModel {
             INSTANCE_ID_FILE, VM_ID_FILE, PUBLIC_KEYS_FILE, CLOUD_IDENTIFIER_FILE, HYPERVISOR_HOST_NAME_FILE));
 
     static final ConfigKey<Integer> MACIdentifier = new ConfigKey<>("Advanced",Integer.class, "mac.identifier", "0",
-            "This value will be used while generating the mac addresses for isolated and shared networks. The hexadecimal equivalent value will be present at the 2nd octet of the mac address. Default value is null which means this feature is disabled.Its scope is global.", true, ConfigKey.Scope.Global);
+            "This value will be used while generating the mac addresses for isolated and shared networks. The hexadecimal equivalent value will be present at the 2nd octet of the mac address. Default value is zero (0) which means that the DB id of the zone will be used.", true, ConfigKey.Scope.Zone);
 
     static final ConfigKey<Boolean> AdminIsAllowedToDeployAnywhere = new ConfigKey<>("Advanced",Boolean.class, "admin.is.allowed.to.deploy.anywhere", "false",
             "This will determine if the root admin is allowed to deploy in networks in subdomains.", true, ConfigKey.Scope.Global);
@@ -114,6 +114,13 @@ public interface NetworkModel {
 
     List<? extends Nic> getNics(long vmId);
 
+    /**
+     * Gets the next available MAC and checks it for global uniqueness in the nics table. It will keep looking until it finds a MAC address that is unique.
+     *
+     * @param networkConfigurationId the id of the network to use the nic in. used for finding the zone
+     * @return a string containing a MAC address
+     * @throws InsufficientAddressCapacityException if no MAC can be returned
+     */
     String getNextAvailableMacAddressInNetwork(long networkConfigurationId) throws InsufficientAddressCapacityException;
 
     PublicIpAddress getPublicIpAddress(long ipAddressId);

engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/dao/EngineDataCenterDao.java

@@ -28,14 +28,6 @@ public interface EngineDataCenterDao extends GenericDao<EngineDataCenterVO, Long
         StateDao<DataCenterResourceEntity.State, DataCenterResourceEntity.State.Event, DataCenterResourceEntity> {
     EngineDataCenterVO findByName(String name);
 
-    /**
-     * @param id data center id
-     * @return a pair of mac address strings.  The first one is private and second is public.
-     */
-    String[] getNextAvailableMacAddressPair(long id);
-
-    String[] getNextAvailableMacAddressPair(long id, long mask);
-
     List<EngineDataCenterVO> findZonesByDomainId(Long domainId);
 
     List<EngineDataCenterVO> listPublicZones(String keyword);

engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/dao/EngineDataCenterDaoImpl.java

@@ -23,7 +23,6 @@ import java.util.Random;
 
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
-import javax.persistence.TableGenerator;
 
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -39,10 +38,8 @@ import com.cloud.utils.db.DB;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
-import com.cloud.utils.db.SequenceFetcher;
 import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.utils.db.UpdateBuilder;
-import com.cloud.utils.net.NetUtils;
 
 /**
  * @config
@@ -66,7 +63,6 @@ public class EngineDataCenterDaoImpl extends GenericDaoBase<EngineDataCenterVO,
 
     protected long _prefix;
     protected Random _rand = new Random(System.currentTimeMillis());
-    protected TableGenerator _tgMacAddress;
 
     @Inject
     protected DcDetailsDao _detailsDao;
@@ -139,25 +135,6 @@ public class EngineDataCenterDaoImpl extends GenericDaoBase<EngineDataCenterVO,
         return listBy(ssc);
     }
 
-    @Override
-    public String[] getNextAvailableMacAddressPair(long id) {
-        return getNextAvailableMacAddressPair(id, 0);
-    }
-
-    @Override
-    public String[] getNextAvailableMacAddressPair(long id, long mask) {
-        SequenceFetcher fetch = SequenceFetcher.getInstance();
-
-        long seq = fetch.getNextSequence(Long.class, _tgMacAddress, id);
-        seq = seq | _prefix | ((id & 0x7f) << 32);
-        seq |= mask;
-        seq |= ((_rand.nextInt(Short.MAX_VALUE) << 16) & 0x00000000ffff0000l);
-        String[] pair = new String[2];
-        pair[0] = NetUtils.long2Mac(seq);
-        pair[1] = NetUtils.long2Mac(seq | 0x1l << 39);
-        return pair;
-    }
-
     @Override
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
         if (!super.configure(name, params)) {
@@ -204,9 +181,6 @@ public class EngineDataCenterDaoImpl extends GenericDaoBase<EngineDataCenterVO,
         UUIDSearch = createSearchBuilder();
         UUIDSearch.and("uuid", UUIDSearch.entity().getUuid(), SearchCriteria.Op.EQ);
         UUIDSearch.done();
-
-        _tgMacAddress = _tgs.get("macAddress");
-        assert _tgMacAddress != null : "Couldn't get mac address table generator";
     }
 
     @Override

engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDao.java

@@ -52,14 +52,6 @@ public interface DataCenterDao extends GenericDao<DataCenterVO, Long> {
 
     DataCenterVO findByName(String name);
 
-    /**
-     * @param id data center id
-     * @return a pair of mac address strings.  The first one is private and second is public.
-     */
-    String[] getNextAvailableMacAddressPair(long id);
-
-    String[] getNextAvailableMacAddressPair(long id, long mask);
-
     PrivateAllocationData allocatePrivateIpAddress(long id, long podId, long instanceId, String reservationId, boolean forSystemVms);
 
     DataCenterIpAddressVO allocatePrivateIpAddress(long id, String reservationId);

engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDaoImpl.java

@@ -24,7 +24,6 @@ import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
-import javax.persistence.TableGenerator;
 
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;
@@ -45,9 +44,7 @@ import com.cloud.utils.db.DB;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
-import com.cloud.utils.db.SequenceFetcher;
 import com.cloud.utils.db.TransactionLegacy;
-import com.cloud.utils.net.NetUtils;
 
 /**
  * @config
@@ -83,7 +80,7 @@ public class DataCenterDaoImpl extends GenericDaoBase<DataCenterVO, Long> implem
 
     protected long _prefix;
     protected Random _rand = new Random(System.currentTimeMillis());
-    protected TableGenerator _tgMacAddress;
+
 
     @Override
     public DataCenterVO findByName(String name) {
@@ -230,25 +227,6 @@ public class DataCenterDaoImpl extends GenericDaoBase<DataCenterVO, Long> implem
         return vo.getVlan();
     }
 
-    @Override
-    public String[] getNextAvailableMacAddressPair(long id) {
-        return getNextAvailableMacAddressPair(id, 0);
-    }
-
-    @Override
-    public String[] getNextAvailableMacAddressPair(long id, long mask) {
-        SequenceFetcher fetch = SequenceFetcher.getInstance();
-
-        long seq = fetch.getNextSequence(Long.class, _tgMacAddress, id);
-        seq = seq | _prefix | ((id & 0x7f) << 32);
-        seq |= mask;
-        seq |= ((_rand.nextInt(Short.MAX_VALUE) << 16) & 0x00000000ffff0000l);
-        String[] pair = new String[2];
-        pair[0] = NetUtils.long2Mac(seq);
-        pair[1] = NetUtils.long2Mac(seq | 0x1l << 39);
-        return pair;
-    }
-
     @Override
     public PrivateAllocationData allocatePrivateIpAddress(long dcId, long podId, long instanceId, String reservationId, boolean forSystemVms) {
         _ipAllocDao.releaseIpAddress(instanceId);
@@ -348,9 +326,6 @@ public class DataCenterDaoImpl extends GenericDaoBase<DataCenterVO, Long> implem
         TokenSearch = createSearchBuilder();
         TokenSearch.and("zoneToken", TokenSearch.entity().getZoneToken(), SearchCriteria.Op.EQ);
         TokenSearch.done();
-
-        _tgMacAddress = _tgs.get("macAddress");
-        assert _tgMacAddress != null : "Couldn't get mac address table generator";
     }
 
     @Override

engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java

@@ -436,9 +436,6 @@ public class NetworkDaoImpl extends GenericDaoBase<NetworkVO, Long>implements Ne
         if(zoneMacIdentifier != null && zoneMacIdentifier.intValue() != 0 ){
             seq = seq | _prefix << 40 | (long)zoneMacIdentifier << 32 | networkConfigId << 16 & 0x00000000ffff0000l;
         }
-        else {
-            seq = seq | _prefix << 40 | _rand.nextInt(Short.MAX_VALUE) << 16 & 0x00000000ffff0000l;
-        }
         return NetUtils.long2Mac(seq);
     }
 

engine/schema/src/main/java/com/cloud/vm/dao/NicDao.java

@@ -87,7 +87,7 @@ public interface NicDao extends GenericDao<NicVO, Long> {
 
     List<NicVO> listByVmIdAndKeyword(long instanceId, String keyword);
 
-    NicVO findByInstanceIdAndMacAddress(long instanceId, String macAddress);
+    NicVO findByMacAddress(String macAddress);
 
     List<NicVO> findNicsByIpv6GatewayIpv6CidrAndReserver(String ipv6Gateway, String ipv6Cidr, String reserverName);
 

engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java

@@ -368,9 +368,8 @@ public class NicDaoImpl extends GenericDaoBase<NicVO, Long> implements NicDao {
     }
 
     @Override
-    public NicVO findByInstanceIdAndMacAddress(long instanceId, String macAddress) {
+    public NicVO findByMacAddress(String macAddress) {
         SearchCriteria<NicVO> sc = AllFieldsSearch.create();
-        sc.setParameters("instance", instanceId);
         sc.setParameters("macAddress", macAddress);
         return findOneBy(sc);
     }

framework/db/src/main/java/com/cloud/utils/db/SequenceFetcher.java

@@ -59,7 +59,7 @@ public class SequenceFetcher {
     }
 
     public <T> T getNextSequence(Class<T> clazz, TableGenerator tg, Object key, boolean isRandom) {
-        Future<T> future = _executors.submit(new Fetcher<T>(clazz, tg, key, isRandom));
+        Future<T> future = _executors.submit(new Fetcher<>(clazz, tg, key, isRandom));
         try {
             return future.get();
         } catch (Exception e) {
@@ -69,7 +69,7 @@ public class SequenceFetcher {
     }
 
     protected SequenceFetcher() {
-        _executors = new ThreadPoolExecutor(100, 100, 120l, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(250), new NamedThreadFactory("SequenceFetcher"));
+        _executors = new ThreadPoolExecutor(100, 100, 120l, TimeUnit.SECONDS, new LinkedBlockingQueue<>(250), new NamedThreadFactory("SequenceFetcher"));
     }
 
     protected static final SequenceFetcher s_instance = new SequenceFetcher();

plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/guru/HypervGuru.java

@@ -50,7 +50,7 @@ public class HypervGuru extends HypervisorGuruBase implements HypervisorGuru {
     @Inject
     private GuestOSDao _guestOsDao;
     @Inject HypervManager _hypervMgr;
-    @Inject NetworkModel _networkMgr;
+    @Inject NetworkModel networkModel;
     int MaxNicSupported = 8;
     @Override
     public final HypervisorType getHypervisorType() {
@@ -120,7 +120,7 @@ public class HypervGuru extends HypervisorGuruBase implements HypervisorGuru {
                     nicTo.setName(profile.getName());
 
                     try {
-                        String mac = _networkMgr.getNextAvailableMacAddressInNetwork(networkId);
+                        String mac = networkModel.getNextAvailableMacAddressInNetwork(networkId);
                         nicTo.setMac(mac);
                     } catch (InsufficientAddressCapacityException e) {
                         throw new CloudRuntimeException("unable to allocate mac address on network: " + networkId);
@@ -136,7 +136,7 @@ public class HypervGuru extends HypervisorGuruBase implements HypervisorGuru {
                     nicTo.setBroadcastUri(profile.getBroadCastUri());
                     nicTo.setIsolationuri(profile.getIsolationUri());
 
-                    Integer networkRate = _networkMgr.getNetworkRate(network.getId(), null);
+                    Integer networkRate = networkModel.getNetworkRate(network.getId(), null);
                     nicTo.setNetworkRateMbps(networkRate);
 
                     expandedNics[i] = nicTo;

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java

@@ -72,7 +72,7 @@ class VmwareVmImplementer {
     @Inject
     NetworkDao networkDao;
     @Inject
-    NetworkModel networkMgr;
+    NetworkModel networkModel;
     @Inject
     NicDao nicDao;
     @Inject
@@ -237,7 +237,7 @@ class VmwareVmImplementer {
                 nicTo.setNetmask("255.255.255.255");
 
                 try {
-                    String mac = networkMgr.getNextAvailableMacAddressInNetwork(networkId);
+                    String mac = networkModel.getNextAvailableMacAddressInNetwork(networkId);
                     nicTo.setMac(mac);
                 } catch (InsufficientAddressCapacityException e) {
                     throw new CloudRuntimeException("unable to allocate mac address on network: " + networkId);
@@ -253,7 +253,7 @@ class VmwareVmImplementer {
                 nicTo.setBroadcastUri(publicNicProfile.getBroadCastUri());
                 nicTo.setIsolationuri(publicNicProfile.getIsolationUri());
 
-                Integer networkRate = networkMgr.getNetworkRate(network.getId(), null);
+                Integer networkRate = networkModel.getNetworkRate(network.getId(), null);
                 nicTo.setNetworkRateMbps(networkRate);
 
                 expandedNics[i] = nicTo;
@@ -296,7 +296,7 @@ class VmwareVmImplementer {
 
         for (NicProfile nicProfile : nicProfiles) {
             if (nicProfile.getTrafficType() == Networks.TrafficType.Guest) {
-                if (networkMgr.isProviderSupportServiceInNetwork(nicProfile.getNetworkId(), Network.Service.Firewall, Network.Provider.CiscoVnmc)) {
+                if (networkModel.isProviderSupportServiceInNetwork(nicProfile.getNetworkId(), Network.Service.Firewall, Network.Provider.CiscoVnmc)) {
                     details.put("ConfigureVServiceInNexus", Boolean.TRUE.toString());
                 }
                 break;

plugins/network-elements/netscaler/src/main/java/com/cloud/network/vm/NetScalerVMManagerImpl.java

@@ -324,7 +324,7 @@ public class NetScalerVMManagerImpl extends ManagerBase implements NetScalerVMMa
         defaultNic2.setIPv4Address("");
         defaultNic2.setIPv4Gateway("");
         defaultNic2.setIPv4Netmask("");
-        String macAddress = _networkDao.getNextAvailableMacAddress(defaultPublicNetwork.getId(), null);
+        String macAddress = _networkModel.getNextAvailableMacAddressInNetwork(defaultPublicNetwork.getId());
         defaultNic2.setMacAddress(macAddress);
 
         networks.put(_networkMgr.setupNetwork(_accountMgr.getSystemAccount(), _networkOfferingDao.findByUniqueName(NetworkOffering.SystemPublicNetwork), plan, null, null, false).get(0),

server/src/main/java/com/cloud/network/NetworkModelImpl.java

@@ -80,7 +80,6 @@ import com.cloud.network.dao.IPAddressVO;
 import com.cloud.network.dao.NetworkAccountDao;
 import com.cloud.network.dao.NetworkAccountVO;
 import com.cloud.network.dao.NetworkDao;
-import com.cloud.network.dao.NetworkDetailsDao;
 import com.cloud.network.dao.NetworkDomainDao;
 import com.cloud.network.dao.NetworkDomainVO;
 import com.cloud.network.dao.NetworkServiceMapDao;
@@ -172,8 +171,6 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
     @Inject
     NetworkDao _networksDao = null;
     @Inject
-    NetworkDetailsDao networkDetailsDao;
-    @Inject
     NicDao _nicDao = null;
     @Inject
     PodVlanMapDao _podVlanMapDao;
@@ -593,13 +590,24 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
     @Override
     public String getNextAvailableMacAddressInNetwork(long networkId) throws InsufficientAddressCapacityException {
         NetworkVO network = _networksDao.findById(networkId);
-        String mac = _networksDao.getNextAvailableMacAddress(networkId, MACIdentifier.value());
+        Integer zoneIdentifier = MACIdentifier.value();
-        if (mac == null) {
+        if (zoneIdentifier.intValue() == 0) {
-            throw new InsufficientAddressCapacityException("Unable to create another mac address", Network.class, networkId);
+            zoneIdentifier = Long.valueOf(network.getDataCenterId()).intValue();
         }
+        String mac;
+        do {
+            mac = _networksDao.getNextAvailableMacAddress(networkId, zoneIdentifier);
+            if (mac == null) {
+                throw new InsufficientAddressCapacityException("Unable to create another mac address", Network.class, networkId);
+            }
+        } while(! isMACUnique(mac));
         return mac;
     }
 
+    private boolean isMACUnique(String mac) {
+        return (_nicDao.findByMacAddress(mac) == null);
+    }
+
     @Override
     @DB
     public Network getNetwork(long id) {

server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java

@@ -59,7 +59,7 @@ public class ControlNetworkGuru extends PodBasedNetworkGuru implements NetworkGu
     @Inject
     ConfigurationDao _configDao;
     @Inject
-    NetworkModel _networkMgr;
+    NetworkModel networkModel;
     String _cidr;
     String _gateway;
 
@@ -114,7 +114,7 @@ public class ControlNetworkGuru extends PodBasedNetworkGuru implements NetworkGu
 
         if (vm.getHypervisorType() == HypervisorType.VMware && !isRouterVm(vm)) {
             NicProfile nicProf = new NicProfile(Nic.ReservationStrategy.Create, null, null, null, null);
-            String mac = _networkMgr.getNextAvailableMacAddressInNetwork(config.getId());
+            String mac = networkModel.getNextAvailableMacAddressInNetwork(config.getId());
             nicProf.setMacAddress(mac);
             return nicProf;
         }
@@ -140,7 +140,7 @@ public class ControlNetworkGuru extends PodBasedNetworkGuru implements NetworkGu
         if (((hType == HypervisorType.VMware) || (hType == HypervisorType.Hyperv)) && isRouterVm(vm)) {
             super.reserve(nic, config, vm, dest, context);
 
-            String mac = _networkMgr.getNextAvailableMacAddressInNetwork(config.getId());
+            String mac = networkModel.getNextAvailableMacAddressInNetwork(config.getId());
             nic.setMacAddress(mac);
             return;
         }