diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 8912fd76a5..70d6cccdde 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -280,15 +280,18 @@ public class ConnectivityService extends IConnectivityManager.Stub // connect anyway?" dialog after the user selects a network that doesn't validate. private static final int PROMPT_UNVALIDATED_DELAY_MS = 8 * 1000; - // Default to 30s linger time-out. Modifiable only for testing. + // Default to 30s linger time-out, and 5s for nascent network. Modifiable only for testing. private static final String LINGER_DELAY_PROPERTY = "persist.netmon.linger"; private static final int DEFAULT_LINGER_DELAY_MS = 30_000; + private static final int DEFAULT_NASCENT_DELAY_MS = 5_000; // The maximum number of network request allowed per uid before an exception is thrown. private static final int MAX_NETWORK_REQUESTS_PER_UID = 100; @VisibleForTesting protected int mLingerDelayMs; // Can't be final, or test subclass constructors can't change it. + @VisibleForTesting + protected int mNascentDelayMs; // How long to delay to removal of a pending intent based request. // See Settings.Secure.CONNECTIVITY_RELEASE_PENDING_INTENT_DELAY_MS @@ -1053,6 +1056,8 @@ public class ConnectivityService extends IConnectivityManager.Stub Settings.Secure.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; mNMS = Objects.requireNonNull(netManager, "missing INetworkManagementService"); mStatsService = Objects.requireNonNull(statsService, "missing INetworkStatsService"); @@ -3311,7 +3316,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // 3. If this network is unneeded (which implies it is not lingering), and there is at least // one lingered request, set inactive. nai.updateInactivityTimer(); - if (nai.isLingering() && nai.numForegroundNetworkRequests() > 0) { + if (nai.isInactive() && nai.numForegroundNetworkRequests() > 0) { if (DBG) log("Unsetting inactive " + nai.toShortString()); nai.unsetInactive(); logNetworkEvent(nai, NetworkEvent.NETWORK_UNLINGER); @@ -3607,7 +3612,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return true; } - if (!nai.everConnected || nai.isVPN() || nai.isLingering() || numRequests > 0) { + if (!nai.everConnected || nai.isVPN() || nai.isInactive() || numRequests > 0) { return false; } for (NetworkRequestInfo nri : mNetworkRequests.values()) { @@ -7192,7 +7197,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Tear the network down. teardownUnneededNetwork(oldNetwork); } else { - // Put the network in the background. + // Put the network in the background if it doesn't satisfy any foreground request. updateCapabilitiesForNetwork(oldNetwork); } } @@ -7408,6 +7413,14 @@ public class ConnectivityService extends IConnectivityManager.Stub } else { if (VDBG || DDBG) log(" accepting network in place of null"); } + + // To prevent constantly CPU wake up for nascent timer, if a network comes up + // and immediately satisfies a request then remove the timer. This will happen for + // all networks except in the case of an underlying network for a VCN. + if (newSatisfier.isNascent()) { + newSatisfier.unlingerRequest(NetworkRequest.REQUEST_ID_NONE); + } + newSatisfier.unlingerRequest(newRequest.requestId); if (!newSatisfier.addRequest(newRequest)) { Log.wtf(TAG, "BUG: " + newSatisfier.toShortString() + " already has " @@ -7546,19 +7559,19 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - // Update the linger state before processing listen callbacks, because the background - // computation depends on whether the network is lingering. Don't send the LOSING callbacks + // Update the inactivity state before processing listen callbacks, because the background + // computation depends on whether the network is inactive. Don't send the LOSING callbacks // just yet though, because they have to be sent after the listens are processed to keep // backward compatibility. - final ArrayList lingeredNetworks = new ArrayList<>(); + final ArrayList inactiveNetworks = new ArrayList<>(); for (final NetworkAgentInfo nai : nais) { - // Rematching may have altered the linger state of some networks, so update all linger - // timers. updateLingerState reads the state from the network agent and does nothing - // if the state has not changed : the source of truth is controlled with - // NetworkAgentInfo#lingerRequest and NetworkAgentInfo#unlingerRequest, which have been - // called while rematching the individual networks above. + // Rematching may have altered the inactivity state of some networks, so update all + // inactivity timers. updateInactivityState reads the state from the network agent + // and does nothing if the state has not changed : the source of truth is controlled + // with NetworkAgentInfo#lingerRequest and NetworkAgentInfo#unlingerRequest, which + // have been called while rematching the individual networks above. if (updateInactivityState(nai, now)) { - lingeredNetworks.add(nai); + inactiveNetworks.add(nai); } } @@ -7575,7 +7588,11 @@ public class ConnectivityService extends IConnectivityManager.Stub processNewlySatisfiedListenRequests(nai); } - for (final NetworkAgentInfo nai : lingeredNetworks) { + for (final NetworkAgentInfo nai : inactiveNetworks) { + // For nascent networks, if connecting with no foreground request, skip broadcasting + // LOSING for backward compatibility. This is typical when mobile data connected while + // wifi connected with mobile data always-on enabled. + if (nai.isNascent()) continue; notifyNetworkLosing(nai, now); } @@ -7816,6 +7833,15 @@ public class ConnectivityService extends IConnectivityManager.Stub // doing. updateSignalStrengthThresholds(networkAgent, "CONNECT", null); + // Before first rematching networks, put an inactivity timer without any request, this + // allows {@code updateInactivityState} to update the state accordingly and prevent + // tearing down for any {@code unneeded} evaluation in this period. + // Note that the timer will not be rescheduled since the expiry time is + // fixed after connection regardless of the network satisfying other requests or not. + // But it will be removed as soon as the network satisfies a request for the first time. + networkAgent.lingerRequest(NetworkRequest.REQUEST_ID_NONE, + SystemClock.elapsedRealtime(), mNascentDelayMs); + // Consider network even though it is not yet validated. rematchAllNetworksAndRequests(); diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index a9a705f07a..bff1a5c99d 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -122,6 +122,13 @@ import java.util.TreeSet; // // When ConnectivityService disconnects a network: // ----------------------------------------------- +// If a network is just connected, ConnectivityService will think it will be used soon, but might +// not be used. Thus, a 5s timer will be held to prevent the network being torn down immediately. +// This "nascent" state is implemented by the "lingering" logic below without relating to any +// request, and is used in some cases where network requests race with network establishment. The +// nascent state ends when the 5-second timer fires, or as soon as the network satisfies a +// request, whichever is earlier. In this state, the network is considered in the background. +// // If a network has no chance of satisfying any requests (even if it were to become validated // and enter state #5), ConnectivityService will disconnect the NetworkAgent's AsyncChannel. // @@ -271,7 +278,8 @@ public class NetworkAgentInfo implements Comparable { // All inactivity timers for this network, sorted by expiry time. A timer is added whenever // a request is moved to a network with a better score, regardless of whether the network is or - // was lingering or not. + // was lingering or not. An inactivity timer is also added when a network connects + // without immediately satisfying any requests. // TODO: determine if we can replace this with a smaller or unsorted data structure. (e.g., // SparseLongArray) combined with the timestamp of when the last timer is scheduled to fire. private final SortedSet mInactivityTimers = new TreeSet<>(); @@ -896,7 +904,12 @@ public class NetworkAgentInfo implements Comparable { /** * Sets the specified requestId to linger on this network for the specified time. Called by - * ConnectivityService when the request is moved to another network with a higher score. + * ConnectivityService when the request is moved to another network with a higher score, or + * when a network is newly created. + * + * @param requestId The requestId of the request that no longer need to be served by this + * network. Or {@link NetworkRequest.REQUEST_ID_NONE} if this is the + * {@code LingerTimer} for a newly created network. */ public void lingerRequest(int requestId, long now, long duration) { if (mInactivityTimerForRequest.get(requestId) != null) { @@ -969,10 +982,23 @@ public class NetworkAgentInfo implements Comparable { mInactive = false; } - public boolean isLingering() { + public boolean isInactive() { return mInactive; } + public boolean isLingering() { + return mInactive && !isNascent(); + } + + /** + * Return whether the network is just connected and about to be torn down because of not + * satisfying any request. + */ + public boolean isNascent() { + return mInactive && mInactivityTimers.size() == 1 + && mInactivityTimers.first().requestId == NetworkRequest.REQUEST_ID_NONE; + } + public void clearInactivityState() { if (mInactivityMessage != null) { mInactivityMessage.cancel(); @@ -1022,7 +1048,7 @@ public class NetworkAgentInfo implements Comparable { + "network{" + network + "} handle{" + network.getNetworkHandle() + "} ni{" + networkInfo.toShortString() + "} " + " Score{" + getCurrentScore() + "} " - + (isLingering() ? " lingering" : "") + + (isNascent() ? " nascent" : (isLingering() ? " lingering" : "")) + (everValidated ? " everValidated" : "") + (lastValidated ? " lastValidated" : "") + (partialConnectivity ? " partialConnectivity" : "") diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index b0cc7f1361..4405e927e0 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -331,11 +331,12 @@ public class ConnectivityServiceTest { private static final String TAG = "ConnectivityServiceTest"; private static final int TIMEOUT_MS = 500; - private static final int TEST_LINGER_DELAY_MS = 300; - // Chosen to be less than the linger timeout. This ensures that we can distinguish between a - // LOST callback that arrives immediately and a LOST callback that arrives after the linger - // timeout. For this, our assertions should run fast enough to leave less than - // (mService.mLingerDelayMs - TEST_CALLBACK_TIMEOUT_MS) between the time callbacks are + private static final int TEST_LINGER_DELAY_MS = 400; + private static final int TEST_NASCENT_DELAY_MS = 300; + // Chosen to be less than the linger and nascent timeout. This ensures that we can distinguish + // between a LOST callback that arrives immediately and a LOST callback that arrives after + // the linger/nascent timeout. For this, our assertions should run fast enough to leave + // less than (mService.mLingerDelayMs - TEST_CALLBACK_TIMEOUT_MS) between the time callbacks are // supposedly fired, and the time we call expectCallback. private static final int TEST_CALLBACK_TIMEOUT_MS = 250; // Chosen to be less than TEST_CALLBACK_TIMEOUT_MS. This ensures that requests have time to @@ -1389,6 +1390,7 @@ public class ConnectivityServiceTest { mMockNetd, mDeps); mService.mLingerDelayMs = TEST_LINGER_DELAY_MS; + mService.mNascentDelayMs = TEST_NASCENT_DELAY_MS; verify(mDeps).makeMultinetworkPolicyTracker(any(), any(), any()); final ArgumentCaptor policyListenerCaptor = @@ -1731,6 +1733,108 @@ public class ConnectivityServiceTest { verifyNoNetwork(); } + /** + * Verify a newly created network will be inactive instead of torn down even if no one is + * requesting. + */ + @Test + public void testNewNetworkInactive() throws Exception { + // Create a callback that monitoring the testing network. + final TestNetworkCallback listenCallback = new TestNetworkCallback(); + mCm.registerNetworkCallback(new NetworkRequest.Builder().build(), listenCallback); + + // 1. Create a network that is not requested by anyone, and does not satisfy any of the + // default requests. Verify that the network will be inactive instead of torn down. + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + mWiFiNetworkAgent.connectWithoutInternet(); + listenCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + listenCallback.assertNoCallback(); + + // Verify that the network will be torn down after nascent expiry. A small period of time + // is added in case of flakiness. + final int nascentTimeoutMs = + mService.mNascentDelayMs + mService.mNascentDelayMs / 4; + listenCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent, nascentTimeoutMs); + + // 2. Create a network that is satisfied by a request comes later. + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + mWiFiNetworkAgent.connectWithoutInternet(); + listenCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + final NetworkRequest wifiRequest = new NetworkRequest.Builder() + .addTransportType(TRANSPORT_WIFI).build(); + final TestNetworkCallback wifiCallback = new TestNetworkCallback(); + mCm.requestNetwork(wifiRequest, wifiCallback); + wifiCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + + // Verify that the network will be kept since the request is still satisfied. And is able + // to get disconnected as usual if the request is released after the nascent timer expires. + listenCallback.assertNoCallback(nascentTimeoutMs); + mCm.unregisterNetworkCallback(wifiCallback); + listenCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); + + // 3. Create a network that is satisfied by a request comes later. + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + mWiFiNetworkAgent.connectWithoutInternet(); + listenCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + mCm.requestNetwork(wifiRequest, wifiCallback); + wifiCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + + // Verify that the network will still be torn down after the request gets removed. + mCm.unregisterNetworkCallback(wifiCallback); + listenCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); + + // There is no need to ensure that LOSING is never sent in the common case that the + // network immediately satisfies a request that was already present, because it is already + // verified anywhere whenever {@code TestNetworkCallback#expectAvailable*} is called. + + mCm.unregisterNetworkCallback(listenCallback); + } + + /** + * Verify a newly created network will be inactive and switch to background if only background + * request is satisfied. + */ + @Test + public void testNewNetworkInactive_bgNetwork() throws Exception { + // Create a callback that monitoring the wifi network. + final TestNetworkCallback wifiListenCallback = new TestNetworkCallback(); + mCm.registerNetworkCallback(new NetworkRequest.Builder() + .addTransportType(TRANSPORT_WIFI).build(), wifiListenCallback); + + // Create callbacks that can monitor background and foreground mobile networks. + // This is done by granting using background networks permission before registration. Thus, + // the service will not add {@code NET_CAPABILITY_FOREGROUND} by default. + grantUsingBackgroundNetworksPermissionForUid(Binder.getCallingUid()); + final TestNetworkCallback bgMobileListenCallback = new TestNetworkCallback(); + final TestNetworkCallback fgMobileListenCallback = new TestNetworkCallback(); + mCm.registerNetworkCallback(new NetworkRequest.Builder() + .addTransportType(TRANSPORT_CELLULAR).build(), bgMobileListenCallback); + mCm.registerNetworkCallback(new NetworkRequest.Builder() + .addTransportType(TRANSPORT_CELLULAR) + .addCapability(NET_CAPABILITY_FOREGROUND).build(), fgMobileListenCallback); + + // Connect wifi, which satisfies default request. + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + mWiFiNetworkAgent.connect(true); + wifiListenCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent); + + // Connect a cellular network, verify that satisfies only the background callback. + setAlwaysOnNetworks(true); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); + mCellNetworkAgent.connect(true); + bgMobileListenCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + fgMobileListenCallback.assertNoCallback(); + assertFalse(isForegroundNetwork(mCellNetworkAgent)); + + mCellNetworkAgent.disconnect(); + bgMobileListenCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); + fgMobileListenCallback.assertNoCallback(); + + mCm.unregisterNetworkCallback(wifiListenCallback); + mCm.unregisterNetworkCallback(bgMobileListenCallback); + mCm.unregisterNetworkCallback(fgMobileListenCallback); + } + @Test public void testValidatedCellularOutscoresUnvalidatedWiFi() throws Exception { // Test bringing up unvalidated WiFi @@ -3888,8 +3992,9 @@ public class ConnectivityServiceTest { setAlwaysOnNetworks(false); testFactory.expectRequestRemove(); - // ... and cell data to be torn down. - cellNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); + // ... and cell data to be torn down after nascent network timeout. + cellNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent, + mService.mNascentDelayMs + TEST_CALLBACK_TIMEOUT_MS); assertLength(1, mCm.getAllNetworks()); } finally { testFactory.terminate();