diff options
| -rw-r--r-- | services/core/java/com/android/server/vcn/Vcn.java | 76 | ||||
| -rw-r--r-- | tests/vcn/java/com/android/server/vcn/VcnTest.java | 79 |
2 files changed, 138 insertions, 17 deletions
diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java index 3f74938005a7..89ed956b3aef 100644 --- a/services/core/java/com/android/server/vcn/Vcn.java +++ b/services/core/java/com/android/server/vcn/Vcn.java @@ -41,6 +41,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -110,6 +111,24 @@ public class Vcn extends Handler { @NonNull private final VcnNetworkRequestListener mRequestListener; @NonNull private final VcnCallback mVcnCallback; + /** + * Map containing all VcnGatewayConnections and their VcnGatewayConnectionConfigs. + * + * <p>Due to potential for race conditions, VcnGatewayConnections MUST only be created and added + * to this map in {@link #handleNetworkRequested(NetworkRequest, int, int)}, when a VCN receives + * a NetworkRequest that matches a VcnGatewayConnectionConfig for this VCN's VcnConfig. + * + * <p>A VcnGatewayConnection instance MUST NEVER overwrite an existing instance - otherwise + * there is potential for a orphaned VcnGatewayConnection instance that does not get properly + * shut down. + * + * <p>Due to potential for race conditions, VcnGatewayConnections MUST only be removed from this + * map once they have finished tearing down, which is reported to this VCN via {@link + * VcnGatewayStatusCallback#onQuit()}. Once this is done, all NetworkRequests are retrieved from + * the NetworkProvider so that another VcnGatewayConnectionConfig can match the + * previously-matched request. + */ + // TODO(b/182533200): remove the invariant on VcnGatewayConnection lifecycles @NonNull private final Map<VcnGatewayConnectionConfig, VcnGatewayConnection> mVcnGatewayConnections = new HashMap<>(); @@ -191,6 +210,19 @@ public class Vcn extends Handler { return Collections.unmodifiableSet(new HashSet<>(mVcnGatewayConnections.values())); } + /** Get current Configs and Gateways for testing purposes */ + @VisibleForTesting(visibility = Visibility.PRIVATE) + public Map<VcnGatewayConnectionConfig, VcnGatewayConnection> + getVcnGatewayConnectionConfigMap() { + return Collections.unmodifiableMap(new HashMap<>(mVcnGatewayConnections)); + } + + /** Set whether this Vcn is active for testing purposes */ + @VisibleForTesting(visibility = Visibility.PRIVATE) + public void setIsActive(boolean isActive) { + mIsActive.set(isActive); + } + private class VcnNetworkRequestListener implements VcnNetworkProvider.NetworkRequestListener { @Override public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) { @@ -202,11 +234,6 @@ public class Vcn extends Handler { @Override public void handleMessage(@NonNull Message msg) { - // Ignore if this Vcn is not active and we're not receiving new configs - if (!isActive() && msg.what != MSG_EVENT_CONFIG_UPDATED) { - return; - } - switch (msg.what) { case MSG_EVENT_CONFIG_UPDATED: handleConfigUpdated((VcnConfig) msg.obj); @@ -237,9 +264,31 @@ public class Vcn extends Handler { mConfig = config; - // TODO(b/181815405): Reevaluate active VcnGatewayConnection(s) + if (mIsActive.getAndSet(true)) { + // VCN is already active - teardown any GatewayConnections whose configs have been + // removed and get all current requests + for (final Entry<VcnGatewayConnectionConfig, VcnGatewayConnection> entry : + mVcnGatewayConnections.entrySet()) { + final VcnGatewayConnectionConfig gatewayConnectionConfig = entry.getKey(); + final VcnGatewayConnection gatewayConnection = entry.getValue(); + + // GatewayConnectionConfigs must match exactly (otherwise authentication or + // connection details may have changed). + if (!mConfig.getGatewayConnectionConfigs().contains(gatewayConnectionConfig)) { + if (gatewayConnection == null) { + Slog.wtf( + getLogTag(), + "Found gatewayConnectionConfig without GatewayConnection"); + } else { + gatewayConnection.teardownAsynchronously(); + } + } + } - if (!mIsActive.getAndSet(true)) { + // Trigger a re-evaluation of all NetworkRequests (to make sure any that can be + // satisfied start a new GatewayConnection) + mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener); + } else { // If this VCN was not previously active, it is exiting Safe Mode. Re-register the // request listener to get NetworkRequests again (and all cached requests). mVcnContext.getVcnNetworkProvider().registerListener(mRequestListener); @@ -259,13 +308,16 @@ public class Vcn extends Handler { private void handleEnterSafeMode() { handleTeardown(); - mVcnGatewayConnections.clear(); - mVcnCallback.onEnteredSafeMode(); } private void handleNetworkRequested( @NonNull NetworkRequest request, int score, int providerId) { + if (!isActive()) { + Slog.v(getLogTag(), "Received NetworkRequest while inactive. Ignore for now"); + return; + } + if (score > getNetworkScore()) { if (VDBG) { Slog.v( @@ -318,8 +370,10 @@ public class Vcn extends Handler { mVcnGatewayConnections.remove(config); // Trigger a re-evaluation of all NetworkRequests (to make sure any that can be satisfied - // start a new GatewayConnection) - mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener); + // start a new GatewayConnection), but only if the Vcn is still active + if (isActive()) { + mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener); + } } private void handleSubscriptionsChanged(@NonNull TelephonySubscriptionSnapshot snapshot) { diff --git a/tests/vcn/java/com/android/server/vcn/VcnTest.java b/tests/vcn/java/com/android/server/vcn/VcnTest.java index 4fa63d4ff640..c853fc50fdf7 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnTest.java @@ -29,6 +29,7 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -139,8 +140,7 @@ public class VcnTest { mTestLooper.dispatchAll(); } - @Test - public void testSubscriptionSnapshotUpdatesVcnGatewayConnections() { + private void verifyUpdateSubscriptionSnapshotNotifiesConnectionGateways(boolean isActive) { final NetworkRequestListener requestListener = verifyAndGetRequestListener(); startVcnGatewayWithCapabilities(requestListener, TEST_CAPS[0]); @@ -150,14 +150,27 @@ public class VcnTest { final TelephonySubscriptionSnapshot updatedSnapshot = mock(TelephonySubscriptionSnapshot.class); + mVcn.setIsActive(isActive); + mVcn.updateSubscriptionSnapshot(updatedSnapshot); mTestLooper.dispatchAll(); for (final VcnGatewayConnection gateway : gatewayConnections) { - verify(gateway).updateSubscriptionSnapshot(eq(updatedSnapshot)); + verify(gateway, isActive ? times(1) : never()) + .updateSubscriptionSnapshot(eq(updatedSnapshot)); } } + @Test + public void testSubscriptionSnapshotUpdatesVcnGatewayConnections() { + verifyUpdateSubscriptionSnapshotNotifiesConnectionGateways(true /* isActive */); + } + + @Test + public void testSubscriptionSnapshotUpdatesVcnGatewayConnectionsWhileInactive() { + verifyUpdateSubscriptionSnapshotNotifiesConnectionGateways(false /* isActive */); + } + private void triggerVcnRequestListeners(NetworkRequestListener requestListener) { for (final int[] caps : TEST_CAPS) { startVcnGatewayWithCapabilities(requestListener, caps); @@ -187,7 +200,6 @@ public class VcnTest { NetworkRequestListener requestListener, Set<VcnGatewayConnection> expectedGatewaysTornDown) { assertFalse(mVcn.isActive()); - assertTrue(mVcn.getVcnGatewayConnections().isEmpty()); for (final VcnGatewayConnection gatewayConnection : expectedGatewaysTornDown) { verify(gatewayConnection).teardownAsynchronously(); } @@ -238,6 +250,51 @@ public class VcnTest { } @Test + public void testGatewayQuitWhileInactive() { + final NetworkRequestListener requestListener = verifyAndGetRequestListener(); + final Set<VcnGatewayConnection> gatewayConnections = + new ArraySet<>(startGatewaysAndGetGatewayConnections(requestListener)); + + mVcn.teardownAsynchronously(); + mTestLooper.dispatchAll(); + + final VcnGatewayStatusCallback statusCallback = mGatewayStatusCallbackCaptor.getValue(); + statusCallback.onQuit(); + mTestLooper.dispatchAll(); + + // Verify that the VCN requests the networkRequests be resent + assertEquals(1, mVcn.getVcnGatewayConnections().size()); + verify(mVcnNetworkProvider, never()).resendAllRequests(requestListener); + } + + @Test + public void testUpdateConfigReevaluatesGatewayConnections() { + final NetworkRequestListener requestListener = verifyAndGetRequestListener(); + startGatewaysAndGetGatewayConnections(requestListener); + assertEquals(2, mVcn.getVcnGatewayConnectionConfigMap().size()); + + // Create VcnConfig with only one VcnGatewayConnectionConfig so a gateway connection is torn + // down + final VcnGatewayConnectionConfig activeConfig = + VcnGatewayConnectionConfigTest.buildTestConfigWithExposedCaps(TEST_CAPS[0]); + final VcnGatewayConnectionConfig removedConfig = + VcnGatewayConnectionConfigTest.buildTestConfigWithExposedCaps(TEST_CAPS[1]); + final VcnConfig updatedConfig = + new VcnConfig.Builder(mContext).addGatewayConnectionConfig(activeConfig).build(); + + mVcn.updateConfig(updatedConfig); + mTestLooper.dispatchAll(); + + final VcnGatewayConnection activeGatewayConnection = + mVcn.getVcnGatewayConnectionConfigMap().get(activeConfig); + final VcnGatewayConnection removedGatewayConnection = + mVcn.getVcnGatewayConnectionConfigMap().get(removedConfig); + verify(activeGatewayConnection, never()).teardownAsynchronously(); + verify(removedGatewayConnection).teardownAsynchronously(); + verify(mVcnNetworkProvider).resendAllRequests(requestListener); + } + + @Test public void testUpdateConfigExitsSafeMode() { final NetworkRequestListener requestListener = verifyAndGetRequestListener(); final Set<VcnGatewayConnection> gatewayConnections = @@ -261,8 +318,8 @@ public class VcnTest { verify(mVcnNetworkProvider, times(2)).registerListener(eq(requestListener)); assertTrue(mVcn.isActive()); for (final int[] caps : TEST_CAPS) { - // Expect each gateway connection created on initial startup, and again with new configs - verify(mDeps, times(2)) + // Expect each gateway connection created only on initial startup + verify(mDeps) .newVcnGatewayConnection( eq(mVcnContext), eq(TEST_SUB_GROUP), @@ -271,4 +328,14 @@ public class VcnTest { any()); } } + + @Test + public void testIgnoreNetworkRequestWhileInactive() { + mVcn.setIsActive(false /* isActive */); + + final NetworkRequestListener requestListener = verifyAndGetRequestListener(); + triggerVcnRequestListeners(requestListener); + + verify(mDeps, never()).newVcnGatewayConnection(any(), any(), any(), any(), any()); + } } |