Skip to content

Commit 5bfa524

Browse files
committed
Direct Routed networks: audit fixes after the isolation-model overhaul
Reviewing every path the overhaul touches surfaced four fixes: - SystemVM public IPv6 is now computed directly in PublicNetworkGuru (EUI-64 from the range's ip6_cidr and the NIC MAC, then /128 + fe80::1). The previously planned ipv6Service.updateNicIpv6() path is unusable here: it is gated on the public network offering's internet protocol (never set on the system public offering) and reserves through one placeholder NIC per network -- on the shared Public network, every SystemVM would have received the same address. - Guest networks and public ranges share one routed-id space (a routed id names a bridge on every host), so collisions between them would merge L2 domains. Both directions are now rejected: a guest network cannot take an id a public range carries, and a public range cannot take an id a guest network holds. - Creating an L3 network on a physical network without the ROUTED isolation method now fails early with a clear message, instead of an opaque no-guru error deep in setupNetwork(). - A direct routed guest NIC's isolation URI now mirrors its routed:// broadcast URI instead of the misleading vlan://<tag> inherited from the Shared allocation path. GuestType.L3 itself was re-verified as still required: some thirty management-server branches (offering validation, subnet handling, zone-wide overlap checks, secondary-IP host routes, security groups) key on the guest type in places where no physical network -- and thus no isolation method -- is in scope. The guest type says what a network is; the isolation method says where it may live.
1 parent 526934c commit 5bfa524

6 files changed

Lines changed: 83 additions & 21 deletions

File tree

‎docs/design/direct-routed-networks.md‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -570,8 +570,11 @@ range (`_dcDao.findVnet()`), and must not collide with another network's broadca
570570
**Uniqueness must be zone-wide, not per physical network.** Bridge names are global on a host, so
571571
two networks with routed id 5828 anywhere in the zone would share `brdr-5828` and merge their L2
572572
domains. The existing zone-wide URI overlap check
573-
(`_networksDao.listByZoneAndUriAndGuestType()`) covers this; with a single `ROUTED` physical
574-
network per zone — the expected deployment — it is equivalent to the per-physnet check anyway.
573+
(`_networksDao.listByZoneAndUriAndGuestType()`) covers guest-vs-guest; with a single `ROUTED`
574+
physical network per zone — the expected deployment — it is equivalent to the per-physnet check
575+
anyway. **Guest networks and public ranges (§8.5) share the same id space** and are guarded in
576+
both directions: creating a guest network whose routed id a public range already carries is
577+
rejected, and so is creating a public range whose id a guest network holds.
575578

576579
Either way the URI is set at creation and **stable for the network's life**: the bridge name never
577580
changes, which is what makes it something host policy can reference.
@@ -816,15 +819,17 @@ systemvm:
816819
must instead receive the host-route form (as `DirectRoutedNetworkGuru.applyDirectRoutedAddressing()`
817820
does for guests) plus `BroadcastDomainType.Routed` and a `routed://<id>` URI — which is also
818821
exactly what steers `BridgeVifDriver`'s Public branch into the brdr-bridge + `modifymacip.sh`
819-
path instead of the public bridge (today that handling sits only in the Guest branch). The IPv6
820-
side needs no new allocation logic: `ipv6Service.updateNicIpv6()` (`PublicNetworkGuru.java:166`)
821-
already computes the address with EUI-64 from the range's subnet and the NIC's MAC (§6.3.4);
822-
only the /128 + `fe80::1` reshaping applies on top, exactly as for guest NICs. Mechanism sketch:
823-
the public IP range's vlan row already carries a tag that `getIp()` copies into the broadcast
824-
URI, so a range created with `routed://<id>` as its "vlan" flows through with almost no new
825-
plumbing (verify `updateNicIpv6()`'s range lookup by broadcast URI matches such a row); the
826-
zone's public network carries one routed id (and thus one `brdr-<id>`) of its own, and systemvm
827-
/32s and /128s are advertised by the host's routing daemon exactly like guest addresses.
822+
path instead of the public bridge (today that handling sits only in the Guest branch). **IPv6
823+
is computed directly in the guru** — `NetUtils.EUI64Address(range ip6_cidr, NIC MAC)` per
824+
§6.3.4, then the /128 + `fe80::1` form. Deliberately *not* via `ipv6Service.updateNicIpv6()`:
825+
auditing that path showed it is gated on the public network offering's internet protocol
826+
(never set on the system public offering, so a no-op in practice) and reserves through **one
827+
placeholder NIC per network** — on the shared Public network every SystemVM would receive the
828+
*same* address. Neither the gate nor the reservation decides anything here: the MAC already
829+
makes the address unique. Mechanism: the public IP range's vlan row carries `routed://<id>` as
830+
its tag, which `getIp()` copies into the broadcast URI; the zone's public network carries one
831+
routed id (and thus one `brdr-<id>`) of its own, and systemvm /32s and /128s are advertised by
832+
the host's routing daemon exactly like guest addresses.
828833

829834
Minor, noted for completeness: `setup_interface_ipv6()` writes `accept_ra 1` — harmless on an
830835
RA-less bridge, but the static default must not depend on it (and may be set to 0 for direct
@@ -1347,8 +1352,9 @@ Checklist to work through:
13471352
- [ ] `systemvm/.../setup/common.sh` — `onlink` on the v4 default route for link-local gateways;
13481353
install the v6 default route from `IP6GW` instead of relying on RA (§8.5)
13491354
- [ ] `PublicNetworkGuru` / `createVlanIpRange` — direct routed public range for systemvm IPs,
1350-
dual-stack: host-route NIC form (v4 and v6), `Routed` broadcast URI; `BridgeVifDriver` Public
1351-
branch handles it; verify `updateNicIpv6()`'s range lookup accepts a `routed://` tag (§8.5)
1355+
dual-stack: host-route NIC form (v4 and v6, EUI-64 computed in the guru), `Routed` broadcast
1356+
URI; `BridgeVifDriver` Public branch handles it; routed ids guarded against guest/public
1357+
collisions in both directions (§8.5, §6.7.2)
13521358
- [ ] `createNetwork`/`createVlanIpRange` — reject an IPv6 CIDR with a prefix longer than /64 for
13531359
L3 networks; EUI-64 needs 64 interface-identifier bits (§6.3.4)
13541360
- [ ] `NetworkServiceImpl.java:657` — stop rejecting DNS for this type as it does for L2 (§6.4)

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2950,6 +2950,15 @@ private Network createGuestNetwork(final long networkOfferingId, final String na
29502950

29512951
if (vlanSpecified) {
29522952
URI uri = encodeVlanIdIntoBroadcastUri(vlanId, pNtwk);
2953+
// A routed id names a bridge on every host (brdr-<id>): a guest network and a
2954+
// public range sharing one id would merge their L2 domains. Public ranges store
2955+
// the routed://<id> URI as their vlan tag; reject any id one already carries.
2956+
// (Guest-vs-guest collisions are caught by the zone-wide URI check below.)
2957+
if (BroadcastDomainType.getSchemeValue(uri) == BroadcastDomainType.Routed
2958+
&& _vlanDao.findByZoneAndVlanId(zoneId, uri.toString()) != null) {
2959+
throw new InvalidParameterValueException(String.format(
2960+
"The routed id %s is already used by a public IP range in zone %s", vlanId, zone.getName()));
2961+
}
29532962
// Aux: generate secondary URI for secondary VLAN ID (if provided) for performing checks
29542963
URI secondaryUri = StringUtils.isNotBlank(isolatedPvlan) ? BroadcastDomainType.fromString(isolatedPvlan) : null;
29552964
if (isSharedNetworkWithoutSpecifyVlan(ntwkOff) || isL3NetworkWithoutSpecifyVlan(ntwkOff) || isPrivateGatewayWithoutSpecifyVlan(ntwkOff)) {

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5935,6 +5935,15 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
59355935
+ zone.getName());
59365936
}
59375937

5938+
// A routed id names a bridge on every host (brdr-<id>): a public range and a guest
5939+
// network sharing one id would merge their L2 domains. The guest-side creation rejects
5940+
// ids held by public ranges; this is the same guard in the other direction.
5941+
if (vlanId != null && vlanId.startsWith(BroadcastDomainType.Routed.scheme() + "://")
5942+
&& !_networkDao.listByZoneAndUriAndGuestType(zoneId, vlanId, null).isEmpty()) {
5943+
throw new InvalidParameterValueException(String.format(
5944+
"The routed id %s is already used by a guest network in zone %s", vlanId, zone.getName()));
5945+
}
5946+
59385947
String ipRange = null;
59395948

59405949
if (ipv4) {

‎server/src/main/java/com/cloud/network/NetworkServiceImpl.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,6 +1573,16 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac
15731573

15741574
ACLType aclType = getAclType(caller, cmd.getAclType(), ntwkOff);
15751575

1576+
// Fail here with a clear message rather than deep in setupNetwork(), where no guru
1577+
// claiming the network produces an opaque error. The physical network is the network
1578+
// operator's opt-in: without ROUTED it carries no direct routed networks.
1579+
if (ntwkOff.getGuestType() == GuestType.L3
1580+
&& (pNtwk.getIsolationMethods() == null || !pNtwk.getIsolationMethods().contains("ROUTED"))) {
1581+
throw new InvalidParameterValueException(String.format(
1582+
"Networks of guest type %s can only be created on a physical network with isolation method ROUTED; physical network %s carries %s",
1583+
GuestType.L3, pNtwk.getName(), pNtwk.getIsolationMethods()));
1584+
}
1585+
15761586
if (ntwkOff.getGuestType() != GuestType.Shared && (!StringUtils.isAllBlank(routerIPv4, routerIPv6))) {
15771587
throw new InvalidParameterValueException("Router IP can be specified only for Shared networks");
15781588
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,12 @@ protected void applyDirectRoutedAddressing(NicProfile nic) {
184184
if (nic == null) {
185185
return;
186186
}
187+
// The inherited allocation labels the NIC's isolation URI vlan://<tag> from the vlan
188+
// row, as a Shared network needs; here there is no VLAN — the only isolation-shaped
189+
// fact about this NIC is its routed://<id> broadcast domain.
190+
if (nic.getBroadCastUri() != null && BroadcastDomainType.getSchemeValue(nic.getBroadCastUri()) == BroadcastDomainType.Routed) {
191+
nic.setIsolationUri(nic.getBroadCastUri());
192+
}
187193
if (nic.getIPv4Address() != null) {
188194
nic.setIPv4Netmask(NetUtils.IPV4_HOST_NETMASK);
189195
nic.setIPv4Gateway(NetUtils.getLinkLocalGateway());

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
import javax.inject.Inject;
2020

21+
import org.apache.commons.lang3.StringUtils;
22+
2123
import com.cloud.dc.dao.VlanDetailsDao;
2224
import com.cloud.network.vpc.dao.VpcDao;
2325
import com.cloud.network.vpc.dao.VpcOfferingDao;
@@ -63,6 +65,7 @@
6365
import com.cloud.vm.ReservationContext;
6466
import com.cloud.vm.VirtualMachine;
6567
import com.cloud.vm.VirtualMachineProfile;
68+
import com.googlecode.ipv6.IPv6Address;
6669

6770
public class PublicNetworkGuru extends AdapterBase implements NetworkGuru {
6871

@@ -156,6 +159,7 @@ protected void getIp(NicProfile nic, DataCenter dc, VirtualMachineProfile vm, Ne
156159
nic.setIsolationUri(BroadcastDomainType.Routed.toUri(ip.getVlanTag()));
157160
nic.setBroadcastUri(BroadcastDomainType.Routed.toUri(ip.getVlanTag()));
158161
nic.setBroadcastType(BroadcastDomainType.Routed);
162+
setRoutedRangeIpv6(nic, dc, ip, network);
159163
} else if (network.getBroadcastDomainType() == BroadcastDomainType.Vxlan) {
160164
nic.setIsolationUri(BroadcastDomainType.Vxlan.toUri(ip.getVlanTag()));
161165
nic.setBroadcastUri(BroadcastDomainType.Vxlan.toUri(ip.getVlanTag()));
@@ -175,14 +179,6 @@ protected void getIp(NicProfile nic, DataCenter dc, VirtualMachineProfile vm, Ne
175179
nic.setIPv4Dns2(dns.second());
176180

177181
ipv6Service.updateNicIpv6(nic, dc, network);
178-
179-
// The IPv6 address itself is computed with EUI-64 from the range's subnet and the NIC's
180-
// MAC (Ipv6ServiceImpl.updateNicIpv6); on a direct routed range only its form changes:
181-
// a /128 with the shared link-local gateway, delivered to the systemvm via boot args.
182-
if (nic.getBroadcastType() == BroadcastDomainType.Routed && nic.getIPv6Address() != null) {
183-
nic.setIPv6Cidr(nic.getIPv6Address() + "/" + NetUtils.IPV6_HOST_PREFIX_LENGTH);
184-
nic.setIPv6Gateway(NetUtils.getIpv6LinkLocalGateway());
185-
}
186182
}
187183

188184
/**
@@ -194,6 +190,32 @@ private boolean isRoutedRange(PublicIp ip) {
194190
return vlanTag != null && vlanTag.startsWith(BroadcastDomainType.Routed.scheme() + "://");
195191
}
196192

193+
/**
194+
* IPv6 on a direct routed public range is computed with EUI-64 from the range's subnet and
195+
* the NIC's MAC — the same stateless model guest NICs use (no pool, no reservation, only the
196+
* result on the NIC) — and takes host-route form: a /128 with the shared link-local gateway.
197+
*
198+
* Deliberately not {@code ipv6Service.updateNicIpv6()}: that path is gated on the public
199+
* network offering's internet protocol, and its per-network placeholder reservation would
200+
* hand every SystemVM on the shared Public network the same address. Neither the gate nor
201+
* the reservation has anything to decide here — the MAC makes the address unique.
202+
*/
203+
private void setRoutedRangeIpv6(NicProfile nic, DataCenter dc, PublicIp ip, Network network) {
204+
String ip6Cidr = ip.vlan() != null ? ip.vlan().getIp6Cidr() : null;
205+
if (StringUtils.isBlank(ip6Cidr) || nic.getIPv6Address() != null) {
206+
return;
207+
}
208+
IPv6Address ipv6Address = NetUtils.EUI64Address(ip6Cidr, nic.getMacAddress());
209+
logger.info("Calculated IPv6 address {} using EUI-64 for direct routed public NIC {}", ipv6Address, nic);
210+
nic.setIPv6Address(ipv6Address.toString());
211+
nic.setIPv6Cidr(ipv6Address + "/" + NetUtils.IPV6_HOST_PREFIX_LENGTH);
212+
nic.setIPv6Gateway(NetUtils.getIpv6LinkLocalGateway());
213+
nic.setFormat(AddressFormat.DualStack);
214+
Pair<String, String> ip6Dns = networkModel.getNetworkIp6Dns(network, dc);
215+
nic.setIPv6Dns1(ip6Dns.first());
216+
nic.setIPv6Dns2(ip6Dns.second());
217+
}
218+
197219
@Override
198220
public void updateNicProfile(NicProfile profile, Network network) {
199221
DataCenter dc = _dcDao.findById(network.getDataCenterId());

0 commit comments

Comments
 (0)