Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/src/main/java/com/cloud/network/NetworkModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ public interface NetworkModel {
*/
String getNextAvailableMacAddressInNetwork(long networkConfigurationId) throws InsufficientAddressCapacityException;

default boolean isMACUnique(String mac, long networkId) {
return true;
}
Comment on lines +128 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this default implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland there are some mock classes implementing it, not implemented it there (I think, we can return default true for them)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but it seems a logical error for anyone wanting to implement a new NetworkModel. They should be forced to implement it and not use this code by default.


PublicIpAddress getPublicIpAddress(long ipAddressId);

List<? extends Vlan> listPodVlans(long podId);
Expand Down Expand Up @@ -362,4 +366,8 @@ List<String[]> generateVmData(String userData, String userDataDetails, String se

boolean checkSecurityGroupSupportForNetwork(Account account, DataCenter zone, List<Long> networkIds,
List<Long> securityGroupsIds);

default long getMacIdentifier(Long dataCenterId) {
return 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ public PublicIp(IPAddressVO addr, VlanVO vlan, long macAddress) {
}

public static PublicIp createFromAddrAndVlan(IPAddressVO addr, VlanVO vlan) {
return new PublicIp(addr, vlan, NetUtils.createSequenceBasedMacAddress(addr.getMacAddress(), NetworkModel.MACIdentifier.value()));
long macIdentifier = NetworkModel.MACIdentifier.valueIn(addr.getDataCenterId());
if (macIdentifier == 0) {
macIdentifier = addr.getDataCenterId();
}
return new PublicIp(addr, vlan, NetUtils.createSequenceBasedMacAddress(addr.getMacAddress(), macIdentifier));
}

@Override
Expand Down Expand Up @@ -274,6 +278,4 @@ public void setRuleState(State ruleState) {
public boolean isForSystemVms() {
return false;
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -1270,7 +1270,7 @@ protected void configureNicProfileBasedOnRequestedIp(NicProfile requestedNicProf
nicProfile.setIPv4Gateway(ipv4Gateway);
nicProfile.setIPv4Netmask(ipv4Netmask);

if (nicProfile.getMacAddress() == null) {
if (nicProfile.getMacAddress() == null || !_networkModel.isMACUnique(nicProfile.getMacAddress(), network.getId())) {
try {
String macAddress = _networkModel.getNextAvailableMacAddressInNetwork(network.getId());
nicProfile.setMacAddress(macAddress);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ private void configureTestConfigureNicProfileBasedOnRequestedIpTests(NicProfile
when(testOrchestrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy);
when(testOrchestrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true);
when(testOrchestrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true);
when(testOrchestrator._networkModel.isMACUnique(Mockito.anyString(), Mockito.anyLong())).thenReturn(true);
try {
when(testOrchestrator._networkModel.getNextAvailableMacAddressInNetwork(Mockito.anyLong())).thenReturn(macAddress);
} catch (InsufficientAddressCapacityException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,8 @@ public List<NetworkVO> getAllPersistentNetworksFromZone(long dataCenterId) {
public String getNextAvailableMacAddress(final long networkConfigId, Integer zoneMacIdentifier) {
final SequenceFetcher fetch = SequenceFetcher.getInstance();
long seq = fetch.getNextSequence(Long.class, _tgMacAddress, networkConfigId);
if(zoneMacIdentifier != null && zoneMacIdentifier.intValue() != 0 ){
seq = seq | _prefix << 40 | (long)zoneMacIdentifier << 32 | networkConfigId << 16 & 0x00000000ffff0000l;
if (zoneMacIdentifier != null && zoneMacIdentifier != 0) {
seq = seq | _prefix << 40 | (long)zoneMacIdentifier << 32 | networkConfigId << 16 & 0x00000000ffff0000L;
}
return NetUtils.long2Mac(seq);
}
Expand Down
2 changes: 1 addition & 1 deletion engine/schema/src/main/java/com/cloud/vm/dao/NicDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public interface NicDao extends GenericDao<NicVO, Long> {

List<NicVO> listByVmIdAndKeyword(long instanceId, String keyword);

NicVO findByMacAddress(String macAddress);
NicVO findByMacAddress(String macAddress, long networkId);

NicVO findByNetworkIdAndMacAddressIncludingRemoved(long networkId, String mac);

Expand Down
3 changes: 2 additions & 1 deletion engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,10 @@ public List<NicVO> listByVmIdAndKeyword(long instanceId, String keyword) {
}

@Override
public NicVO findByMacAddress(String macAddress) {
public NicVO findByMacAddress(String macAddress, long networkId) {
SearchCriteria<NicVO> sc = AllFieldsSearch.create();
sc.setParameters("macAddress", macAddress);
sc.setParameters("network", networkId);
return findOneBy(sc);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2321,7 +2321,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Insuff
nic.setBroadcastUri(BroadcastDomainType.Vlan.toUri(ip.getVlanTag()));
nic.setFormat(AddressFormat.Ip4);
nic.setReservationId(String.valueOf(ip.getVlanTag()));
if(nic.getMacAddress() == null) {
if (nic.getMacAddress() == null) {
nic.setMacAddress(ip.getMacAddress());
}
}
Expand Down Expand Up @@ -2372,7 +2372,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Insuff

nic.setBroadcastUri(network.getBroadcastUri());
nic.setFormat(AddressFormat.Ip4);
if(nic.getMacAddress() == null) {
if (nic.getMacAddress() == null || !_networkModel.isMACUnique(nic.getMacAddress(), network.getId())) {
nic.setMacAddress(_networkModel.getNextAvailableMacAddressInNetwork(network.getId()));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ private void setNicPropertiesFromNetwork(NicProfile nic, Network network) throws
if (nic.getBroadCastUri() == null) {
nic.setBroadcastUri(network.getBroadcastUri());
}
if (nic.getMacAddress() == null) {
if (nic.getMacAddress() == null || !_networkModel.isMACUnique(nic.getMacAddress(), network.getId())) {
nic.setMacAddress(_networkModel.getNextAvailableMacAddressInNetwork(network.getId()));
}
}
Expand Down
29 changes: 22 additions & 7 deletions server/src/main/java/com/cloud/network/NetworkModelImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -593,22 +593,23 @@ public List<? extends Nic> getNics(long vmId) {
@Override
public String getNextAvailableMacAddressInNetwork(long networkId) throws InsufficientAddressCapacityException {
NetworkVO network = _networksDao.findById(networkId);
Integer zoneIdentifier = MACIdentifier.value();
if (zoneIdentifier.intValue() == 0) {
zoneIdentifier = Long.valueOf(network.getDataCenterId()).intValue();
if (network == null) {
throw new CloudRuntimeException("Could not find network with id " + networkId);
}

Integer zoneMacIdentifier = Long.valueOf(getMacIdentifier(network.getDataCenterId())).intValue();
String mac;
do {
mac = _networksDao.getNextAvailableMacAddress(networkId, zoneIdentifier);
mac = _networksDao.getNextAvailableMacAddress(networkId, zoneMacIdentifier);
if (mac == null) {
throw new InsufficientAddressCapacityException("Unable to create another mac address", Network.class, networkId);
}
} while(! isMACUnique(mac));
} while (!isMACUnique(mac, networkId));
return mac;
}

private boolean isMACUnique(String mac) {
return (_nicDao.findByMacAddress(mac) == null);
public boolean isMACUnique(String mac, long networkId) {
return (_nicDao.findByMacAddress(mac, networkId) == null);
}

@Override
Expand Down Expand Up @@ -2815,4 +2816,18 @@ public boolean checkSecurityGroupSupportForNetwork(Account account, DataCenter z
}
return false;
}

@Override
public long getMacIdentifier(Long dataCenterId) {
long macAddress = 0;
if (dataCenterId == null) {
macAddress = NetworkModel.MACIdentifier.value();
} else {
macAddress = NetworkModel.MACIdentifier.valueIn(dataCenterId);
if (macAddress == 0) {
macAddress = dataCenterId;
}
}
return macAddress;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public NicProfile allocate(Network network, NicProfile nic, VirtualMachineProfil
allocateDirectIp(nic, network, vm, dc, nic.getRequestedIPv4(), nic.getRequestedIPv6());
nic.setReservationStrategy(ReservationStrategy.Create);

if (nic.getMacAddress() == null) {
if (nic.getMacAddress() == null || !_networkModel.isMACUnique(nic.getMacAddress(), network.getId())) {
nic.setMacAddress(_networkModel.getNextAvailableMacAddressInNetwork(network.getId()));
if (nic.getMacAddress() == null) {
throw new InsufficientAddressCapacityException("Unable to allocate more mac addresses", Network.class, network.getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public class PodBasedNetworkGuru extends AdapterBase implements NetworkGuru {
DataCenterDao _dcDao;
@Inject
StorageNetworkManager _sNwMgr;
@Inject
NetworkModel _networkModel;

Random _rand = new Random(System.currentTimeMillis());

Expand Down Expand Up @@ -131,7 +133,11 @@ public void reserve(NicProfile nic, Network config, VirtualMachineProfile vm, De
Integer vlan = result.getVlan();

nic.setIPv4Address(result.getIpAddress());
nic.setMacAddress(NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(result.getMacAddress(), NetworkModel.MACIdentifier.value())));
String macAddress = NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(result.getMacAddress(), _networkModel.getMacIdentifier(dest.getDataCenter().getId())));
if (!_networkModel.isMACUnique(macAddress, config.getId())) {
macAddress = _networkModel.getNextAvailableMacAddressInNetwork(config.getId());
}
nic.setMacAddress(macAddress);
Comment on lines +136 to +140
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern seems to be recurring (also in PrivateNetworkGuru, StorageNetworkGuru and NetProfileHelperImpl). Is it worth codifying it in a method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's repeating. will check.

nic.setIPv4Gateway(pod.getGateway());
nic.setFormat(AddressFormat.Ip4);
String netmask = NetUtils.getCidrNetmask(pod.getCidrSize());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,12 @@ protected void getIp(NicProfile nic, DataCenter dc, Network network) throws Insu
PrivateIpVO ipVO = _privateIpDao.allocateIpAddress(network.getDataCenterId(), network.getId(), null);
String vlanTag = BroadcastDomainType.getValue(network.getBroadcastUri());
String netmask = NetUtils.getCidrNetmask(network.getCidr());
String macAddress = NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ipVO.getMacAddress(), networkModel.getMacIdentifier(network.getDataCenterId())));
if (!networkModel.isMACUnique(macAddress, network.getId())) {
macAddress = networkModel.getNextAvailableMacAddressInNetwork(network.getId());
}
PrivateIpAddress ip =
new PrivateIpAddress(ipVO, vlanTag, network.getGateway(), netmask, NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ipVO.getMacAddress(), NetworkModel.MACIdentifier.value())));
new PrivateIpAddress(ipVO, vlanTag, network.getGateway(), netmask, macAddress);

nic.setIPv4Address(ip.getIpAddress());
nic.setIPv4Gateway(ip.getGateway());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public class StorageNetworkGuru extends PodBasedNetworkGuru implements NetworkGu
@Inject
StorageNetworkManager _sNwMgr;
@Inject
NetworkModel _networkModel;
@Inject
NetworkDao _nwDao;

protected StorageNetworkGuru() {
Expand Down Expand Up @@ -130,7 +132,11 @@ public void reserve(NicProfile nic, Network network, VirtualMachineProfile vm, D

vlan = ip.getVlan();
nic.setIPv4Address(ip.getIpAddress());
nic.setMacAddress(NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ip.getMac(), NetworkModel.MACIdentifier.value())));
String macAddress = NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ip.getMac(), _networkModel.getMacIdentifier(dest.getDataCenter().getId())));
if (!_networkModel.isMACUnique(macAddress, network.getId())) {
macAddress = _networkModel.getNextAvailableMacAddressInNetwork(network.getId());
}
nic.setMacAddress(macAddress);
nic.setFormat(AddressFormat.Ip4);
nic.setIPv4Netmask(ip.getNetmask());
nic.setBroadcastType(BroadcastDomainType.Storage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// under the License.
package com.cloud.network.router;

import com.cloud.exception.InsufficientAddressCapacityException;
import org.apache.cloudstack.network.router.deployment.RouterDeploymentDefinition;

import com.cloud.network.Network;
Expand All @@ -24,7 +25,7 @@

public interface NicProfileHelper {

public abstract NicProfile createPrivateNicProfileForGateway(final VpcGateway privateGateway, final VirtualRouter router);
public abstract NicProfile createPrivateNicProfileForGateway(final VpcGateway privateGateway, final VirtualRouter router) throws InsufficientAddressCapacityException;

public abstract NicProfile createGuestNicProfileForVpcRouter(final RouterDeploymentDefinition vpcRouterDeploymentDefinition,
Network guestNetwork);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import javax.inject.Inject;

import com.cloud.exception.InsufficientAddressCapacityException;
import com.cloud.vm.NicVO;
import com.cloud.vm.VirtualMachine;
import org.apache.cloudstack.network.router.deployment.RouterDeploymentDefinition;
Expand Down Expand Up @@ -61,7 +62,7 @@ public class NicProfileHelperImpl implements NicProfileHelper {

@Override
@DB
public NicProfile createPrivateNicProfileForGateway(final VpcGateway privateGateway, final VirtualRouter router) {
public NicProfile createPrivateNicProfileForGateway(final VpcGateway privateGateway, final VirtualRouter router) throws InsufficientAddressCapacityException {
final Network privateNetwork = _networkModel.getNetwork(privateGateway.getNetworkId());

PrivateIpVO ipVO = _privateIpDao.allocateIpAddress(privateNetwork.getDataCenterId(), privateNetwork.getId(), privateGateway.getIp4Address());
Expand Down Expand Up @@ -90,14 +91,20 @@ public NicProfile createPrivateNicProfileForGateway(final VpcGateway privateGate
privateNicProfile.setDeviceId(null);

if (router.getIsRedundantRouter()) {
String newMacAddress = NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ipVO.getMacAddress(), NetworkModel.MACIdentifier.value()));
String newMacAddress = NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ipVO.getMacAddress(), _networkModel.getMacIdentifier(privateNetwork.getDataCenterId())));
if (!_networkModel.isMACUnique(newMacAddress, privateNetwork.getId())) {
newMacAddress = _networkModel.getNextAvailableMacAddressInNetwork(privateNetwork.getId());
}
privateNicProfile.setMacAddress(newMacAddress);
}
} else {
final String netmask = NetUtils.getCidrNetmask(privateNetwork.getCidr());
String newMacAddress = NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ipVO.getMacAddress(), _networkModel.getMacIdentifier(privateNetwork.getDataCenterId())));
if (!_networkModel.isMACUnique(newMacAddress, privateNetwork.getId())) {
newMacAddress = _networkModel.getNextAvailableMacAddressInNetwork(privateNetwork.getId());
}
final PrivateIpAddress ip =
new PrivateIpAddress(ipVO, privateNetwork.getBroadcastUri().toString(), privateNetwork.getGateway(), netmask,
NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ipVO.getMacAddress(), NetworkModel.MACIdentifier.value())));
new PrivateIpAddress(ipVO, privateNetwork.getBroadcastUri().toString(), privateNetwork.getGateway(), netmask, newMacAddress);

final URI netUri = BroadcastDomainType.fromString(ip.getBroadcastUri());
privateNicProfile.setIPv4Address(ip.getIpAddress());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,10 @@ public void setNICIPv6AddressTest() throws InsufficientAddressCapacityException

Mockito.when(network.getIp6Cidr()).thenReturn("2001:db8:100::/64");
Mockito.when(network.getIp6Gateway()).thenReturn("2001:db8:100::1");
Mockito.when(network.getId()).thenReturn(1L);

Mockito.when(networkModel.getNetworkIp6Dns(network, dc)).thenReturn(new Pair<>("2001:db8::53:1", "2001:db8::53:2"));
Mockito.when(networkModel.isMACUnique("1e:00:b1:00:0a:f6", 1L)).thenReturn(true);

String expected = "2001:db8:100:0:1c00:b1ff:fe00:af6";

Expand Down
6 changes: 3 additions & 3 deletions utils/src/main/java/com/cloud/utils/net/NetUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,17 +131,17 @@ public static boolean isIpv6EnabledProtocol(InternetProtocol protocol) {
}
}

public static long createSequenceBasedMacAddress(final long macAddress, long globalConfig) {
public static long createSequenceBasedMacAddress(final long macAddress, long macIdentifier) {
/*
Logic for generating MAC address:
Mac = B1:B2:B3:B4:B5:B6 (Bx is a byte).
B1 -> Presently controlled by prefix variable. The value should be such that the MAC is local and unicast.
B2 -> This will be configurable for each deployment/installation. Controlled by the global config MACIdentifier
B2 -> This will be configurable for each deployment/installation. Controlled by the 'mac.identifier' zone-level config
B3 -> A randomly generated number between 0 - 255
B4,5,6 -> These bytes are based on the unique DB identifier associated with the IP address for which MAC is generated (refer to mac_address field in user_ip_address table).
*/

return macAddress | prefix<<40 | globalConfig << 32 & 0x00ff00000000l | (long)s_rand.nextInt(255) << 24;
return macAddress | prefix << 40 | macIdentifier << 32 & 0x00ff00000000L | (long)s_rand.nextInt(255) << 24;
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The random number generation uses s_rand.nextInt(255) which generates values from 0 to 254 (exclusive upper bound). According to the comment on line 140, B3 should be "A randomly generated number between 0 - 255". To include 255, this should be s_rand.nextInt(256).

Suggested change
return macAddress | prefix << 40 | macIdentifier << 32 & 0x00ff00000000L | (long)s_rand.nextInt(255) << 24;
return macAddress | prefix << 40 | macIdentifier << 32 & 0x00ff00000000L | (long)s_rand.nextInt(256) << 24;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@sureshanaparti sureshanaparti Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weizhouapache @DaanHoogland can update this as per the comment here? previously, it used to generate till 254 only.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the previous programming has been either overly defensive or sloppy. But in general I’d rather see us iterate than randomly go through the range. After half is full the hit-and-miss rate becomes pretty high like this. Either way, it is functional as is, so address in another PR, i’d say.

}

public static String getHostName() {
Expand Down
Loading