From b3ce7501b78065a19d8e57283bf7f3c699b99648 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 20 Feb 2020 07:32:12 +0000 Subject: [PATCH] Revert "[NS D05] Rework how to tear down networks" Revert submission 10343065 Reason for revert: This is part of a feature that was punted out of R. Reverted Changes: Ic9a3d3363:[NS D05] Rework how to tear down networks I7d815f873:[NS D06] Implement more policies I561098476:[NS D07] Use the unmodified legacy score Change-Id: I8d2696d15999265d79abfc9163e7e5ccad873cfe --- .../android/server/ConnectivityService.java | 20 +++--- .../server/connectivity/NetworkAgentInfo.java | 67 +++++++------------ .../server/ConnectivityServiceTest.java | 9 ++- 3 files changed, 39 insertions(+), 57 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f7eabac3b2..7083281eaa 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 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 3860904a3f..4612cfd0f7 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 @@ public class NetworkAgentInfo implements Comparable { 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 @@ 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; } @@ -611,41 +629,6 @@ 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/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 5aabc60b22..08869751b9 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -2054,15 +2054,14 @@ public class ConnectivityServiceTest { 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());