Merge "[VCN03] Linger the newly connected network for a short time"

services/core/java/com/android/server/ConnectivityService.java

@@ -282,15 +282,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
@@ -1064,6 +1067,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");
@@ -3335,7 +3340,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);
@@ -3629,7 +3634,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()) {
@@ -7242,7 +7247,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);
         }
     }
@@ -7496,6 +7501,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 "
@@ -7638,19 +7651,19 @@ public class ConnectivityService extends IConnectivityManager.Stub
             }
         }
 
-        // Update the linger state before processing listen callbacks, because the background
+        // Update the inactivity state before processing listen callbacks, because the background
-        // computation depends on whether the network is lingering. Don't send the LOSING callbacks
+        // 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
+            // Rematching may have altered the inactivity state of some networks, so update all
-            // timers. updateLingerState reads the state from the network agent and does nothing
+            // inactivity timers. updateInactivityState reads the state from the network agent
-            // if the state has not changed : the source of truth is controlled with
+            // and does nothing if the state has not changed : the source of truth is controlled
-            // NetworkAgentInfo#lingerRequest and NetworkAgentInfo#unlingerRequest, which have been
+            // with NetworkAgentInfo#lingerRequest and NetworkAgentInfo#unlingerRequest, which
-            // called while rematching the individual networks above.
+            // have been called while rematching the individual networks above.
             if (updateInactivityState(nai, now)) {
-                lingeredNetworks.add(nai);
+                inactiveNetworks.add(nai);
             }
         }
 
@@ -7667,7 +7680,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);
         }
 
@@ -7908,6 +7925,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();
 

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<NetworkAgentInfo> {
 
     // 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 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
 
     /**
      * 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<NetworkAgentInfo> {
         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<NetworkAgentInfo> {
                 + "network{" + network + "}  handle{" + network.getNetworkHandle() + "}  ni{"
                 + networkInfo.toShortString() + "} "
                 + "  Score{" + getCurrentScore() + "} "
-                + (isLingering() ? " lingering" : "")
+                + (isNascent() ? " nascent" : (isLingering() ? " lingering" : ""))
                 + (everValidated ? " everValidated" : "")
                 + (lastValidated ? " lastValidated" : "")
                 + (partialConnectivity ? " partialConnectivity" : "")

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;
+    private static final int TEST_LINGER_DELAY_MS = 400;
-    // Chosen to be less than the linger timeout. This ensures that we can distinguish between a
+    private static final int TEST_NASCENT_DELAY_MS = 300;
-    // LOST callback that arrives immediately and a LOST callback that arrives after the linger
+    // Chosen to be less than the linger and nascent timeout. This ensures that we can distinguish
-    // timeout. For this, our assertions should run fast enough to leave less than
+    // between a LOST callback that arrives immediately and a LOST callback that arrives after
-    // (mService.mLingerDelayMs - TEST_CALLBACK_TIMEOUT_MS) between the time callbacks are
+    // 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
@@ -1395,6 +1396,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<INetworkPolicyListener> policyListenerCaptor =
@@ -1737,6 +1739,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
@@ -3894,8 +3998,9 @@ public class ConnectivityServiceTest {
             setAlwaysOnNetworks(false);
             testFactory.expectRequestRemove();
 
-            // ...  and cell data to be torn down.
+            // ...  and cell data to be torn down after nascent network timeout.
-            cellNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent);
+            cellNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent,
+                    mService.mNascentDelayMs + TEST_CALLBACK_TIMEOUT_MS);
             assertLength(1, mCm.getAllNetworks());
         } finally {
             testFactory.terminate();