FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

kvm/bridge: Allow Link Local Cidr (cloud0 interface) to be configured by wido · Pull Request #3500 · apache/cloudstack · GitHub

/ cloudstack Public

kvm/bridge: Allow Link Local Cidr (cloud0 interface) to be configured #3500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yadvr merged 1 commit into from
Jul 25, 2019
Conversation 44 Commits 1 Checks 0 Files changed
Merged

kvm/bridge: Allow Link Local Cidr (cloud0 interface) to be configured #3500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension .java  (9) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
28 changes: 17 additions & 11 deletions plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@

import javax.naming.ConfigurationException;

import com.cloud.utils.StringUtils;
import com.cloud.utils.net.NetUtils;
import com.cloud.utils.script.OutputInterpreter;
import com.google.common.base.Strings;
import org.apache.log4j.Logger;
import org.libvirt.LibvirtException;

import com.google.common.base.Strings;

import com.cloud.agent.api.to.NicTO;
import com.cloud.exception.InternalErrorException;
import com.cloud.network.Networks;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.net.NetUtils;
import com.cloud.utils.script.OutputInterpreter;
import com.cloud.utils.script.Script;

public class BridgeVifDriver extends VifDriverBase {
Expand All @@ -49,7 +49,7 @@ public class BridgeVifDriver extends VifDriverBase {
private final Object _vnetBridgeMonitor = new Object();
private String _modifyVlanPath;
private String _modifyVxlanPath;
private String bridgeNameSchema;
private String _controlCidr = NetUtils.getLinkLocalCIDR();
private Long libvirtVersion;

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

bridgeNameSchema = (String)params.get("network.bridge.name.schema");
String controlCidr = (String)params.get("control.cidr");
if (StringUtils.isNotBlank(controlCidr)) {
_controlCidr = controlCidr;
}

String value = (String)params.get("scripts.timeout");
_timeout = NumbersUtil.parseInt(value, 30 * 60) * 1000;
Expand Down Expand Up @@ -384,7 +387,7 @@ private void deleteVnetBr(String brName) {
private void deleteExistingLinkLocalRouteTable(String linkLocalBr) {
Script command = new Script("/bin/bash", _timeout);
command.add("-c");
command.add("ip route | grep " + NetUtils.getLinkLocalCIDR());
command.add("ip route | grep " + _controlCidr);
OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
String result = command.execute(parser);
boolean foundLinkLocalBr = false;
Expand All @@ -397,15 +400,16 @@ private void deleteExistingLinkLocalRouteTable(String linkLocalBr) {
}
final String device = tokens[2];
if (!Strings.isNullOrEmpty(device) && !device.equalsIgnoreCase(linkLocalBr)) {
Script.runSimpleBashScript("ip route del " + NetUtils.getLinkLocalCIDR() + " dev " + tokens[2]);
Script.runSimpleBashScript("ip route del " + _controlCidr + " dev " + tokens[2]);
} else {
foundLinkLocalBr = true;
}
}
}

if (!foundLinkLocalBr) {
Script.runSimpleBashScript("ip address add 169.254.0.1/16 dev " + linkLocalBr + ";" + "ip route add " + NetUtils.getLinkLocalCIDR() + " dev " + linkLocalBr + " src " +
NetUtils.getLinkLocalGateway());
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + linkLocalBr);
Script.runSimpleBashScript("ip route add " + _controlCidr + " dev " + linkLocalBr + " src " + NetUtils.getLinkLocalGateway(_controlCidr));
}
}

Expand All @@ -417,7 +421,9 @@ private void createControlNetwork() {
public void createControlNetwork(String privBrName) {
deleteExistingLinkLocalRouteTable(privBrName);
if (!isExistingBridge(privBrName)) {
Script.runSimpleBashScript("brctl addbr " + privBrName + "; ip link set " + privBrName + " up; ip address add 169.254.0.1/16 dev " + privBrName, _timeout);
Script.runSimpleBashScript("ip link add name " + privBrName + " type bridge");
Script.runSimpleBashScript("ip link set " + privBrName + " up");
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + privBrName);
}
}

Expand Down
20 changes: 11 additions & 9 deletions plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import javax.naming.ConfigurationException;

import com.cloud.utils.StringUtils;
import org.apache.log4j.Logger;
import org.libvirt.LibvirtException;

Expand All @@ -46,7 +47,7 @@ public class IvsVifDriver extends VifDriverBase {
private String _modifyVlanPath;
private String _modifyVxlanPath;
private String _ivsIfUpPath;
private Long libvirtVersion;
private String _controlCidr = NetUtils.getLinkLocalCIDR();

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

libvirtVersion = (Long) params.get("libvirtVersion");

nvazquez Jul 24, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks like libvirtVersion was not being used on this class, is that right?

wido Jul 24, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Not being used indeed. Dead variable which was never called
if (libvirtVersion == null) {
libvirtVersion = 0L;
String controlCidr = (String)params.get("control.cidr");
if (StringUtils.isNotBlank(controlCidr)) {
_controlCidr = controlCidr;
}
}

Expand Down Expand Up @@ -256,7 +257,7 @@ private void deleteVnetBr(String brName) {
private void deleteExitingLinkLocalRouteTable(String linkLocalBr) {
Script command = new Script("/bin/bash", _timeout);
command.add("-c");
command.add("ip route | grep " + NetUtils.getLinkLocalCIDR());
command.add("ip route | grep " + _controlCidr);
OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
String result = command.execute(parser);
boolean foundLinkLocalBr = false;
Expand All @@ -265,23 +266,24 @@ private void deleteExitingLinkLocalRouteTable(String linkLocalBr) {
for (String line : lines) {
String[] tokens = line.split(" ");
if (!tokens[2].equalsIgnoreCase(linkLocalBr)) {
Script.runSimpleBashScript("ip route del " + NetUtils.getLinkLocalCIDR());
Script.runSimpleBashScript("ip route del " + _controlCidr);
} else {
foundLinkLocalBr = true;
}
}
}
if (!foundLinkLocalBr) {
Script.runSimpleBashScript("ip address add 169.254.0.1/16 dev " + linkLocalBr + ";" + "ip route add " + NetUtils.getLinkLocalCIDR() + " dev " + linkLocalBr + " src " +
NetUtils.getLinkLocalGateway());
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + linkLocalBr);
Script.runSimpleBashScript("ip route add " + _controlCidr + " dev " + linkLocalBr + " src " + NetUtils.getLinkLocalGateway(_controlCidr));
}
}

@Override
public void createControlNetwork(String privBrName) {
deleteExitingLinkLocalRouteTable(privBrName);
if (!isBridgeExists(privBrName)) {
Script.runSimpleBashScript("brctl addbr " + privBrName + "; ip link set " + privBrName + " up; ip address add 169.254.0.1/16 dev " + privBrName, _timeout);
Script.runSimpleBashScript("brctl addbr " + privBrName + "; ip link set " + privBrName + " up");
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + privBrName, _timeout);
}
}

Expand Down
16 changes: 11 additions & 5 deletions plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
public class OvsVifDriver extends VifDriverBase {
private static final Logger s_logger = Logger.getLogger(OvsVifDriver.class);
private int _timeout;
private String _controlCidr = NetUtils.getLinkLocalCIDR();
private DpdkDriver dpdkDriver;

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

String controlCidr = (String)params.get("control.cidr");
if (com.cloud.utils.StringUtils.isNotBlank(controlCidr)) {
_controlCidr = controlCidr;
}

String value = (String)params.get("scripts.timeout");
_timeout = NumbersUtil.parseInt(value, 30 * 60) * 1000;
}
Expand Down Expand Up @@ -213,7 +219,7 @@ public void detach(LibvirtVMDef.InterfaceDef iface) {
private void deleteExitingLinkLocalRouteTable(String linkLocalBr) {
Script command = new Script("/bin/bash", _timeout);
command.add("-c");
command.add("ip route | grep " + NetUtils.getLinkLocalCIDR());
command.add("ip route | grep " + _controlCidr);
OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
String result = command.execute(parser);
boolean foundLinkLocalBr = false;
Expand All @@ -222,23 +228,23 @@ private void deleteExitingLinkLocalRouteTable(String linkLocalBr) {
for (String line : lines) {
String[] tokens = line.split(" ");
if (!tokens[2].equalsIgnoreCase(linkLocalBr)) {
Script.runSimpleBashScript("ip route del " + NetUtils.getLinkLocalCIDR());
Script.runSimpleBashScript("ip route del " + _controlCidr);
} else {
foundLinkLocalBr = true;
}
}
}
if (!foundLinkLocalBr) {
Script.runSimpleBashScript("ip address add 169.254.0.1/16 dev " + linkLocalBr + ";" + "ip route add " + NetUtils.getLinkLocalCIDR() + " dev " + linkLocalBr + " src " +
NetUtils.getLinkLocalGateway());
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + linkLocalBr + ";" + "ip route add " + _controlCidr + " dev " + linkLocalBr + " src " +
NetUtils.getLinkLocalGateway(_controlCidr));
}
}

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

Expand Down
16 changes: 1 addition & 15 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1613,7 +1613,7 @@ public HostPodVO doInTransaction(final TransactionStatus status) {
_zoneDao.addPrivateIpAddress(zoneId, pod.getId(), startIp, endIpFinal, false, null);
}

final String[] linkLocalIpRanges = getLinkLocalIPRange();
final String[] linkLocalIpRanges = NetUtils.getLinkLocalIPRange(_configDao.getValue(Config.ControlCidr.key()));
if (linkLocalIpRanges != null) {
_zoneDao.addLinkLocalIpAddress(zoneId, pod.getId(), linkLocalIpRanges[0], linkLocalIpRanges[1]);
}
Expand Down Expand Up @@ -4489,20 +4489,6 @@ private String getZoneName(final long zoneId) {
}
}

private String[] getLinkLocalIPRange() {
final String ipNums = _configDao.getValue("linkLocalIp.nums");
final int nums = Integer.parseInt(ipNums);
if (nums > 16 || nums <= 0) {
throw new InvalidParameterValueException("The linkLocalIp.nums: " + nums + "is wrong, should be 1~16");
}
/* local link ip address starts from 169.254.0.2 - 169.254.(nums) */
final String[] ipRanges = NetUtils.getLinkLocalIPRange(nums);
if (ipRanges == null) {
throw new InvalidParameterValueException("The linkLocalIp.nums: " + nums + "may be wrong, should be 1~16");
}
return ipRanges;
}

@Override
@ActionEvent(eventType = EventTypes.EVENT_VLAN_IP_RANGE_DELETE, eventDescription = "deleting vlan ip range", async = false)
public boolean deleteVlanIpRange(final DeleteVlanIpRangeCmd cmd) {
Expand Down
11 changes: 8 additions & 3 deletions server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,16 @@ public void reserve(NicProfile nic, Network config, VirtualMachineProfile vm, De
if (ip == null) {
throw new InsufficientAddressCapacityException("Insufficient link local address capacity", DataCenter.class, dest.getDataCenter().getId());
}

String netmask = NetUtils.cidr2Netmask(_cidr);

s_logger.debug(String.format("Reserved NIC for %s [ipv4:%s netmask:%s gateway:%s]", vm.getInstanceName(), ip, netmask, _gateway));

nic.setIPv4Address(ip);
nic.setMacAddress(NetUtils.long2Mac(NetUtils.ip2Long(ip) | (14l << 40)));
nic.setIPv4Netmask("255.255.0.0");
nic.setIPv4Netmask(netmask);
nic.setFormat(AddressFormat.Ip4);
nic.setIPv4Gateway(NetUtils.getLinkLocalGateway());
nic.setIPv4Gateway(_gateway);
}

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

_cidr = dbParams.get(Config.ControlCidr.toString());
if (_cidr == null) {
_cidr = "169.254.0.0/16";
_cidr = NetUtils.getLinkLocalCIDR();
}

_gateway = dbParams.get(Config.ControlGateway.toString());
Expand Down
8 changes: 2 additions & 6 deletions server/src/main/java/com/cloud/server/ConfigurationServerImpl.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -904,12 +904,8 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Intern
throw new InvalidParameterValueException("The linkLocalIp.nums: " + nums + "is wrong, should be 1~16");
}
/* local link ip address starts from 169.254.0.2 - 169.254.(nums) */
String[] linkLocalIpRanges = NetUtils.getLinkLocalIPRange(nums);
if (linkLocalIpRanges == null) {
throw new InvalidParameterValueException("The linkLocalIp.nums: " + nums + "may be wrong, should be 1~16");
} else {
_zoneDao.addLinkLocalIpAddress(zoneId, pod.getId(), linkLocalIpRanges[0], linkLocalIpRanges[1]);
}
String[] linkLocalIpRanges = NetUtils.getLinkLocalIPRange(_configDao.getValue(Config.ControlCidr.key()));
_zoneDao.addLinkLocalIpAddress(zoneId, pod.getId(), linkLocalIpRanges[0], linkLocalIpRanges[1]);
}
});
} catch (Exception e) {
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/test/IPRangeConfig.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ public List<String> saveIPRange(String type, long podId, long zoneId, long vlanD
problemIPs = savePrivateIPRange(txn, startIPLong, endIPLong, podId, zoneId);
}

String[] linkLocalIps = NetUtils.getLinkLocalIPRange(10);
String[] linkLocalIps = NetUtils.getLinkLocalIPRange("169.254.0.0/16");
long startLinkLocalIp = NetUtils.ip2Long(linkLocalIps[0]);
long endLinkLocalIp = NetUtils.ip2Long(linkLocalIps[1]);

Expand Down
33 changes: 20 additions & 13 deletions utils/src/main/java/com/cloud/utils/net/NetUtils.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -961,27 +961,34 @@ public static String getLinkLocalNetMask() {
return "255.255.0.0";
}

public static String getLinkLocalGateway(String cidr) {
return getLinkLocalFirstAddressFromCIDR(cidr);
}

public static String getLinkLocalGateway() {
return "169.254.0.1";
return getLinkLocalGateway(getLinkLocalCIDR());
}

public static String getLinkLocalCIDR() {
return "169.254.0.0/16";

nvazquez Jul 18, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Same as the comment above, instead of returning a hardcoded value, this method can return the configured CIDR and can make getLinkLocalGateway() above to be consistent with it.

wido Jul 22, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Do we have access to the configuration here? I don't think so. Therefor I changed it this way.

nvazquez Jul 24, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good
}

public static String[] getLinkLocalIPRange(final int size) {
if (size > 16 || size <= 0) {
return null;
}
/* reserve gateway */
final String[] range = getIpRangeFromCidr(getLinkLocalGateway(), MAX_CIDR - size);
public static String getLinkLocalFirstAddressFromCIDR(final String cidr) {
SubnetUtils subnetUtils = new SubnetUtils(cidr);
return subnetUtils.getInfo().getLowAddress();
}

public static String getLinkLocalAddressFromCIDR(final String cidr) {
return getLinkLocalFirstAddressFromCIDR(cidr) + "/" + cidr2Netmask(cidr);
}

public static String[] getLinkLocalIPRange(final String cidr) {
final SubnetUtils subnetUtils = new SubnetUtils(cidr);
final String[] addresses = subnetUtils.getInfo().getAllAddresses();
final String[] range = new String[2];
range[0] = addresses[1];
range[1] = subnetUtils.getInfo().getHighAddress();

if (range[0].equalsIgnoreCase(getLinkLocalGateway())) {
/* remove the gateway */
long ip = ip2Long(range[0]);
ip += 1;
range[0] = long2Ip(ip);
}
return range;
}

Expand Down
22 changes: 22 additions & 0 deletions utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -709,4 +709,26 @@ public void testIsIPv6EUI64() {
assertFalse(NetUtils.isIPv6EUI64("2001:db8::100:1"));
assertFalse(NetUtils.isIPv6EUI64("2a01:4f9:2a:185f::2"));
}

@Test
public void testLinkLocal() {
final String cidr = NetUtils.getLinkLocalCIDR();
assertEquals("255.255.0.0", NetUtils.getLinkLocalNetMask());
assertEquals("169.254.0.1", NetUtils.getLinkLocalGateway());
assertEquals("169.254.0.0/16", cidr);
assertEquals("169.254.0.1", NetUtils.getLinkLocalFirstAddressFromCIDR(cidr));
assertEquals("169.254.0.1/255.255.0.0", NetUtils.getLinkLocalAddressFromCIDR(cidr));
assertEquals("169.254.240.1/255.255.240.0", NetUtils.getLinkLocalAddressFromCIDR("169.254.240.0/20"));

String[] range = NetUtils.getLinkLocalIPRange("169.254.0.0/16");
assertEquals("169.254.0.2", range[0]);
assertEquals("169.254.255.254", range[1]);
}

@Test
public void testCidrNetmask() {
assertEquals("255.255.255.0", NetUtils.cidr2Netmask("192.168.0.0/24"));
assertEquals("255.255.0.0", NetUtils.cidr2Netmask("169.254.0.0/16"));
assertEquals("255.255.240.0", NetUtils.cidr2Netmask("169.254.240.0/20"));
}
}
Back | FazBrowse Home | New Git URL