[NS D05] Rework how to tear down networks
Networks are torn down if they don't satisfy requests but only if
they are found unable to ever do so. This is important because
general-purpose networks typically turn up unvalidated, which
means they would get torn down immediately in the presence of
some worse network that is already validated.
Note that functionally it's the same as before for the existing
scores, except that
• An exiting network that doesn't satisfy any request is always
torn down immediately – this is WAI
• An unmetered network is never torn down when compared to a
metered network – this is a bugfix (previously the platform
would tear down an unmetered cellular on grounds that it
could not beat the performance of a metered Ethernet, but
that's wrong ; the bug was never realized because Android
always thinks Ethernet is unmetered)
• An unvalidated network will not be torn down if the current
satisfier is also unvalidated, even if the satisfier is much
faster. This is the reason for the change in the test. It's
wrong to tear down in this case because even if much slower
the slower network should win if it validates and the other
doesn't.
Test: ConnectivityServiceTest
Change-Id: Ic9a3d336306a25d1272976ce467aa7c908af7bef
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 7083281..f7eabac 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3430,16 +3430,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 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 4612cfd..3860904 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 @@
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 @@
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 @@
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/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 0886975..5aabc60 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -2054,14 +2054,15 @@
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());