From 4153670ea2093547d67caf2b0ad50d8f5317a24c Mon Sep 17 00:00:00 2001 From: Yan Yan Date: Sat, 19 Nov 2022 00:18:42 +0000 Subject: [PATCH 1/2] Store priority class as final in UnderlyingNetworkRecord This commit refactors UnderlyingNetworkRecord to store its priority class as a final field and calculate it during construction. This commit improves the code maintainabiliy in the network selection logic. Bug: 245618320 Test: FrameworksVcnTests, CtsVcnTestCases Change-Id: I346323cb18de935626b2dc6343be41c21fc7f055 --- .../UnderlyingNetworkController.java | 19 +-- .../UnderlyingNetworkRecord.java | 150 +++++++++--------- .../server/vcn/VcnGatewayConnectionTest.java | 8 +- .../vcn/VcnGatewayConnectionTestBase.java | 18 ++- .../NetworkPriorityClassifierTest.java | 66 ++++---- .../UnderlyingNetworkControllerTest.java | 70 +++++--- 6 files changed, 183 insertions(+), 148 deletions(-) diff --git a/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkController.java b/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkController.java index d474c5d33a93f..3acbb5e17e13e 100644 --- a/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkController.java +++ b/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkController.java @@ -356,7 +356,7 @@ public class UnderlyingNetworkController { if (!allNetworkPriorities.isEmpty()) { allNetworkPriorities += ", "; } - allNetworkPriorities += record.network + ": " + record.getPriorityClass(); + allNetworkPriorities += record.network + ": " + record.priorityClass; } logInfo( "Selected network changed to " @@ -393,19 +393,20 @@ public class UnderlyingNetworkController { private TreeSet getSortedUnderlyingNetworks() { TreeSet sorted = - new TreeSet<>( - UnderlyingNetworkRecord.getComparator( + new TreeSet<>(UnderlyingNetworkRecord.getComparator()); + + for (UnderlyingNetworkRecord.Builder builder : + mUnderlyingNetworkRecordBuilders.values()) { + if (builder.isValid()) { + final UnderlyingNetworkRecord record = + builder.build( mVcnContext, mConnectionConfig.getVcnUnderlyingNetworkPriorities(), mSubscriptionGroup, mLastSnapshot, mCurrentRecord, - mCarrierConfig)); - - for (UnderlyingNetworkRecord.Builder builder : - mUnderlyingNetworkRecordBuilders.values()) { - if (builder.isValid()) { - sorted.add(builder.build()); + mCarrierConfig); + sorted.add(record); } } diff --git a/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkRecord.java b/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkRecord.java index 319680e0b01c3..aea9f4d2dbae2 100644 --- a/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkRecord.java +++ b/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkRecord.java @@ -42,53 +42,58 @@ import java.util.Objects; * @hide */ public class UnderlyingNetworkRecord { - private static final int PRIORITY_CLASS_INVALID = Integer.MAX_VALUE; - @NonNull public final Network network; @NonNull public final NetworkCapabilities networkCapabilities; @NonNull public final LinkProperties linkProperties; public final boolean isBlocked; - - private int mPriorityClass = PRIORITY_CLASS_INVALID; + public final boolean isSelected; + public final int priorityClass; @VisibleForTesting(visibility = Visibility.PRIVATE) public UnderlyingNetworkRecord( @NonNull Network network, @NonNull NetworkCapabilities networkCapabilities, @NonNull LinkProperties linkProperties, - boolean isBlocked) { - this.network = network; - this.networkCapabilities = networkCapabilities; - this.linkProperties = linkProperties; - this.isBlocked = isBlocked; - } - - private int getOrCalculatePriorityClass( + boolean isBlocked, VcnContext vcnContext, List underlyingNetworkTemplates, ParcelUuid subscriptionGroup, TelephonySubscriptionSnapshot snapshot, UnderlyingNetworkRecord currentlySelected, PersistableBundleWrapper carrierConfig) { - // Never changes after the underlying network record is created. - if (mPriorityClass == PRIORITY_CLASS_INVALID) { - mPriorityClass = - NetworkPriorityClassifier.calculatePriorityClass( - vcnContext, - this, - underlyingNetworkTemplates, - subscriptionGroup, - snapshot, - currentlySelected, - carrierConfig); - } + this.network = network; + this.networkCapabilities = networkCapabilities; + this.linkProperties = linkProperties; + this.isBlocked = isBlocked; - return mPriorityClass; + this.isSelected = isSelected(this.network, currentlySelected); + + priorityClass = + NetworkPriorityClassifier.calculatePriorityClass( + vcnContext, + this, + underlyingNetworkTemplates, + subscriptionGroup, + snapshot, + currentlySelected, + carrierConfig); } - // Used in UnderlyingNetworkController - int getPriorityClass() { - return mPriorityClass; + @VisibleForTesting(visibility = Visibility.PRIVATE) + public UnderlyingNetworkRecord( + @NonNull Network network, + @NonNull NetworkCapabilities networkCapabilities, + @NonNull LinkProperties linkProperties, + boolean isBlocked, + boolean isSelected, + int priorityClass) { + this.network = network; + this.networkCapabilities = networkCapabilities; + this.linkProperties = linkProperties; + this.isBlocked = isBlocked; + this.isSelected = isSelected; + + this.priorityClass = priorityClass; } @Override @@ -108,40 +113,32 @@ public class UnderlyingNetworkRecord { return Objects.hash(network, networkCapabilities, linkProperties, isBlocked); } - static Comparator getComparator( - VcnContext vcnContext, - List underlyingNetworkTemplates, - ParcelUuid subscriptionGroup, - TelephonySubscriptionSnapshot snapshot, - UnderlyingNetworkRecord currentlySelected, - PersistableBundleWrapper carrierConfig) { + /** Returns if two records are equal including their priority classes. */ + public static boolean isEqualIncludingPriorities( + UnderlyingNetworkRecord left, UnderlyingNetworkRecord right) { + if (left != null && right != null) { + return left.equals(right) + && left.isSelected == right.isSelected + && left.priorityClass == right.priorityClass; + } + + return left == right; + } + + static Comparator getComparator() { return (left, right) -> { - final int leftIndex = - left.getOrCalculatePriorityClass( - vcnContext, - underlyingNetworkTemplates, - subscriptionGroup, - snapshot, - currentlySelected, - carrierConfig); - final int rightIndex = - right.getOrCalculatePriorityClass( - vcnContext, - underlyingNetworkTemplates, - subscriptionGroup, - snapshot, - currentlySelected, - carrierConfig); + final int leftIndex = left.priorityClass; + final int rightIndex = right.priorityClass; // In the case of networks in the same priority class, prioritize based on other // criteria (eg. actively selected network, link metrics, etc) if (leftIndex == rightIndex) { // TODO: Improve the strategy of network selection when both UnderlyingNetworkRecord // fall into the same priority class. - if (isSelected(left, currentlySelected)) { + if (left.isSelected) { return -1; } - if (isSelected(left, currentlySelected)) { + if (right.isSelected) { return 1; } } @@ -150,11 +147,11 @@ public class UnderlyingNetworkRecord { } private static boolean isSelected( - UnderlyingNetworkRecord recordToCheck, UnderlyingNetworkRecord currentlySelected) { + Network networkToCheck, UnderlyingNetworkRecord currentlySelected) { if (currentlySelected == null) { return false; } - if (currentlySelected.network == recordToCheck.network) { + if (currentlySelected.network.equals(networkToCheck)) { return true; } return false; @@ -172,16 +169,8 @@ public class UnderlyingNetworkRecord { pw.println("UnderlyingNetworkRecord:"); pw.increaseIndent(); - final int priorityIndex = - getOrCalculatePriorityClass( - vcnContext, - underlyingNetworkTemplates, - subscriptionGroup, - snapshot, - currentlySelected, - carrierConfig); - - pw.println("Priority index: " + priorityIndex); + pw.println("priorityClass: " + priorityClass); + pw.println("isSelected: " + isSelected); pw.println("mNetwork: " + network); pw.println("mNetworkCapabilities: " + networkCapabilities); pw.println("mLinkProperties: " + linkProperties); @@ -198,8 +187,6 @@ public class UnderlyingNetworkRecord { boolean mIsBlocked; boolean mWasIsBlockedSet; - @Nullable private UnderlyingNetworkRecord mCached; - Builder(@NonNull Network network) { mNetwork = network; } @@ -211,7 +198,6 @@ public class UnderlyingNetworkRecord { void setNetworkCapabilities(@NonNull NetworkCapabilities networkCapabilities) { mNetworkCapabilities = networkCapabilities; - mCached = null; } @Nullable @@ -221,32 +207,40 @@ public class UnderlyingNetworkRecord { void setLinkProperties(@NonNull LinkProperties linkProperties) { mLinkProperties = linkProperties; - mCached = null; } void setIsBlocked(boolean isBlocked) { mIsBlocked = isBlocked; mWasIsBlockedSet = true; - mCached = null; } boolean isValid() { return mNetworkCapabilities != null && mLinkProperties != null && mWasIsBlockedSet; } - UnderlyingNetworkRecord build() { + UnderlyingNetworkRecord build( + VcnContext vcnContext, + List underlyingNetworkTemplates, + ParcelUuid subscriptionGroup, + TelephonySubscriptionSnapshot snapshot, + UnderlyingNetworkRecord currentlySelected, + PersistableBundleWrapper carrierConfig) { if (!isValid()) { throw new IllegalArgumentException( "Called build before UnderlyingNetworkRecord was valid"); } - if (mCached == null) { - mCached = - new UnderlyingNetworkRecord( - mNetwork, mNetworkCapabilities, mLinkProperties, mIsBlocked); - } - - return mCached; + return new UnderlyingNetworkRecord( + mNetwork, + mNetworkCapabilities, + mLinkProperties, + mIsBlocked, + vcnContext, + underlyingNetworkTemplates, + subscriptionGroup, + snapshot, + currentlySelected, + carrierConfig); } } } diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java index a4ee2de9f433e..692c8a8f0898b 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTest.java @@ -143,9 +143,9 @@ public class VcnGatewayConnectionTest extends VcnGatewayConnectionTestBase { capBuilder.setLinkDownstreamBandwidthKbps(TEST_DOWNSTREAM_BANDWIDTH); capBuilder.setAdministratorUids(new int[] {TEST_UID}); final Network underlyingNetwork = mock(Network.class, CALLS_REAL_METHODS); - UnderlyingNetworkRecord record = new UnderlyingNetworkRecord( - underlyingNetwork, - capBuilder.build(), new LinkProperties(), false); + UnderlyingNetworkRecord record = + getTestNetworkRecord( + underlyingNetwork, capBuilder.build(), new LinkProperties(), false); final NetworkCapabilities vcnCaps = VcnGatewayConnection.buildNetworkCapabilities( VcnGatewayConnectionConfigTest.buildTestConfig(), @@ -211,7 +211,7 @@ public class VcnGatewayConnectionTest extends VcnGatewayConnectionTestBase { doReturn(TEST_DNS_ADDRESSES).when(childSessionConfig).getInternalDnsServers(); UnderlyingNetworkRecord record = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mock(Network.class, CALLS_REAL_METHODS), new NetworkCapabilities.Builder().build(), underlyingLp, diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java index 7bafd243799f5..0a18c3d1990d3 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java @@ -109,9 +109,23 @@ public class VcnGatewayConnectionTestBase { protected static final long ELAPSED_REAL_TIME = 123456789L; protected static final String TEST_IPSEC_TUNNEL_IFACE = "IPSEC_IFACE"; + protected static UnderlyingNetworkRecord getTestNetworkRecord( + Network network, + NetworkCapabilities networkCapabilities, + LinkProperties linkProperties, + boolean isBlocked) { + return new UnderlyingNetworkRecord( + network, + networkCapabilities, + linkProperties, + isBlocked, + false /* isSelected */, + 0 /* priorityClass */); + } + protected static final String TEST_TCP_BUFFER_SIZES_1 = "1,2,3,4"; protected static final UnderlyingNetworkRecord TEST_UNDERLYING_NETWORK_RECORD_1 = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mock(Network.class, CALLS_REAL_METHODS), new NetworkCapabilities(), new LinkProperties(), @@ -124,7 +138,7 @@ public class VcnGatewayConnectionTestBase { protected static final String TEST_TCP_BUFFER_SIZES_2 = "2,3,4,5"; protected static final UnderlyingNetworkRecord TEST_UNDERLYING_NETWORK_RECORD_2 = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mock(Network.class, CALLS_REAL_METHODS), new NetworkCapabilities(), new LinkProperties(), diff --git a/tests/vcn/java/com/android/server/vcn/routeselection/NetworkPriorityClassifierTest.java b/tests/vcn/java/com/android/server/vcn/routeselection/NetworkPriorityClassifierTest.java index b0d68952c39d1..bb7da5604e169 100644 --- a/tests/vcn/java/com/android/server/vcn/routeselection/NetworkPriorityClassifierTest.java +++ b/tests/vcn/java/com/android/server/vcn/routeselection/NetworkPriorityClassifierTest.java @@ -25,7 +25,6 @@ import static android.net.vcn.VcnUnderlyingNetworkTemplateTestBase.TEST_MIN_EXIT import static com.android.server.vcn.VcnTestUtils.setupSystemService; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.PRIORITY_ANY; -import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.calculatePriorityClass; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.checkMatchesCellPriorityRule; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.checkMatchesPriorityRule; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.checkMatchesWifiPriorityRule; @@ -48,6 +47,7 @@ import android.net.TelephonyNetworkSpecifier; import android.net.vcn.VcnCellUnderlyingNetworkTemplate; import android.net.vcn.VcnGatewayConnectionConfig; import android.net.vcn.VcnManager; +import android.net.vcn.VcnUnderlyingNetworkTemplate; import android.net.vcn.VcnWifiUnderlyingNetworkTemplate; import android.os.ParcelUuid; import android.os.PersistableBundle; @@ -64,6 +64,7 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.List; import java.util.Set; import java.util.UUID; @@ -135,25 +136,35 @@ public class NetworkPriorityClassifierTest { false /* isInTestMode */)); doNothing().when(mVcnContext).ensureRunningOnLooperThread(); - mWifiNetworkRecord = - new UnderlyingNetworkRecord( - mNetwork, - WIFI_NETWORK_CAPABILITIES, - LINK_PROPERTIES, - false /* isBlocked */); - - mCellNetworkRecord = - new UnderlyingNetworkRecord( - mNetwork, - CELL_NETWORK_CAPABILITIES, - LINK_PROPERTIES, - false /* isBlocked */); - setupSystemService( mockContext, mTelephonyManager, Context.TELEPHONY_SERVICE, TelephonyManager.class); when(mTelephonyManager.createForSubscriptionId(SUB_ID)).thenReturn(mTelephonyManager); when(mTelephonyManager.getNetworkOperator()).thenReturn(PLMN_ID); when(mTelephonyManager.getSimSpecificCarrierId()).thenReturn(CARRIER_ID); + + mWifiNetworkRecord = + getTestNetworkRecord( + WIFI_NETWORK_CAPABILITIES, + VcnGatewayConnectionConfig.DEFAULT_UNDERLYING_NETWORK_TEMPLATES); + mCellNetworkRecord = + getTestNetworkRecord( + CELL_NETWORK_CAPABILITIES, + VcnGatewayConnectionConfig.DEFAULT_UNDERLYING_NETWORK_TEMPLATES); + } + + private UnderlyingNetworkRecord getTestNetworkRecord( + NetworkCapabilities nc, List underlyingNetworkTemplates) { + return new UnderlyingNetworkRecord( + mNetwork, + nc, + LINK_PROPERTIES, + false /* isBlocked */, + mVcnContext, + underlyingNetworkTemplates, + SUB_GROUP, + mSubscriptionSnapshot, + null /* currentlySelected */, + null /* carrierConfig */); } @Test @@ -490,24 +501,9 @@ public class NetworkPriorityClassifierTest { mSubscriptionSnapshot)); } - private void verifyCalculatePriorityClass( - UnderlyingNetworkRecord networkRecord, int expectedIndex) { - final int priorityIndex = - calculatePriorityClass( - mVcnContext, - networkRecord, - VcnGatewayConnectionConfig.DEFAULT_UNDERLYING_NETWORK_TEMPLATES, - SUB_GROUP, - mSubscriptionSnapshot, - null /* currentlySelected */, - null /* carrierConfig */); - - assertEquals(expectedIndex, priorityIndex); - } - @Test public void testCalculatePriorityClass() throws Exception { - verifyCalculatePriorityClass(mCellNetworkRecord, 2); + assertEquals(2, mCellNetworkRecord.priorityClass); } @Test @@ -518,9 +514,11 @@ public class NetworkPriorityClassifierTest { .setSignalStrength(WIFI_RSSI_LOW) .setSsid(SSID) .build(); - final UnderlyingNetworkRecord wifiNetworkRecord = - new UnderlyingNetworkRecord(mNetwork, nc, LINK_PROPERTIES, false /* isBlocked */); - verifyCalculatePriorityClass(wifiNetworkRecord, PRIORITY_ANY); + final UnderlyingNetworkRecord wifiNetworkRecord = + getTestNetworkRecord( + nc, VcnGatewayConnectionConfig.DEFAULT_UNDERLYING_NETWORK_TEMPLATES); + + assertEquals(PRIORITY_ANY, wifiNetworkRecord.priorityClass); } } diff --git a/tests/vcn/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java b/tests/vcn/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java index fad9669911bb1..73d0a280916a9 100644 --- a/tests/vcn/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java +++ b/tests/vcn/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java @@ -21,7 +21,9 @@ import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.WI import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.WIFI_EXIT_RSSI_THRESHOLD_DEFAULT; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; @@ -321,29 +323,55 @@ public class UnderlyingNetworkControllerTest { .unregisterNetworkCallback(any(UnderlyingNetworkListener.class)); } + private static UnderlyingNetworkRecord getTestNetworkRecord( + Network network, + NetworkCapabilities networkCapabilities, + LinkProperties linkProperties, + boolean isBlocked) { + return new UnderlyingNetworkRecord( + network, + networkCapabilities, + linkProperties, + isBlocked, + false /* isSelected */, + 0 /* priorityClass */); + } + @Test public void testUnderlyingNetworkRecordEquals() { UnderlyingNetworkRecord recordA = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mNetwork, INITIAL_NETWORK_CAPABILITIES, INITIAL_LINK_PROPERTIES, false /* isBlocked */); UnderlyingNetworkRecord recordB = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mNetwork, INITIAL_NETWORK_CAPABILITIES, INITIAL_LINK_PROPERTIES, false /* isBlocked */); UnderlyingNetworkRecord recordC = new UnderlyingNetworkRecord( + mNetwork, + INITIAL_NETWORK_CAPABILITIES, + INITIAL_LINK_PROPERTIES, + false /* isBlocked */, + true /* isSelected */, + -1 /* priorityClass */); + UnderlyingNetworkRecord recordD = + getTestNetworkRecord( mNetwork, UPDATED_NETWORK_CAPABILITIES, UPDATED_LINK_PROPERTIES, false /* isBlocked */); assertEquals(recordA, recordB); - assertNotEquals(recordA, recordC); + assertEquals(recordA, recordC); + assertNotEquals(recordA, recordD); + + assertTrue(UnderlyingNetworkRecord.isEqualIncludingPriorities(recordA, recordB)); + assertFalse(UnderlyingNetworkRecord.isEqualIncludingPriorities(recordA, recordC)); } @Test @@ -366,6 +394,10 @@ public class UnderlyingNetworkControllerTest { .build(); } + private void verifyOnSelectedUnderlyingNetworkChanged(UnderlyingNetworkRecord expectedRecord) { + verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + } + private UnderlyingNetworkListener verifyRegistrationOnAvailableAndGetCallback( NetworkCapabilities networkCapabilities) { verify(mConnectivityManager) @@ -384,12 +416,12 @@ public class UnderlyingNetworkControllerTest { cb.onBlockedStatusChanged(mNetwork, false /* isFalse */); UnderlyingNetworkRecord expectedRecord = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mNetwork, responseNetworkCaps, INITIAL_LINK_PROPERTIES, false /* isBlocked */); - verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); return cb; } @@ -402,12 +434,12 @@ public class UnderlyingNetworkControllerTest { cb.onCapabilitiesChanged(mNetwork, responseNetworkCaps); UnderlyingNetworkRecord expectedRecord = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mNetwork, responseNetworkCaps, INITIAL_LINK_PROPERTIES, false /* isBlocked */); - verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); } @Test @@ -417,12 +449,12 @@ public class UnderlyingNetworkControllerTest { cb.onLinkPropertiesChanged(mNetwork, UPDATED_LINK_PROPERTIES); UnderlyingNetworkRecord expectedRecord = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mNetwork, buildResponseNwCaps(INITIAL_NETWORK_CAPABILITIES, INITIAL_SUB_IDS), UPDATED_LINK_PROPERTIES, false /* isBlocked */); - verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); } @Test @@ -434,18 +466,16 @@ public class UnderlyingNetworkControllerTest { cb.onCapabilitiesChanged(mNetwork, responseNetworkCaps); UnderlyingNetworkRecord expectedRecord = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mNetwork, responseNetworkCaps, INITIAL_LINK_PROPERTIES, false /* isBlocked */); - verify(mNetworkControllerCb, times(1)) - .onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); // onSelectedUnderlyingNetworkChanged() won't be fired twice if network capabilities doesn't // change. cb.onCapabilitiesChanged(mNetwork, responseNetworkCaps); - verify(mNetworkControllerCb, times(1)) - .onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); } @Test @@ -458,18 +488,16 @@ public class UnderlyingNetworkControllerTest { cb.onCapabilitiesChanged(mNetwork, responseNetworkCaps); UnderlyingNetworkRecord expectedRecord = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mNetwork, responseNetworkCaps, INITIAL_LINK_PROPERTIES, false /* isBlocked */); - verify(mNetworkControllerCb, times(1)) - .onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); // onSelectedUnderlyingNetworkChanged() won't be fired twice if network capabilities doesn't // change. cb.onCapabilitiesChanged(mNetwork, responseNetworkCaps); - verify(mNetworkControllerCb, times(1)) - .onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); } @Test @@ -479,12 +507,12 @@ public class UnderlyingNetworkControllerTest { cb.onBlockedStatusChanged(mNetwork, true /* isBlocked */); UnderlyingNetworkRecord expectedRecord = - new UnderlyingNetworkRecord( + getTestNetworkRecord( mNetwork, buildResponseNwCaps(INITIAL_NETWORK_CAPABILITIES, INITIAL_SUB_IDS), INITIAL_LINK_PROPERTIES, true /* isBlocked */); - verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(expectedRecord)); + verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); } @Test From 4b3eb31c805bfa6ff62f573921c6c56d08e1ddee Mon Sep 17 00:00:00 2001 From: Yan Yan Date: Sat, 19 Nov 2022 00:18:42 +0000 Subject: [PATCH 2/2] Support selecting non-INTERNET networks as underlying networks This commit allows VCN to select underlying Cell networks according to caller configured network capabilities instead of always only selecting INTERNET networks as underlying networks. This commit also makes sure that if all of the network candidates fail to match caller configured network templates, VCN can still select an INTERNET network as a fallback but will never select a non-INTERNET network. Bug: 245618320 Test: FrameworksVcnTests(new tests), CtsVcnTestCases Change-Id: I6eca0331e4bc6804a500fbed9909bb91b1e80d2d --- .../vcn/VcnCellUnderlyingNetworkTemplate.java | 5 + .../NetworkPriorityClassifier.java | 44 ++- .../UnderlyingNetworkController.java | 123 +++++-- .../VcnCellUnderlyingNetworkTemplateTest.java | 3 +- .../vcn/VcnGatewayConnectionConfigTest.java | 12 +- .../NetworkPriorityClassifierTest.java | 76 ++++- .../UnderlyingNetworkControllerTest.java | 314 +++++++++++++++++- 7 files changed, 519 insertions(+), 58 deletions(-) diff --git a/core/java/android/net/vcn/VcnCellUnderlyingNetworkTemplate.java b/core/java/android/net/vcn/VcnCellUnderlyingNetworkTemplate.java index c3dba33ab8811..38b3174abd4c1 100644 --- a/core/java/android/net/vcn/VcnCellUnderlyingNetworkTemplate.java +++ b/core/java/android/net/vcn/VcnCellUnderlyingNetworkTemplate.java @@ -353,6 +353,11 @@ public final class VcnCellUnderlyingNetworkTemplate extends VcnUnderlyingNetwork return mCapabilitiesMatchCriteria.get(NET_CAPABILITY_RCS); } + /** @hide */ + public Map getCapabilitiesMatchCriteria() { + return Collections.unmodifiableMap(new HashMap<>(mCapabilitiesMatchCriteria)); + } + @Override public int hashCode() { return Objects.hash( diff --git a/services/core/java/com/android/server/vcn/routeselection/NetworkPriorityClassifier.java b/services/core/java/com/android/server/vcn/routeselection/NetworkPriorityClassifier.java index 2f84fddc72789..2141eba3be50c 100644 --- a/services/core/java/com/android/server/vcn/routeselection/NetworkPriorityClassifier.java +++ b/services/core/java/com/android/server/vcn/routeselection/NetworkPriorityClassifier.java @@ -15,6 +15,7 @@ */ package com.android.server.vcn.routeselection; +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.TRANSPORT_CELLULAR; @@ -45,6 +46,7 @@ import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscription import com.android.server.vcn.VcnContext; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; @@ -69,9 +71,23 @@ class NetworkPriorityClassifier { @VisibleForTesting(visibility = Visibility.PRIVATE) static final int WIFI_EXIT_RSSI_THRESHOLD_DEFAULT = -74; - /** Priority for any other networks (including unvalidated, etc) */ + /** + * Priority for networks that VCN can fall back to. + * + *

If none of the network candidates are validated or match any template, VCN will fall back + * to any INTERNET network. + */ @VisibleForTesting(visibility = Visibility.PRIVATE) - static final int PRIORITY_ANY = Integer.MAX_VALUE; + static final int PRIORITY_FALLBACK = Integer.MAX_VALUE; + + /** + * Priority for networks that cannot be selected as VCN's underlying networks. + * + *

VCN MUST never select a non-INTERNET network that are unvalidated or fail to match any + * template as the underlying network. + */ + @VisibleForTesting(visibility = Visibility.PRIVATE) + static final int PRIORITY_INVALID = -1; /** Gives networks a priority class, based on configured VcnGatewayConnectionConfig */ public static int calculatePriorityClass( @@ -86,12 +102,12 @@ class NetworkPriorityClassifier { if (networkRecord.isBlocked) { logWtf("Network blocked for System Server: " + networkRecord.network); - return PRIORITY_ANY; + return PRIORITY_INVALID; } if (snapshot == null) { logWtf("Got null snapshot"); - return PRIORITY_ANY; + return PRIORITY_INVALID; } int priorityIndex = 0; @@ -108,7 +124,13 @@ class NetworkPriorityClassifier { } priorityIndex++; } - return PRIORITY_ANY; + + final NetworkCapabilities caps = networkRecord.networkCapabilities; + if (caps.hasCapability(NET_CAPABILITY_INTERNET) + || (vcnContext.isInTestMode() && caps.hasTransport(TRANSPORT_TEST))) { + return PRIORITY_FALLBACK; + } + return PRIORITY_INVALID; } @VisibleForTesting(visibility = Visibility.PRIVATE) @@ -297,6 +319,18 @@ class NetworkPriorityClassifier { return false; } + for (Map.Entry entry : + networkPriority.getCapabilitiesMatchCriteria().entrySet()) { + final int cap = entry.getKey(); + final int matchCriteria = entry.getValue(); + + if (matchCriteria == MATCH_REQUIRED && !caps.hasCapability(cap)) { + return false; + } else if (matchCriteria == MATCH_FORBIDDEN && caps.hasCapability(cap)) { + return false; + } + } + return true; } diff --git a/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkController.java b/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkController.java index 3acbb5e17e13e..6afa795e96fa1 100644 --- a/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkController.java +++ b/services/core/java/com/android/server/vcn/routeselection/UnderlyingNetworkController.java @@ -16,6 +16,9 @@ package com.android.server.vcn.routeselection; +import static android.net.vcn.VcnUnderlyingNetworkTemplate.MATCH_ANY; +import static android.net.vcn.VcnUnderlyingNetworkTemplate.MATCH_FORBIDDEN; +import static android.net.vcn.VcnUnderlyingNetworkTemplate.MATCH_REQUIRED; import static android.telephony.TelephonyCallback.ActiveDataSubscriptionIdListener; import static com.android.server.VcnManagementService.LOCAL_LOG; @@ -32,6 +35,7 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkRequest; import android.net.TelephonyNetworkSpecifier; +import android.net.vcn.VcnCellUnderlyingNetworkTemplate; import android.net.vcn.VcnGatewayConnectionConfig; import android.net.vcn.VcnUnderlyingNetworkTemplate; import android.os.Handler; @@ -40,6 +44,7 @@ import android.os.ParcelUuid; import android.telephony.TelephonyCallback; import android.telephony.TelephonyManager; import android.util.ArrayMap; +import android.util.ArraySet; import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; @@ -49,6 +54,7 @@ import com.android.server.vcn.VcnContext; import com.android.server.vcn.util.LogUtils; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -126,6 +132,63 @@ public class UnderlyingNetworkController { registerOrUpdateNetworkRequests(); } + private static class CapabilityMatchCriteria { + public final int capability; + public final int matchCriteria; + + CapabilityMatchCriteria(int capability, int matchCriteria) { + this.capability = capability; + this.matchCriteria = matchCriteria; + } + + @Override + public int hashCode() { + return Objects.hash(capability, matchCriteria); + } + + @Override + public boolean equals(@Nullable Object other) { + if (!(other instanceof CapabilityMatchCriteria)) { + return false; + } + + final CapabilityMatchCriteria rhs = (CapabilityMatchCriteria) other; + return capability == rhs.capability && matchCriteria == rhs.matchCriteria; + } + } + + private static Set> dedupAndGetCapRequirementsForCell( + VcnGatewayConnectionConfig connectionConfig) { + final Set> dedupedCapsMatchSets = new ArraySet<>(); + + for (VcnUnderlyingNetworkTemplate template : + connectionConfig.getVcnUnderlyingNetworkPriorities()) { + if (template instanceof VcnCellUnderlyingNetworkTemplate) { + final Set capsMatchSet = new ArraySet<>(); + + for (Map.Entry entry : + ((VcnCellUnderlyingNetworkTemplate) template) + .getCapabilitiesMatchCriteria() + .entrySet()) { + + final int capability = entry.getKey(); + final int matchCriteria = entry.getValue(); + if (matchCriteria != MATCH_ANY) { + capsMatchSet.add(new CapabilityMatchCriteria(capability, matchCriteria)); + } + } + + dedupedCapsMatchSets.add(capsMatchSet); + } + } + + dedupedCapsMatchSets.add( + Collections.singleton( + new CapabilityMatchCriteria( + NetworkCapabilities.NET_CAPABILITY_INTERNET, MATCH_REQUIRED))); + return dedupedCapsMatchSets; + } + private void registerOrUpdateNetworkRequests() { NetworkCallback oldRouteSelectionCallback = mRouteSelectionCallback; NetworkCallback oldWifiCallback = mWifiBringupCallback; @@ -158,11 +221,14 @@ public class UnderlyingNetworkController { getWifiNetworkRequest(), mWifiBringupCallback, mHandler); for (final int subId : mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup)) { - final NetworkBringupCallback cb = new NetworkBringupCallback(); - mCellBringupCallbacks.add(cb); + for (Set capsMatchCriteria : + dedupAndGetCapRequirementsForCell(mConnectionConfig)) { + final NetworkBringupCallback cb = new NetworkBringupCallback(); + mCellBringupCallbacks.add(cb); - mConnectivityManager.requestBackgroundNetwork( - getCellNetworkRequestForSubId(subId), cb, mHandler); + mConnectivityManager.requestBackgroundNetwork( + getCellNetworkRequestForSubId(subId, capsMatchCriteria), cb, mHandler); + } } } else { mRouteSelectionCallback = null; @@ -214,6 +280,13 @@ public class UnderlyingNetworkController { .build(); } + private NetworkRequest.Builder getBaseWifiNetworkRequestBuilder() { + return getBaseNetworkRequestBuilder() + .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .setSubscriptionIds(mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup)); + } + /** * Builds the WiFi bringup request * @@ -224,10 +297,7 @@ public class UnderlyingNetworkController { * but will NEVER bring up a Carrier WiFi network itself. */ private NetworkRequest getWifiNetworkRequest() { - return getBaseNetworkRequestBuilder() - .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) - .setSubscriptionIds(mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup)) - .build(); + return getBaseWifiNetworkRequestBuilder().build(); } /** @@ -238,9 +308,7 @@ public class UnderlyingNetworkController { * pace to effectively select a short-lived WiFi offload network. */ private NetworkRequest getWifiEntryRssiThresholdNetworkRequest() { - return getBaseNetworkRequestBuilder() - .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) - .setSubscriptionIds(mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup)) + return getBaseWifiNetworkRequestBuilder() // Ensure wifi updates signal strengths when crossing this threshold. .setSignalStrength(getWifiEntryRssiThreshold(mCarrierConfig)) .build(); @@ -254,9 +322,7 @@ public class UnderlyingNetworkController { * pace to effectively select away from a failing WiFi network. */ private NetworkRequest getWifiExitRssiThresholdNetworkRequest() { - return getBaseNetworkRequestBuilder() - .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) - .setSubscriptionIds(mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup)) + return getBaseWifiNetworkRequestBuilder() // Ensure wifi updates signal strengths when crossing this threshold. .setSignalStrength(getWifiExitRssiThreshold(mCarrierConfig)) .build(); @@ -273,11 +339,25 @@ public class UnderlyingNetworkController { *

Since this request MUST make it to the TelephonyNetworkFactory, subIds are not specified * in the NetworkCapabilities, but rather in the TelephonyNetworkSpecifier. */ - private NetworkRequest getCellNetworkRequestForSubId(int subId) { - return getBaseNetworkRequestBuilder() - .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) - .setNetworkSpecifier(new TelephonyNetworkSpecifier(subId)) - .build(); + private NetworkRequest getCellNetworkRequestForSubId( + int subId, Set capsMatchCriteria) { + final NetworkRequest.Builder nrBuilder = + getBaseNetworkRequestBuilder() + .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) + .setNetworkSpecifier(new TelephonyNetworkSpecifier(subId)); + + for (CapabilityMatchCriteria capMatchCriteria : capsMatchCriteria) { + final int cap = capMatchCriteria.capability; + final int matchCriteria = capMatchCriteria.matchCriteria; + + if (matchCriteria == MATCH_REQUIRED) { + nrBuilder.addCapability(cap); + } else if (matchCriteria == MATCH_FORBIDDEN) { + nrBuilder.addForbiddenCapability(cap); + } + } + + return nrBuilder.build(); } /** @@ -285,7 +365,6 @@ public class UnderlyingNetworkController { */ 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); @@ -406,7 +485,9 @@ public class UnderlyingNetworkController { mLastSnapshot, mCurrentRecord, mCarrierConfig); - sorted.add(record); + if (record.priorityClass != NetworkPriorityClassifier.PRIORITY_INVALID) { + sorted.add(record); + } } } diff --git a/tests/vcn/java/android/net/vcn/VcnCellUnderlyingNetworkTemplateTest.java b/tests/vcn/java/android/net/vcn/VcnCellUnderlyingNetworkTemplateTest.java index 1f6bb2150643d..1569613123239 100644 --- a/tests/vcn/java/android/net/vcn/VcnCellUnderlyingNetworkTemplateTest.java +++ b/tests/vcn/java/android/net/vcn/VcnCellUnderlyingNetworkTemplateTest.java @@ -32,7 +32,8 @@ public class VcnCellUnderlyingNetworkTemplateTest extends VcnUnderlyingNetworkTe private static final Set ALLOWED_PLMN_IDS = new HashSet<>(); private static final Set ALLOWED_CARRIER_IDS = new HashSet<>(); - private static VcnCellUnderlyingNetworkTemplate.Builder getTestNetworkTemplateBuilder() { + // Public for use in UnderlyingNetworkControllerTest + public static VcnCellUnderlyingNetworkTemplate.Builder getTestNetworkTemplateBuilder() { return new VcnCellUnderlyingNetworkTemplate.Builder() .setMetered(MATCH_FORBIDDEN) .setMinUpstreamBandwidthKbps( diff --git a/tests/vcn/java/android/net/vcn/VcnGatewayConnectionConfigTest.java b/tests/vcn/java/android/net/vcn/VcnGatewayConnectionConfigTest.java index 40408880a2c68..1883c85b5249f 100644 --- a/tests/vcn/java/android/net/vcn/VcnGatewayConnectionConfigTest.java +++ b/tests/vcn/java/android/net/vcn/VcnGatewayConnectionConfigTest.java @@ -100,14 +100,20 @@ public class VcnGatewayConnectionConfigTest { EXPOSED_CAPS); } - // Public for use in VcnGatewayConnectionTest - public static VcnGatewayConnectionConfig buildTestConfig() { + // Public for use in UnderlyingNetworkControllerTest + public static VcnGatewayConnectionConfig buildTestConfig( + List nwTemplates) { final VcnGatewayConnectionConfig.Builder builder = - newBuilder().setVcnUnderlyingNetworkPriorities(UNDERLYING_NETWORK_TEMPLATES); + newBuilder().setVcnUnderlyingNetworkPriorities(nwTemplates); return buildTestConfigWithExposedCaps(builder, EXPOSED_CAPS); } + // Public for use in VcnGatewayConnectionTest + public static VcnGatewayConnectionConfig buildTestConfig() { + return buildTestConfig(UNDERLYING_NETWORK_TEMPLATES); + } + private static VcnGatewayConnectionConfig.Builder newBuilder() { // Append a unique identifier to the name prefix to guarantee that all created // VcnGatewayConnectionConfigs have a unique name (required by VcnConfig). diff --git a/tests/vcn/java/com/android/server/vcn/routeselection/NetworkPriorityClassifierTest.java b/tests/vcn/java/com/android/server/vcn/routeselection/NetworkPriorityClassifierTest.java index bb7da5604e169..629e988495ccc 100644 --- a/tests/vcn/java/com/android/server/vcn/routeselection/NetworkPriorityClassifierTest.java +++ b/tests/vcn/java/com/android/server/vcn/routeselection/NetworkPriorityClassifierTest.java @@ -16,6 +16,7 @@ package com.android.server.vcn.routeselection; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.vcn.VcnUnderlyingNetworkTemplate.MATCH_FORBIDDEN; import static android.net.vcn.VcnUnderlyingNetworkTemplate.MATCH_REQUIRED; import static android.net.vcn.VcnUnderlyingNetworkTemplateTestBase.TEST_MIN_ENTRY_DOWNSTREAM_BANDWIDTH_KBPS; @@ -24,7 +25,8 @@ import static android.net.vcn.VcnUnderlyingNetworkTemplateTestBase.TEST_MIN_EXIT import static android.net.vcn.VcnUnderlyingNetworkTemplateTestBase.TEST_MIN_EXIT_UPSTREAM_BANDWIDTH_KBPS; import static com.android.server.vcn.VcnTestUtils.setupSystemService; -import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.PRIORITY_ANY; +import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.PRIORITY_FALLBACK; +import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.PRIORITY_INVALID; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.checkMatchesCellPriorityRule; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.checkMatchesPriorityRule; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.checkMatchesWifiPriorityRule; @@ -64,6 +66,7 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.Collections; import java.util.List; import java.util.Set; import java.util.UUID; @@ -103,6 +106,7 @@ public class NetworkPriorityClassifierTest { private static final NetworkCapabilities CELL_NETWORK_CAPABILITIES = new NetworkCapabilities.Builder() .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .addCapability(NetworkCapabilities.NET_CAPABILITY_DUN) .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) .setSubscriptionIds(Set.of(SUB_ID)) .setNetworkSpecifier(TEL_NETWORK_SPECIFIER) @@ -501,24 +505,72 @@ public class NetworkPriorityClassifierTest { mSubscriptionSnapshot)); } + private void verifyMatchCellWithRequiredCapabilities( + VcnCellUnderlyingNetworkTemplate template, boolean expectMatch) { + assertEquals( + expectMatch, + checkMatchesCellPriorityRule( + mVcnContext, + template, + mCellNetworkRecord, + SUB_GROUP, + mSubscriptionSnapshot)); + } + + @Test + public void testMatchCell() { + final VcnCellUnderlyingNetworkTemplate template = + getCellNetworkPriorityBuilder().setInternet(MATCH_REQUIRED).build(); + verifyMatchCellWithRequiredCapabilities(template, true /* expectMatch */); + } + + @Test + public void testMatchCellFail_RequiredCapabilitiesMissing() { + final VcnCellUnderlyingNetworkTemplate template = + getCellNetworkPriorityBuilder().setCbs(MATCH_REQUIRED).build(); + verifyMatchCellWithRequiredCapabilities(template, false /* expectMatch */); + } + + @Test + public void testMatchCellFail_ForbiddenCapabilitiesFound() { + final VcnCellUnderlyingNetworkTemplate template = + getCellNetworkPriorityBuilder().setDun(MATCH_FORBIDDEN).build(); + verifyMatchCellWithRequiredCapabilities(template, false /* expectMatch */); + } + @Test public void testCalculatePriorityClass() throws Exception { assertEquals(2, mCellNetworkRecord.priorityClass); } - @Test - public void testCalculatePriorityClassFailToMatchAny() throws Exception { - final NetworkCapabilities nc = + private void checkCalculatePriorityClassFailToMatchAny( + boolean hasInternet, int expectedPriorityClass) throws Exception { + final List templatesRequireDun = + Collections.singletonList( + new VcnCellUnderlyingNetworkTemplate.Builder() + .setDun(MATCH_REQUIRED) + .build()); + + final NetworkCapabilities.Builder ncBuilder = new NetworkCapabilities.Builder() - .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) - .setSignalStrength(WIFI_RSSI_LOW) - .setSsid(SSID) - .build(); + .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR); + if (hasInternet) { + ncBuilder.addCapability(NET_CAPABILITY_INTERNET); + } - final UnderlyingNetworkRecord wifiNetworkRecord = - getTestNetworkRecord( - nc, VcnGatewayConnectionConfig.DEFAULT_UNDERLYING_NETWORK_TEMPLATES); + final UnderlyingNetworkRecord nonDunNetworkRecord = + getTestNetworkRecord(ncBuilder.build(), templatesRequireDun); - assertEquals(PRIORITY_ANY, wifiNetworkRecord.priorityClass); + assertEquals(expectedPriorityClass, nonDunNetworkRecord.priorityClass); + } + + @Test + public void testCalculatePriorityClassFailToMatchAny_InternetNetwork() throws Exception { + checkCalculatePriorityClassFailToMatchAny(true /* hasInternet */, PRIORITY_FALLBACK); + } + + @Test + public void testCalculatePriorityClassFailToMatchAny_NonInternetNetwork() throws Exception { + checkCalculatePriorityClassFailToMatchAny(false /* hasInternet */, PRIORITY_INVALID); } } diff --git a/tests/vcn/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java b/tests/vcn/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java index 73d0a280916a9..2941fdea20bb9 100644 --- a/tests/vcn/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java +++ b/tests/vcn/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java @@ -16,6 +16,14 @@ package com.android.server.vcn.routeselection; +import static android.net.NetworkCapabilities.NET_CAPABILITY_CBS; +import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; +import static android.net.NetworkCapabilities.NET_CAPABILITY_MMS; +import static android.net.vcn.VcnCellUnderlyingNetworkTemplate.MATCH_ANY; +import static android.net.vcn.VcnCellUnderlyingNetworkTemplate.MATCH_FORBIDDEN; +import static android.net.vcn.VcnCellUnderlyingNetworkTemplate.MATCH_REQUIRED; + import static com.android.server.vcn.VcnTestUtils.setupSystemService; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.WIFI_ENTRY_RSSI_THRESHOLD_DEFAULT; import static com.android.server.vcn.routeselection.NetworkPriorityClassifier.WIFI_EXIT_RSSI_THRESHOLD_DEFAULT; @@ -30,6 +38,7 @@ import static org.mockito.Mockito.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -44,7 +53,10 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkRequest; import android.net.TelephonyNetworkSpecifier; +import android.net.vcn.VcnCellUnderlyingNetworkTemplate; +import android.net.vcn.VcnCellUnderlyingNetworkTemplateTest; import android.net.vcn.VcnGatewayConnectionConfigTest; +import android.net.vcn.VcnUnderlyingNetworkTemplate; import android.os.ParcelUuid; import android.os.test.TestLooper; import android.telephony.CarrierConfigManager; @@ -66,7 +78,10 @@ import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.UUID; @@ -97,11 +112,39 @@ public class UnderlyingNetworkControllerTest { .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) .build(); + private static final NetworkCapabilities DUN_NETWORK_CAPABILITIES = + new NetworkCapabilities.Builder() + .addCapability(NetworkCapabilities.NET_CAPABILITY_DUN) + .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED) + .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) + .removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) + .build(); + + private static final NetworkCapabilities CBS_NETWORK_CAPABILITIES = + new NetworkCapabilities.Builder() + .addCapability(NetworkCapabilities.NET_CAPABILITY_CBS) + .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED) + .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) + .removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) + .build(); + private static final LinkProperties INITIAL_LINK_PROPERTIES = getLinkPropertiesWithName("initial_iface"); private static final LinkProperties UPDATED_LINK_PROPERTIES = getLinkPropertiesWithName("updated_iface"); + private static final VcnCellUnderlyingNetworkTemplate CELL_TEMPLATE_DUN = + new VcnCellUnderlyingNetworkTemplate.Builder() + .setInternet(MATCH_ANY) + .setDun(MATCH_REQUIRED) + .build(); + + private static final VcnCellUnderlyingNetworkTemplate CELL_TEMPLATE_CBS = + new VcnCellUnderlyingNetworkTemplate.Builder() + .setInternet(MATCH_ANY) + .setCbs(MATCH_REQUIRED) + .build(); + @Mock private Context mContext; @Mock private VcnNetworkProvider mVcnNetworkProvider; @Mock private ConnectivityManager mConnectivityManager; @@ -203,6 +246,107 @@ public class UnderlyingNetworkControllerTest { any()); } + private void verifyRequestBackgroundNetwork( + ConnectivityManager cm, + int expectedSubId, + Set expectedRequiredCaps, + Set expectedForbiddenCaps) { + verify(cm) + .requestBackgroundNetwork( + eq( + getCellRequestForSubId( + expectedSubId, + expectedRequiredCaps, + expectedForbiddenCaps)), + any(NetworkBringupCallback.class), + any()); + } + + @Test + public void testNetworkCallbacksRegisteredOnStartupForNonInternetCapabilities() { + final ConnectivityManager cm = mock(ConnectivityManager.class); + setupSystemService(mContext, cm, Context.CONNECTIVITY_SERVICE, ConnectivityManager.class); + + // Build network templates + final List networkTemplates = new ArrayList(); + + networkTemplates.add( + VcnCellUnderlyingNetworkTemplateTest.getTestNetworkTemplateBuilder() + .setDun(MATCH_REQUIRED) + .setInternet(MATCH_ANY) + .build()); + + networkTemplates.add( + VcnCellUnderlyingNetworkTemplateTest.getTestNetworkTemplateBuilder() + .setMms(MATCH_REQUIRED) + .setCbs(MATCH_FORBIDDEN) + .setInternet(MATCH_ANY) + .build()); + + // Start UnderlyingNetworkController + new UnderlyingNetworkController( + mVcnContext, + VcnGatewayConnectionConfigTest.buildTestConfig(networkTemplates), + SUB_GROUP, + mSubscriptionSnapshot, + mNetworkControllerCb); + + // Verifications + for (final int subId : INITIAL_SUB_IDS) { + verifyRequestBackgroundNetwork( + cm, + subId, + Collections.singleton(NET_CAPABILITY_INTERNET), + Collections.emptySet()); + verifyRequestBackgroundNetwork( + cm, subId, Collections.singleton(NET_CAPABILITY_DUN), Collections.emptySet()); + verifyRequestBackgroundNetwork( + cm, + subId, + Collections.singleton(NET_CAPABILITY_MMS), + Collections.singleton(NET_CAPABILITY_CBS)); + } + } + + @Test + public void testNetworkCallbacksRegisteredOnStartupWithDedupedtCapabilities() { + final ConnectivityManager cm = mock(ConnectivityManager.class); + setupSystemService(mContext, cm, Context.CONNECTIVITY_SERVICE, ConnectivityManager.class); + + // Build network templates + final List networkTemplates = new ArrayList(); + final VcnCellUnderlyingNetworkTemplate.Builder builder = + new VcnCellUnderlyingNetworkTemplate.Builder() + .setMms(MATCH_REQUIRED) + .setCbs(MATCH_FORBIDDEN) + .setInternet(MATCH_ANY); + + networkTemplates.add(builder.setMetered(MATCH_REQUIRED).build()); + networkTemplates.add(builder.setMetered(MATCH_FORBIDDEN).build()); + + // Start UnderlyingNetworkController + new UnderlyingNetworkController( + mVcnContext, + VcnGatewayConnectionConfigTest.buildTestConfig(networkTemplates), + SUB_GROUP, + mSubscriptionSnapshot, + mNetworkControllerCb); + + // Verifications + for (final int subId : INITIAL_SUB_IDS) { + verifyRequestBackgroundNetwork( + cm, + subId, + Collections.singleton(NET_CAPABILITY_INTERNET), + Collections.emptySet()); + verifyRequestBackgroundNetwork( + cm, + subId, + Collections.singleton(NET_CAPABILITY_MMS), + Collections.singleton(NET_CAPABILITY_CBS)); + } + } + private void verifyNetworkRequestsRegistered(Set expectedSubIds) { verify(mConnectivityManager) .requestBackgroundNetwork( @@ -212,8 +356,13 @@ public class UnderlyingNetworkControllerTest { for (final int subId : expectedSubIds) { verify(mConnectivityManager) .requestBackgroundNetwork( - eq(getCellRequestForSubId(subId)), - any(NetworkBringupCallback.class), any()); + eq( + getCellRequestForSubId( + subId, + Collections.singleton(NET_CAPABILITY_INTERNET), + Collections.emptySet())), + any(NetworkBringupCallback.class), + any()); } verify(mConnectivityManager) @@ -255,6 +404,7 @@ public class UnderlyingNetworkControllerTest { private NetworkRequest getWifiRequest(Set netCapsSubIds) { return getExpectedRequestBase() .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) .setSubscriptionIds(netCapsSubIds) .build(); } @@ -263,6 +413,7 @@ public class UnderlyingNetworkControllerTest { // TODO (b/187991063): Add tests for carrier-config based thresholds return getExpectedRequestBase() .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) .setSubscriptionIds(netCapsSubIds) .setSignalStrength(WIFI_ENTRY_RSSI_THRESHOLD_DEFAULT) .build(); @@ -272,16 +423,27 @@ public class UnderlyingNetworkControllerTest { // TODO (b/187991063): Add tests for carrier-config based thresholds return getExpectedRequestBase() .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) .setSubscriptionIds(netCapsSubIds) .setSignalStrength(WIFI_EXIT_RSSI_THRESHOLD_DEFAULT) .build(); } - private NetworkRequest getCellRequestForSubId(int subId) { - return getExpectedRequestBase() - .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) - .setNetworkSpecifier(new TelephonyNetworkSpecifier(subId)) - .build(); + private NetworkRequest getCellRequestForSubId( + int subId, Set requiredCaps, Set forbiddenCaps) { + final NetworkRequest.Builder nqBuilder = + getExpectedRequestBase() + .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) + .setNetworkSpecifier(new TelephonyNetworkSpecifier(subId)); + + for (int cap : requiredCaps) { + nqBuilder.addCapability(cap); + } + for (int cap : forbiddenCaps) { + nqBuilder.addForbiddenCapability(cap); + } + + return nqBuilder.build(); } private NetworkRequest getRouteSelectionRequest(Set netCapsSubIds) { @@ -303,7 +465,6 @@ public class UnderlyingNetworkControllerTest { private NetworkRequest.Builder getExpectedRequestBase() { 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); @@ -506,13 +667,7 @@ public class UnderlyingNetworkControllerTest { cb.onBlockedStatusChanged(mNetwork, true /* isBlocked */); - UnderlyingNetworkRecord expectedRecord = - getTestNetworkRecord( - mNetwork, - buildResponseNwCaps(INITIAL_NETWORK_CAPABILITIES, INITIAL_SUB_IDS), - INITIAL_LINK_PROPERTIES, - true /* isBlocked */); - verifyOnSelectedUnderlyingNetworkChanged(expectedRecord); + verifyOnSelectedUnderlyingNetworkChanged(null); } @Test @@ -548,5 +703,132 @@ public class UnderlyingNetworkControllerTest { verify(mNetworkControllerCb, times(1)).onSelectedUnderlyingNetworkChanged(any()); } - // TODO (b/187991063): Add tests for network prioritization + private UnderlyingNetworkListener setupControllerAndGetNetworkListener( + List networkTemplates) { + final ConnectivityManager cm = mock(ConnectivityManager.class); + setupSystemService(mContext, cm, Context.CONNECTIVITY_SERVICE, ConnectivityManager.class); + + new UnderlyingNetworkController( + mVcnContext, + VcnGatewayConnectionConfigTest.buildTestConfig(networkTemplates), + SUB_GROUP, + mSubscriptionSnapshot, + mNetworkControllerCb); + + verify(cm) + .registerNetworkCallback( + eq(getRouteSelectionRequest(INITIAL_SUB_IDS)), + mUnderlyingNetworkListenerCaptor.capture(), + any()); + + return mUnderlyingNetworkListenerCaptor.getValue(); + } + + private UnderlyingNetworkRecord bringupNetworkAndGetRecord( + UnderlyingNetworkListener cb, + NetworkCapabilities requestNetworkCaps, + List underlyingNetworkTemplates, + UnderlyingNetworkRecord currentlySelected) { + final Network network = mock(Network.class); + final NetworkCapabilities responseNetworkCaps = + buildResponseNwCaps(requestNetworkCaps, INITIAL_SUB_IDS); + + cb.onAvailable(network); + cb.onCapabilitiesChanged(network, responseNetworkCaps); + cb.onLinkPropertiesChanged(network, INITIAL_LINK_PROPERTIES); + cb.onBlockedStatusChanged(network, false /* isFalse */); + return new UnderlyingNetworkRecord( + network, + responseNetworkCaps, + INITIAL_LINK_PROPERTIES, + false /* isBlocked */, + mVcnContext, + underlyingNetworkTemplates, + SUB_GROUP, + mSubscriptionSnapshot, + currentlySelected, + null /* carrierConfig */); + } + + @Test + public void testSelectMorePreferredNetwork() { + final List networkTemplates = new ArrayList(); + networkTemplates.add(CELL_TEMPLATE_DUN); + networkTemplates.add(CELL_TEMPLATE_CBS); + + UnderlyingNetworkListener cb = setupControllerAndGetNetworkListener(networkTemplates); + + // Bring up CBS network + final UnderlyingNetworkRecord cbsNetworkRecord = + bringupNetworkAndGetRecord( + cb, + CBS_NETWORK_CAPABILITIES, + networkTemplates, + null /* currentlySelected */); + verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(cbsNetworkRecord)); + + // Bring up DUN network + final UnderlyingNetworkRecord dunNetworkRecord = + bringupNetworkAndGetRecord( + cb, DUN_NETWORK_CAPABILITIES, networkTemplates, cbsNetworkRecord); + verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(dunNetworkRecord)); + } + + @Test + public void testNeverSelectLessPreferredNetwork() { + final List networkTemplates = new ArrayList(); + networkTemplates.add(CELL_TEMPLATE_DUN); + networkTemplates.add(CELL_TEMPLATE_CBS); + + UnderlyingNetworkListener cb = setupControllerAndGetNetworkListener(networkTemplates); + + // Bring up DUN network + final UnderlyingNetworkRecord dunNetworkRecord = + bringupNetworkAndGetRecord( + cb, + DUN_NETWORK_CAPABILITIES, + networkTemplates, + null /* currentlySelected */); + verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(dunNetworkRecord)); + + // Bring up CBS network + final UnderlyingNetworkRecord cbsNetworkRecord = + bringupNetworkAndGetRecord( + cb, CBS_NETWORK_CAPABILITIES, networkTemplates, dunNetworkRecord); + verify(mNetworkControllerCb, never()) + .onSelectedUnderlyingNetworkChanged(eq(cbsNetworkRecord)); + } + + @Test + public void testFailtoMatchTemplateAndFallBackToInternetNetwork() { + final List networkTemplates = new ArrayList(); + + networkTemplates.add( + new VcnCellUnderlyingNetworkTemplate.Builder().setDun(MATCH_REQUIRED).build()); + UnderlyingNetworkListener cb = setupControllerAndGetNetworkListener(networkTemplates); + + // Bring up an Internet network without DUN capability + final UnderlyingNetworkRecord networkRecord = + bringupNetworkAndGetRecord( + cb, + INITIAL_NETWORK_CAPABILITIES, + networkTemplates, + null /* currentlySelected */); + verify(mNetworkControllerCb).onSelectedUnderlyingNetworkChanged(eq(networkRecord)); + } + + @Test + public void testFailtoMatchTemplateAndNeverFallBackToNonInternetNetwork() { + final List networkTemplates = new ArrayList(); + + networkTemplates.add( + new VcnCellUnderlyingNetworkTemplate.Builder().setDun(MATCH_REQUIRED).build()); + UnderlyingNetworkListener cb = setupControllerAndGetNetworkListener(networkTemplates); + + bringupNetworkAndGetRecord( + cb, CBS_NETWORK_CAPABILITIES, networkTemplates, null /* currentlySelected */); + + verify(mNetworkControllerCb, never()) + .onSelectedUnderlyingNetworkChanged(any(UnderlyingNetworkRecord.class)); + } }