[VCN03] Linger the newly connected network for a short time
This is mainly used by VCN, where VCN-eligible networks might
be registered before VCN management service can fire request
to prevent the newly created network from being torndown
immediately by ConnectivityService.
Test: atest ConnectivityServiceTest#testNewNetworkLingering
atest ConnectivityServiceTest#testNewNetworkLingering_bgNetwork
atest FrameworksNetTests --rerun-until-failure 20
Bug: 175180558
Change-Id: I7768daf4cf2d8f6d8057b5107412e06104ba82e0
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 8912fd7..70d6ccc 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -280,15 +280,18 @@
// 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 @@
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 @@
// 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 @@
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 @@
// 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 @@
} 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 @@
}
}
- // 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<NetworkAgentInfo> lingeredNetworks = new ArrayList<>();
+ final ArrayList<NetworkAgentInfo> 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 @@
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 @@
// 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 a9a705f..bff1a5c 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 @@
//
// 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 @@
// 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<InactivityTimer> mInactivityTimers = new TreeSet<>();
@@ -896,7 +904,12 @@
/**
* 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 @@
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 @@
+ "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 b0cc7f1..4405e92 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -331,11 +331,12 @@
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 @@
mMockNetd,
mDeps);
mService.mLingerDelayMs = TEST_LINGER_DELAY_MS;
+ mService.mNascentDelayMs = TEST_NASCENT_DELAY_MS;
verify(mDeps).makeMultinetworkPolicyTracker(any(), any(), any());
final ArgumentCaptor<INetworkPolicyListener> policyListenerCaptor =
@@ -1731,6 +1733,108 @@
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 @@
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();