Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,7 @@ public class ApiConstants {
public static final String LINMIN_APID = "linminapid";
public static final String DHCP_SERVER_TYPE = "dhcpservertype";
public static final String LINK_LOCAL_IP = "linklocalip";
public static final String LINK_LOCAL_IP6 = "linklocalip6";
public static final String LINK_LOCAL_MAC_ADDRESS = "linklocalmacaddress";
public static final String LINK_LOCAL_MAC_NETMASK = "linklocalnetmask";
public static final String LINK_LOCAL_NETWORK_ID = "linklocalnetworkid";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ public class SystemVmResponse extends BaseResponseWithAnnotations {
@Param(description = "The Control IP address for the System VM")
private String linkLocalIp;

@SerializedName(ApiConstants.LINK_LOCAL_IP6)
@Param(description = "The Control IPv6 link-local address for the System VM, calculated from the link local MAC address", since = "4.23.0")
private String linkLocalIp6;

@SerializedName(ApiConstants.LINK_LOCAL_MAC_ADDRESS)
@Param(description = "The link local MAC address for the System VM")
private String linkLocalMacAddress;
Expand Down Expand Up @@ -427,6 +431,14 @@ public void setLinkLocalIp(String linkLocalIp) {
this.linkLocalIp = linkLocalIp;
}

public String getLinkLocalIp6() {
return linkLocalIp6;
}

public void setLinkLocalIp6(String linkLocalIp6) {
this.linkLocalIp6 = linkLocalIp6;
}

public String getLinkLocalMacAddress() {
return linkLocalMacAddress;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ public void createControlNetwork(String privBrName) {
Script.runSimpleBashScript("ip link set " + privBrName + " up");
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + privBrName);
}
enableBridgeIpv6LinkLocal(privBrName);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ public void createControlNetwork(String privBrName) {
Script.runSimpleBashScript("ip link add " + privBrName + " type bridge; ip link set " + privBrName + " up");
Script.runSimpleBashScript("ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + privBrName, _timeout);
}
enableBridgeIpv6LinkLocal(privBrName);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ public void createControlNetwork(String privBrName) {
if (!isExistingBridge(privBrName)) {
Script.runSimpleBashScript("ovs-vsctl add-br " + privBrName + "; ip link set " + privBrName + " up; ip address add " + NetUtils.getLinkLocalAddressFromCIDR(_controlCidr) + " dev " + privBrName, _timeout);
}
enableBridgeIpv6LinkLocal(privBrName);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import com.cloud.agent.api.to.NicTO;
import com.cloud.exception.InternalErrorException;
import com.cloud.utils.script.Script;

public abstract class VifDriverBase implements VifDriver {

Expand Down Expand Up @@ -81,6 +82,18 @@ public boolean isExistingBridge(String bridgeName) {
return false;
}

/**
* Enable IPv6 on the control network bridge so the host can reach the
* IPv6 link-local address system VMs listen on. Only link-local is wanted,
* so Router Advertisements and SLAAC are disabled on the bridge.
*/
protected void enableBridgeIpv6LinkLocal(String bridgeName) {
logger.info("Enabling IPv6 link-local on bridge {}", bridgeName);
Script.runSimpleBashScript("sysctl -qw net.ipv6.conf." + bridgeName + ".accept_ra=0" +
" net.ipv6.conf." + bridgeName + ".autoconf=0" +
" net.ipv6.conf." + bridgeName + ".disable_ipv6=0");
}

protected static int getNetworkRateKbps(NicTO nic) {
if (nic.getNetworkRateMbps() != null && nic.getNetworkRateMbps().intValue() != -1) {
return nic.getNetworkRateMbps().intValue() * bitsPerMbpsToKbps;
Expand Down
3 changes: 3 additions & 0 deletions server/src/main/java/com/cloud/api/ApiResponseHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -1886,6 +1886,9 @@ public SystemVmResponse createSystemVmResponse(VirtualMachine vm) {
vmResponse.setLinkLocalIp(singleNicProfile.getIPv4Address());
vmResponse.setLinkLocalMacAddress(singleNicProfile.getMacAddress());
vmResponse.setLinkLocalNetmask(singleNicProfile.getIPv4Netmask());
if (singleNicProfile.getMacAddress() != null) {
vmResponse.setLinkLocalIp6(NetUtils.ipv6LinkLocal(singleNicProfile.getMacAddress()).toString());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is calculated from the mac, it never checks the vm actually has the address. so the field always shows something even when ipv6 is off on the systemvm.

on my lab both systemvms reported a linklocalip6 while nothing was listening on it.

is that the intent, a value you can always compute? if so maybe say so in the field description, otherwise someone will read it as "this address works".

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.

The calculated address is always the same. It never changes or will be different. This is an RFC for IPv6 where the link-local is persistent. That's the great thing about it.

Calculate instead of store. Saves a lot of code.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

makes sense that it never changes. can we say in the field description that its calculated, so nobody reads it as the address being up?

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.

Well, isn't this a basic IPv6 knowledge? Link Local IPv6 always exists and you don't need to do anything about it.

}
} else if (network.getTrafficType() == TrafficType.Public) {
vmResponse.setPublicIp(singleNicProfile.getIPv4Address());
vmResponse.setPublicMacAddress(singleNicProfile.getMacAddress());
Expand Down
4 changes: 4 additions & 0 deletions systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,10 @@ def add_ip6_chain(self, address_family, table, chain, hook, action):
if hook == "input" or hook == "output":
CsHelper.execute("nft add rule %s %s %s icmpv6 type { echo-request, echo-reply, \
nd-neighbor-solicit, nd-router-advert, nd-neighbor-advert } accept" % (address_family, table, chain))
if hook == "input":
# sshd listens on the IPv6 link-local address of the control interface,
# only allow this over link-local so no global address can reach it
CsHelper.execute("nft add rule %s %s %s ip6 saddr fe80::/10 ip6 daddr fe80::/10 tcp dport 3922 accept" % (address_family, table, chain))
if hook == "input" or hook == "forward":
CsHelper.execute("nft add rule %s %s %s ct state established,related accept" % (address_family, table, chain))

Expand Down
34 changes: 33 additions & 1 deletion systemvm/debian/opt/cloud/bin/setup/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -573,10 +573,42 @@ setup_dnsmasq() {
fi
}

enable_ipv6_link_local() {
local eth=$1
log_it "Enabling IPv6 link-local on interface $eth"
# Generate the link-local address with EUI-64 based on the MAC address so
# the address can be calculated by the Management Server
sysctl -w net.ipv6.conf.${eth}.addr_gen_mode=0
# Only a link-local address is wanted, no SLAAC/RA configuration
sysctl -w net.ipv6.conf.${eth}.accept_ra=0
sysctl -w net.ipv6.conf.${eth}.autoconf=0
sysctl -w net.ipv6.conf.${eth}.disable_ipv6=0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

tested this on a lab and it gets undone a few seconds later, so the feature doesnt survive a boot.

this sets the runtime value only. the template ships net.ipv6.conf.all.disable_ipv6 = 1 in /etc/sysctl.conf, and bootstrap.sh:104 runs sysctl -p after setup_sshd. boot order on the ssvm:

00:43:15  Configuring sshd
00:43:15  Enabling IPv6 link-local on interface eth0
00:43:17  Interface eth0 has IPv6 link-local address fe80::c00:a9ff:fefe:cadc
00:43:17  Configuring sshd to also listen on fe80::c00:a9ff:fefe:cadc%eth0
00:43:19  Executing cloud-early-config
00:43:20  Bootstrapping systemvm appliance      <- sysctl -p in here

end state, address gone and sshd only on v4:

$ ip -6 addr show dev eth0
(nothing)
$ sysctl -n net.ipv6.conf.eth0.disable_ipv6
1
$ ss -tln | grep 3922
LISTEN 0  128  169.254.202.220:3922  0.0.0.0:*

showed it directly on the vm:

$ sysctl -qw net.ipv6.conf.eth0.disable_ipv6=0
$ ip -6 addr show dev eth0 scope link
    inet6 fe80::c00:a9ff:fefe:cadc/64 scope link
$ sysctl -p
$ ip -6 addr show dev eth0 scope link
(gone)

so listSystemVms shows a linklocalip6 that nothing answers on. ping6 and ssh to it both time out from the host.

the design is fine by the way, once i left ipv6 on and restarted sshd it worked end to end over fe80::...%cloud0 on 3922.

can we clear the persistent one too? common.sh:126-129 already does that for net.ipv6.conf.all.disable_ipv6 including rewriting sysctl.conf.

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.

Thanks! I had a manual script checking a few things, didn't notice it was gone right after. Let me look into that, I now have a better lab to test this again.

My goal is to remove IPv4 entirely. We now have a complete allocation mechanism for IPv4 for this control cidr which is a lot of code and database entries which shouldn't be needed.

First step is adding IPv6, making sure its stable and then remove IPv4 in a future PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sounds good, happy to test again once its updated


# Wait for Duplicate Address Detection to complete so the address can be bound
LINK_LOCAL_IP6=""
local i
for i in $(seq 1 10); do
LINK_LOCAL_IP6=$(ip -6 addr show dev ${eth} scope link -tentative | grep -Po '(?<=inet6 )fe80:[0-9a-f:]+' | head -1)
[ -n "$LINK_LOCAL_IP6" ] && break
sleep 1
done

if [ -n "$LINK_LOCAL_IP6" ]; then
log_it "Interface $eth has IPv6 link-local address $LINK_LOCAL_IP6"
else
log_it "No IPv6 link-local address appeared on interface $eth"
fi
}

setup_sshd(){
local ip=$1
local eth=$2
[ -f /etc/ssh/sshd_config ] && sed -i -e "s/^[#]*ListenAddress.*$/ListenAddress $ip/" /etc/ssh/sshd_config
[ -f /etc/ssh/sshd_config ] && sed -i -e "/^ListenAddress fe80/d" -e "s/^[#]*ListenAddress.*$/ListenAddress $ip/" /etc/ssh/sshd_config
enable_ipv6_link_local $eth
if [ -n "$LINK_LOCAL_IP6" ]; then
log_it "Configuring sshd to also listen on ${LINK_LOCAL_IP6}%${eth}"
sed -i -e "/^ListenAddress $ip$/a ListenAddress ${LINK_LOCAL_IP6}%${eth}" /etc/ssh/sshd_config

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this writes the v6 ListenAddress but nothing rechecks the address is still there when sshd actually starts. on my lab it ended up pointing at an address that no longer existed:

$ grep -i ListenAddress /etc/ssh/sshd_config
ListenAddress 169.254.202.220
ListenAddress fe80::c00:a9ff:fefe:cadc%eth0

$ ss -tln | grep 3922
LISTEN 0  128  169.254.202.220:3922  0.0.0.0:*

sshd was fine with it and just bound v4, so no harm this time. but if it ever refused to start we'd have a systemvm with no way in.

worth only adding the line when the address is actually up at sshd start?

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.

Same goes for IPv4 ofcourse, we never check if the address exists, we assume it does. This needs fixing somewhere else.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fair enough, ok to fix that separately

fi
sed -i "/3922/s/eth./$eth/" /etc/iptables/rules.v4
}

Expand Down
1 change: 1 addition & 0 deletions ui/public/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,7 @@
"label.link": "Link",
"label.link.domain.to.ldap": "Link domain to LDAP",
"label.linklocalip": "Link-local/Control IP address",
"label.linklocalip6": "Link-local/Control IPv6 address",
"label.linux": "Linux",
"label.list.ciscoasa1000v": "ASA 1000v",
"label.list.ciscovnmc": "Cisco VNMC",
Expand Down
4 changes: 2 additions & 2 deletions ui/src/config/section/infra/systemVms.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ export default {
docHelp: 'adminguide/systemvm.html',
permission: ['listSystemVms'],
searchFilters: ['name', 'zoneid', 'podid', 'hostid', 'systemvmtype', 'storageid', 'arch'],
columns: ['name', 'state', 'agentstate', 'systemvmtype', 'publicip', 'privateip', 'linklocalip', 'version', 'hostname', 'arch', 'zonename'],
details: ['name', 'id', 'agentstate', 'systemvmtype', 'publicip', 'privateip', 'linklocalip', 'gateway', 'hostname', 'arch', 'version', 'zonename', 'created', 'activeviewersessions', 'isdynamicallyscalable', 'hostcontrolstate', 'storageip'],
columns: ['name', 'state', 'agentstate', 'systemvmtype', 'publicip', 'privateip', 'linklocalip', 'linklocalip6', 'version', 'hostname', 'arch', 'zonename'],
details: ['name', 'id', 'agentstate', 'systemvmtype', 'publicip', 'privateip', 'linklocalip', 'linklocalip6', 'gateway', 'hostname', 'arch', 'version', 'zonename', 'created', 'activeviewersessions', 'isdynamicallyscalable', 'hostcontrolstate', 'storageip'],
resourceType: 'SystemVm',
filters: () => {
const filters = ['starting', 'running', 'stopping', 'stopped', 'destroyed', 'expunging', 'migrating', 'error', 'unknown', 'shutdown']
Expand Down
Loading