Merge changes from topic "revert-10343065-HRWTWGOFZV"
* changes:
Revert "[NS D05] Rework how to tear down networks"
Revert "[NS D06] Implement more policies"
Revert "[NS D07] Use the unmodified legacy score"
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index e48ef5a..7cf3fae 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3456,16 +3456,16 @@
// 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 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))) {
+ (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())) {
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 3860904..4612cfd 100644
--- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
@@ -16,8 +16,6 @@
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;
@@ -477,16 +475,24 @@
return networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN);
}
- /** Gets the current score */
- public int getCurrentScore() {
+ 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.
+
// If this network is explicitly selected and the user has decided to use it even if it's
- // unvalidated, give it the maximum score.
- if (networkAgentConfig.explicitlySelected && networkAgentConfig.acceptUnvalidated) {
+ // 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)) {
return ConnectivityConstants.EXPLICITLY_SELECTED_NETWORK_SCORE;
}
int score = mNetworkScore.getLegacyScore();
- if (!lastValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) {
+ if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) {
score -= ConnectivityConstants.UNVALIDATED_SCORE_PENALTY;
}
if (score < 0) score = 0;
@@ -502,6 +508,18 @@
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;
}
@@ -611,41 +629,6 @@
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 80d46e0..c536ab2 100644
--- a/services/core/java/com/android/server/connectivity/NetworkRanker.java
+++ b/services/core/java/com/android/server/connectivity/NetworkRanker.java
@@ -16,9 +16,6 @@
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;
@@ -45,20 +42,13 @@
@NonNull final Collection<NetworkAgentInfo> nais) {
final ArrayList<NetworkAgentInfo> candidates = new ArrayList<>(nais);
candidates.removeIf(nai -> !nai.satisfies(request));
-
- // 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);
+ // Enforce policy.
+ filterBadWifiAvoidancePolicy(candidates);
NetworkAgentInfo bestNetwork = null;
int bestScore = Integer.MIN_VALUE;
for (final NetworkAgentInfo nai : candidates) {
- final int score = nai.getNetworkScore().getLegacyScore();
+ final int score = nai.getCurrentScore();
if (score > bestScore) {
bestNetwork = nai;
bestScore = score;
@@ -67,27 +57,9 @@
return bestNetwork;
}
- // If a network is a VPN it has priority.
- private void filterVpn(@NonNull final ArrayList<NetworkAgentInfo> 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<NetworkAgentInfo> 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<NetworkAgentInfo> candidates) {
+ private void filterBadWifiAvoidancePolicy(
+ @NonNull final ArrayList<NetworkAgentInfo> candidates) {
final NetworkAgentInfo wifi = findFirst(candidates,
nai -> nai.networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)
&& nai.everValidated
@@ -99,16 +71,4 @@
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<NetworkAgentInfo> 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 6d4a1b2..4e75f2a2 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -2058,15 +2058,14 @@
assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
- // Bring up validated wifi.
+ // Bring up wifi with a score of 70.
// Cell is lingered because it would not satisfy any request, even if it validated.
mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
- mWiFiNetworkAgent.connect(true); // Score: 60
+ mWiFiNetworkAgent.adjustScore(50);
+ mWiFiNetworkAgent.connect(false); // Score: 70
callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
- // TODO: Investigate sending validated before losing.
callback.expectCallback(CallbackEntry.LOSING, mCellNetworkAgent);
- callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
- defaultCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent);
+ defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
@@ -5850,7 +5849,7 @@
mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
mWiFiNetworkAgent.connect(true);
- trustedCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent);
+ trustedCallback.expectAvailableDoubleValidatedCallbacks(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 d2532c2..a6b371a 100644
--- a/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt
+++ b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt
@@ -16,18 +16,14 @@
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
@@ -37,24 +33,10 @@
class NetworkRankerTest {
private val ranker = NetworkRanker()
- 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
+ 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()
}
@Test