diff options
4 files changed, 129 insertions, 176 deletions
diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index cd2d6d4e4585..d66d82d1f23f 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -36,7 +36,6 @@ import android.content.IntentFilter; import android.net.ConnectivityManager; import android.net.LinkProperties; import android.net.NetworkCapabilities; -import android.net.TelephonyNetworkSpecifier; import android.net.vcn.IVcnManagementService; import android.net.vcn.IVcnStatusCallback; import android.net.vcn.IVcnUnderlyingNetworkPolicyListener; @@ -716,19 +715,29 @@ public class VcnManagementService extends IVcnManagementService.Stub { }); } - private int getSubIdForNetworkCapabilities(@NonNull NetworkCapabilities networkCapabilities) { - if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) - && networkCapabilities.getNetworkSpecifier() instanceof TelephonyNetworkSpecifier) { - TelephonyNetworkSpecifier telephonyNetworkSpecifier = - (TelephonyNetworkSpecifier) networkCapabilities.getNetworkSpecifier(); - return telephonyNetworkSpecifier.getSubscriptionId(); - } else if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) - && networkCapabilities.getTransportInfo() instanceof WifiInfo) { - WifiInfo wifiInfo = (WifiInfo) networkCapabilities.getTransportInfo(); - return mDeps.getSubIdForWifiInfo(wifiInfo); + private ParcelUuid getSubGroupForNetworkCapabilities( + @NonNull NetworkCapabilities networkCapabilities) { + ParcelUuid subGrp = null; + final TelephonySubscriptionSnapshot snapshot; + + // Always access mLastSnapshot under lock. Technically this can be treated as a volatile + // but for consistency and safety, always access under lock. + synchronized (mLock) { + snapshot = mLastSnapshot; + } + + // If multiple subscription IDs exist, they MUST all point to the same subscription + // group. Otherwise undefined behavior may occur. + for (int subId : networkCapabilities.getSubIds()) { + // Verify that all subscriptions point to the same group + if (subGrp != null && !subGrp.equals(snapshot.getGroupForSubId(subId))) { + Slog.wtf(TAG, "Got multiple subscription groups for a single network"); + } + + subGrp = snapshot.getGroupForSubId(subId); } - return SubscriptionManager.INVALID_SUBSCRIPTION_ID; + return subGrp; } /** @@ -753,23 +762,19 @@ public class VcnManagementService extends IVcnManagementService.Stub { // mutates final NetworkCapabilities ncCopy = new NetworkCapabilities(networkCapabilities); - final int subId = getSubIdForNetworkCapabilities(ncCopy); + final ParcelUuid subGrp = getSubGroupForNetworkCapabilities(ncCopy); boolean isVcnManagedNetwork = false; boolean isRestrictedCarrierWifi = false; - if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) { - synchronized (mLock) { - ParcelUuid subGroup = mLastSnapshot.getGroupForSubId(subId); - - final Vcn vcn = mVcns.get(subGroup); - if (vcn != null) { - if (vcn.isActive()) { - isVcnManagedNetwork = true; - } + synchronized (mLock) { + final Vcn vcn = mVcns.get(subGrp); + if (vcn != null) { + if (vcn.isActive()) { + isVcnManagedNetwork = true; + } - if (ncCopy.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) { - // Carrier WiFi always restricted if VCN exists (even in safe mode). - isRestrictedCarrierWifi = true; - } + if (ncCopy.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) { + // Carrier WiFi always restricted if VCN exists (even in safe mode). + isRestrictedCarrierWifi = true; } } } diff --git a/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java b/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java index 6ff8d6f8ba94..069c5c3fc714 100644 --- a/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java +++ b/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java @@ -27,15 +27,14 @@ import android.net.NetworkRequest; import android.net.TelephonyNetworkSpecifier; import android.os.Handler; import android.os.ParcelUuid; -import android.util.ArrayMap; -import android.util.ArraySet; import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting.Visibility; import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; -import java.util.Map; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; import java.util.Set; @@ -59,9 +58,9 @@ public class UnderlyingNetworkTracker { @NonNull private final Handler mHandler; @NonNull private final ConnectivityManager mConnectivityManager; - @NonNull private final Map<Integer, NetworkCallback> mCellBringupCallbacks = new ArrayMap<>(); - @NonNull private final NetworkCallback mWifiBringupCallback = new NetworkBringupCallback(); - @NonNull private final NetworkCallback mRouteSelectionCallback = new RouteSelectionCallback(); + @NonNull private final List<NetworkCallback> mCellBringupCallbacks = new ArrayList<>(); + @Nullable private NetworkCallback mWifiBringupCallback; + @Nullable private NetworkCallback mRouteSelectionCallback; @NonNull private TelephonySubscriptionSnapshot mLastSnapshot; private boolean mIsQuitting = false; @@ -105,36 +104,59 @@ public class UnderlyingNetworkTracker { mConnectivityManager = mVcnContext.getContext().getSystemService(ConnectivityManager.class); - registerNetworkRequests(); + registerOrUpdateNetworkRequests(); } - private void registerNetworkRequests() { - // register bringup requests for underlying Networks - mConnectivityManager.requestBackgroundNetwork( - getWifiNetworkRequest(), mHandler, mWifiBringupCallback); - updateSubIdsAndCellularRequests(); - - // Register Network-selection request used to decide selected underlying Network. All - // underlying networks must be VCN managed in order to be used. - mConnectivityManager.requestBackgroundNetwork( - getBaseNetworkRequest(true /* requireVcnManaged */).build(), - mHandler, - mRouteSelectionCallback); + private void registerOrUpdateNetworkRequests() { + NetworkCallback oldRouteSelectionCallback = mRouteSelectionCallback; + NetworkCallback oldWifiCallback = mWifiBringupCallback; + List<NetworkCallback> oldCellCallbacks = new ArrayList<>(mCellBringupCallbacks); + mCellBringupCallbacks.clear(); + + // Register new callbacks. Make-before-break; always register new callbacks before removal + // of old callbacks + if (!mIsQuitting) { + mRouteSelectionCallback = new RouteSelectionCallback(); + mConnectivityManager.requestBackgroundNetwork( + getBaseNetworkRequestBuilder().build(), mHandler, mRouteSelectionCallback); + + mWifiBringupCallback = new NetworkBringupCallback(); + mConnectivityManager.requestBackgroundNetwork( + getWifiNetworkRequest(), mHandler, mWifiBringupCallback); + + for (final int subId : mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup)) { + final NetworkBringupCallback cb = new NetworkBringupCallback(); + mCellBringupCallbacks.add(cb); + + mConnectivityManager.requestBackgroundNetwork( + getCellNetworkRequestForSubId(subId), mHandler, cb); + } + } else { + mRouteSelectionCallback = null; + mWifiBringupCallback = null; + // mCellBringupCallbacks already cleared above. + } + + // Unregister old callbacks (as necessary) + if (oldRouteSelectionCallback != null) { + mConnectivityManager.unregisterNetworkCallback(oldRouteSelectionCallback); + } + if (oldWifiCallback != null) { + mConnectivityManager.unregisterNetworkCallback(oldWifiCallback); + } + for (NetworkCallback cellBringupCallback : oldCellCallbacks) { + mConnectivityManager.unregisterNetworkCallback(cellBringupCallback); + } } private NetworkRequest getWifiNetworkRequest() { - // Request exclusively VCN managed networks to ensure that we only ever keep carrier wifi - // alive. - return getBaseNetworkRequest(true /* requireVcnManaged */) + return getBaseNetworkRequestBuilder() .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) .build(); } private NetworkRequest getCellNetworkRequestForSubId(int subId) { - // Do not request NOT_VCN_MANAGED to ensure that the TelephonyNetworkFactory has a - // fulfillable request to bring up underlying cellular Networks even if the VCN is already - // connected. - return getBaseNetworkRequest(false /* requireVcnManaged */) + return getBaseNetworkRequestBuilder() .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) .setNetworkSpecifier(new TelephonyNetworkSpecifier(subId)) .build(); @@ -143,67 +165,19 @@ public class UnderlyingNetworkTracker { /** * Builds and returns a NetworkRequest builder common to all Underlying Network requests * - * <p>A NetworkRequest may either (1) Require the presence of a capability by using - * addCapability(), (2) require the absence of a capability using unwanted capabilities, or (3) - * allow any state. Underlying networks are never desired to have the NOT_VCN_MANAGED - * capability, and only cases (2) and (3) are used. - * - * @param requireVcnManaged whether the underlying network is required to be VCN managed to - * match this request. If {@code true}, the NOT_VCN_MANAGED capability will be set as - * unwanted. Else, the NOT_VCN_MANAGED capability will be removed, and any state is - * acceptable. + * <p>This request is guaranteed to select carrier-owned, non-VCN underlying networks by virtue + * of a populated set of subIds as expressed in NetworkCapabilities#getSubIds(). Only carrier + * owned networks may be selected, as the request specifies only subIds in the VCN's + * subscription group, while the VCN networks are excluded by virtue of not having subIds set on + * the VCN-exposed networks. */ - private NetworkRequest.Builder getBaseNetworkRequest(boolean requireVcnManaged) { - NetworkRequest.Builder requestBase = - new NetworkRequest.Builder() - .removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) - .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) - .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED); - - for (int capability : mRequiredUnderlyingNetworkCapabilities) { - requestBase.addCapability(capability); - } - - if (requireVcnManaged) { - requestBase.addUnwantedCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED); - } - - return requestBase; - } - - /** - * Update the current subIds and Cellular bringup requests for this UnderlyingNetworkTracker. - */ - private void updateSubIdsAndCellularRequests() { - mVcnContext.ensureRunningOnLooperThread(); - - // Don't bother re-filing NetworkRequests if this Tracker has been torn down. - if (mIsQuitting) { - return; - } - - final Set<Integer> subIdsInSubGroup = mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup); - - // new subIds to track = (updated list of subIds) - (currently tracked subIds) - final Set<Integer> subIdsToRegister = new ArraySet<>(subIdsInSubGroup); - subIdsToRegister.removeAll(mCellBringupCallbacks.keySet()); - - // subIds to stop tracking = (currently tracked subIds) - (updated list of subIds) - final Set<Integer> subIdsToUnregister = new ArraySet<>(mCellBringupCallbacks.keySet()); - subIdsToUnregister.removeAll(subIdsInSubGroup); - - for (final int subId : subIdsToRegister) { - final NetworkBringupCallback cb = new NetworkBringupCallback(); - mCellBringupCallbacks.put(subId, cb); - - mConnectivityManager.requestBackgroundNetwork( - getCellNetworkRequestForSubId(subId), mHandler, cb); - } - - for (final int subId : subIdsToUnregister) { - final NetworkCallback cb = mCellBringupCallbacks.remove(subId); - mConnectivityManager.unregisterNetworkCallback(cb); - } + private NetworkRequest.Builder getBaseNetworkRequestBuilder() { + return new NetworkRequest.Builder() + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) + .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) + .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED) + .setSubIds(mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup)); } /** @@ -217,22 +191,16 @@ public class UnderlyingNetworkTracker { Objects.requireNonNull(snapshot, "Missing snapshot"); mLastSnapshot = snapshot; - updateSubIdsAndCellularRequests(); + registerOrUpdateNetworkRequests(); } /** Tears down this Tracker, and releases all underlying network requests. */ public void teardown() { mVcnContext.ensureRunningOnLooperThread(); - - mConnectivityManager.unregisterNetworkCallback(mWifiBringupCallback); - mConnectivityManager.unregisterNetworkCallback(mRouteSelectionCallback); - - for (final NetworkCallback cb : mCellBringupCallbacks.values()) { - mConnectivityManager.unregisterNetworkCallback(cb); - } - mCellBringupCallbacks.clear(); - mIsQuitting = true; + + // Will unregister all existing callbacks, but not register new ones due to quitting flag. + registerOrUpdateNetworkRequests(); } /** Returns whether the currently selected Network matches the given network. */ diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index f15d4204d125..c88b0c154712 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -34,8 +34,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; @@ -45,7 +45,6 @@ import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import android.annotation.NonNull; import android.app.AppOpsManager; @@ -63,7 +62,6 @@ import android.net.vcn.VcnConfig; import android.net.vcn.VcnConfigTest; import android.net.vcn.VcnManager; import android.net.vcn.VcnUnderlyingNetworkPolicy; -import android.net.wifi.WifiInfo; import android.os.IBinder; import android.os.ParcelUuid; import android.os.PersistableBundle; @@ -708,22 +706,12 @@ public class VcnManagementServiceTest { int subId, ParcelUuid subGrp, boolean isVcnActive, int transport) { setupSubscriptionAndStartVcn(subId, subGrp, isVcnActive); - final NetworkCapabilities.Builder ncBuilder = new NetworkCapabilities.Builder(); - ncBuilder.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); - if (transport == TRANSPORT_CELLULAR) { - ncBuilder - .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) - .setNetworkSpecifier(new TelephonyNetworkSpecifier(TEST_SUBSCRIPTION_ID)); - } else if (transport == TRANSPORT_WIFI) { - WifiInfo wifiInfo = mock(WifiInfo.class); - when(wifiInfo.makeCopy(anyLong())).thenReturn(wifiInfo); - when(mMockDeps.getSubIdForWifiInfo(eq(wifiInfo))).thenReturn(TEST_SUBSCRIPTION_ID); - - ncBuilder - .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) - .setTransportInfo(wifiInfo); - } else { - throw new IllegalArgumentException("Unknown transport"); + final NetworkCapabilities.Builder ncBuilder = + new NetworkCapabilities.Builder() + .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + .addTransportType(transport); + if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) { + ncBuilder.setSubIds(Collections.singleton(subId)); } return mVcnMgmtSvc.getUnderlyingNetworkPolicy(ncBuilder.build(), new LinkProperties()); diff --git a/tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java b/tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java index 631c862d2bfd..ed2e4d9b9946 100644 --- a/tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java +++ b/tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java @@ -146,40 +146,34 @@ public class UnderlyingNetworkTrackerTest { @Test public void testNetworkCallbacksRegisteredOnStartup() { - // verify NetworkCallbacks registered when instantiated - verify(mConnectivityManager) - .requestBackgroundNetwork( - eq(getWifiRequest()), - any(), - any(NetworkBringupCallback.class)); - verifyBackgroundCellRequests(mSubscriptionSnapshot, SUB_GROUP, INITIAL_SUB_IDS); + verifyNetworkRequestsRegistered(INITIAL_SUB_IDS); + } + private void verifyNetworkRequestsRegistered(Set<Integer> expectedSubIds) { verify(mConnectivityManager) .requestBackgroundNetwork( - eq(getRouteSelectionRequest()), + eq(getWifiRequest(expectedSubIds)), any(), - any(RouteSelectionCallback.class)); - } - - private void verifyBackgroundCellRequests( - TelephonySubscriptionSnapshot snapshot, - ParcelUuid subGroup, - Set<Integer> expectedSubIds) { - verify(snapshot).getAllSubIdsInGroup(eq(subGroup)); - + any(NetworkBringupCallback.class)); for (final int subId : expectedSubIds) { verify(mConnectivityManager) .requestBackgroundNetwork( - eq(getCellRequestForSubId(subId)), + eq(getCellRequestForSubId(subId, expectedSubIds)), any(), any(NetworkBringupCallback.class)); } + + verify(mConnectivityManager) + .requestBackgroundNetwork( + eq(getRouteSelectionRequest(expectedSubIds)), + any(), + any(RouteSelectionCallback.class)); } @Test public void testUpdateSubscriptionSnapshot() { // Verify initial cell background requests filed - verifyBackgroundCellRequests(mSubscriptionSnapshot, SUB_GROUP, INITIAL_SUB_IDS); + verifyNetworkRequestsRegistered(INITIAL_SUB_IDS); TelephonySubscriptionSnapshot subscriptionUpdate = mock(TelephonySubscriptionSnapshot.class); @@ -187,40 +181,38 @@ public class UnderlyingNetworkTrackerTest { mUnderlyingNetworkTracker.updateSubscriptionSnapshot(subscriptionUpdate); - // verify that initially-filed bringup requests are unregistered - verify(mConnectivityManager, times(INITIAL_SUB_IDS.size())) + // verify that initially-filed bringup requests are unregistered (cell + wifi) + verify(mConnectivityManager, times(INITIAL_SUB_IDS.size() + 1)) .unregisterNetworkCallback(any(NetworkBringupCallback.class)); - verifyBackgroundCellRequests(subscriptionUpdate, SUB_GROUP, UPDATED_SUB_IDS); + verify(mConnectivityManager).unregisterNetworkCallback(any(RouteSelectionCallback.class)); + verifyNetworkRequestsRegistered(UPDATED_SUB_IDS); } - private NetworkRequest getWifiRequest() { - return getExpectedRequestBase(true) + private NetworkRequest getWifiRequest(Set<Integer> netCapsSubIds) { + return getExpectedRequestBase(netCapsSubIds) .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) .build(); } - private NetworkRequest getCellRequestForSubId(int subId) { - return getExpectedRequestBase(false) + private NetworkRequest getCellRequestForSubId(int subId, Set<Integer> netCapsSubIds) { + return getExpectedRequestBase(netCapsSubIds) .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) .setNetworkSpecifier(new TelephonyNetworkSpecifier(subId)) .build(); } - private NetworkRequest getRouteSelectionRequest() { - return getExpectedRequestBase(true).build(); + private NetworkRequest getRouteSelectionRequest(Set<Integer> netCapsSubIds) { + return getExpectedRequestBase(netCapsSubIds).build(); } - private NetworkRequest.Builder getExpectedRequestBase(boolean requireVcnManaged) { + private NetworkRequest.Builder getExpectedRequestBase(Set<Integer> subIds) { final NetworkRequest.Builder builder = new NetworkRequest.Builder() .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) .removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) - .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED); - - if (requireVcnManaged) { - builder.addUnwantedCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED); - } + .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED) + .setSubIds(subIds); return builder; } @@ -274,7 +266,7 @@ public class UnderlyingNetworkTrackerTest { NetworkCapabilities networkCapabilities) { verify(mConnectivityManager) .requestBackgroundNetwork( - eq(getRouteSelectionRequest()), + eq(getRouteSelectionRequest(INITIAL_SUB_IDS)), any(), mRouteSelectionCallbackCaptor.capture()); |