From 580fda26702b3d9fec941794bddc7bd6462a0dfc Mon Sep 17 00:00:00 2001 From: Chiachang Wang Date: Tue, 13 Apr 2021 18:34:51 +0800 Subject: [PATCH 1/5] Remove hidden connectivity method access in FrameworksVcnTests Due to connectivity modularization work, hidden connectivity methods and members are not accessible outside the module. Remove the corresponding usage. The test case in VcnNetworkProviderTest are not able to create NetworkRequests with request id assigned. The loop to create different request is removed. After that, the test does not provide more test coverage than other tests. Thus, Remove the test case directly. Bug: 182859030 Test: atest FrameworksVcnTests Merged-In: I488f62089d1dbe93c232f892885d944bef896df6 Change-Id: I488f62089d1dbe93c232f892885d944bef896df6 (cherry picked from commit 18f3a26efad77c1da290c01e67f38b88ca4156d8) --- .../server/VcnManagementServiceTest.java | 6 ++-- .../server/vcn/VcnGatewayConnectionTest.java | 30 +++++++++--------- .../vcn/VcnGatewayConnectionTestBase.java | 5 +-- .../server/vcn/VcnNetworkProviderTest.java | 31 ------------------- 4 files changed, 22 insertions(+), 50 deletions(-) diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index bbc9bb600f6d9..9a663436f9830 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -39,6 +39,7 @@ import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.any; import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.doAnswer; @@ -59,7 +60,6 @@ import android.net.ConnectivityManager; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; -import android.net.NetworkCapabilities.Transport; import android.net.NetworkRequest; import android.net.TelephonyNetworkSpecifier; import android.net.vcn.IVcnStatusCallback; @@ -657,7 +657,7 @@ public class VcnManagementServiceTest { private void verifyMergedNetworkCapabilities( NetworkCapabilities mergedCapabilities, - @Transport int transportType, + int transportType, boolean isVcnManaged, boolean isRestricted) { assertTrue(mergedCapabilities.hasTransport(transportType)); @@ -779,7 +779,7 @@ public class VcnManagementServiceTest { .registerNetworkCallback( eq(new NetworkRequest.Builder().clearCapabilities().build()), captor.capture()); - captor.getValue().onCapabilitiesChanged(new Network(0), caps); + captor.getValue().onCapabilitiesChanged(mock(Network.class, CALLS_REAL_METHODS), caps); } @Test diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java index d08af9dd33708..ced8745e89c9a 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java @@ -25,7 +25,10 @@ import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -83,31 +86,30 @@ public class VcnGatewayConnectionTest extends VcnGatewayConnectionTestBase { super.setUp(); mWifiInfo = mock(WifiInfo.class); + doReturn(mWifiInfo).when(mWifiInfo).makeCopy(anyLong()); } private void verifyBuildNetworkCapabilitiesCommon(int transportType) { - final NetworkCapabilities underlyingCaps = new NetworkCapabilities(); - underlyingCaps.addTransportType(transportType); - underlyingCaps.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); - underlyingCaps.addCapability(NET_CAPABILITY_NOT_METERED); - underlyingCaps.addCapability(NET_CAPABILITY_NOT_ROAMING); + final NetworkCapabilities.Builder capBuilder = new NetworkCapabilities.Builder(); + capBuilder.addTransportType(transportType); + capBuilder.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); + capBuilder.addCapability(NET_CAPABILITY_NOT_METERED); + capBuilder.addCapability(NET_CAPABILITY_NOT_ROAMING); if (transportType == TRANSPORT_WIFI) { - underlyingCaps.setTransportInfo(mWifiInfo); - underlyingCaps.setOwnerUid(TEST_UID); + capBuilder.setTransportInfo(mWifiInfo); + capBuilder.setOwnerUid(TEST_UID); } else if (transportType == TRANSPORT_CELLULAR) { - underlyingCaps.setAdministratorUids(new int[] {TEST_UID}); - underlyingCaps.setNetworkSpecifier( + capBuilder.setNetworkSpecifier( new TelephonyNetworkSpecifier(TEST_SUBSCRIPTION_ID_1)); } - - UnderlyingNetworkRecord record = - new UnderlyingNetworkRecord( - new Network(0), underlyingCaps, new LinkProperties(), false); + capBuilder.setAdministratorUids(new int[] {TEST_UID}); + UnderlyingNetworkRecord record = new UnderlyingNetworkRecord( + mock(Network.class, CALLS_REAL_METHODS), + capBuilder.build(), new LinkProperties(), false); final NetworkCapabilities vcnCaps = VcnGatewayConnection.buildNetworkCapabilities( VcnGatewayConnectionConfigTest.buildTestConfig(), record); - assertTrue(vcnCaps.hasTransport(TRANSPORT_CELLULAR)); assertTrue(vcnCaps.hasCapability(NET_CAPABILITY_NOT_METERED)); assertTrue(vcnCaps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java index dc73be25ffa3c..98d553dab01ab 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -93,7 +94,7 @@ public class VcnGatewayConnectionTestBase { protected static final UnderlyingNetworkRecord TEST_UNDERLYING_NETWORK_RECORD_1 = new UnderlyingNetworkRecord( - new Network(0), + mock(Network.class, CALLS_REAL_METHODS), new NetworkCapabilities(), new LinkProperties(), false /* blocked */); @@ -104,7 +105,7 @@ public class VcnGatewayConnectionTestBase { protected static final UnderlyingNetworkRecord TEST_UNDERLYING_NETWORK_RECORD_2 = new UnderlyingNetworkRecord( - new Network(1), + mock(Network.class, CALLS_REAL_METHODS), new NetworkCapabilities(), new LinkProperties(), false /* blocked */); diff --git a/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java b/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java index c2c6200fd5f99..f943f34c9d523 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java @@ -22,8 +22,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import android.annotation.NonNull; import android.content.Context; -import android.net.ConnectivityManager; -import android.net.NetworkCapabilities; import android.net.NetworkRequest; import android.os.test.TestLooper; @@ -36,9 +34,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import java.util.ArrayList; -import java.util.List; - /** Tests for TelephonySubscriptionTracker */ @RunWith(AndroidJUnit4.class) @SmallTest @@ -46,8 +41,6 @@ public class VcnNetworkProviderTest { private static final int TEST_SCORE_UNSATISFIED = 0; private static final int TEST_SCORE_HIGH = 100; private static final int TEST_PROVIDER_ID = 1; - private static final int TEST_LEGACY_TYPE = ConnectivityManager.TYPE_MOBILE; - private static final NetworkRequest.Type TEST_REQUEST_TYPE = NetworkRequest.Type.REQUEST; @NonNull private final Context mContext; @NonNull private final TestLooper mTestLooper; @@ -94,28 +87,4 @@ public class VcnNetworkProviderTest { mVcnNetworkProvider.onNetworkRequested(request, TEST_SCORE_UNSATISFIED, TEST_PROVIDER_ID); verifyNoMoreInteractions(mListener); } - - @Test - public void testCachedRequestsPassedOnRegister() throws Exception { - final List requests = new ArrayList<>(); - - for (int i = 0; i < 10; i++) { - final NetworkRequest request = - new NetworkRequest( - new NetworkCapabilities(), - TEST_LEGACY_TYPE, - i /* requestId */, - TEST_REQUEST_TYPE); - - requests.add(request); - mVcnNetworkProvider.onNetworkRequested(request, i, i + 1); - } - - mVcnNetworkProvider.registerListener(mListener); - for (int i = 0; i < requests.size(); i++) { - final NetworkRequest request = requests.get(i); - verify(mListener).onNetworkRequested(request, i, i + 1); - } - verifyNoMoreInteractions(mListener); - } } From 061c84ba27077175aa49da27d34707d1084dca6b Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 8 Apr 2021 03:18:45 -0700 Subject: [PATCH 2/5] Disable INTERNET/DUN when mobile data toggled off This change disables INTERNET and DUN capabilities once the mobile data toggle is turned off. Additionally, when the toggle is flipped, any existing networks advertising either of those two capabilities will be restarted in order to remove the capabilities. Bug: 184864726 Test: atest FrameworksVcnTests Merged-In: I81efeaa4b129443b08a597ff45e0037ddefeb414 Change-Id: I81efeaa4b129443b08a597ff45e0037ddefeb414 (cherry picked from commit fcacb06752e4b9dccbd49f4abbb8e486b1aefc2a) --- .../core/java/com/android/server/vcn/Vcn.java | 152 +++++++++++++++++- .../server/vcn/VcnGatewayConnection.java | 22 ++- ...atewayConnectionDisconnectedStateTest.java | 1 + .../server/vcn/VcnGatewayConnectionTest.java | 28 +++- .../vcn/VcnGatewayConnectionTestBase.java | 1 + .../java/com/android/server/vcn/VcnTest.java | 142 +++++++++++++++- 6 files changed, 326 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java index 7bc6056f91f30..70a8a7ff7bb31 100644 --- a/services/core/java/com/android/server/vcn/Vcn.java +++ b/services/core/java/com/android/server/vcn/Vcn.java @@ -16,6 +16,8 @@ package com.android.server.vcn; +import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_ACTIVE; @@ -26,14 +28,20 @@ import static com.android.server.VcnManagementService.VDBG; import android.annotation.NonNull; import android.annotation.Nullable; +import android.content.ContentResolver; +import android.database.ContentObserver; import android.net.NetworkCapabilities; import android.net.NetworkRequest; +import android.net.Uri; import android.net.vcn.VcnConfig; import android.net.vcn.VcnGatewayConnectionConfig; import android.net.vcn.VcnManager.VcnErrorCode; import android.os.Handler; import android.os.Message; import android.os.ParcelUuid; +import android.provider.Settings; +import android.telephony.TelephonyManager; +import android.util.ArraySet; import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; @@ -42,9 +50,11 @@ import com.android.internal.util.IndentingPrintWriter; import com.android.server.VcnManagementService.VcnCallback; import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; @@ -61,6 +71,9 @@ import java.util.Set; public class Vcn extends Handler { private static final String TAG = Vcn.class.getSimpleName(); + private static final List CAPS_REQUIRING_MOBILE_DATA = + Arrays.asList(NET_CAPABILITY_INTERNET, NET_CAPABILITY_DUN); + private static final int MSG_EVENT_BASE = 0; private static final int MSG_CMD_BASE = 100; @@ -110,6 +123,15 @@ public class Vcn extends Handler { */ private static final int MSG_EVENT_SAFE_MODE_STATE_CHANGED = MSG_EVENT_BASE + 4; + /** + * Triggers reevaluation of mobile data enabled conditions. + * + *

Upon this notification, the VCN will check if any of the underlying subIds have mobile + * data enabled. If not, the VCN will restart any GatewayConnections providing INTERNET or DUN + * with the current mobile data toggle status. + */ + private static final int MSG_EVENT_MOBILE_DATA_TOGGLED = MSG_EVENT_BASE + 5; + /** Triggers an immediate teardown of the entire Vcn, including GatewayConnections. */ private static final int MSG_CMD_TEARDOWN = MSG_CMD_BASE; @@ -118,6 +140,8 @@ public class Vcn extends Handler { @NonNull private final Dependencies mDeps; @NonNull private final VcnNetworkRequestListener mRequestListener; @NonNull private final VcnCallback mVcnCallback; + @NonNull private final VcnContentResolver mContentResolver; + @NonNull private final ContentObserver mMobileDataSettingsObserver; /** * Map containing all VcnGatewayConnections and their VcnGatewayConnectionConfigs. @@ -154,6 +178,8 @@ public class Vcn extends Handler { // Accessed from different threads, but always under lock in VcnManagementService private volatile int mCurrentStatus = VCN_STATUS_CODE_ACTIVE; + private boolean mIsMobileDataEnabled = false; + public Vcn( @NonNull VcnContext vcnContext, @NonNull ParcelUuid subscriptionGroup, @@ -177,10 +203,19 @@ public class Vcn extends Handler { mVcnCallback = Objects.requireNonNull(vcnCallback, "Missing vcnCallback"); mDeps = Objects.requireNonNull(deps, "Missing deps"); mRequestListener = new VcnNetworkRequestListener(); + mContentResolver = mDeps.newVcnContentResolver(mVcnContext); + mMobileDataSettingsObserver = new VcnMobileDataContentObserver(this /* handler */); + + final Uri uri = Settings.Global.getUriFor(Settings.Global.MOBILE_DATA); + mContentResolver.registerContentObserver( + uri, true /* notifyForDescendants */, mMobileDataSettingsObserver); mConfig = Objects.requireNonNull(config, "Missing config"); mLastSnapshot = Objects.requireNonNull(snapshot, "Missing snapshot"); + // Update mIsMobileDataEnabled before starting handling of NetworkRequests. + mIsMobileDataEnabled = getMobileDataStatus(); + // Register to receive cached and future NetworkRequests mVcnContext.getVcnNetworkProvider().registerListener(mRequestListener); } @@ -260,6 +295,9 @@ public class Vcn extends Handler { case MSG_EVENT_SAFE_MODE_STATE_CHANGED: handleSafeModeStatusChanged(); break; + case MSG_EVENT_MOBILE_DATA_TOGGLED: + handleMobileDataToggled(); + break; case MSG_CMD_TEARDOWN: handleTeardown(); break; @@ -366,18 +404,37 @@ public class Vcn extends Handler { if (isRequestSatisfiedByGatewayConnectionConfig(request, gatewayConnectionConfig)) { Slog.v(getLogTag(), "Bringing up new VcnGatewayConnection for request " + request); + if (getExposedCapabilitiesForMobileDataState(gatewayConnectionConfig).isEmpty()) { + // Skip; this network does not provide any services if mobile data is disabled. + continue; + } + final VcnGatewayConnection vcnGatewayConnection = mDeps.newVcnGatewayConnection( mVcnContext, mSubscriptionGroup, mLastSnapshot, gatewayConnectionConfig, - new VcnGatewayStatusCallbackImpl(gatewayConnectionConfig)); + new VcnGatewayStatusCallbackImpl(gatewayConnectionConfig), + mIsMobileDataEnabled); mVcnGatewayConnections.put(gatewayConnectionConfig, vcnGatewayConnection); } } } + private Set getExposedCapabilitiesForMobileDataState( + VcnGatewayConnectionConfig gatewayConnectionConfig) { + if (mIsMobileDataEnabled) { + return gatewayConnectionConfig.getAllExposedCapabilities(); + } + + final Set exposedCapsWithoutMobileData = + new ArraySet<>(gatewayConnectionConfig.getAllExposedCapabilities()); + exposedCapsWithoutMobileData.removeAll(CAPS_REQUIRING_MOBILE_DATA); + + return exposedCapsWithoutMobileData; + } + private void handleGatewayConnectionQuit(VcnGatewayConnectionConfig config) { Slog.v(getLogTag(), "VcnGatewayConnection quit: " + config); mVcnGatewayConnections.remove(config); @@ -396,12 +453,55 @@ public class Vcn extends Handler { } } + private void handleMobileDataToggled() { + final boolean oldMobileDataEnabledStatus = mIsMobileDataEnabled; + mIsMobileDataEnabled = getMobileDataStatus(); + + if (oldMobileDataEnabledStatus != mIsMobileDataEnabled) { + // Teardown any GatewayConnections that advertise INTERNET or DUN. If they provide other + // services, the VcnGatewayConnections will be restarted without advertising INTERNET or + // DUN. + for (Entry entry : + mVcnGatewayConnections.entrySet()) { + final VcnGatewayConnectionConfig gatewayConnectionConfig = entry.getKey(); + final VcnGatewayConnection gatewayConnection = entry.getValue(); + + final Set exposedCaps = + gatewayConnectionConfig.getAllExposedCapabilities(); + if (exposedCaps.contains(NET_CAPABILITY_INTERNET) + || exposedCaps.contains(NET_CAPABILITY_DUN)) { + if (gatewayConnection == null) { + Slog.wtf( + getLogTag(), + "Found gatewayConnectionConfig without GatewayConnection"); + } else { + // TODO(b/184868850): Optimize by restarting NetworkAgents without teardown. + gatewayConnection.teardownAsynchronously(); + } + } + } + } + } + + private boolean getMobileDataStatus() { + final TelephonyManager genericTelMan = + mVcnContext.getContext().getSystemService(TelephonyManager.class); + + for (int subId : mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup)) { + if (genericTelMan.createForSubscriptionId(subId).isDataEnabled()) { + return true; + } + } + + return false; + } + private boolean isRequestSatisfiedByGatewayConnectionConfig( @NonNull NetworkRequest request, @NonNull VcnGatewayConnectionConfig config) { final NetworkCapabilities.Builder builder = new NetworkCapabilities.Builder(); builder.addTransportType(TRANSPORT_CELLULAR); builder.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); - for (int cap : config.getAllExposedCapabilities()) { + for (int cap : getExposedCapabilitiesForMobileDataState(config)) { builder.addCapability(cap); } @@ -432,6 +532,16 @@ public class Vcn extends Handler { pw.decreaseIndent(); } + @VisibleForTesting(visibility = Visibility.PRIVATE) + public boolean isMobileDataEnabled() { + return mIsMobileDataEnabled; + } + + @VisibleForTesting(visibility = Visibility.PRIVATE) + public void setMobileDataEnabled(boolean isMobileDataEnabled) { + mIsMobileDataEnabled = isMobileDataEnabled; + } + /** Retrieves the network score for a VCN Network */ // Package visibility for use in VcnGatewayConnection static int getNetworkScore() { @@ -485,6 +595,17 @@ public class Vcn extends Handler { } } + private class VcnMobileDataContentObserver extends ContentObserver { + private VcnMobileDataContentObserver(Handler handler) { + super(handler); + } + + @Override + public void onChange(boolean selfChange) { + sendMessage(obtainMessage(MSG_EVENT_MOBILE_DATA_TOGGLED)); + } + } + /** External dependencies used by Vcn, for injection in tests */ @VisibleForTesting(visibility = Visibility.PRIVATE) public static class Dependencies { @@ -494,13 +615,36 @@ public class Vcn extends Handler { ParcelUuid subscriptionGroup, TelephonySubscriptionSnapshot snapshot, VcnGatewayConnectionConfig connectionConfig, - VcnGatewayStatusCallback gatewayStatusCallback) { + VcnGatewayStatusCallback gatewayStatusCallback, + boolean isMobileDataEnabled) { return new VcnGatewayConnection( vcnContext, subscriptionGroup, snapshot, connectionConfig, - gatewayStatusCallback); + gatewayStatusCallback, + isMobileDataEnabled); + } + + /** Builds a new VcnContentResolver instance */ + public VcnContentResolver newVcnContentResolver(VcnContext vcnContext) { + return new VcnContentResolver(vcnContext); + } + } + + /** Proxy Implementation of NetworkAgent, used for testing. */ + @VisibleForTesting(visibility = Visibility.PRIVATE) + public static class VcnContentResolver { + private final ContentResolver mImpl; + + public VcnContentResolver(VcnContext vcnContext) { + mImpl = vcnContext.getContext().getContentResolver(); + } + + /** Registers the content observer */ + public void registerContentObserver( + @NonNull Uri uri, boolean notifyForDescendants, @NonNull ContentObserver observer) { + mImpl.registerContentObserver(uri, notifyForDescendants, observer); } } } diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index df31221b5d605..8847dda71c491 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -16,6 +16,8 @@ package com.android.server.vcn; +import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING; @@ -517,6 +519,7 @@ public class VcnGatewayConnection extends StateMachine { @NonNull private final VcnGatewayStatusCallback mGatewayStatusCallback; @NonNull private final Dependencies mDeps; @NonNull private final VcnUnderlyingNetworkTrackerCallback mUnderlyingNetworkTrackerCallback; + private final boolean mIsMobileDataEnabled; @NonNull private final IpSecManager mIpSecManager; @@ -626,13 +629,15 @@ public class VcnGatewayConnection extends StateMachine { @NonNull ParcelUuid subscriptionGroup, @NonNull TelephonySubscriptionSnapshot snapshot, @NonNull VcnGatewayConnectionConfig connectionConfig, - @NonNull VcnGatewayStatusCallback gatewayStatusCallback) { + @NonNull VcnGatewayStatusCallback gatewayStatusCallback, + boolean isMobileDataEnabled) { this( vcnContext, subscriptionGroup, snapshot, connectionConfig, gatewayStatusCallback, + isMobileDataEnabled, new Dependencies()); } @@ -643,6 +648,7 @@ public class VcnGatewayConnection extends StateMachine { @NonNull TelephonySubscriptionSnapshot snapshot, @NonNull VcnGatewayConnectionConfig connectionConfig, @NonNull VcnGatewayStatusCallback gatewayStatusCallback, + boolean isMobileDataEnabled, @NonNull Dependencies deps) { super(TAG, Objects.requireNonNull(vcnContext, "Missing vcnContext").getLooper()); mVcnContext = vcnContext; @@ -650,6 +656,7 @@ public class VcnGatewayConnection extends StateMachine { mConnectionConfig = Objects.requireNonNull(connectionConfig, "Missing connectionConfig"); mGatewayStatusCallback = Objects.requireNonNull(gatewayStatusCallback, "Missing gatewayStatusCallback"); + mIsMobileDataEnabled = isMobileDataEnabled; mDeps = Objects.requireNonNull(deps, "Missing deps"); mLastSnapshot = Objects.requireNonNull(snapshot, "Missing snapshot"); @@ -1502,7 +1509,7 @@ public class VcnGatewayConnection extends StateMachine { @NonNull VcnNetworkAgent agent, @NonNull VcnChildSessionConfiguration childConfig) { final NetworkCapabilities caps = - buildNetworkCapabilities(mConnectionConfig, mUnderlying); + buildNetworkCapabilities(mConnectionConfig, mUnderlying, mIsMobileDataEnabled); final LinkProperties lp = buildConnectedLinkProperties( mConnectionConfig, tunnelIface, childConfig, mUnderlying); @@ -1515,7 +1522,7 @@ public class VcnGatewayConnection extends StateMachine { @NonNull IpSecTunnelInterface tunnelIface, @NonNull VcnChildSessionConfiguration childConfig) { final NetworkCapabilities caps = - buildNetworkCapabilities(mConnectionConfig, mUnderlying); + buildNetworkCapabilities(mConnectionConfig, mUnderlying, mIsMobileDataEnabled); final LinkProperties lp = buildConnectedLinkProperties( mConnectionConfig, tunnelIface, childConfig, mUnderlying); @@ -1843,7 +1850,8 @@ public class VcnGatewayConnection extends StateMachine { @VisibleForTesting(visibility = Visibility.PRIVATE) static NetworkCapabilities buildNetworkCapabilities( @NonNull VcnGatewayConnectionConfig gatewayConnectionConfig, - @Nullable UnderlyingNetworkRecord underlying) { + @Nullable UnderlyingNetworkRecord underlying, + boolean isMobileDataEnabled) { final NetworkCapabilities.Builder builder = new NetworkCapabilities.Builder(); builder.addTransportType(TRANSPORT_CELLULAR); @@ -1853,6 +1861,12 @@ public class VcnGatewayConnection extends StateMachine { // Add exposed capabilities for (int cap : gatewayConnectionConfig.getAllExposedCapabilities()) { + // Skip adding INTERNET or DUN if mobile data is disabled. + if (!isMobileDataEnabled + && (cap == NET_CAPABILITY_INTERNET || cap == NET_CAPABILITY_DUN)) { + continue; + } + builder.addCapability(cap); } diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java index 5f27fabb62b0d..ac0edaa3b5796 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java @@ -65,6 +65,7 @@ public class VcnGatewayConnectionDisconnectedStateTest extends VcnGatewayConnect TEST_SUBSCRIPTION_SNAPSHOT, mConfig, mGatewayStatusCallback, + true /* isMobileDataEnabled */, mDeps); vgc.setIsQuitting(true); diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java index ced8745e89c9a..9705f0fc6bbc6 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java @@ -16,6 +16,8 @@ package com.android.server.vcn; +import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED; @@ -89,7 +91,8 @@ public class VcnGatewayConnectionTest extends VcnGatewayConnectionTestBase { doReturn(mWifiInfo).when(mWifiInfo).makeCopy(anyLong()); } - private void verifyBuildNetworkCapabilitiesCommon(int transportType) { + private void verifyBuildNetworkCapabilitiesCommon( + int transportType, boolean isMobileDataEnabled) { final NetworkCapabilities.Builder capBuilder = new NetworkCapabilities.Builder(); capBuilder.addTransportType(transportType); capBuilder.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); @@ -109,10 +112,22 @@ public class VcnGatewayConnectionTest extends VcnGatewayConnectionTestBase { capBuilder.build(), new LinkProperties(), false); final NetworkCapabilities vcnCaps = VcnGatewayConnection.buildNetworkCapabilities( - VcnGatewayConnectionConfigTest.buildTestConfig(), record); + VcnGatewayConnectionConfigTest.buildTestConfig(), + record, + isMobileDataEnabled); + assertTrue(vcnCaps.hasTransport(TRANSPORT_CELLULAR)); assertTrue(vcnCaps.hasCapability(NET_CAPABILITY_NOT_METERED)); assertTrue(vcnCaps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + + for (int cap : VcnGatewayConnectionConfigTest.EXPOSED_CAPS) { + if (cap == NET_CAPABILITY_INTERNET || cap == NET_CAPABILITY_DUN) { + assertEquals(isMobileDataEnabled, vcnCaps.hasCapability(cap)); + } else { + assertTrue(vcnCaps.hasCapability(cap)); + } + } + assertArrayEquals(new int[] {TEST_UID}, vcnCaps.getAdministratorUids()); assertTrue(vcnCaps.getTransportInfo() instanceof VcnTransportInfo); @@ -126,12 +141,17 @@ public class VcnGatewayConnectionTest extends VcnGatewayConnectionTestBase { @Test public void testBuildNetworkCapabilitiesUnderlyingWifi() throws Exception { - verifyBuildNetworkCapabilitiesCommon(TRANSPORT_WIFI); + verifyBuildNetworkCapabilitiesCommon(TRANSPORT_WIFI, true /* isMobileDataEnabled */); } @Test public void testBuildNetworkCapabilitiesUnderlyingCell() throws Exception { - verifyBuildNetworkCapabilitiesCommon(TRANSPORT_CELLULAR); + verifyBuildNetworkCapabilitiesCommon(TRANSPORT_CELLULAR, true /* isMobileDataEnabled */); + } + + @Test + public void testBuildNetworkCapabilitiesMobileDataDisabled() throws Exception { + verifyBuildNetworkCapabilitiesCommon(TRANSPORT_CELLULAR, false /* isMobileDataEnabled */); } @Test diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java index 98d553dab01ab..284f1f88658e4 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java @@ -202,6 +202,7 @@ public class VcnGatewayConnectionTestBase { TEST_SUBSCRIPTION_SNAPSHOT, mConfig, mGatewayStatusCallback, + true /* isMobileDataEnabled */, mDeps); } diff --git a/tests/vcn/java/com/android/server/vcn/VcnTest.java b/tests/vcn/java/com/android/server/vcn/VcnTest.java index 90eb75e865e73..7c2a5a1cfc5d6 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnTest.java @@ -16,16 +16,24 @@ package com.android.server.vcn; +import static android.net.NetworkCapabilities.NET_CAPABILITY_CBS; import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; +import static android.net.NetworkCapabilities.NET_CAPABILITY_FOTA; +import static android.net.NetworkCapabilities.NET_CAPABILITY_IMS; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_MMS; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_ACTIVE; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_SAFE_MODE; +import static com.android.server.vcn.Vcn.VcnContentResolver; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; @@ -35,12 +43,16 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import android.content.Context; +import android.database.ContentObserver; import android.net.NetworkRequest; +import android.net.Uri; import android.net.vcn.VcnConfig; import android.net.vcn.VcnGatewayConnectionConfig; import android.net.vcn.VcnGatewayConnectionConfigTest; import android.os.ParcelUuid; import android.os.test.TestLooper; +import android.provider.Settings; +import android.telephony.TelephonyManager; import android.util.ArraySet; import com.android.server.VcnManagementService.VcnCallback; @@ -53,7 +65,10 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.UUID; @@ -62,14 +77,21 @@ public class VcnTest { private static final ParcelUuid TEST_SUB_GROUP = new ParcelUuid(new UUID(0, 0)); private static final int NETWORK_SCORE = 0; private static final int PROVIDER_ID = 5; + private static final boolean MOBILE_DATA_ENABLED = true; + private static final Set TEST_SUB_IDS_IN_GROUP = + new ArraySet<>(Arrays.asList(1, 2, 3)); private static final int[][] TEST_CAPS = new int[][] { - new int[] {NET_CAPABILITY_MMS, NET_CAPABILITY_INTERNET}, - new int[] {NET_CAPABILITY_DUN} + new int[] {NET_CAPABILITY_IMS, NET_CAPABILITY_INTERNET, NET_CAPABILITY_DUN}, + new int[] {NET_CAPABILITY_CBS, NET_CAPABILITY_INTERNET}, + new int[] {NET_CAPABILITY_FOTA, NET_CAPABILITY_DUN}, + new int[] {NET_CAPABILITY_MMS} }; private Context mContext; private VcnContext mVcnContext; + private TelephonyManager mTelephonyManager; + private VcnContentResolver mContentResolver; private TelephonySubscriptionSnapshot mSubscriptionSnapshot; private VcnNetworkProvider mVcnNetworkProvider; private VcnCallback mVcnCallback; @@ -86,6 +108,9 @@ public class VcnTest { public void setUp() { mContext = mock(Context.class); mVcnContext = mock(VcnContext.class); + mTelephonyManager = + setupAndGetTelephonyManager(MOBILE_DATA_ENABLED /* isMobileDataEnabled */); + mContentResolver = mock(VcnContentResolver.class); mSubscriptionSnapshot = mock(TelephonySubscriptionSnapshot.class); mVcnNetworkProvider = mock(VcnNetworkProvider.class); mVcnCallback = mock(VcnCallback.class); @@ -97,12 +122,15 @@ public class VcnTest { doReturn(mContext).when(mVcnContext).getContext(); doReturn(mTestLooper.getLooper()).when(mVcnContext).getLooper(); doReturn(mVcnNetworkProvider).when(mVcnContext).getVcnNetworkProvider(); + doReturn(mContentResolver).when(mDeps).newVcnContentResolver(eq(mVcnContext)); // Setup VcnGatewayConnection instance generation doAnswer((invocation) -> { // Mock-within a doAnswer is safe, because it doesn't actually run nested. return mock(VcnGatewayConnection.class); - }).when(mDeps).newVcnGatewayConnection(any(), any(), any(), any(), any()); + }).when(mDeps).newVcnGatewayConnection(any(), any(), any(), any(), any(), anyBoolean()); + + doReturn(TEST_SUB_IDS_IN_GROUP).when(mSubscriptionSnapshot).getAllSubIdsInGroup(any()); mGatewayStatusCallbackCaptor = ArgumentCaptor.forClass(VcnGatewayStatusCallback.class); @@ -123,6 +151,16 @@ public class VcnTest { mDeps); } + private TelephonyManager setupAndGetTelephonyManager(boolean isMobileDataEnabled) { + final TelephonyManager telephonyManager = mock(TelephonyManager.class); + VcnTestUtils.setupSystemService( + mContext, telephonyManager, Context.TELEPHONY_SERVICE, TelephonyManager.class); + doReturn(telephonyManager).when(telephonyManager).createForSubscriptionId(anyInt()); + doReturn(isMobileDataEnabled).when(telephonyManager).isDataEnabled(); + + return telephonyManager; + } + private NetworkRequestListener verifyAndGetRequestListener() { ArgumentCaptor mNetworkRequestListenerCaptor = ArgumentCaptor.forClass(NetworkRequestListener.class); @@ -163,6 +201,39 @@ public class VcnTest { } } + @Test + public void testContentObserverRegistered() { + // Validate state from setUp() + final Uri uri = Settings.Global.getUriFor(Settings.Global.MOBILE_DATA); + verify(mContentResolver) + .registerContentObserver(eq(uri), eq(true), any(ContentObserver.class)); + } + + @Test + public void testMobileDataStateCheckedOnInitialization_enabled() { + // Validate state from setUp() + assertTrue(mVcn.isMobileDataEnabled()); + verify(mTelephonyManager).isDataEnabled(); + } + + @Test + public void testMobileDataStateCheckedOnInitialization_disabled() { + // Build and setup new telephonyManager to ensure method call count is reset. + final TelephonyManager telephonyManager = + setupAndGetTelephonyManager(false /* isMobileDataEnabled */); + final Vcn vcn = + new Vcn( + mVcnContext, + TEST_SUB_GROUP, + mConfig, + mSubscriptionSnapshot, + mVcnCallback, + mDeps); + + assertFalse(vcn.isMobileDataEnabled()); + verify(mTelephonyManager).isDataEnabled(); + } + @Test public void testSubscriptionSnapshotUpdatesVcnGatewayConnections() { verifyUpdateSubscriptionSnapshotNotifiesGatewayConnections(VCN_STATUS_CODE_ACTIVE); @@ -193,7 +264,8 @@ public class VcnTest { eq(TEST_SUB_GROUP), eq(mSubscriptionSnapshot), any(), - mGatewayStatusCallbackCaptor.capture()); + mGatewayStatusCallbackCaptor.capture(), + eq(MOBILE_DATA_ENABLED)); return gatewayConnections; } @@ -256,20 +328,21 @@ public class VcnTest { mTestLooper.dispatchAll(); // Verify that the VCN requests the networkRequests be resent - assertEquals(1, mVcn.getVcnGatewayConnections().size()); + assertEquals(gatewayConnections.size() - 1, mVcn.getVcnGatewayConnections().size()); verify(mVcnNetworkProvider).resendAllRequests(requestListener); // Verify that the VcnGatewayConnection is restarted if a request exists for it triggerVcnRequestListeners(requestListener); mTestLooper.dispatchAll(); - assertEquals(2, mVcn.getVcnGatewayConnections().size()); + assertEquals(gatewayConnections.size(), mVcn.getVcnGatewayConnections().size()); verify(mDeps, times(gatewayConnections.size() + 1)) .newVcnGatewayConnection( eq(mVcnContext), eq(TEST_SUB_GROUP), eq(mSubscriptionSnapshot), any(), - mGatewayStatusCallbackCaptor.capture()); + mGatewayStatusCallbackCaptor.capture(), + anyBoolean()); } @Test @@ -286,7 +359,7 @@ public class VcnTest { public void testUpdateConfigReevaluatesGatewayConnections() { final NetworkRequestListener requestListener = verifyAndGetRequestListener(); startGatewaysAndGetGatewayConnections(requestListener); - assertEquals(2, mVcn.getVcnGatewayConnectionConfigMap().size()); + assertEquals(TEST_CAPS.length, mVcn.getVcnGatewayConnectionConfigMap().size()); // Create VcnConfig with only one VcnGatewayConnectionConfig so a gateway connection is torn // down. Reuse existing VcnGatewayConnectionConfig so that the gateway connection name @@ -309,4 +382,57 @@ public class VcnTest { verify(removedGatewayConnection).teardownAsynchronously(); verify(mVcnNetworkProvider).resendAllRequests(requestListener); } + + private void verifyMobileDataToggled(boolean startingToggleState, boolean endingToggleState) { + final ArgumentCaptor captor = + ArgumentCaptor.forClass(ContentObserver.class); + verify(mContentResolver).registerContentObserver(any(), anyBoolean(), captor.capture()); + final ContentObserver contentObserver = captor.getValue(); + + // Start VcnGatewayConnections + mVcn.setMobileDataEnabled(startingToggleState); + triggerVcnRequestListeners(verifyAndGetRequestListener()); + final Map gateways = + mVcn.getVcnGatewayConnectionConfigMap(); + + // Trigger data toggle change. + doReturn(endingToggleState).when(mTelephonyManager).isDataEnabled(); + contentObserver.onChange(false /* selfChange, ignored */); + mTestLooper.dispatchAll(); + + // Verify that data toggle changes restart ONLY INTERNET or DUN networks, and only if the + // toggle state changed. + for (Entry entry : gateways.entrySet()) { + final Set exposedCaps = entry.getKey().getAllExposedCapabilities(); + if (startingToggleState != endingToggleState + && (exposedCaps.contains(NET_CAPABILITY_INTERNET) + || exposedCaps.contains(NET_CAPABILITY_DUN))) { + verify(entry.getValue()).teardownAsynchronously(); + } else { + verify(entry.getValue(), never()).teardownAsynchronously(); + } + } + + assertEquals(endingToggleState, mVcn.isMobileDataEnabled()); + } + + @Test + public void testMobileDataEnabled() { + verifyMobileDataToggled(false /* startingToggleState */, true /* endingToggleState */); + } + + @Test + public void testMobileDataDisabled() { + verifyMobileDataToggled(true /* startingToggleState */, false /* endingToggleState */); + } + + @Test + public void testMobileDataObserverFiredWithoutChanges_dataEnabled() { + verifyMobileDataToggled(false /* startingToggleState */, false /* endingToggleState */); + } + + @Test + public void testMobileDataObserverFiredWithoutChanges_dataDisabled() { + verifyMobileDataToggled(true /* startingToggleState */, true /* endingToggleState */); + } } From 7bdcb188b203aa03590ecc29a981a070f950c91e Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Mon, 12 Apr 2021 14:41:01 -0700 Subject: [PATCH 3/5] Improve dump detail This change improves the formatting and detail provided by the VCN dump Bug: 8675309 Test: atest FrameworksVcnTests Merged-In: I8f13a0d8fe54e9bf8948e9576e9705024fe79611 Change-Id: I8f13a0d8fe54e9bf8948e9576e9705024fe79611 (cherry picked from commit 1ce20d65e0ef111e0614c87e08d467109267bc94) --- .../android/server/VcnManagementService.java | 36 +++++++++++++++++++ .../vcn/TelephonySubscriptionTracker.java | 12 +++++++ .../server/vcn/UnderlyingNetworkTracker.java | 13 +++++++ .../server/vcn/VcnGatewayConnection.java | 6 ++++ .../server/vcn/VcnNetworkProvider.java | 4 +++ 5 files changed, 71 insertions(+) diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index 9c10658e7c0e9..45717f200c454 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -936,13 +936,31 @@ public class VcnManagementService extends IVcnManagementService.Stub { pw.println("VcnManagementService dump:"); pw.increaseIndent(); + pw.println("mNetworkProvider:"); + pw.increaseIndent(); mNetworkProvider.dump(pw); + pw.decreaseIndent(); + pw.println(); + + pw.println("mTrackingNetworkCallback:"); + pw.increaseIndent(); + mTrackingNetworkCallback.dump(pw); + pw.decreaseIndent(); + pw.println(); synchronized (mLock) { + pw.println("mLastSnapshot:"); + pw.increaseIndent(); + mLastSnapshot.dump(pw); + pw.decreaseIndent(); + pw.println(); + pw.println("mVcns:"); + pw.increaseIndent(); for (Vcn vcn : mVcns.values()) { vcn.dump(pw); } + pw.decreaseIndent(); pw.println(); } @@ -1004,6 +1022,24 @@ public class VcnManagementService extends IVcnManagementService.Stub { return false; } + + /** Dumps the state of this snapshot for logging and debugging purposes. */ + public void dump(IndentingPrintWriter pw) { + pw.println("TrackingNetworkCallback:"); + pw.increaseIndent(); + + pw.println("mCaps:"); + pw.increaseIndent(); + synchronized (mCaps) { + for (Entry entry : mCaps.entrySet()) { + pw.println(entry.getKey() + ": " + entry.getValue()); + } + } + pw.decreaseIndent(); + pw.println(); + + pw.decreaseIndent(); + } } /** VcnCallbackImpl for Vcn signals sent up to VcnManagementService. */ diff --git a/services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java b/services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java index d8a145d9ae339..19fbdbd86099d 100644 --- a/services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java +++ b/services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java @@ -43,6 +43,7 @@ import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting.Visibility; +import com.android.internal.util.IndentingPrintWriter; import java.util.Collections; import java.util.HashMap; @@ -320,6 +321,17 @@ public class TelephonySubscriptionTracker extends BroadcastReceiver { && mPrivilegedPackages.equals(other.mPrivilegedPackages); } + /** Dumps the state of this snapshot for logging and debugging purposes. */ + public void dump(IndentingPrintWriter pw) { + pw.println("TelephonySubscriptionSnapshot:"); + pw.increaseIndent(); + + pw.println("mSubIdToGroupMap: " + mSubIdToGroupMap); + pw.println("mPrivilegedPackages: " + mPrivilegedPackages); + + pw.decreaseIndent(); + } + @Override public String toString() { return "TelephonySubscriptionSnapshot{ " diff --git a/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java b/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java index ab214e8329ef5..48ccad33e6319 100644 --- a/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java +++ b/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java @@ -31,6 +31,7 @@ import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting.Visibility; +import com.android.internal.util.IndentingPrintWriter; import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; import java.util.ArrayList; @@ -396,6 +397,18 @@ public class UnderlyingNetworkTracker { return Objects.hash(network, networkCapabilities, linkProperties, isBlocked); } + /** Dumps the state of this record for logging and debugging purposes. */ + public void dump(IndentingPrintWriter pw) { + pw.println("UnderlyingNetworkRecord:"); + pw.increaseIndent(); + + pw.println("mNetwork: " + network); + pw.println("mNetworkCapabilities: " + networkCapabilities); + pw.println("mLinkProperties: " + linkProperties); + + pw.decreaseIndent(); + } + /** Builder to incrementally construct an UnderlyingNetworkRecord. */ private static class Builder { @NonNull private final Network mNetwork; diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index 8847dda71c491..d8a085b092ef6 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -2054,6 +2054,12 @@ public class VcnGatewayConnection extends StateMachine { "mNetworkAgent.getNetwork(): " + (mNetworkAgent == null ? null : mNetworkAgent.getNetwork())); + pw.println("mUnderlying:"); + pw.increaseIndent(); + mUnderlying.dump(pw); + pw.decreaseIndent(); + pw.println(); + pw.decreaseIndent(); } diff --git a/services/core/java/com/android/server/vcn/VcnNetworkProvider.java b/services/core/java/com/android/server/vcn/VcnNetworkProvider.java index be0deb57ee769..4de0fdaba676b 100644 --- a/services/core/java/com/android/server/vcn/VcnNetworkProvider.java +++ b/services/core/java/com/android/server/vcn/VcnNetworkProvider.java @@ -163,15 +163,19 @@ public class VcnNetworkProvider extends NetworkProvider { pw.increaseIndent(); pw.println("mListeners:"); + pw.increaseIndent(); for (NetworkRequestListener listener : mListeners) { pw.println(listener); } + pw.decreaseIndent(); pw.println(); pw.println("mRequests.values:"); + pw.increaseIndent(); for (NetworkRequestEntry entry : mRequests.values()) { entry.dump(pw); } + pw.decreaseIndent(); pw.println(); pw.decreaseIndent(); From 39659bf29aab57de87f329aa57b825b6e773d55e Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Mon, 12 Apr 2021 23:18:27 -0700 Subject: [PATCH 4/5] Remove use of network scores, and provider IDs in VcnNetworkProvider This change updates the VcnNetworkProvider and associated requestListeners to no longer use the network score and provider IDs, in order to match the new NetworkProvider APIs. Bug: 185204197 Test: atest FrameworksVcnTests Merged-In: I0cabce702cfd8457fd1a152af07a847b87d67028 Change-Id: I0cabce702cfd8457fd1a152af07a847b87d67028 (cherry picked from commit fff0f8ba339a4079e223d2c5f16bbd21fd25ff02) --- .../core/java/com/android/server/vcn/Vcn.java | 24 ++----- .../server/vcn/VcnNetworkProvider.java | 66 ++++--------------- .../server/vcn/VcnNetworkProviderTest.java | 41 ++++++++---- .../java/com/android/server/vcn/VcnTest.java | 4 +- 4 files changed, 47 insertions(+), 88 deletions(-) diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java index 70a8a7ff7bb31..c7c538521de8e 100644 --- a/services/core/java/com/android/server/vcn/Vcn.java +++ b/services/core/java/com/android/server/vcn/Vcn.java @@ -265,10 +265,10 @@ public class Vcn extends Handler { private class VcnNetworkRequestListener implements VcnNetworkProvider.NetworkRequestListener { @Override - public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) { + public void onNetworkRequested(@NonNull NetworkRequest request) { Objects.requireNonNull(request, "Missing request"); - sendMessage(obtainMessage(MSG_EVENT_NETWORK_REQUESTED, score, providerId, request)); + sendMessage(obtainMessage(MSG_EVENT_NETWORK_REQUESTED, request)); } } @@ -284,7 +284,7 @@ public class Vcn extends Handler { handleConfigUpdated((VcnConfig) msg.obj); break; case MSG_EVENT_NETWORK_REQUESTED: - handleNetworkRequested((NetworkRequest) msg.obj, msg.arg1, msg.arg2); + handleNetworkRequested((NetworkRequest) msg.obj); break; case MSG_EVENT_SUBSCRIPTIONS_CHANGED: handleSubscriptionsChanged((TelephonySubscriptionSnapshot) msg.obj); @@ -365,25 +365,9 @@ public class Vcn extends Handler { } } - private void handleNetworkRequested( - @NonNull NetworkRequest request, int score, int providerId) { + private void handleNetworkRequested(@NonNull NetworkRequest request) { Slog.v(getLogTag(), "Received request " + request); - if (score > getNetworkScore()) { - if (VDBG) { - Slog.v( - getLogTag(), - "Request already satisfied by higher-scoring (" - + score - + ") network from " - + "provider " - + providerId - + ": " - + request); - } - return; - } - // If preexisting VcnGatewayConnection(s) satisfy request, return for (VcnGatewayConnectionConfig gatewayConnectionConfig : mVcnGatewayConnections.keySet()) { if (isRequestSatisfiedByGatewayConnectionConfig(request, gatewayConnectionConfig)) { diff --git a/services/core/java/com/android/server/vcn/VcnNetworkProvider.java b/services/core/java/com/android/server/vcn/VcnNetworkProvider.java index 4de0fdaba676b..8adcf95e780f5 100644 --- a/services/core/java/com/android/server/vcn/VcnNetworkProvider.java +++ b/services/core/java/com/android/server/vcn/VcnNetworkProvider.java @@ -23,7 +23,6 @@ import android.content.Context; import android.net.NetworkProvider; import android.net.NetworkRequest; import android.os.Looper; -import android.util.ArrayMap; import android.util.ArraySet; import android.util.Slog; @@ -31,7 +30,6 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting.Visibility; import com.android.internal.util.IndentingPrintWriter; -import java.util.Objects; import java.util.Set; /** @@ -48,11 +46,11 @@ public class VcnNetworkProvider extends NetworkProvider { private final Set mListeners = new ArraySet<>(); /** - * Cache of NetworkRequest(s), scores and network providers, keyed by NetworkRequest + * Cache of NetworkRequest(s). * *

NetworkRequests are immutable once created, and therefore can be used as stable keys. */ - private final ArrayMap mRequests = new ArrayMap<>(); + private final Set mRequests = new ArraySet<>(); public VcnNetworkProvider(Context context, Looper looper) { super(context, looper, VcnNetworkProvider.class.getSimpleName()); @@ -80,38 +78,28 @@ public class VcnNetworkProvider extends NetworkProvider { /** Sends all cached NetworkRequest(s) to the specified listener. */ @VisibleForTesting(visibility = Visibility.PACKAGE) public void resendAllRequests(@NonNull NetworkRequestListener listener) { - for (NetworkRequestEntry entry : mRequests.values()) { - notifyListenerForEvent(listener, entry); + for (NetworkRequest request : mRequests) { + notifyListenerForEvent(listener, request); } } private void notifyListenerForEvent( - @NonNull NetworkRequestListener listener, @NonNull NetworkRequestEntry entry) { - listener.onNetworkRequested(entry.mRequest, entry.mScore, entry.mProviderId); + @NonNull NetworkRequestListener listener, @NonNull NetworkRequest request) { + listener.onNetworkRequested(request); } @Override public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) { if (VDBG) { - Slog.v( - TAG, - "Network requested: Request = " - + request - + ", score = " - + score - + ", providerId = " - + providerId); + Slog.v(TAG, "Network requested: Request = " + request); } - final NetworkRequestEntry entry = new NetworkRequestEntry(request, score, providerId); - - // NetworkRequests are immutable once created, and therefore can be used as stable keys. - mRequests.put(request, entry); + mRequests.add(request); // TODO(b/176939047): Intelligently route requests to prioritized VcnInstances (based on // Default Data Sub, or similar) for (NetworkRequestListener listener : mListeners) { - notifyListenerForEvent(listener, entry); + notifyListenerForEvent(listener, request); } } @@ -120,37 +108,9 @@ public class VcnNetworkProvider extends NetworkProvider { mRequests.remove(request); } - private static class NetworkRequestEntry { - public final NetworkRequest mRequest; - public final int mScore; - public final int mProviderId; - - private NetworkRequestEntry(@NonNull NetworkRequest request, int score, int providerId) { - mRequest = Objects.requireNonNull(request, "Missing request"); - mScore = score; - mProviderId = providerId; - } - - /** - * Dumps the state of this NetworkRequestEntry for logging and debugging purposes. - * - *

PII and credentials MUST NEVER be dumped here. - */ - public void dump(IndentingPrintWriter pw) { - pw.println("NetworkRequestEntry:"); - pw.increaseIndent(); - - pw.println("mRequest: " + mRequest); - pw.println("mScore: " + mScore); - pw.println("mProviderId: " + mProviderId); - - pw.decreaseIndent(); - } - } - // package-private interface NetworkRequestListener { - void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId); + void onNetworkRequested(@NonNull NetworkRequest request); } /** @@ -170,10 +130,10 @@ public class VcnNetworkProvider extends NetworkProvider { pw.decreaseIndent(); pw.println(); - pw.println("mRequests.values:"); + pw.println("mRequests:"); pw.increaseIndent(); - for (NetworkRequestEntry entry : mRequests.values()) { - entry.dump(pw); + for (NetworkRequest request : mRequests) { + pw.println(request); } pw.decreaseIndent(); pw.println(); diff --git a/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java b/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java index f943f34c9d523..79cf746f105fa 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java @@ -34,12 +34,14 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.ArrayList; +import java.util.List; + /** Tests for TelephonySubscriptionTracker */ @RunWith(AndroidJUnit4.class) @SmallTest public class VcnNetworkProviderTest { private static final int TEST_SCORE_UNSATISFIED = 0; - private static final int TEST_SCORE_HIGH = 100; private static final int TEST_PROVIDER_ID = 1; @NonNull private final Context mContext; @@ -65,17 +67,7 @@ public class VcnNetworkProviderTest { final NetworkRequest request = mock(NetworkRequest.class); mVcnNetworkProvider.onNetworkRequested(request, TEST_SCORE_UNSATISFIED, TEST_PROVIDER_ID); - verify(mListener).onNetworkRequested(request, TEST_SCORE_UNSATISFIED, TEST_PROVIDER_ID); - } - - @Test - public void testRequestsPassedToRegisteredListeners_satisfiedByHighScoringProvider() - throws Exception { - mVcnNetworkProvider.registerListener(mListener); - - final NetworkRequest request = mock(NetworkRequest.class); - mVcnNetworkProvider.onNetworkRequested(request, TEST_SCORE_HIGH, TEST_PROVIDER_ID); - verify(mListener).onNetworkRequested(request, TEST_SCORE_HIGH, TEST_PROVIDER_ID); + verify(mListener).onNetworkRequested(request); } @Test @@ -87,4 +79,29 @@ public class VcnNetworkProviderTest { mVcnNetworkProvider.onNetworkRequested(request, TEST_SCORE_UNSATISFIED, TEST_PROVIDER_ID); verifyNoMoreInteractions(mListener); } + + @Test + public void testCachedRequestsPassedOnRegister() throws Exception { + final List requests = new ArrayList<>(); + + for (int i = 0; i < 10; i++) { + // Build unique network requests; in this case, iterate down the capabilities as a way + // to unique-ify requests. + final NetworkRequest request = + new NetworkRequest.Builder().clearCapabilities().addCapability(i).build(); + + requests.add(request); + mVcnNetworkProvider.onNetworkRequested(request, i, i + 1); + } + + // Remove one, and verify that it is never sent to the listeners. + final NetworkRequest removed = requests.remove(0); + mVcnNetworkProvider.onNetworkRequestWithdrawn(removed); + + mVcnNetworkProvider.registerListener(mListener); + for (NetworkRequest request : requests) { + verify(mListener).onNetworkRequested(request); + } + verifyNoMoreInteractions(mListener); + } } diff --git a/tests/vcn/java/com/android/server/vcn/VcnTest.java b/tests/vcn/java/com/android/server/vcn/VcnTest.java index 7c2a5a1cfc5d6..736fabdb1ac54 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnTest.java @@ -75,8 +75,6 @@ import java.util.UUID; public class VcnTest { private static final String PKG_NAME = VcnTest.class.getPackage().getName(); private static final ParcelUuid TEST_SUB_GROUP = new ParcelUuid(new UUID(0, 0)); - private static final int NETWORK_SCORE = 0; - private static final int PROVIDER_ID = 5; private static final boolean MOBILE_DATA_ENABLED = true; private static final Set TEST_SUB_IDS_IN_GROUP = new ArraySet<>(Arrays.asList(1, 2, 3)); @@ -177,7 +175,7 @@ public class VcnTest { requestBuilder.addCapability(netCapability); } - requestListener.onNetworkRequested(requestBuilder.build(), NETWORK_SCORE, PROVIDER_ID); + requestListener.onNetworkRequested(requestBuilder.build()); mTestLooper.dispatchAll(); } From 78f9acd7e7d600b2f04c041c0c23a9f46bb91808 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Mon, 12 Apr 2021 23:42:59 -0700 Subject: [PATCH 5/5] Switch to using NetworkScore instead of legacy integer This change switches the VcnNetworkAgent from using the legacy score integer, and switches to using the new NetworkScore, using the legacy integer as a internal representation. Bug: 185204197 Test: FrameworksVcnTests Merged-In: Ib0690243f176a2f074358fcc8430f1fd7c3d0d3c Change-Id: Ib0690243f176a2f074358fcc8430f1fd7c3d0d3c (cherry picked from commit ffc912dace5db15f957880b084d418db77ce310b) --- services/core/java/com/android/server/vcn/Vcn.java | 11 ++++++----- .../com/android/server/vcn/VcnGatewayConnection.java | 5 +++-- .../vcn/VcnGatewayConnectionConnectedStateTest.java | 10 +++++----- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java index c7c538521de8e..cccb0968fc6ab 100644 --- a/services/core/java/com/android/server/vcn/Vcn.java +++ b/services/core/java/com/android/server/vcn/Vcn.java @@ -32,6 +32,7 @@ import android.content.ContentResolver; import android.database.ContentObserver; import android.net.NetworkCapabilities; import android.net.NetworkRequest; +import android.net.NetworkScore; import android.net.Uri; import android.net.vcn.VcnConfig; import android.net.vcn.VcnGatewayConnectionConfig; @@ -71,6 +72,8 @@ import java.util.Set; public class Vcn extends Handler { private static final String TAG = Vcn.class.getSimpleName(); + private static final int VCN_LEGACY_SCORE_INT = 52; + private static final List CAPS_REQUIRING_MOBILE_DATA = Arrays.asList(NET_CAPABILITY_INTERNET, NET_CAPABILITY_DUN); @@ -527,11 +530,9 @@ public class Vcn extends Handler { } /** Retrieves the network score for a VCN Network */ - // Package visibility for use in VcnGatewayConnection - static int getNetworkScore() { - // TODO: STOPSHIP (b/173549607): Make this use new NetworkSelection, or some magic "max in - // subGrp" value - return 52; + // Package visibility for use in VcnGatewayConnection and VcnNetworkProvider + static NetworkScore getNetworkScore() { + return new NetworkScore.Builder().setLegacyInt(VCN_LEGACY_SCORE_INT).build(); } /** Callback used for passing status signals from a VcnGatewayConnection to its managing Vcn. */ diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index d8a085b092ef6..38f5dd6c3a8d2 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -49,6 +49,7 @@ import android.net.NetworkAgent; import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; import android.net.NetworkProvider; +import android.net.NetworkScore; import android.net.RouteInfo; import android.net.TelephonyNetworkSpecifier; import android.net.Uri; @@ -2203,7 +2204,7 @@ public class VcnGatewayConnection extends StateMachine { @NonNull String tag, @NonNull NetworkCapabilities caps, @NonNull LinkProperties lp, - @NonNull int score, + @NonNull NetworkScore score, @NonNull NetworkAgentConfig nac, @NonNull NetworkProvider provider, @NonNull Consumer networkUnwantedCallback, @@ -2344,7 +2345,7 @@ public class VcnGatewayConnection extends StateMachine { @NonNull String tag, @NonNull NetworkCapabilities caps, @NonNull LinkProperties lp, - @NonNull int score, + @NonNull NetworkScore score, @NonNull NetworkAgentConfig nac, @NonNull NetworkProvider provider, @NonNull Consumer networkUnwantedCallback, diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java index a2223e8c1e9ac..95a972652bf47 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java @@ -89,7 +89,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection mNetworkAgent = mock(VcnNetworkAgent.class); doReturn(mNetworkAgent) .when(mDeps) - .newNetworkAgent(any(), any(), any(), any(), anyInt(), any(), any(), any(), any()); + .newNetworkAgent(any(), any(), any(), any(), any(), any(), any(), any(), any()); mGatewayConnection.setUnderlyingNetwork(TEST_UNDERLYING_NETWORK_RECORD_1); @@ -216,7 +216,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection any(), any(), any(), - anyInt(), + any(), any(), any(), any(), @@ -244,7 +244,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection any(String.class), ncCaptor.capture(), lpCaptor.capture(), - anyInt(), + any(), argThat(nac -> nac.getLegacyType() == ConnectivityManager.TYPE_MOBILE), any(), any(), @@ -297,7 +297,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection startingInternalAddrs.equals(lp.getLinkAddresses()) && Collections.singletonList(TEST_DNS_ADDR) .equals(lp.getDnsServers())), - anyInt(), + any(), any(), any(), any(), @@ -356,7 +356,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection any(), any(), any(), - anyInt(), + any(), any(), any(), unwantedCallbackCaptor.capture(),