From 0702f98edc8ae01854231da6dd87f861c7c18128 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 16 Sep 2021 21:50:07 +0900 Subject: [PATCH] Add a mode for cell radios unable to time share Upon changing the default SIM card, the radio will create a new connection to the new subscription. If that subscription works correctly, the stack will prefer it to the old one as the new subscription will be marked with a Primary policy flag it its score. Normally, at this point the old network lingers to give apps an opportunity to gracefully migrate their connections. But with some radios, this may have a dramatic effect on the performance of the new connection. This patch introduces a flag so that devices with such radios can be marked. In this case the stack will move to a degraded mode and eschew the grace delay for apps and give them a hard break instead, so that the new network can reach a good performance immediately. Apps with existing connections will suffer a worse experience. If there is a request that can only be served by the old connection, still keep it, as arguably the user still expects their MMS be sent on the old connection, even if the new connection doesn't work well until it's done. Test: new test in this patch, and add relevant tests in both modes also manually change the value of the flag and run FrameworksNetTests and CtsNetTestCasesLatestSdk Bug: 200226979 Change-Id: I4ace82f90e873bf06298cc689bb1d794ed5124bd --- .../src/android/net/NetworkCapabilities.java | 8 + .../res/values/config.xml | 11 + .../res/values/overlayable.xml | 1 + .../android/server/ConnectivityService.java | 46 +++- .../android/server/NetworkAgentWrapper.java | 5 + .../server/ConnectivityServiceTest.java | 221 +++++++++++++++++- 6 files changed, 280 insertions(+), 12 deletions(-) diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index 17fc3b4521..ae5cfda76d 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -1055,6 +1055,14 @@ public final class NetworkCapabilities implements Parcelable { return isValidTransport(transportType) && ((mTransportTypes & (1 << transportType)) != 0); } + /** + * Returns true iff this NetworkCapabilities has the specified transport and no other. + * @hide + */ + public boolean hasSingleTransport(@Transport int transportType) { + return mTransportTypes == (1 << transportType); + } + private void combineTransportTypes(NetworkCapabilities nc) { this.mTransportTypes |= nc.mTransportTypes; } diff --git a/service/ServiceConnectivityResources/res/values/config.xml b/service/ServiceConnectivityResources/res/values/config.xml index b22457a9bb..f8f86a257a 100644 --- a/service/ServiceConnectivityResources/res/values/config.xml +++ b/service/ServiceConnectivityResources/res/values/config.xml @@ -125,4 +125,15 @@ details on what is happening. --> false + + true diff --git a/service/ServiceConnectivityResources/res/values/overlayable.xml b/service/ServiceConnectivityResources/res/values/overlayable.xml index 5af13d764b..e5010d7429 100644 --- a/service/ServiceConnectivityResources/res/values/overlayable.xml +++ b/service/ServiceConnectivityResources/res/values/overlayable.xml @@ -36,6 +36,7 @@ + diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 9f63191c2f..c69af0601e 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -80,6 +80,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_TEST; import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.NetworkRequest.Type.LISTEN_FOR_BEST; +import static android.net.NetworkScore.POLICY_TRANSPORT_PRIMARY; import static android.net.OemNetworkPreferences.OEM_NETWORK_PREFERENCE_TEST; import static android.net.OemNetworkPreferences.OEM_NETWORK_PREFERENCE_TEST_ONLY; import static android.net.shared.NetworkMonitorUtils.isPrivateDnsValidationRequired; @@ -335,6 +336,9 @@ public class ConnectivityService extends IConnectivityManager.Stub protected int mLingerDelayMs; // Can't be final, or test subclass constructors can't change it. @VisibleForTesting protected int mNascentDelayMs; + // True if the cell radio of the device is capable of time-sharing. + @VisibleForTesting + protected boolean mCellularRadioTimesharingCapable = true; // How long to delay to removal of a pending intent based request. // See ConnectivitySettingsManager.CONNECTIVITY_RELEASE_PENDING_INTENT_DELAY_MS @@ -1375,6 +1379,12 @@ public class ConnectivityService extends IConnectivityManager.Stub NetworkCapabilities.NET_CAPABILITY_VEHICLE_INTERNAL, NetworkRequest.Type.BACKGROUND_REQUEST); + mLingerDelayMs = mSystemProperties.getInt(LINGER_DELAY_PROPERTY, DEFAULT_LINGER_DELAY_MS); + // TODO: Consider making the timer customizable. + mNascentDelayMs = DEFAULT_NASCENT_DELAY_MS; + mCellularRadioTimesharingCapable = + mResources.get().getBoolean(R.bool.config_cellular_radio_timesharing_capable); + mHandlerThread = mDeps.makeHandlerThread(); mHandlerThread.start(); mHandler = new InternalHandler(mHandlerThread.getLooper()); @@ -1385,10 +1395,6 @@ public class ConnectivityService extends IConnectivityManager.Stub mReleasePendingIntentDelayMs = Settings.Secure.getInt(context.getContentResolver(), ConnectivitySettingsManager.CONNECTIVITY_RELEASE_PENDING_INTENT_DELAY_MS, 5_000); - mLingerDelayMs = mSystemProperties.getInt(LINGER_DELAY_PROPERTY, DEFAULT_LINGER_DELAY_MS); - // TODO: Consider making the timer customizable. - mNascentDelayMs = DEFAULT_NASCENT_DELAY_MS; - mStatsManager = mContext.getSystemService(NetworkStatsManager.class); mPolicyManager = mContext.getSystemService(NetworkPolicyManager.class); mDnsResolver = Objects.requireNonNull(dnsresolver, "missing IDnsResolver"); @@ -7824,6 +7830,30 @@ public class ConnectivityService extends IConnectivityManager.Stub bundle.putParcelable(t.getClass().getSimpleName(), t); } + /** + * Returns whether reassigning a request from an NAI to another can be done gracefully. + * + * When a request should be assigned to a new network, it is normally lingered to give + * time for apps to gracefully migrate their connections. When both networks are on the same + * radio, but that radio can't do time-sharing efficiently, this may end up being + * counter-productive because any traffic on the old network may drastically reduce the + * performance of the new network. + * The stack supports a configuration to let modem vendors state that their radio can't + * do time-sharing efficiently. If this configuration is set, the stack assumes moving + * from one cell network to another can't be done gracefully. + * + * @param oldNai the old network serving the request + * @param newNai the new network serving the request + * @return whether the switch can be graceful + */ + private boolean canSupportGracefulNetworkSwitch(@NonNull final NetworkAgentInfo oldSatisfier, + @NonNull final NetworkAgentInfo newSatisfier) { + if (mCellularRadioTimesharingCapable) return true; + return !oldSatisfier.networkCapabilities.hasSingleTransport(TRANSPORT_CELLULAR) + || !newSatisfier.networkCapabilities.hasSingleTransport(TRANSPORT_CELLULAR) + || !newSatisfier.getScore().hasPolicy(POLICY_TRANSPORT_PRIMARY); + } + private void teardownUnneededNetwork(NetworkAgentInfo nai) { if (nai.numRequestNetworkRequests() != 0) { for (int i = 0; i < nai.numNetworkRequests(); i++) { @@ -8084,7 +8114,13 @@ public class ConnectivityService extends IConnectivityManager.Stub log(" accepting network in place of " + previousSatisfier.toShortString()); } previousSatisfier.removeRequest(previousRequest.requestId); - previousSatisfier.lingerRequest(previousRequest.requestId, now); + if (canSupportGracefulNetworkSwitch(previousSatisfier, newSatisfier)) { + // If this network switch can't be supported gracefully, the request is not + // lingered. This allows letting go of the network sooner to reclaim some + // performance on the new network, since the radio can't do both at the same + // time while preserving good performance. + previousSatisfier.lingerRequest(previousRequest.requestId, now); + } } else { if (VDBG || DDBG) log(" accepting network in place of null"); } diff --git a/tests/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/integration/util/com/android/server/NetworkAgentWrapper.java index 970b7d2e58..4dc86fffb5 100644 --- a/tests/integration/util/com/android/server/NetworkAgentWrapper.java +++ b/tests/integration/util/com/android/server/NetworkAgentWrapper.java @@ -27,6 +27,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_WIFI_AWARE; import static com.android.server.ConnectivityServiceTestUtils.transportToLegacyType; +import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertEquals; @@ -310,6 +311,10 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { assertTrue(mDisconnected.block(timeoutMs)); } + public void assertNotDisconnected(long timeoutMs) { + assertFalse(mDisconnected.block(timeoutMs)); + } + public void sendLinkProperties(LinkProperties lp) { mNetworkAgent.sendLinkProperties(lp); } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 2aba0b45c3..1c7ac44c48 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -1792,6 +1792,8 @@ public class ConnectivityServiceTest { doReturn(R.integer.config_networkAvoidBadWifi).when(mResources) .getIdentifier(eq("config_networkAvoidBadWifi"), eq("integer"), any()); doReturn(1).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi); + doReturn(true).when(mResources) + .getBoolean(R.bool.config_cellular_radio_timesharing_capable); final ConnectivityResources connRes = mock(ConnectivityResources.class); doReturn(mResources).when(connRes).get(); @@ -2273,8 +2275,22 @@ public class ConnectivityServiceTest { // After waitForIdle(), the message was processed and the service didn't crash. } + // TODO : migrate to @Parameterized @Test - public void testValidatedCellularOutscoresUnvalidatedWiFi() throws Exception { + public void testValidatedCellularOutscoresUnvalidatedWiFi_CanTimeShare() throws Exception { + // The behavior of this test should be the same whether the radio can time share or not. + doTestValidatedCellularOutscoresUnvalidatedWiFi(true); + } + + // TODO : migrate to @Parameterized + @Test + public void testValidatedCellularOutscoresUnvalidatedWiFi_CannotTimeShare() throws Exception { + doTestValidatedCellularOutscoresUnvalidatedWiFi(false); + } + + public void doTestValidatedCellularOutscoresUnvalidatedWiFi( + final boolean cellRadioTimesharingCapable) throws Exception { + mService.mCellularRadioTimesharingCapable = cellRadioTimesharingCapable; // Test bringing up unvalidated WiFi mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); ExpectedBroadcast b = registerConnectivityBroadcast(1); @@ -2308,8 +2324,21 @@ public class ConnectivityServiceTest { verifyNoNetwork(); } + // TODO : migrate to @Parameterized @Test - public void testUnvalidatedWifiOutscoresUnvalidatedCellular() throws Exception { + public void testUnvalidatedWifiOutscoresUnvalidatedCellular_CanTimeShare() throws Exception { + doTestUnvalidatedWifiOutscoresUnvalidatedCellular(true); + } + + // TODO : migrate to @Parameterized + @Test + public void testUnvalidatedWifiOutscoresUnvalidatedCellular_CannotTimeShare() throws Exception { + doTestUnvalidatedWifiOutscoresUnvalidatedCellular(false); + } + + public void doTestUnvalidatedWifiOutscoresUnvalidatedCellular( + final boolean cellRadioTimesharingCapable) throws Exception { + mService.mCellularRadioTimesharingCapable = cellRadioTimesharingCapable; // Test bringing up unvalidated cellular. mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); ExpectedBroadcast b = registerConnectivityBroadcast(1); @@ -2334,8 +2363,21 @@ public class ConnectivityServiceTest { verifyNoNetwork(); } + // TODO : migrate to @Parameterized @Test - public void testUnlingeringDoesNotValidate() throws Exception { + public void testUnlingeringDoesNotValidate_CanTimeShare() throws Exception { + doTestUnlingeringDoesNotValidate(true); + } + + // TODO : migrate to @Parameterized + @Test + public void testUnlingeringDoesNotValidate_CannotTimeShare() throws Exception { + doTestUnlingeringDoesNotValidate(false); + } + + public void doTestUnlingeringDoesNotValidate( + final boolean cellRadioTimesharingCapable) throws Exception { + mService.mCellularRadioTimesharingCapable = cellRadioTimesharingCapable; // Test bringing up unvalidated WiFi. mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); ExpectedBroadcast b = registerConnectivityBroadcast(1); @@ -2362,8 +2404,134 @@ public class ConnectivityServiceTest { NET_CAPABILITY_VALIDATED)); } + // TODO : migrate to @Parameterized @Test - public void testCellularOutscoresWeakWifi() throws Exception { + public void testRequestMigrationToSameTransport_CanTimeShare() throws Exception { + // Simulate a device where the cell radio is capable of time sharing + mService.mCellularRadioTimesharingCapable = true; + doTestRequestMigrationToSameTransport(TRANSPORT_CELLULAR, true); + doTestRequestMigrationToSameTransport(TRANSPORT_WIFI, true); + doTestRequestMigrationToSameTransport(TRANSPORT_ETHERNET, true); + } + + // TODO : migrate to @Parameterized + @Test + public void testRequestMigrationToSameTransport_CannotTimeShare() throws Exception { + // Simulate a device where the cell radio is not capable of time sharing + mService.mCellularRadioTimesharingCapable = false; + doTestRequestMigrationToSameTransport(TRANSPORT_CELLULAR, false); + doTestRequestMigrationToSameTransport(TRANSPORT_WIFI, true); + doTestRequestMigrationToSameTransport(TRANSPORT_ETHERNET, true); + } + + public void doTestRequestMigrationToSameTransport(final int transport, + final boolean expectLingering) throws Exception { + // To speed up tests the linger delay is very short by default in tests but this + // test needs to make sure the delay is not incurred so a longer value is safer (it + // reduces the risk that a bug exists but goes undetected). The alarm manager in the test + // throws and crashes CS if this is set to anything more than the below constant though. + mService.mLingerDelayMs = UNREASONABLY_LONG_ALARM_WAIT_MS; + + final TestNetworkCallback generalCb = new TestNetworkCallback(); + final TestNetworkCallback defaultCb = new TestNetworkCallback(); + mCm.registerNetworkCallback( + new NetworkRequest.Builder().addTransportType(transport | transport).build(), + generalCb); + mCm.registerDefaultNetworkCallback(defaultCb); + + // Bring up net agent 1 + final TestNetworkAgentWrapper net1 = new TestNetworkAgentWrapper(transport); + net1.connect(true); + // Make sure the default request is on net 1 + generalCb.expectAvailableThenValidatedCallbacks(net1); + defaultCb.expectAvailableThenValidatedCallbacks(net1); + + // Bring up net 2 with primary and mms + final TestNetworkAgentWrapper net2 = new TestNetworkAgentWrapper(transport); + net2.addCapability(NET_CAPABILITY_MMS); + net2.setScore(new NetworkScore.Builder().setTransportPrimary(true).build()); + net2.connect(true); + + // Make sure the default request goes to net 2 + generalCb.expectAvailableCallbacksUnvalidated(net2); + if (expectLingering) { + generalCb.expectCallback(CallbackEntry.LOSING, net1); + } + generalCb.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, net2); + defaultCb.expectAvailableDoubleValidatedCallbacks(net2); + + // Make sure cell 1 is unwanted immediately if the radio can't time share, but only + // after some delay if it can. + if (expectLingering) { + net1.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS); // always incurs the timeout + generalCb.assertNoCallback(); + // assertNotDisconnected waited for TEST_CALLBACK_TIMEOUT_MS, so waiting for the + // linger period gives TEST_CALLBACK_TIMEOUT_MS time for the event to process. + net1.expectDisconnected(UNREASONABLY_LONG_ALARM_WAIT_MS); + } else { + net1.expectDisconnected(TEST_CALLBACK_TIMEOUT_MS); + } + net1.disconnect(); + generalCb.expectCallback(CallbackEntry.LOST, net1); + + // Remove primary from net 2 + net2.setScore(new NetworkScore.Builder().build()); + // Request MMS + final TestNetworkCallback mmsCallback = new TestNetworkCallback(); + mCm.requestNetwork(new NetworkRequest.Builder().addCapability(NET_CAPABILITY_MMS).build(), + mmsCallback); + mmsCallback.expectAvailableCallbacksValidated(net2); + + // Bring up net 3 with primary but without MMS + final TestNetworkAgentWrapper net3 = new TestNetworkAgentWrapper(transport); + net3.setScore(new NetworkScore.Builder().setTransportPrimary(true).build()); + net3.connect(true); + + // Make sure default goes to net 3, but the MMS request doesn't + generalCb.expectAvailableThenValidatedCallbacks(net3); + defaultCb.expectAvailableDoubleValidatedCallbacks(net3); + mmsCallback.assertNoCallback(); + net2.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS); // Always incurs the timeout + + // Revoke MMS request and make sure net 2 is torn down with the appropriate delay + mCm.unregisterNetworkCallback(mmsCallback); + if (expectLingering) { + // If the radio can time share, the linger delay hasn't elapsed yet, so apps will + // get LOSING. If the radio can't time share, this is a hard loss, since the last + // request keeping up this network has been removed and the network isn't lingering + // for any other request. + generalCb.expectCallback(CallbackEntry.LOSING, net2); + net2.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS); + generalCb.assertNoCallback(); + net2.expectDisconnected(UNREASONABLY_LONG_ALARM_WAIT_MS); + } else { + net2.expectDisconnected(TEST_CALLBACK_TIMEOUT_MS); + } + net2.disconnect(); + generalCb.expectCallback(CallbackEntry.LOST, net2); + defaultCb.assertNoCallback(); + + net3.disconnect(); + mCm.unregisterNetworkCallback(defaultCb); + mCm.unregisterNetworkCallback(generalCb); + } + + // TODO : migrate to @Parameterized + @Test + public void testCellularOutscoresWeakWifi_CanTimeShare() throws Exception { + // The behavior of this test should be the same whether the radio can time share or not. + doTestCellularOutscoresWeakWifi(true); + } + + // TODO : migrate to @Parameterized + @Test + public void testCellularOutscoresWeakWifi_CannotTimeShare() throws Exception { + doTestCellularOutscoresWeakWifi(false); + } + + public void doTestCellularOutscoresWeakWifi( + final boolean cellRadioTimesharingCapable) throws Exception { + mService.mCellularRadioTimesharingCapable = cellRadioTimesharingCapable; // Test bringing up validated cellular. mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); ExpectedBroadcast b = registerConnectivityBroadcast(1); @@ -2388,8 +2556,21 @@ public class ConnectivityServiceTest { verifyActiveNetwork(TRANSPORT_WIFI); } + // TODO : migrate to @Parameterized @Test - public void testReapingNetwork() throws Exception { + public void testReapingNetwork_CanTimeShare() throws Exception { + doTestReapingNetwork(true); + } + + // TODO : migrate to @Parameterized + @Test + public void testReapingNetwork_CannotTimeShare() throws Exception { + doTestReapingNetwork(false); + } + + public void doTestReapingNetwork( + final boolean cellRadioTimesharingCapable) throws Exception { + mService.mCellularRadioTimesharingCapable = cellRadioTimesharingCapable; // Test bringing up WiFi without NET_CAPABILITY_INTERNET. // Expect it to be torn down immediately because it satisfies no requests. mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); @@ -2417,8 +2598,21 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.expectDisconnected(); } + // TODO : migrate to @Parameterized @Test - public void testCellularFallback() throws Exception { + public void testCellularFallback_CanTimeShare() throws Exception { + doTestCellularFallback(true); + } + + // TODO : migrate to @Parameterized + @Test + public void testCellularFallback_CannotTimeShare() throws Exception { + doTestCellularFallback(false); + } + + public void doTestCellularFallback( + final boolean cellRadioTimesharingCapable) throws Exception { + mService.mCellularRadioTimesharingCapable = cellRadioTimesharingCapable; // Test bringing up validated cellular. mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); ExpectedBroadcast b = registerConnectivityBroadcast(1); @@ -2455,8 +2649,21 @@ public class ConnectivityServiceTest { verifyActiveNetwork(TRANSPORT_WIFI); } + // TODO : migrate to @Parameterized @Test - public void testWiFiFallback() throws Exception { + public void testWiFiFallback_CanTimeShare() throws Exception { + doTestWiFiFallback(true); + } + + // TODO : migrate to @Parameterized + @Test + public void testWiFiFallback_CannotTimeShare() throws Exception { + doTestWiFiFallback(false); + } + + public void doTestWiFiFallback( + final boolean cellRadioTimesharingCapable) throws Exception { + mService.mCellularRadioTimesharingCapable = cellRadioTimesharingCapable; // Test bringing up unvalidated WiFi. mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); ExpectedBroadcast b = registerConnectivityBroadcast(1);