Skip to content

Commit 35f55cc

Browse files
Dogface2kDogface2k
committed
OVS: preserve GRE key range and isolate VPC lookups
1 parent 3742216 commit 35f55cc

2 files changed

Lines changed: 166 additions & 14 deletions

File tree

‎plugins/network-elements/ovs/src/main/java/com/cloud/network/ovs/OvsTunnelManagerImpl.java‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.ArrayList;
2020
import java.util.List;
2121
import java.util.Map;
22+
import java.util.Set;
2223
import java.util.concurrent.Executors;
2324
import java.util.concurrent.ScheduledExecutorService;
2425

@@ -93,6 +94,11 @@
9394
@Component
9495
public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManager, StateListener<VirtualMachine.State, VirtualMachine.Event, VirtualMachine> {
9596

97+
private static final long MIN_GRE_KEY = 0L;
98+
private static final long MAX_GRE_KEY = 4294967295L;
99+
private static final Set<Network.State> VPC_TOPOLOGY_NETWORK_STATES = Set.of(
100+
Network.State.Setup, Network.State.Implementing, Network.State.Implemented);
101+
96102
// boolean _isEnabled;
97103
ScheduledExecutorService _executorPool;
98104
ScheduledExecutorService _cleanupExecutor;
@@ -690,11 +696,11 @@ private void handleVmStateChange(VMInstanceVO vm) {
690696
}
691697

692698
for (Long vpcId: vpcIds) {
693-
if (!isOvsDistributedRouterVpc(vpcId)) {
694-
continue;
695-
}
696-
697699
try {
700+
if (!isOvsDistributedRouterVpc(vpcId)) {
701+
continue;
702+
}
703+
698704
// get the list of hosts on which VPC spans (i.e hosts that need to be aware of VPC topology change update)
699705
List<Long> vpcSpannedHostIds = _ovsNetworkToplogyGuru.getVpcSpannedHosts(vpcId);
700706
String bridgeName=generateBridgeNameForVpc(vpcId);
@@ -740,6 +746,12 @@ OvsVpcPhysicalTopologyConfigCommand prepareVpcTopologyUpdate(long vpcId) {
740746
assert (vpc != null): "invalid vpc id";
741747

742748
List<? extends Network> vpcNetworks = _vpcMgr.getVpcNetworks(vpcId);
749+
List<Network> topologyNetworks = new ArrayList<>();
750+
for (Network network : vpcNetworks) {
751+
if (VPC_TOPOLOGY_NETWORK_STATES.contains(network.getState())) {
752+
topologyNetworks.add(network);
753+
}
754+
}
743755
List<Long> hostIds = _ovsNetworkToplogyGuru.getVpcSpannedHosts(vpcId);
744756
List<Long> vmIds = _ovsNetworkToplogyGuru.getAllActiveVmsInVpc(vpcId);
745757

@@ -750,7 +762,7 @@ OvsVpcPhysicalTopologyConfigCommand prepareVpcTopologyUpdate(long vpcId) {
750762
for (Long hostId : hostIds) {
751763
HostVO hostDetails = _hostDao.findById(hostId);
752764
String remoteIp = null;
753-
for (Network network: vpcNetworks) {
765+
for (Network network: topologyNetworks) {
754766
try {
755767
remoteIp = getGreEndpointIP(hostDetails, network);
756768
} catch (Exception e) {
@@ -762,7 +774,7 @@ OvsVpcPhysicalTopologyConfigCommand prepareVpcTopologyUpdate(long vpcId) {
762774
hosts.add(host);
763775
}
764776

765-
for (Network network: vpcNetworks) {
777+
for (Network network: topologyNetworks) {
766778
if (network.getBroadcastDomainType() != BroadcastDomainType.Vswitch || network.getBroadcastUri() == null) {
767779
throw new CloudRuntimeException(String.format(
768780
"OVS distributed-router VPC %s contains network %s without a Vswitch broadcast URI",
@@ -775,13 +787,19 @@ OvsVpcPhysicalTopologyConfigCommand prepareVpcTopologyUpdate(long vpcId) {
775787
"OVS distributed-router network %s has invalid broadcast key %s for VPC %s",
776788
network.getUuid(), key, vpc.getUuid()));
777789
}
778-
int greKey;
790+
String greKeyValue = key.substring(expectedPrefix.length());
791+
long greKey;
779792
try {
780-
greKey = Integer.parseInt(key.substring(expectedPrefix.length()));
793+
greKey = Long.parseLong(greKeyValue);
781794
} catch (NumberFormatException e) {
782795
throw new CloudRuntimeException(String.format(
783796
"OVS distributed-router network %s has non-numeric GRE key %s",
784-
network.getUuid(), key.substring(expectedPrefix.length())), e);
797+
network.getUuid(), greKeyValue), e);
798+
}
799+
if (greKey < MIN_GRE_KEY || greKey > MAX_GRE_KEY) {
800+
throw new CloudRuntimeException(String.format(
801+
"OVS distributed-router network %s has GRE key %s outside the supported range %s-%s",
802+
network.getUuid(), greKeyValue, MIN_GRE_KEY, MAX_GRE_KEY));
785803
}
786804
NicVO nic = _nicDao.findByIp4AddressAndNetworkId(network.getGateway(), network.getId());
787805
if (nic == null) {

‎plugins/network-elements/ovs/src/test/java/com/cloud/network/ovs/OvsTunnelManagerImplTest.java‎

Lines changed: 139 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,14 +159,21 @@ public void testPostStateTransitionEventContinuesAfterNonOvsVpc() {
159159
when(vpcManager.isProviderSupportServiceInVpc(VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
160160
.thenReturn(false);
161161
when(vpcManager.isProviderSupportServiceInVpc(SECOND_VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
162-
.thenReturn(false);
162+
.thenReturn(true);
163+
when(secondVpc.getUuid()).thenReturn("second-vpc-uuid");
164+
when(secondVpc.getCidr()).thenReturn("10.1.0.0/16");
163165
when(transition.getCurrentState()).thenReturn(VirtualMachine.State.Starting);
164166
when(transition.getEvent()).thenReturn(VirtualMachine.Event.OperationSucceeded);
165167
when(transition.getToState()).thenReturn(VirtualMachine.State.Running);
168+
when(topologyGuru.getVpcSpannedHosts(SECOND_VPC_ID)).thenReturn(Collections.emptyList());
169+
when(topologyGuru.getAllActiveVmsInVpc(SECOND_VPC_ID)).thenReturn(Collections.emptyList());
170+
doReturn(Collections.emptyList()).when(vpcManager).getVpcNetworks(SECOND_VPC_ID);
171+
prepareSequenceNumber(SECOND_VPC_ID);
166172

167173
assertTrue(manager.postStateTransitionEvent(transition, vm, true, null));
168174

169-
verify(vpcDao).findById(SECOND_VPC_ID);
175+
verify(vpcManager).getVpcNetworks(SECOND_VPC_ID);
176+
verify(manager._vpcDrSeqNoDao).update(1L, sequenceNumber);
170177
}
171178

172179
@Test
@@ -185,7 +192,9 @@ public void testPostStateTransitionEventContainsMalformedOvsTopologyAndContinues
185192
when(vpcManager.isProviderSupportServiceInVpc(VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
186193
.thenReturn(true);
187194
when(vpcManager.isProviderSupportServiceInVpc(SECOND_VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
188-
.thenReturn(false);
195+
.thenReturn(true);
196+
when(secondVpc.getUuid()).thenReturn("second-vpc-uuid");
197+
when(secondVpc.getCidr()).thenReturn("10.1.0.0/16");
189198
when(transition.getCurrentState()).thenReturn(VirtualMachine.State.Starting);
190199
when(transition.getEvent()).thenReturn(VirtualMachine.Event.OperationSucceeded);
191200
when(transition.getToState()).thenReturn(VirtualMachine.State.Running);
@@ -195,12 +204,51 @@ public void testPostStateTransitionEventContainsMalformedOvsTopologyAndContinues
195204
doReturn(List.of(malformedNetwork)).when(vpcManager).getVpcNetworks(VPC_ID);
196205
when(firstVpc.getUuid()).thenReturn("vpc-uuid");
197206
when(firstVpc.getCidr()).thenReturn("10.0.0.0/16");
207+
when(malformedNetwork.getState()).thenReturn(Network.State.Implemented);
198208
when(malformedNetwork.getUuid()).thenReturn("network-uuid");
199209
when(malformedNetwork.getBroadcastDomainType()).thenReturn(BroadcastDomainType.NSX);
210+
when(topologyGuru.getVpcSpannedHosts(SECOND_VPC_ID)).thenReturn(Collections.emptyList());
211+
when(topologyGuru.getAllActiveVmsInVpc(SECOND_VPC_ID)).thenReturn(Collections.emptyList());
212+
doReturn(Collections.emptyList()).when(vpcManager).getVpcNetworks(SECOND_VPC_ID);
213+
prepareSequenceNumber(SECOND_VPC_ID);
214+
215+
assertTrue(manager.postStateTransitionEvent(transition, vm, true, null));
216+
217+
verify(vpcManager).getVpcNetworks(SECOND_VPC_ID);
218+
verify(manager._vpcDrSeqNoDao).update(1L, sequenceNumber);
219+
}
220+
221+
@Test
222+
public void testPostStateTransitionEventContainsProviderLookupFailureAndProcessesLaterOvsVpc() {
223+
VpcVO firstVpc = mock(VpcVO.class);
224+
VpcVO secondVpc = mock(VpcVO.class);
225+
VMInstanceVO vm = mock(VMInstanceVO.class);
226+
@SuppressWarnings("unchecked")
227+
StateMachine2.Transition<VirtualMachine.State, VirtualMachine.Event> transition = mock(StateMachine2.Transition.class);
228+
when(vm.getId()).thenReturn(11L);
229+
when(topologyGuru.getVpcIdsVmIsPartOf(11L)).thenReturn(List.of(VPC_ID, SECOND_VPC_ID));
230+
when(vpcDao.findById(VPC_ID)).thenReturn(firstVpc);
231+
when(vpcDao.findById(SECOND_VPC_ID)).thenReturn(secondVpc);
232+
when(firstVpc.usesDistributedRouter()).thenReturn(true);
233+
when(secondVpc.usesDistributedRouter()).thenReturn(true);
234+
when(vpcManager.isProviderSupportServiceInVpc(VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
235+
.thenThrow(new CloudRuntimeException("provider lookup failed"));
236+
when(vpcManager.isProviderSupportServiceInVpc(SECOND_VPC_ID, Network.Service.Connectivity, Network.Provider.Ovs))
237+
.thenReturn(true);
238+
when(secondVpc.getUuid()).thenReturn("second-vpc-uuid");
239+
when(secondVpc.getCidr()).thenReturn("10.1.0.0/16");
240+
when(transition.getCurrentState()).thenReturn(VirtualMachine.State.Starting);
241+
when(transition.getEvent()).thenReturn(VirtualMachine.Event.OperationSucceeded);
242+
when(transition.getToState()).thenReturn(VirtualMachine.State.Running);
243+
when(topologyGuru.getVpcSpannedHosts(SECOND_VPC_ID)).thenReturn(Collections.emptyList());
244+
when(topologyGuru.getAllActiveVmsInVpc(SECOND_VPC_ID)).thenReturn(Collections.emptyList());
245+
doReturn(Collections.emptyList()).when(vpcManager).getVpcNetworks(SECOND_VPC_ID);
246+
prepareSequenceNumber(SECOND_VPC_ID);
200247

201248
assertTrue(manager.postStateTransitionEvent(transition, vm, true, null));
202249

203-
verify(vpcDao).findById(SECOND_VPC_ID);
250+
verify(vpcManager).getVpcNetworks(SECOND_VPC_ID);
251+
verify(manager._vpcDrSeqNoDao).update(1L, sequenceNumber);
204252
}
205253

206254
@Test
@@ -278,12 +326,48 @@ public void testPrepareVpcTopologyUpdateRejectsNonVswitchTier() {
278326
doReturn(List.of(network)).when(vpcManager).getVpcNetworks(VPC_ID);
279327
when(topologyGuru.getVpcSpannedHosts(VPC_ID)).thenReturn(Collections.emptyList());
280328
when(topologyGuru.getAllActiveVmsInVpc(VPC_ID)).thenReturn(Collections.emptyList());
329+
when(network.getState()).thenReturn(Network.State.Implemented);
281330
when(network.getUuid()).thenReturn("network-uuid");
282331
when(network.getBroadcastDomainType()).thenReturn(BroadcastDomainType.NSX);
283332

284333
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
285334
}
286335

336+
@Test
337+
public void testPrepareVpcTopologyUpdateSkipsAllocatedTierWithoutBroadcastUri() {
338+
VpcVO vpc = mock(VpcVO.class);
339+
Network network = mock(Network.class);
340+
when(vpcDao.findById(VPC_ID)).thenReturn(vpc);
341+
when(vpc.getCidr()).thenReturn("10.0.0.0/16");
342+
doReturn(List.of(network)).when(vpcManager).getVpcNetworks(VPC_ID);
343+
when(topologyGuru.getVpcSpannedHosts(VPC_ID)).thenReturn(Collections.emptyList());
344+
when(topologyGuru.getAllActiveVmsInVpc(VPC_ID)).thenReturn(Collections.emptyList());
345+
when(network.getState()).thenReturn(Network.State.Allocated);
346+
347+
OvsVpcPhysicalTopologyConfigCommand command = manager.prepareVpcTopologyUpdate(VPC_ID);
348+
349+
assertTrue(command.getVpcConfigInJson().contains("\"tiers\":[]"));
350+
verify(network, never()).getBroadcastDomainType();
351+
verify(nicDao, never()).findByIp4AddressAndNetworkId(
352+
org.mockito.ArgumentMatchers.anyString(), org.mockito.ArgumentMatchers.anyLong());
353+
}
354+
355+
@Test
356+
public void testPrepareVpcTopologyUpdateRejectsImplementedTierWithoutBroadcastUri() {
357+
VpcVO vpc = mock(VpcVO.class);
358+
Network network = mock(Network.class);
359+
when(vpcDao.findById(VPC_ID)).thenReturn(vpc);
360+
when(vpc.getUuid()).thenReturn("vpc-uuid");
361+
doReturn(List.of(network)).when(vpcManager).getVpcNetworks(VPC_ID);
362+
when(topologyGuru.getVpcSpannedHosts(VPC_ID)).thenReturn(Collections.emptyList());
363+
when(topologyGuru.getAllActiveVmsInVpc(VPC_ID)).thenReturn(Collections.emptyList());
364+
when(network.getState()).thenReturn(Network.State.Implemented);
365+
when(network.getUuid()).thenReturn("network-uuid");
366+
when(network.getBroadcastDomainType()).thenReturn(BroadcastDomainType.Vswitch);
367+
368+
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
369+
}
370+
287371
@Test
288372
public void testPrepareVpcTopologyUpdateRejectsBroadcastKeyForAnotherVpc() {
289373
Network network = prepareVswitchNetwork("8.123");
@@ -322,10 +406,52 @@ public void testPrepareVpcTopologyUpdateRejectsTrailingDelimiterInBroadcastKey()
322406
}
323407

324408
@Test
325-
public void testPrepareVpcTopologyUpdateRejectsGreKeyOutsideIntegerRange() {
409+
public void testPrepareVpcTopologyUpdateAcceptsGreKeyAboveSignedIntegerRange() {
326410
prepareVswitchNetwork("7.2147483648");
411+
NicVO gatewayNic = prepareGatewayNic();
412+
413+
OvsVpcPhysicalTopologyConfigCommand command = manager.prepareVpcTopologyUpdate(VPC_ID);
414+
415+
assertTrue(command.getVpcConfigInJson().contains("\"grekey\":2147483648"));
416+
verify(gatewayNic).getMacAddress();
417+
}
418+
419+
@Test
420+
public void testPrepareVpcTopologyUpdateAcceptsMaximumUnsignedGreKey() {
421+
prepareVswitchNetwork("7.4294967295");
422+
NicVO gatewayNic = prepareGatewayNic();
423+
424+
OvsVpcPhysicalTopologyConfigCommand command = manager.prepareVpcTopologyUpdate(VPC_ID);
425+
426+
assertTrue(command.getVpcConfigInJson().contains("\"grekey\":4294967295"));
427+
verify(gatewayNic).getMacAddress();
428+
}
429+
430+
@Test
431+
public void testPrepareVpcTopologyUpdateAcceptsZeroGreKey() {
432+
prepareVswitchNetwork("7.0");
433+
NicVO gatewayNic = prepareGatewayNic();
434+
435+
OvsVpcPhysicalTopologyConfigCommand command = manager.prepareVpcTopologyUpdate(VPC_ID);
436+
437+
assertTrue(command.getVpcConfigInJson().contains("\"grekey\":0"));
438+
verify(gatewayNic).getMacAddress();
439+
}
440+
441+
@Test
442+
public void testPrepareVpcTopologyUpdateRejectsGreKeyAboveUnsignedRange() {
443+
prepareVswitchNetwork("7.4294967296");
327444

328445
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
446+
verify(nicDao, never()).findByIp4AddressAndNetworkId("10.0.1.1", 13L);
447+
}
448+
449+
@Test
450+
public void testPrepareVpcTopologyUpdateRejectsNegativeGreKey() {
451+
prepareVswitchNetwork("7.-1");
452+
453+
assertThrows(CloudRuntimeException.class, () -> manager.prepareVpcTopologyUpdate(VPC_ID));
454+
verify(nicDao, never()).findByIp4AddressAndNetworkId("10.0.1.1", 13L);
329455
}
330456

331457
@Test
@@ -363,11 +489,19 @@ private Network prepareVswitchNetwork(String broadcastKey) {
363489
when(network.getUuid()).thenReturn("network-uuid");
364490
when(network.getGateway()).thenReturn("10.0.1.1");
365491
when(network.getCidr()).thenReturn("10.0.1.0/24");
492+
when(network.getState()).thenReturn(Network.State.Implemented);
366493
when(network.getBroadcastDomainType()).thenReturn(BroadcastDomainType.Vswitch);
367494
when(network.getBroadcastUri()).thenReturn(BroadcastDomainType.Vswitch.toUri(broadcastKey));
368495
return network;
369496
}
370497

498+
private NicVO prepareGatewayNic() {
499+
NicVO gatewayNic = mock(NicVO.class);
500+
when(nicDao.findByIp4AddressAndNetworkId("10.0.1.1", 13L)).thenReturn(gatewayNic);
501+
when(gatewayNic.getMacAddress()).thenReturn("02:00:00:00:00:01");
502+
return gatewayNic;
503+
}
504+
371505
private void prepareSequenceNumber(long vpcId) {
372506
sequenceNumber = mock(VpcDistributedRouterSeqNoVO.class);
373507
when(sequenceNumber.getId()).thenReturn(1L);

0 commit comments

Comments
 (0)