Skip to content

Commit 1c9e6d2

Browse files
widowido
committed
kvm/bridge: Allow Link Local Cidr (cloud0 interface) to be configured
There are certain scenarios where the 169.254.0.0/16 subnet is used for different purposes then CloudStack on a hypervisor. Once of such scenarios is a BGP+EVPN+VXLAN setup using BGP Unnumbered where the 169.254.0.1 address is used by Frr/Zebra BGP routing to send traffic to the neighboring router. The following settings can be changed in the agent.properties (default values added): control.cidr=169.254.0.0/16 Make sure the global setting 'control.cidr' matches the values defined in the agent.propeties! In the future the mgmt server can send this parameter to a KVM Agent on startup, but at the moment this framework is not in place and thus these values can't be send to the Agent in a proper manner. This commit also: - Adds Unit Tests - Removes the dead IvsVifDriver Signed-off-by: Wido den Hollander <wido@widodh.nl>
1 parent c1d3f98 commit 1c9e6d2

File tree

9 files changed

+93
-63
lines changed

9 files changed

+93
-63
lines changed

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,17 @@
2828

2929
import javax.naming.ConfigurationException;
3030

31+
import com.cloud.utils.StringUtils;
32+
import com.cloud.utils.net.NetUtils;
33+
import com.cloud.utils.script.OutputInterpreter;
34+
import com.google.common.base.Strings;
3135
import org.apache.log4j.Logger;
3236
import org.libvirt.LibvirtException;
3337

34-
import com.google.common.base.Strings;
35-
3638
import com.cloud.agent.api.to.NicTO;
3739
import com.cloud.exception.InternalErrorException;
3840
import com.cloud.network.Networks;
3941
import com.cloud.utils.NumbersUtil;
40-
import com.cloud.utils.net.NetUtils;
41-
import com.cloud.utils.script.OutputInterpreter;
4242
import com.cloud.utils.script.Script;
4343

4444
public class BridgeVifDriver extends VifDriverBase {
@@ -49,7 +49,7 @@ public class BridgeVifDriver extends VifDriverBase {
4949
private final Object _vnetBridgeMonitor = new Object();
5050
private String _modifyVlanPath;
5151
private String _modifyVxlanPath;
52-
private String bridgeNameSchema;
52+
private String _controlCidr = NetUtils.getLinkLocalCIDR();
5353
private Long libvirtVersion;
5454

5555
@Override
@@ -67,7 +67,10 @@ public void configure(Map<String, Object> params) throws ConfigurationException
6767
networkScriptsDir = "scripts/vm/network/vnet";
6868
}
6969

70-
bridgeNameSchema = (String)params.get("network.bridge.name.schema");
70+
String controlCidr = (String)params.get("control.cidr");
71+
if (StringUtils.isNotBlank(controlCidr)) {
72+
_controlCidr = controlCidr;
73+
}
7174

7275
String value = (String)params.get("scripts.timeout");
7376
_timeout = NumbersUtil.parseInt(value, 30 * 60) * 1000;
@@ -384,7 +387,7 @@ private void deleteVnetBr(String brName) {
384387
private void deleteExistingLinkLocalRouteTable(String linkLocalBr) {
385388
Script command = new Script("/bin/bash", _timeout);
386389
command.add("-c");
387-
command.add("ip route | grep " + NetUtils.getLinkLocalCIDR());
390+
command.add("ip route | grep " + _controlCidr);
388391
OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
389392
String result = command.execute(parser);
390393
boolean foundLinkLocalBr = false;
@@ -397,15 +400,16 @@ private void deleteExistingLinkLocalRouteTable(String linkLocalBr) {
397400
}
398401
final String device = tokens[2];
399402
if (!Strings.isNullOrEmpty(device) && !device.equalsIgnoreCase(linkLocalBr)) {
400-
Script.runSimpleBashScript("ip route del " + NetUtils.getLinkLocalCIDR() + " dev " + tokens[2]);
403+
Script.runSimpleBashScript("ip route del " + _controlCidr + " dev " + tokens[2]);
401404
} else {
402405
foundLinkLocalBr = true;
403406
}
404407
}
405408
}
409+
406410
if (!foundLinkLocalBr) {
407-
Script.runSimpleBashScript("ip address add 169.254.0.1/16 dev " + linkLocalBr + ";" + "ip route add " + NetUtils.getLinkLocalCIDR() + " dev " + linkLocalBr + " src " +
408-
NetUtils.getLinkLocalGateway());
411+
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + linkLocalBr);
412+
Script.runSimpleBashScript("ip route add " + _controlCidr + " dev " + linkLocalBr + " src " + NetUtils.getLinkLocalGateway(_controlCidr));
409413
}
410414
}
411415

@@ -417,7 +421,9 @@ private void createControlNetwork() {
417421
public void createControlNetwork(String privBrName) {
418422
deleteExistingLinkLocalRouteTable(privBrName);
419423
if (!isExistingBridge(privBrName)) {
420-
Script.runSimpleBashScript("brctl addbr " + privBrName + "; ip link set " + privBrName + " up; ip address add 169.254.0.1/16 dev " + privBrName, _timeout);
424+
Script.runSimpleBashScript("ip link add name " + privBrName + " type bridge");
425+
Script.runSimpleBashScript("ip link set " + privBrName + " up");
426+
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + privBrName);
421427
}
422428
}
423429

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import javax.naming.ConfigurationException;
2828

29+
import com.cloud.utils.StringUtils;
2930
import org.apache.log4j.Logger;
3031
import org.libvirt.LibvirtException;
3132

@@ -46,7 +47,7 @@ public class IvsVifDriver extends VifDriverBase {
4647
private String _modifyVlanPath;
4748
private String _modifyVxlanPath;
4849
private String _ivsIfUpPath;
49-
private Long libvirtVersion;
50+
private String _controlCidr = NetUtils.getLinkLocalCIDR();
5051

5152
@Override
5253
public void configure(Map<String, Object> params) throws ConfigurationException {
@@ -70,9 +71,9 @@ public void configure(Map<String, Object> params) throws ConfigurationException
7071
}
7172
_ivsIfUpPath = Script.findScript(utilScriptsDir, "qemu-ivs-ifup");
7273

73-
libvirtVersion = (Long) params.get("libvirtVersion");
74-
if (libvirtVersion == null) {
75-
libvirtVersion = 0L;
74+
String controlCidr = (String)params.get("control.cidr");
75+
if (StringUtils.isNotBlank(controlCidr)) {
76+
_controlCidr = controlCidr;
7677
}
7778
}
7879

@@ -256,7 +257,7 @@ private void deleteVnetBr(String brName) {
256257
private void deleteExitingLinkLocalRouteTable(String linkLocalBr) {
257258
Script command = new Script("/bin/bash", _timeout);
258259
command.add("-c");
259-
command.add("ip route | grep " + NetUtils.getLinkLocalCIDR());
260+
command.add("ip route | grep " + _controlCidr);
260261
OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
261262
String result = command.execute(parser);
262263
boolean foundLinkLocalBr = false;
@@ -265,23 +266,24 @@ private void deleteExitingLinkLocalRouteTable(String linkLocalBr) {
265266
for (String line : lines) {
266267
String[] tokens = line.split(" ");
267268
if (!tokens[2].equalsIgnoreCase(linkLocalBr)) {
268-
Script.runSimpleBashScript("ip route del " + NetUtils.getLinkLocalCIDR());
269+
Script.runSimpleBashScript("ip route del " + _controlCidr);
269270
} else {
270271
foundLinkLocalBr = true;
271272
}
272273
}
273274
}
274275
if (!foundLinkLocalBr) {
275-
Script.runSimpleBashScript("ip address add 169.254.0.1/16 dev " + linkLocalBr + ";" + "ip route add " + NetUtils.getLinkLocalCIDR() + " dev " + linkLocalBr + " src " +
276-
NetUtils.getLinkLocalGateway());
276+
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + linkLocalBr);
277+
Script.runSimpleBashScript("ip route add " + _controlCidr + " dev " + linkLocalBr + " src " + NetUtils.getLinkLocalGateway(_controlCidr));
277278
}
278279
}
279280

280281
@Override
281282
public void createControlNetwork(String privBrName) {
282283
deleteExitingLinkLocalRouteTable(privBrName);
283284
if (!isBridgeExists(privBrName)) {
284-
Script.runSimpleBashScript("brctl addbr " + privBrName + "; ip link set " + privBrName + " up; ip address add 169.254.0.1/16 dev " + privBrName, _timeout);
285+
Script.runSimpleBashScript("brctl addbr " + privBrName + "; ip link set " + privBrName + " up");
286+
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + privBrName, _timeout);
285287
}
286288
}
287289

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
public class OvsVifDriver extends VifDriverBase {
4545
private static final Logger s_logger = Logger.getLogger(OvsVifDriver.class);
4646
private int _timeout;
47+
private String _controlCidr = NetUtils.getLinkLocalCIDR();
4748
private DpdkDriver dpdkDriver;
4849

4950
@Override
@@ -62,6 +63,11 @@ public void configure(Map<String, Object> params) throws ConfigurationException
6263
dpdkDriver = new DpdkDriverImpl();
6364
}
6465

66+
String controlCidr = (String)params.get("control.cidr");
67+
if (com.cloud.utils.StringUtils.isNotBlank(controlCidr)) {
68+
_controlCidr = controlCidr;
69+
}
70+
6571
String value = (String)params.get("scripts.timeout");
6672
_timeout = NumbersUtil.parseInt(value, 30 * 60) * 1000;
6773
}
@@ -213,7 +219,7 @@ public void detach(LibvirtVMDef.InterfaceDef iface) {
213219
private void deleteExitingLinkLocalRouteTable(String linkLocalBr) {
214220
Script command = new Script("/bin/bash", _timeout);
215221
command.add("-c");
216-
command.add("ip route | grep " + NetUtils.getLinkLocalCIDR());
222+
command.add("ip route | grep " + _controlCidr);
217223
OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
218224
String result = command.execute(parser);
219225
boolean foundLinkLocalBr = false;
@@ -222,23 +228,23 @@ private void deleteExitingLinkLocalRouteTable(String linkLocalBr) {
222228
for (String line : lines) {
223229
String[] tokens = line.split(" ");
224230
if (!tokens[2].equalsIgnoreCase(linkLocalBr)) {
225-
Script.runSimpleBashScript("ip route del " + NetUtils.getLinkLocalCIDR());
231+
Script.runSimpleBashScript("ip route del " + _controlCidr);
226232
} else {
227233
foundLinkLocalBr = true;
228234
}
229235
}
230236
}
231237
if (!foundLinkLocalBr) {
232-
Script.runSimpleBashScript("ip address add 169.254.0.1/16 dev " + linkLocalBr + ";" + "ip route add " + NetUtils.getLinkLocalCIDR() + " dev " + linkLocalBr + " src " +
233-
NetUtils.getLinkLocalGateway());
238+
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + linkLocalBr + ";" + "ip route add " + _controlCidr + " dev " + linkLocalBr + " src " +
239+
NetUtils.getLinkLocalGateway(_controlCidr));
234240
}
235241
}
236242

237243
@Override
238244
public void createControlNetwork(String privBrName) {
239245
deleteExitingLinkLocalRouteTable(privBrName);
240246
if (!isExistingBridge(privBrName)) {
241-
Script.runSimpleBashScript("ovs-vsctl add-br " + privBrName + "; ip link set " + privBrName + " up; ip address add 169.254.0.1/16 dev " + privBrName, _timeout);
247+
Script.runSimpleBashScript("ovs-vsctl add-br " + privBrName + "; ip link set " + privBrName + " up; ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + privBrName, _timeout);
242248
}
243249
}
244250

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,7 +1613,7 @@ public HostPodVO doInTransaction(final TransactionStatus status) {
16131613
_zoneDao.addPrivateIpAddress(zoneId, pod.getId(), startIp, endIpFinal, false, null);
16141614
}
16151615

1616-
final String[] linkLocalIpRanges = getLinkLocalIPRange();
1616+
final String[] linkLocalIpRanges = NetUtils.getLinkLocalIPRange(_configDao.getValue(Config.ControlCidr.key()));
16171617
if (linkLocalIpRanges != null) {
16181618
_zoneDao.addLinkLocalIpAddress(zoneId, pod.getId(), linkLocalIpRanges[0], linkLocalIpRanges[1]);
16191619
}
@@ -4489,20 +4489,6 @@ private String getZoneName(final long zoneId) {
44894489
}
44904490
}
44914491

4492-
private String[] getLinkLocalIPRange() {
4493-
final String ipNums = _configDao.getValue("linkLocalIp.nums");
4494-
final int nums = Integer.parseInt(ipNums);
4495-
if (nums > 16 || nums <= 0) {
4496-
throw new InvalidParameterValueException("The linkLocalIp.nums: " + nums + "is wrong, should be 1~16");
4497-
}
4498-
/* local link ip address starts from 169.254.0.2 - 169.254.(nums) */
4499-
final String[] ipRanges = NetUtils.getLinkLocalIPRange(nums);
4500-
if (ipRanges == null) {
4501-
throw new InvalidParameterValueException("The linkLocalIp.nums: " + nums + "may be wrong, should be 1~16");
4502-
}
4503-
return ipRanges;
4504-
}
4505-
45064492
@Override
45074493
@ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_DELETE, eventDescription = "deleting vlan ip range", async = false)
45084494
public boolean deleteVlanIpRange(final DeleteVlanIpRangeCmd cmd) {

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,16 @@ public void reserve(NicProfile nic, Network config, VirtualMachineProfile vm, De
160160
if (ip == null) {
161161
throw new InsufficientAddressCapacityException("Insufficient link local address capacity", DataCenter.class, dest.getDataCenter().getId());
162162
}
163+
164+
String netmask = NetUtils.cidr2Netmask(_cidr);
165+
166+
s_logger.debug(String.format("Reserved NIC for %s [ipv4:%s netmask:%s gateway:%s]", vm.getInstanceName(), ip, netmask, _gateway));
167+
163168
nic.setIPv4Address(ip);
164169
nic.setMacAddress(NetUtils.long2Mac(NetUtils.ip2Long(ip) | (14l << 40)));
165-
nic.setIPv4Netmask("255.255.0.0");
170+
nic.setIPv4Netmask(netmask);
166171
nic.setFormat(AddressFormat.Ip4);
167-
nic.setIPv4Gateway(NetUtils.getLinkLocalGateway());
172+
nic.setIPv4Gateway(_gateway);
168173
}
169174

170175
@Override
@@ -223,7 +228,7 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
223228

224229
_cidr = dbParams.get(Config.ControlCidr.toString());
225230
if (_cidr == null) {
226-
_cidr = "169.254.0.0/16";
231+
_cidr = NetUtils.getLinkLocalCIDR();
227232
}
228233

229234
_gateway = dbParams.get(Config.ControlGateway.toString());

‎server/src/main/java/com/cloud/server/ConfigurationServerImpl.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -904,12 +904,8 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Intern
904904
throw new InvalidParameterValueException("The linkLocalIp.nums: " + nums + "is wrong, should be 1~16");
905905
}
906906
/* local link ip address starts from 169.254.0.2 - 169.254.(nums) */
907-
String[] linkLocalIpRanges = NetUtils.getLinkLocalIPRange(nums);
908-
if (linkLocalIpRanges == null) {
909-
throw new InvalidParameterValueException("The linkLocalIp.nums: " + nums + "may be wrong, should be 1~16");
910-
} else {
911-
_zoneDao.addLinkLocalIpAddress(zoneId, pod.getId(), linkLocalIpRanges[0], linkLocalIpRanges[1]);
912-
}
907+
String[] linkLocalIpRanges = NetUtils.getLinkLocalIPRange(_configDao.getValue(Config.ControlCidr.key()));
908+
_zoneDao.addLinkLocalIpAddress(zoneId, pod.getId(), linkLocalIpRanges[0], linkLocalIpRanges[1]);
913909
}
914910
});
915911
} catch (Exception e) {

‎server/src/main/java/com/cloud/test/IPRangeConfig.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ public List<String> saveIPRange(String type, long podId, long zoneId, long vlanD
436436
problemIPs = savePrivateIPRange(txn, startIPLong, endIPLong, podId, zoneId);
437437
}
438438

439-
String[] linkLocalIps = NetUtils.getLinkLocalIPRange(10);
439+
String[] linkLocalIps = NetUtils.getLinkLocalIPRange("169.254.0.0/16");
440440
long startLinkLocalIp = NetUtils.ip2Long(linkLocalIps[0]);
441441
long endLinkLocalIp = NetUtils.ip2Long(linkLocalIps[1]);
442442

‎utils/src/main/java/com/cloud/utils/net/NetUtils.java‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -961,27 +961,34 @@ public static String getLinkLocalNetMask() {
961961
return "255.255.0.0";
962962
}
963963

964+
public static String getLinkLocalGateway(String cidr) {
965+
return getLinkLocalFirstAddressFromCIDR(cidr);
966+
}
967+
964968
public static String getLinkLocalGateway() {
965-
return "169.254.0.1";
969+
return getLinkLocalGateway(getLinkLocalCIDR());
966970
}
967971

968972
public static String getLinkLocalCIDR() {
969973
return "169.254.0.0/16";
970974
}
971975

972-
public static String[] getLinkLocalIPRange(final int size) {
973-
if (size > 16 || size <= 0) {
974-
return null;
975-
}
976-
/* reserve gateway */
977-
final String[] range = getIpRangeFromCidr(getLinkLocalGateway(), MAX_CIDR - size);
976+
public static String getLinkLocalFirstAddressFromCIDR(final String cidr) {
977+
SubnetUtils subnetUtils = new SubnetUtils(cidr);
978+
return subnetUtils.getInfo().getLowAddress();
979+
}
980+
981+
public static String getLinkLocalAddressFromCIDR(final String cidr) {
982+
return getLinkLocalFirstAddressFromCIDR(cidr) + "/" + cidr2Netmask(cidr);
983+
}
984+
985+
public static String[] getLinkLocalIPRange(final String cidr) {
986+
final SubnetUtils subnetUtils = new SubnetUtils(cidr);
987+
final String[] addresses = subnetUtils.getInfo().getAllAddresses();
988+
final String[] range = new String[2];
989+
range[0] = addresses[1];
990+
range[1] = subnetUtils.getInfo().getHighAddress();
978991

979-
if (range[0].equalsIgnoreCase(getLinkLocalGateway())) {
980-
/* remove the gateway */
981-
long ip = ip2Long(range[0]);
982-
ip += 1;
983-
range[0] = long2Ip(ip);
984-
}
985992
return range;
986993
}
987994

‎utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,4 +709,26 @@ public void testIsIPv6EUI64() {
709709
assertFalse(NetUtils.isIPv6EUI64("2001:db8::100:1"));
710710
assertFalse(NetUtils.isIPv6EUI64("2a01:4f9:2a:185f::2"));
711711
}
712+
713+
@Test
714+
public void testLinkLocal() {
715+
final String cidr = NetUtils.getLinkLocalCIDR();
716+
assertEquals("255.255.0.0", NetUtils.getLinkLocalNetMask());
717+
assertEquals("169.254.0.1", NetUtils.getLinkLocalGateway());
718+
assertEquals("169.254.0.0/16", cidr);
719+
assertEquals("169.254.0.1", NetUtils.getLinkLocalFirstAddressFromCIDR(cidr));
720+
assertEquals("169.254.0.1/255.255.0.0", NetUtils.getLinkLocalAddressFromCIDR(cidr));
721+
assertEquals("169.254.240.1/255.255.240.0", NetUtils.getLinkLocalAddressFromCIDR("169.254.240.0/20"));
722+
723+
String[] range = NetUtils.getLinkLocalIPRange("169.254.0.0/16");
724+
assertEquals("169.254.0.2", range[0]);
725+
assertEquals("169.254.255.254", range[1]);
726+
}
727+
728+
@Test
729+
public void testCidrNetmask() {
730+
assertEquals("255.255.255.0", NetUtils.cidr2Netmask("192.168.0.0/24"));
731+
assertEquals("255.255.0.0", NetUtils.cidr2Netmask("169.254.0.0/16"));
732+
assertEquals("255.255.240.0", NetUtils.cidr2Netmask("169.254.240.0/20"));
733+
}
712734
}

0 commit comments

Comments
 (0)