diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 7083281eaa7e9..f7eabac3b21f6 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3430,16 +3430,16 @@ public class ConnectivityService extends IConnectivityManager.Stub // there is hope for it to become one if it validated, then it is needed. ensureRunningOnConnectivityServiceThread(); if (nri.request.isRequest() && nai.satisfies(nri.request) && - (nai.isSatisfyingRequest(nri.request.requestId) || - // Note that this catches two important cases: - // 1. Unvalidated cellular will not be reaped when unvalidated WiFi - // is currently satisfying the request. This is desirable when - // cellular ends up validating but WiFi does not. - // 2. Unvalidated WiFi will not be reaped when validated cellular - // is currently satisfying the request. This is desirable when - // WiFi ends up validating and out scoring cellular. - nri.mSatisfier.getCurrentScore() - < nai.getCurrentScoreAsValidated())) { + (nai.isSatisfyingRequest(nri.request.requestId) + // Note that canPossiblyBeat catches two important cases: + // 1. Unvalidated slow networks will not be reaped when an unvalidated fast + // network is currently satisfying the request. This is desirable for example + // when cellular ends up validating but WiFi/Ethernet does not. + // 2. Fast networks will not be reaped when a validated slow network is + // currently satisfying the request. This is desirable for example when + // Ethernet ends up validating and out scoring WiFi, or WiFi/Ethernet ends + // up validating and out scoring cellular. + || nai.canPossiblyBeat(nri.mSatisfier))) { return false; } } diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 4612cfd0f7cbb..3860904a3fed9 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -16,6 +16,8 @@ package com.android.server.connectivity; +import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; +import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; import static android.net.NetworkCapabilities.transportNamesOf; import android.annotation.NonNull; @@ -475,24 +477,16 @@ public class NetworkAgentInfo implements Comparable { return networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN); } - private int getCurrentScore(boolean pretendValidated) { - // TODO: We may want to refactor this into a NetworkScore class that takes a base score from - // the NetworkAgent and signals from the NetworkAgent and uses those signals to modify the - // score. The NetworkScore class would provide a nice place to centralize score constants - // so they are not scattered about the transports. - + /** Gets the current score */ + public int getCurrentScore() { // If this network is explicitly selected and the user has decided to use it even if it's - // unvalidated, give it the maximum score. Also give it the maximum score if it's explicitly - // selected and we're trying to see what its score could be. This ensures that we don't tear - // down an explicitly selected network before the user gets a chance to prefer it when - // a higher-scoring network (e.g., Ethernet) is available. - if (networkAgentConfig.explicitlySelected - && (networkAgentConfig.acceptUnvalidated || pretendValidated)) { + // unvalidated, give it the maximum score. + if (networkAgentConfig.explicitlySelected && networkAgentConfig.acceptUnvalidated) { return ConnectivityConstants.EXPLICITLY_SELECTED_NETWORK_SCORE; } int score = mNetworkScore.getLegacyScore(); - if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) { + if (!lastValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) { score -= ConnectivityConstants.UNVALIDATED_SCORE_PENALTY; } if (score < 0) score = 0; @@ -508,18 +502,6 @@ public class NetworkAgentInfo implements Comparable { return isWifi && !avoidBadWifi && everValidated; } - // Get the current score for this Network. This may be modified from what the - // NetworkAgent sent, as it has modifiers applied to it. - public int getCurrentScore() { - return getCurrentScore(false); - } - - // Get the current score for this Network as if it was validated. This may be modified from - // what the NetworkAgent sent, as it has modifiers applied to it. - public int getCurrentScoreAsValidated() { - return getCurrentScore(true); - } - public void setNetworkScore(@NonNull NetworkScore ns) { mNetworkScore = ns; } @@ -629,6 +611,41 @@ public class NetworkAgentInfo implements Comparable { mLingering = false; } + /** + * Returns whether this NAI has any chance of ever beating this other agent. + * + * The chief use case of this is the decision to tear down this network. ConnectivityService + * tears down networks that don't satisfy any request, unless they have a chance to beat any + * existing satisfier. + * + * @param other the agent to beat + * @return whether this should be given more time to try and beat the other agent + * TODO : remove this and migrate to a ranker-based approach + */ + public boolean canPossiblyBeat(@NonNull final NetworkAgentInfo other) { + // Any explicitly selected network should be held on. + if (networkAgentConfig.explicitlySelected) return true; + // An outscored exiting network should be torn down. + if (mNetworkScore.isExiting()) return false; + // If this network is validated it can be torn down as it can't hope to be better than + // it already is. + if (networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)) return false; + // If neither network is validated, keep both until at least one does. + if (!other.networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)) return true; + // If this network is not metered but the other is, it should be preferable if it validates. + if (networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED) + && !other.networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED)) { + return true; + } + + // If the control comes here : + // • This network is neither exiting or explicitly selected + // • This network is not validated, but the other is + // • This network is metered, or both networks are unmetered + // Keep it if it's expected to be faster than the other., should it validate. + return mNetworkScore.probablyFasterThan(other.mNetworkScore); + } + public void dumpLingerTimers(PrintWriter pw) { for (LingerTimer timer : mLingerTimers) { pw.println(timer); } } diff --git a/services/core/java/com/android/server/connectivity/NetworkRanker.java b/services/core/java/com/android/server/connectivity/NetworkRanker.java index c536ab25e9254..80d46e0370b4d 100644 --- a/services/core/java/com/android/server/connectivity/NetworkRanker.java +++ b/services/core/java/com/android/server/connectivity/NetworkRanker.java @@ -16,6 +16,9 @@ package com.android.server.connectivity; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; +import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; +import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkScore.POLICY_IGNORE_ON_WIFI; import static com.android.internal.util.FunctionalUtils.findFirst; @@ -42,13 +45,20 @@ public class NetworkRanker { @NonNull final Collection nais) { final ArrayList candidates = new ArrayList<>(nais); candidates.removeIf(nai -> !nai.satisfies(request)); - // Enforce policy. - filterBadWifiAvoidancePolicy(candidates); + + // Enforce policy. The order in which the policy is computed is essential, because each + // step may remove some of the candidates. For example, filterValidated drops non-validated + // networks in presence of validated networks for INTERNET requests, but the bad wifi + // avoidance policy takes priority over this, so it must be done before. + filterVpn(candidates); + filterExplicitlySelected(candidates); + filterBadWifiAvoidance(candidates); + filterValidated(request, candidates); NetworkAgentInfo bestNetwork = null; int bestScore = Integer.MIN_VALUE; for (final NetworkAgentInfo nai : candidates) { - final int score = nai.getCurrentScore(); + final int score = nai.getNetworkScore().getLegacyScore(); if (score > bestScore) { bestNetwork = nai; bestScore = score; @@ -57,9 +67,27 @@ public class NetworkRanker { return bestNetwork; } - // If some network with wifi transport is present, drop all networks with POLICY_IGNORE_ON_WIFI. - private void filterBadWifiAvoidancePolicy( + // If a network is a VPN it has priority. + private void filterVpn(@NonNull final ArrayList candidates) { + final NetworkAgentInfo vpn = findFirst(candidates, + nai -> nai.networkCapabilities.hasTransport(TRANSPORT_VPN)); + if (null == vpn) return; // No VPN : this policy doesn't apply. + candidates.removeIf(nai -> !nai.networkCapabilities.hasTransport(TRANSPORT_VPN)); + } + + // If some network is explicitly selected and set to accept unvalidated connectivity, then + // drop all networks that are not explicitly selected. + private void filterExplicitlySelected( @NonNull final ArrayList candidates) { + final NetworkAgentInfo explicitlySelected = findFirst(candidates, + nai -> nai.networkAgentConfig.explicitlySelected + && nai.networkAgentConfig.acceptUnvalidated); + if (null == explicitlySelected) return; // No explicitly selected network accepting unvalid + candidates.removeIf(nai -> !nai.networkAgentConfig.explicitlySelected); + } + + // If some network with wifi transport is present, drop all networks with POLICY_IGNORE_ON_WIFI. + private void filterBadWifiAvoidance(@NonNull final ArrayList candidates) { final NetworkAgentInfo wifi = findFirst(candidates, nai -> nai.networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) && nai.everValidated @@ -71,4 +99,16 @@ public class NetworkRanker { if (null == wifi) return; // No wifi : this policy doesn't apply candidates.removeIf(nai -> nai.getNetworkScore().hasPolicy(POLICY_IGNORE_ON_WIFI)); } + + // If some network is validated and the request asks for INTERNET, drop all networks that are + // not validated. + private void filterValidated(@NonNull final NetworkRequest request, + @NonNull final ArrayList candidates) { + if (!request.hasCapability(NET_CAPABILITY_INTERNET)) return; + final NetworkAgentInfo validated = findFirst(candidates, + nai -> nai.networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)); + if (null == validated) return; // No validated network + candidates.removeIf(nai -> + !nai.networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)); + } } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 08869751b93e7..220cdce0d1785 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -2054,14 +2054,15 @@ public class ConnectivityServiceTest { assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); - // Bring up wifi with a score of 70. + // Bring up validated wifi. // Cell is lingered because it would not satisfy any request, even if it validated. mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); - mWiFiNetworkAgent.adjustScore(50); - mWiFiNetworkAgent.connect(false); // Score: 70 + mWiFiNetworkAgent.connect(true); // Score: 60 callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + // TODO: Investigate sending validated before losing. callback.expectCallback(CallbackEntry.LOSING, mCellNetworkAgent); - defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent); + defaultCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); @@ -5845,7 +5846,7 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(true); - trustedCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); + trustedCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent); verify(mNetworkManagementService).setDefaultNetId(eq(mWiFiNetworkAgent.getNetwork().netId)); reset(mNetworkManagementService); diff --git a/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt index a6b371a23b587..d2532c2ce3d32 100644 --- a/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt +++ b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt @@ -16,14 +16,18 @@ package com.android.server.connectivity +import android.net.ConnectivityManager.TYPE_WIFI +import android.net.LinkProperties +import android.net.Network +import android.net.NetworkAgentConfig import android.net.NetworkCapabilities +import android.net.NetworkInfo import android.net.NetworkRequest +import android.net.NetworkScore import androidx.test.filters.SmallTest import androidx.test.runner.AndroidJUnit4 import org.junit.Test import org.junit.runner.RunWith -import org.mockito.ArgumentMatchers.any -import org.mockito.Mockito.doReturn import org.mockito.Mockito.mock import kotlin.test.assertEquals import kotlin.test.assertNull @@ -33,10 +37,24 @@ import kotlin.test.assertNull class NetworkRankerTest { private val ranker = NetworkRanker() - private fun makeNai(satisfy: Boolean, score: Int) = mock(NetworkAgentInfo::class.java).also { - doReturn(satisfy).`when`(it).satisfies(any()) - doReturn(score).`when`(it).currentScore - it.networkCapabilities = NetworkCapabilities() + private fun makeNai(satisfy: Boolean, score: Int) = object : NetworkAgentInfo( + null /* messenger */, + null /* asyncChannel*/, + Network(100), + NetworkInfo(TYPE_WIFI, 0 /* subtype */, "" /* typename */, "" /* subtypename */), + LinkProperties(), + NetworkCapabilities(), + NetworkScore.Builder().setLegacyScore(score).build(), + null /* context */, + null /* handler */, + NetworkAgentConfig(), + null /* connectivityService */, + null /* netd */, + null /* dnsResolver */, + null /* networkManagementService */, + 0 /* factorySerialNumber */) { + override fun satisfies(request: NetworkRequest?): Boolean = satisfy + override fun getCurrentScore(): Int = score } @Test